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

Tue Aug 4 04:26:25 PDT 2020

Hi All,

I also made the same mistake with this mail when replying to Pete.
Again, apologies about that! I didn't mean to take the conversation
privately off-list, I think it should be had publicly.

________________________________

Hi Pete,

My concern with this approach is the expectation that the original
author of a patch must provide fixes for someone else's workflow,
despite possibly never performing builds that way themselves.

Let me put it this way: multi-config builds are a part of my
development workflow, would it therefore be equally acceptable if I
started reverting any patch that broke multi-config builds and expect
the original developer to fix it? Is that something I should start
feeling able to do now?

Or to take a more extreme example: we have some people in the community
that develop on MacOS. Should they start feeling able to revert every
patch that breaks MacOS builds and expect the original developer (who
may very well not have a Mac, they are quite expensive after all) to
fix the issue?

Personally I don't think this is a sustainable process. If something
breaks a person's build workflow but passes all the buildbots and
passes review, I believe it should be on the person whose workflow was
broken to try and fix (of course, asking the original author for help
is perfectly reasonable) and not the author of every patch. I don't
believe any other way of working scales to a community of more than a
handful of people; after all, pretty much everyone's workflow is
different!

Thanks
David Truby

On Mon, 2020-08-03 at 15:25 +0000, Peter Steinfeld wrote:
> Thanks, David.
>
> Out-of-tree builds are critically important for my development/review
> process.  When they get broken, I want them to be fixed quickly.
>
> One way to do that is to identify and revert the patch that broke
> them.  Then the author of the patch creates a solution that both
> implements the original intention of the patch and also does not
> break out-of-tree builds.  This new patch would then be reviewed by
> the person who reverted the breaking patch.
>
> It would be great if we could prevent out-of-tree builds from
> breaking.  One step in that direction would be to have buildbots for
> them.  But I have no idea of how difficult this would be and whether
> it would be worth the effort.
>
> Pete
>
> -----Original Message-----
> From: David Truby <David.Truby at arm.com>
> Sent: Monday, August 3, 2020 6:06 AM
> To: Peter Steinfeld <psteinfeld at nvidia.com>; Steve Scalpone <
> sscalpone at nvidia.com>; isuruf at gmail.com
> Subject: Re: [flang-dev] Out-of-tree flang builds
>
> External email: Use caution opening links or attachments
>
>
> Hi all,
>
> 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.
>
> 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).
> 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.  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.
> > > > 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.  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
> > > >
> > >
> > > _______________________________________________
> > > flang-dev mailing list
> > > flang-dev at lists.llvm.org
> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose
> the contents to any other person, use it for any purpose, or store or
> copy the information in any medium. Thank you.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/flang-dev/attachments/20200804/40eb9a76/attachment-0001.html>