[flang-dev] Fw: Out-of-tree flang builds

Tue Aug 4 07:56:42 PDT 2020

Hi, David,

I realize that this is frustrating, but in general, we do realize that 
buildbots don't cover everything and we do revert actively even if the 
breakage is not observable on the buildbots. It is incumbent that every 
reasonable attempt is made by the reporter to provide a reproducer for 
the problem, and failing that, to proactively test future versions. 
Otherwise, forward progress will stall. It seems as though, in this 
case, we have a reproducer that can be widely tested.

I would not view this as treating one configuration as more important 
than another. It looks like our general practice of trying not to allow 
the fixing of one thing to break some currently-working thing.

We really do need to have a buildbot that tests out of tree builds. I 
was just on a call yesterday within my organization where we were 
discussing how to setup more builders with more configurations. Nick, we 
should make note of this in particular.

Regardless, if it is our consensus that out-of-tree builds are 
supported, and it seems like it is, then it is everyone's responsibility 
to test sufficiently and make sure that they work. Reverting as needed.

  -Hal

On 8/4/20 6:24 AM, David Truby via flang-dev wrote:
> Hi all, I meant to send this mail yesterday morning and instead managed to
> direct reply to a limited number of people instead of the list. Sorry 
> about that!
>
> Just to clarify, I'm not in any way opposed to out-of-tree builds and
> if people rely on them then obviously we should accept patches to keep
> them working. What I am opposed to here is the idea that everyone must
> test every patch both in and out of tree or have their patch reverted,
> especially when out of tree configurations are not being built by any
> buildbots.
>
> To take this particular case as an example, I posted a patch fixing a
> build configuration that was broken previously (note: I did not revert
> the patch that broke my build configuration, I simply posted a new
> patch to fix it). This patch underwent a review for 2 weeks before
> being approved and passed all the buildbots we have and as such I
> believe did not violate the criteria for a patch to be reverted
> according to the LLVM Developer Policy:
> https://llvm.org/docs/DeveloperPolicy.html#quality 
> <https://llvm.org/docs/DeveloperPolicy.html#quality>.
>
> I did however attempt to address the issue by providing pointers to
> what a quick fix might look like (such a fix has now kindly been
> applied by Andrzej Warzynski in https://reviews.llvm.org/D85078 
> <https://reviews.llvm.org/D85078>).
> However the patch was reverted rather than an attempt at a fix being
> made, which I believe would have been by far the more preferable option
> (such a fix could even have been committed without review given how
> simple it was).
>
> Obviously the revert broke my build configuration again, so the revert
> somewhat makes it feel that a decision was made off-list to consider
> one build configuration as more important than another. I think if we
> want out-of-tree builds to be considered more important than multi-
> config builds then that should be a decision made by the community not
> unilaterally. I also think if that decision is made then it must be
> defended by buildbots; if my patch had broken a buildbot I would have
> reverted it myself!
>
> Thanks
> David Truby
>
>
> On Fri, 2020-07-31 at 22:14 +0000, Peter Steinfeld wrote:
> > I agree that having every possible build configuration working in
> > every patch is difficult.
> >
> > With respect to your patch being reverted – This is the crux of the
> > matter – whether out-of-tree builds are a critical feature.  If so,
> > Tim was right to revert.
> >
> > It’s too bad that the buildbots don’t do out-of-tree builds.  In the
> > meanwhile, you can either do one yourself, as is documented here --
> > https://github.com/llvm/llvm-project/tree/master/flang 
> <https://github.com/llvm/llvm-project/tree/master/flang>. Or you can
> > ask someone who does them to review your changes. I volunteer!
> >
> > Pete
> > From: flang-dev <flang-dev-bounces at lists.llvm.org> On Behalf Of David
> > Truby via flang-dev
> > Sent: Friday, July 31, 2020 2:53 PM
> > To: Steve Scalpone <sscalpone at nvidia.com>; Isuru Fernando <
> > isuruf at gmail.com>
> > Cc: Peter Steinfeld via flang-dev <flang-dev at lists.llvm.org>
> > Subject: Re: [flang-dev] Out-of-tree flang builds
> >
> > External email: Use caution opening links or attachments
> >
> > Hi all,
> >
> > I agree with Michael here: having every possible build configuration
> > fixed in every commit is an impossible task. I submitted the patch in
> > question because earlier builds broke my build configuration (and any
> > other builds with multi-config generators, e.g. msbuild), and I
> > believe submitting such a patch to fix these configurations was
> > reasonable, just as submitting patches to fix out-of-tree builds is
> > reasonable.
> >
> > I do not believe that this patch should have been reverted, as such a
> > revert seems to suggest that one (non-standard?) build configuration
> > (out of tree builds) is more important than another (non-standard?)
> > build configuration (multi-config builds). The patch in question
> > didn't fail any build bots and doesn't (I believe) fall short of the
> > LLVM Developer Guidelines guidance on quality of patches. I believe a
> > better course of action here would have been a patch from someone
> > that requires out-of-tree builds to work, and I submitted some
> > suggestions on places to look to develop an easy fix.
> >
> > In response to out of tree builds being a critical feature that no
> > patch should break: I think in general it is unreasonable to state
> > that _any_ feature that is not defended by buildbots is a critical
> > feature that no patch should break. This puts a really unreasonable
> > expectation on the developer of every patch to manually test with
> > every possible configuration that some people might care about,
> > rather than using the automated structure we already have to detect
> > regressions.
> >
> > Thanks
> > David Truby
> > From: flang-dev <flang-dev-bounces at lists.llvm.org> on behalf of Isuru
> > Fernando via flang-dev <flang-dev at lists.llvm.org>
> > Sent: 31 July 2020 20:52
> > To: Steve Scalpone <sscalpone at nvidia.com>
> > Cc: Peter Steinfeld via flang-dev <flang-dev at lists.llvm.org>
> > Subject: Re: [flang-dev] Out-of-tree flang builds
> >
> > > Support for out-of-tree builds, like BUILD_SHARED_LIBS or gn
> > builds, are helpful for developers, but not relevant for release
> > builds.
> >
> > I disagree. Out-of-tree builds are also very useful for downstream
> > distributors when packaging individual components of the LLVM
> > project.
> >
> > Isuru
> >
> > On Fri, Jul 31, 2020 at 2:45 PM Steve Scalpone via flang-dev <
> > flang-dev at lists.llvm.org> wrote:
> > > +1 on Pete's and Tim's reply. I am often building multiple branches
> > > at the same time.
> > > From: flang-dev <flang-dev-bounces at lists.llvm.org> on behalf of
> > > Peter Steinfeld via flang-dev <flang-dev at lists.llvm.org>
> > > Sent: Friday, July 31, 2020 12:23 PM
> > > To: Michael Kruse <llvm at meinersbur.de>
> > > Cc: flang-dev at lists.llvm.org <flang-dev at lists.llvm.org>
> > > Subject: Re: [flang-dev] Out-of-tree flang builds
> > >
> > > External email: Use caution opening links or attachments
> > >
> > > Iterative builds are very useful and work well.
> > >
> > >
> > >
> > > But when I’m reviewing a change from someone else or starting on a
> > > new change for myself, I build all of flang. In the last six
> > > hours, I’ve done four non-iterative builds, for example.
> > >
> > >
> > >
> > > Pete
> > >
> > >
> > >
> > > From: Michael Kruse <llvm at meinersbur.de>
> > > Sent: Friday, July 31, 2020 12:01 PM
> > > To: Peter Steinfeld <psteinfeld at nvidia.com>
> > > Cc: flang-dev at lists.llvm.org
> > > Subject: Re: [flang-dev] Out-of-tree flang builds
> > >
> > >
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > >
> > > Support for out-of-tree builds, like BUILD_SHARED_LIBS or gn
> > > builds, are helpful for developers, but not relevant for release
> > > builds. Requiring to maintain all combinations of build
> > > configurations is not feasible before every commit, this is why
> > > they are only best effort. When you notice a out-of-tree build
> > > breaking, you are free to fix the problem (and assuming the fix is
> > > straightforward, low-risk, without review).
> > >
> > >
> > >
> > > However, I am wondering, why isn't an iterative build sufficient?
> > > Assuming you are not touching any non-flang files, make/ninja
> > > should not try to rebuild them.
> > >
> > >
> > > Michael
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > Am Fr., 31. Juli 2020 um 13:41 Uhr schrieb Peter Steinfeld via
> > > flang-dev <flang-dev at lists.llvm.org>:
> > >
> > > > Out-of-tree builds are a way to build only the flang code using a
> > > > pre-existing full build of llvm. Out-of-tree flang builds are
> > > > four times faster than full builds. Thus, for those of us
> > > > actively developing code and reviewing other people's changes,
> > > > out-of-tree builds significantly improve our productivity.
> > > >
> > > >
> > > >
> > > > Unfortunately, out-of-tree builds are not currently supported by
> > > > the llvm buildbots.  Thus, people changing build files must test
> > > > this feature themselves.  The process of doing an out-of-tree
> > > > build is described in the flang overview document --
> > > > https://github.com/llvm/llvm-project/tree/master/flang 
> <https://github.com/llvm/llvm-project/tree/master/flang>.
> > > > Alternatively, people changing build files could ask someone
> > > > familiar with out-of-tree builds to review their changes.
> > > >
> > > >
> > > >
> > > > A recent update from David Truby broke out-of-tree builds --
> > > > https://reviews.llvm.org/D84022 
> <https://reviews.llvm.org/D84022>. In the conversation in the
> > > > Phabricator review, David states that he understands that "out-
> > > > of-tree builds are considered a "best effort" feature that isn't
> > > > guaranteed to work".  But since out-of-tree builds are critical
> > > > for my development, I don't consider them optional.
> > > >
> > > >
> > > >
> > > > I'm writing this note to bring this issue to the broader flang
> > > > community.  I consider out-of-tree builds to be a critical
> > > > feature that no change should break. What do you think?
> > > >
> > > > _______________________________________________
> > > > flang-dev mailing list
> > > > flang-dev at lists.llvm.org
> > > > https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev 
> <https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev>
> > > >
> > >
> > > _______________________________________________
> > > flang-dev mailing list
> > > flang-dev at lists.llvm.org
> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev 
> <https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev>
>
> _______________________________________________
> flang-dev mailing list
> flang-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/flang-dev/attachments/20200804/0f45a2e3/attachment-0001.html>