llvm dev - Dec 2020 - RFC: Contributing Bazel BUILD files in the "peripheral" support tier

On Fri, Dec 4, 2020 at 8:07 PM Tom Stellard <tstellar at redhat.com>
wrote:
> On 12/4/20 7:19 PM, Mehdi AMINI wrote:
> >
> >
> > On Fri, Dec 4, 2020 at 6:42 PM Tom Stellard via llvm-dev
> > <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
> >
> >     On 12/3/20 4:27 PM, Geoffrey Martin-Noble wrote:
> >      > Apologies for the delayed response here. I was out of the
> "office".
> >      >
> >      > Thanks for taking another look :-)
> >      >
> >      > I want to respond first to the process question of pitch vs
RFC.
> My
> >      > impression was that the pitch process should be used in the
case
> >     that an
> >      > RFC couldn't reach consensus. I asked a few times in the
last
> thread
> >      >
> >     (https://groups.google.com/g/llvm-dev/c/u07o3QREVUg/m/uVlV3pMTBAAJ
> and
> >      >
https://groups.google.com/g/llvm-dev/c/u07o3QREVUg/m/wF5mu-dpBAAJ
> )
> >      > whether I should move this to a pitch, but feel like there
wasn't
> a
> >      > clear response in the context of Renato's support tiers
RFC.
> >      >
> >      > It seems like Tom and Renato still disagree about whether I
> >     should move
> >      > this to a pitch. I would appreciate some consensus on that
point
> at
> >      > least :-D I do see the appeal of a living document for this
sort
> of
> >      > thing, so definitely see the appeal there, but also it seems
like
> >     the
> >      > pitch process is a heavier-weight and more unusual one, so I
was
> >      > hesitant. My inclination is to continue this as an RFC
unless we
> are
> >      > unable to reach consensus on the issue as outlined in the
pitch
> >     process
> >      > description. It does feel like this is really not quite as
big a
> >      > decision as you seem to be suggesting. It's also an
easily
> >     reversible
> >      > one since there are no build dependencies and everything is
> >     contained.
> >      >
> >
> >     I still think this should be a pitch.  The original mailing list
> >     discussion was controversial and that's when an RFC should be
> escalated
> >     to a pitch according to: [1].
> >
> >
> > You may have missed it, but in the meantime there has been another RFC
> > clarifying our policy though: https://llvm.org/docs/SupportPolicy.html
> > It seems fair to me to revisit this RFC as is in light of the policy
> change.
> >
>
> I don't think the questions about whether or not this should be
included
> in the project are answered by this new policy.

I'll quote the policy:
> Section: "What is covered"
> The peripheral tier is composed of:
> Experimental targets and options that haven’t been enable by default yet.
> Main repository projects that don’t get released or regularly tested.
> Legacy tools and scripts that aren’t used in upstream validation.
> *Alternative build systems (ex. GN, Bazel) and related infrastructure.*
The intent of the policy is to cover exactly this proposal.



> To me the part about
> how the bazel build files were going to be supported and what
> responsibility the community had for maintaining them was always very
> clear.
>
> > I'd actually like to request that the objections are reiterated
and
> > positioned in terms of the policy before we escalate this.
> >
>
> I don't think it's really fair to ask people to re-object to the
> proposal.

Why?
The objections were mostly answered and have been addressed in the policy.
I don't quite get what you would put in a "pitch" while the
informations are outdated by the policy.
On the contrary it seems not only fair to me, but necessary.


> In my opinion, one of the problems with RFCs in the past is
> that they turn into an endurance test, because there is no process for
> making a decision.  Either the proposer gets tired of asking and gives
> up or the objectors get tired of objecting and give up.  We have a
> decision process now with the pitch process, and I think we should use it

We have to use it when we can't do otherwise. And again, I disagree that
this is a case without having objection formulated in light of the policy.




>
> >
> >     Thank you for responding to my technical concerns, and I agree
that
> >     working out most of those details may be better left for a patch
> review
> >     discussion.  But I think at least the presence of build
information
> for
> >     other projects and the sub-module alternative should be mentioned
in
> >     the
> >     pitch.
> >
> >     If there were only technical or support policy issues like these
to
> >     resolve then I don't think this would be controversial and
require a
> >     pitch.
> >
> >     My main issue with this RFC, (which I tried to address at the end
of
> my
> >     previous mail), is the precedent this sets for what gets included
in
> >     tree.  Essentially, we have a subset of our community that chose
to
> >     go a
> >     different direction from upstream, as always there are costs and
> >     benefits with this decision.  The question for the community is do
we
> >     want to help or encourage this in the future by removing some of
the
> >     costs of these decisions and allowing alternative implementations
to
> >     live in tree.
> >
> >     Maybe for build systems this is OK, and for other things this is
not,
> >     I don't know.  But if we are going to be setting a precedent,
to me,
> >     the
> >     best way to do this is through the pitch process.
> >
> >
> > Why are you considering this "setting a precedent" while
there is
> > already GN in tree?
> >
>
> You are right we are not really setting a precedent here, because GN is
> already in tree.  However, I don't think we should now just allow any
> build system to be added to the tree just because GN is there.  We need
> to have some kind of process and criteria for deciding what gets added
> and what doesn't.  I think a pitch will help accomplish this.
>
> I'll be honest, I don't really understand why there is so much push
back
> on turning this into a pitch.  Is it really that much extra work?
>
> -Tom
>
> > --
> > Mehdi
> >
> >
> >
> >
> >     -Tom
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >     [1]
> >
>
https://github.com/llvm/llvm-www/blob/master/proposals/LP0001-LLVMDecisionMaking.md
> >
> >      > On Mon, Nov 16, 2020 at 9:41 PM Tom Stellard <tstellar at
redhat.com
> >     <mailto:tstellar at redhat.com>
> >      > <mailto:tstellar at redhat.com <mailto:tstellar at
redhat.com>>> wrote:
> >      >
> >      >      > This should have approximately the same impact on
the
> >     community
> >      >     as the
> >      >      > current GN build in `llvm/utils/gn` does today.
That is, it
> >      >     should not
> >      >      > affect anyone who doesn't care.
> >      >      >
> >      >
> >      >     I want to push back on this a little bit, because having
the
> >     code in
> >      >     tree does impact everyone, even people who don't
care about
> >     it.  It
> >      >     increases disk usage, commit traffic, checkout times,
> >     bugzilla / issue
> >      >     traffic, and CI builds to name a few things.  There are
costs
> >     to having
> >      >     this in tree, the question (as always) is do the
benefits
> >     outweigh the
> >      >     costs?
> >      >
> >      > Yes my apologies that this was poorly phrased. I was aiming
for a
> >     pithy
> >      > summary and a clear statement that our goal here is not to
> >     significantly
> >      > impact contributors uninterested in Bazel. My impression is
that
> >     the GN
> >      > build has achieved that goal. I definitely agree that any
> >     addition to
> >      > the monorepo should have a clear weighing of costs vs
benefits
> >     and that
> >      > the costs are never actually zero. I do think the costs here
are
> >     really
> >      > quite low however. I am happy to address your concerns and
also
> >     think
> >      > that it is important to note that if additional issues arise
we
> are
> >      > still agreeing to be on the hook for addressing them (e.g.
if in
> >      > practice this causes some unforseen issue with the release)
and
> >     deleting
> >      > this contribution if we cannot do so in a timely manner (`rm
-rf
> >      > utils/bazel` is all it requires).
> >      >
> >      >     Personally, I do not think we should have alternative
build
> >     systems in
> >      >     tree.  However, I still think you should try to propose
this
> >     as a pitch.
> >      >     I would much rather this go through a fair process and
land
> >     than for it
> >      >     to be rejected based on a contentious thread.
> >      >
> >      >     Here is why I'm not convinced this should be in
tree:
> >      >
> >      >     To me it's not clear why having the build files
in-tree is
> >     better than
> >      >     having a separate repo with an llvm-project sub-module. 
The
> >     in tree
> >      >     bazel files will be broken from time to time, since most
> >     developers will
> >      >     not be updating them, however, with the sub-module
approach
> >     you can
> >      >     ensure that the build will always work by pinning the
> llvm-bazel
> >      >     repo to
> >      >     a known-working commit of llvm-project.  Can you expand
on the
> >      >     pros/cons
> >      >     of in-tree vs out-of-tree with sub-modules.
> >      >
> >      > Out-of-tree with a submodule is the current approach we have
with
> >      > https://github.com/google/llvm-bazel. It's certainly
doable, but
> >      > involves quite a bit of bookkeeping to track which version
> >     corresponds
> >      > to a given version of LLVM such that someone can fetch the
correct
> >      > configuration (you'll note that the repository has about
7k tags
> >     at the
> >      > moment). To make things somewhat more complicated, the
typical
> >     way to
> >      > fetch something for use in Bazel is with an http_archive
> >      >
> >     <
> https://docs.bazel.build/versions/master/repo/http.html#http_archive
> > which
> >
> >      > requires one to specify the archive digest to avoid
refetching on
> >     each
> >      > build. This doesn't work particularly well with tags
that change
> >     which
> >      > commit they point to. I'm not saying these issues
aren't
> >     solvable, but
> >      > they add quite a bit of complexity.
> >      >
> >      > The other point is that I think this makes contributing to
the
> Bazel
> >      > configuration quite a bit more complex because you have to
apply
> >     patches
> >      > across multiple repositories to also be kept in sync. Given
that
> >     LLVM
> >      > has a monorepo, it still seems like the logical place for a
build
> >      > configuration of LLVM used by multiple projects.
> >      >
> >      >     Other concerns I have from reviewing the patch:
> >      >
> >      > It seems like these are mostly concerns with the specific
> >      > implementation. Would you be alright with saving the
specific
> >     details
> >      > for an eventual review on the patch if this moves forward?
I've
> made
> >      > brief responses below.
> >      >
> >      >     * It looks like there is a build configuration for at
least
> one
> >      >     external
> >      >     project (zlib) and possibly another (vulkan-headers?). 
Do we
> >     really
> >      >     want to have build configurations for non-LLVM projects
in our
> >      >     tree?  Is
> >      >     there any limit to the number of external projects that
can
> >     and will be
> >      >     added?
> >      >
> >      > These are dependencies of the LLVM Project and LLVM keeps
its
> >      > dependencies pretty tightly managed AFAIU. These
configurations
> >     are also
> >      > pretty trivial, "here are the source files", type
things, so I
> think
> >      > it's even a bit generous to call them configurations:
we're just
> >      > informing Bazel where the files are located.
> >      >
> >      >
> >      >     * There are 3 files (abi-breaking.h.cmake,
config.h.cmake,
> >      >     llvm-config.h.cmake) that have been copied from the llvm
tree
> >     into
> >      >     utils/bazel/, is there some way we can avoid carrying
multiple
> >      >     copies of
> >      >     the same file in tree?
> >      >
> >      >
> >      >     * Similarly, there are some files that are normally
generated
> >     at build
> >      >     time clang/Config/config.h, llvm/Config/config.h,
> >      >     llvm/Config/llvm-config.h that have been copied into
> >     utils/bazel.
> >      >     Is it
> >      >     really necessary
> >      >     to have these in tree?  Especially since some of the
> >     templates, like
> >      >     llvm-config.h.cmake, are also in utils/bazel?
> >      >
> >      > The copy here is pretty much orthogonal to the actual build
> >      > configuration. The intent is to have a literal change
detector
> >     test for
> >      > changes to these cmake configurations, since they would
invalidate
> >      > assumptions in the Bazel configuration. Chandler and I went
back
> and
> >      > forth on a few different ways to do this. We can certainly
look
> >     at other
> >      > options. The issue is that I don't think there's
actually a
> >     useful way
> >      > to interpret the .cmake template files since changes to them
are
> >     also
> >      > made as changes to the cmake configuration and without these
> >     being in
> >      > sync the files just drift. Happy to discuss other options
for how
> to
> >      > handle this. We could, for instance, have some other process
that
> >     just
> >      > looks at the git diff/log for these files.
> >      >
> >      >     * I still worry about the bazel files causing merging
> >     conflicts when
> >      >     backported to the stable branch.  If these are added to
tree,
> >     could we
> >      >     have a rule where commits to utils/bazel/ cannot include
> >     changes to
> >      >     other files?
> >      >
> >      > I'd certainly be open to discussing restrictions that
would avoid
> >      > additional burden on release managers. I think that one
makes
> >      > contributing to the Bazel configuration more difficult
because you
> >      > cannot do it as part of a patch that requires a change, but
if
> it's
> >      > something that would cause issues with the release then we
can
> >     avoid it.
> >      > My intuition is that this wouldn't actually come up
often,
> >     however. For
> >      > example, just looking at the gn directory I see several
commits
> >     in the
> >      > last week that touch this and other files. Have you actually
run
> >     into
> >      > issues? Since this is unsupported the conflicts could also
be
> >     resolved
> >      > pretty much however you wanted (e.g. delete the conflict
markers,
> >     delete
> >      > the file), so they seem pretty trivial to deal with if they
only
> >     happen
> >      > occasionally. My preference would therefore be to see if
this is
> >      > actually a problem in practice before putting rules in
place.
> >      >
> >      >
> >      > On Tue, Nov 17, 2020 at 2:27 AM Renato Golin <rengolin at
gmail.com
> >     <mailto:rengolin at gmail.com>
> >      > <mailto:rengolin at gmail.com <mailto:rengolin at
gmail.com>>> wrote:
> >      >
> >      >     Hi Geoffrey,
> >      >
> >      >     Thanks for the re-submission.
> >      >
> >      >     I have some comments below that may sound negative, but
> they're
> >      >     probably just a reflection of my own ignorance. I want
to
> >     make sure
> >      >     the submission is clear, so it can be accepted on its
own
> right.
> >      >
> >      >     On Tue, 17 Nov 2020 at 03:02, Geoffrey Martin-Noble via
> llvm-dev
> >      >     <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
> >     <mailto:llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>>>
> >     wrote:
> >      >
> >      >           This should not affect development of core tier
> components.
> >      >         One reason we propose adding this to the root utils/
> >     directory
> >      >         instead of under llvm/utils (where GN is located) is
to
> avoid
> >      >         unnecessarily sending messages to llvm-commits.
Others
> have
> >      >         raised the concern that the existence of an
alternative
> build
> >      >         system might lead to lack of maintenance for the
CMake
> build
> >      >         system. Given that supporting CMake will remain a
> requirement
> >      >         and maintenance of a Bazel build system will
continue to
> >     happen
> >      >         regardless, we do not expect any significant impact
in
> >     this way.
> >      >
> >      >
> >      >     I was under the impression that "utils" was
actually
> >     "llvm/utils",
> >      >     which would be in the same place as GN. I don't
think we
> should
> >      >     treat GN and Bazel as different and I really
wouldn't like to
> >     have a
> >      >     different quality control (for post commit reviews).
> >      >
> >      >     If the Bazel commits are too verbose (for example,
committing
> >      >     auto-generated code), then we should really clean that
up and
> >     commit
> >      >     the script that generates them and make that part of the
> build.
> >      >
> >      >     I understand the need to move the noise away, but move
it too
> far
> >      >     away and it's no better than in a separate repo.
> >      >
> >      > I am happy to put this in either location and agree it
should be
> >     in the
> >      > same place as GN. If we were to decide that it should go
`utils/`
> >     then I
> >      > would also propose we move GN to there as well. I believe
the GN
> >     files
> >      > were contributed prior to the existence of the monorepo, so
a
> >     top-level
> >      > `utils/` wouldn't have been an option. I think living
under the
> root
> >      > `utils/` directory makes more sense because these are not
> >     configurations
> >      > for only the LLVM subproject (we also build MLIR and Clang
with
> >     perhaps
> >      > more to come). I believe it was Mehdi's suggestion that
this
> >     would help
> >      > mitigate some of the costs to having it in the monorepo
because
> Tom
> >      > mentioned commit list traffic as a concern. I don't
think I agree
> >     that
> >      > one directory up is akin to a separate repo though :-D
> >      >
> >      > That said, this is a really minor point for me. I'm
happy to put
> >     this
> >      > wherever people prefer :-)
> >      >
> >      >
> >      >         A number of people raised the question of "why
not a
> separate
> >      >         repository". This is indeed possible: It's
what we've
> >     done with
> >      > https://github.com/google/llvm-bazel, which is currently
used by
> >      > https://github.com/google/iree. It is significantly more
> >      >         infrastructure, coordination, and complexity for
> >     something that
> >      >         is specifically a configuration for the LLVM project
> >     itself, not
> >      >         its own dependent or adjacent project.
> >      >
> >      >
> >      >     I was also under the impression that one of the big
reasons
> >     why we
> >      >     needed it to be in LLVM is that, like CMake, it needed
files
> all
> >      >     over the place. This would indeed be a major
infrastructure
> >     undertaking.
> >      >
> >      >     But given that it's all being hosted in a single
directory,
> and
> >      >     outside of the LLVM tree, I really can't see
what's so much
> >     harder
> >      >     about an extra checkout in the same tree.
> >      >
> >      > Bazel *wants* the build files to be all over the place, but
I've
> >     tricked
> >      > it with some repository rule symlinking. That's also
true of the
> >     LLVM GN
> >      > configuration, I believe. My assumption is that having BUILD
files
> >      > actually throughout the repository would be something that
would
> >     receive
> >      > quite a bit of pushback and would be confusing for people
who
> would
> >      > naturally expect these BUILD files to be maintained as a
> >     supported build
> >      > system. I would happily put a BUILD.bazel file at the root
of each
> >      > subproject and drop the symlinking madness, but I suspect
this
> >     would not
> >      > be embraced as a solution ;-P
> >      >
> >      >
> >      >         I believe this contribution will significantly
improve the
> >      >         situation for downstream users that use Bazel while
having
> >      >         minimal impact on the community at large.
> >      >
> >      >
> >      >     It's not clear to me yet if LLVM/Bazel is only used
in Google
> >      >     projects or any other non-Google project. All that you
listed
> >     so far
> >      >     seem to be exclusive to Google.
> >      >
> >      >     This is not a problem per se, but it does promote the
idea
> that
> >      >     Google could common it up internally instead.
> >      >
> >      >     The main reasons why it would be upstream are that
it's
> either a
> >      >     product by or requirement to the project itself, or it
helps
> >     unite
> >      >     cross-industry collaboration that wouldn't be
possible
> otherwise.
> >      >
> >      >     It's clearly not the former (and why it's in the
periphery
> tier),
> >      >     but it's also not clear it's in the latter
either.
> >      >
> >      > I can really only speak for Google projects. I have also
noticed
> >     several
> >      > other Bazel build configurations in the wild, e.g. PlaidML
> >      >
> >     <
> https://github.com/plaidml/plaidml/blob/master/vendor/llvm/llvm.BUILD
> > (Intel)
> >
> >      > or this bazel_llvm
> >     <https://github.com/ChrisCummins/bazel_llvm> project
> >      > that I found after someone contributed a doc fix. I believe
in
> >     the last
> >      > thread someone from Facebook mentioned that Bazel build
files
> >     would also
> >      > be relatively easily translatable to their internal
Bazel-derived
> >     build
> >      > system, Buck. Someone from Lyft also expressed interest in
using
> >     a Bazel
> >      > build configuration if it was in-tree. But I can't
really speak
> >     to the
> >      > motivations, road maps, etc. for any of these people,
companies,
> or
> >      > projects (if you're reading, please chime in ;-P).
> >      >
> >
> >     _______________________________________________
> >     LLVM Developers mailing list
> >     llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
> >     https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201204/e156cadd/attachment-0001.html>

On Fri, Dec 4, 2020 at 8:17 PM Mehdi AMINI <joker.eph at gmail.com> wrote:
>
>
> On Fri, Dec 4, 2020 at 8:07 PM Tom Stellard <tstellar at redhat.com>
wrote:
>
>> On 12/4/20 7:19 PM, Mehdi AMINI wrote:
>> >
>> >
>> > On Fri, Dec 4, 2020 at 6:42 PM Tom Stellard via llvm-dev
>> > <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>> >
>> >     On 12/3/20 4:27 PM, Geoffrey Martin-Noble wrote:
>> >      > Apologies for the delayed response here. I was out of
the
>> "office".
>> >      >
>> >      > Thanks for taking another look :-)
>> >      >
>> >      > I want to respond first to the process question of pitch
vs RFC.
>> My
>> >      > impression was that the pitch process should be used in
the case
>> >     that an
>> >      > RFC couldn't reach consensus. I asked a few times in
the last
>> thread
>> >      >
>> >    
(https://groups.google.com/g/llvm-dev/c/u07o3QREVUg/m/uVlV3pMTBAAJ
>> and
>> >      >
>> https://groups.google.com/g/llvm-dev/c/u07o3QREVUg/m/wF5mu-dpBAAJ)
>> >      > whether I should move this to a pitch, but feel like
there
>> wasn't a
>> >      > clear response in the context of Renato's support
tiers RFC.
>> >      >
>> >      > It seems like Tom and Renato still disagree about
whether I
>> >     should move
>> >      > this to a pitch. I would appreciate some consensus on
that point
>> at
>> >      > least :-D I do see the appeal of a living document for
this sort
>> of
>> >      > thing, so definitely see the appeal there, but also it
seems like
>> >     the
>> >      > pitch process is a heavier-weight and more unusual one,
so I was
>> >      > hesitant. My inclination is to continue this as an RFC
unless we
>> are
>> >      > unable to reach consensus on the issue as outlined in
the pitch
>> >     process
>> >      > description. It does feel like this is really not quite
as big a
>> >      > decision as you seem to be suggesting. It's also an
easily
>> >     reversible
>> >      > one since there are no build dependencies and everything
is
>> >     contained.
>> >      >
>> >
>> >     I still think this should be a pitch.  The original mailing
list
>> >     discussion was controversial and that's when an RFC should
be
>> escalated
>> >     to a pitch according to: [1].
>> >
>> >
>> > You may have missed it, but in the meantime there has been another
RFC
>> > clarifying our policy though:
https://llvm.org/docs/SupportPolicy.html
>> > It seems fair to me to revisit this RFC as is in light of the
policy
>> change.
>> >
>>
>> I don't think the questions about whether or not this should be
included
>> in the project are answered by this new policy.
>
>
> I'll quote the policy:
>
> > Section: "What is covered"
> > The peripheral tier is composed of:
> > Experimental targets and options that haven’t been enable by default
yet.
> > Main repository projects that don’t get released or regularly tested.
> > Legacy tools and scripts that aren’t used in upstream validation.
> > *Alternative build systems (ex. GN, Bazel) and related
infrastructure.*
>
> The intent of the policy is to cover exactly this proposal.
>
>
>
>
>> To me the part about
>> how the bazel build files were going to be supported and what
>> responsibility the community had for maintaining them was always very
>> clear.
>>
>> > I'd actually like to request that the objections are
reiterated and
>> > positioned in terms of the policy before we escalate this.
>> >
>>
>> I don't think it's really fair to ask people to re-object to
the
>> proposal.
>
>
> Why?
> The objections were mostly answered and have been addressed in the policy.
> I don't quite get what you would put in a "pitch" while the
> informations are outdated by the policy.
> On the contrary it seems not only fair to me, but necessary.
>
>
>
>> In my opinion, one of the problems with RFCs in the past is
>> that they turn into an endurance test, because there is no process for
>> making a decision.  Either the proposer gets tired of asking and gives
>> up or the objectors get tired of objecting and give up.  We have a
>> decision process now with the pitch process, and I think we should use
it
>
>
> We have to use it when we can't do otherwise. And again, I disagree
that
> this is a case without having objection formulated in light of the policy.
>
Another spin to it: the point of working on the policy and putting it in
place was also to help make sure that such proposals aren't automatically
controversial to the point where we can't resolve them. If the policy does
not help us here, that's quite a failure IMO.


>
>
>
>
>
>>
>> >
>> >     Thank you for responding to my technical concerns, and I agree
that
>> >     working out most of those details may be better left for a
patch
>> review
>> >     discussion.  But I think at least the presence of build
information
>> for
>> >     other projects and the sub-module alternative should be
mentioned in
>> >     the
>> >     pitch.
>> >
>> >     If there were only technical or support policy issues like
these to
>> >     resolve then I don't think this would be controversial and
require a
>> >     pitch.
>> >
>> >     My main issue with this RFC, (which I tried to address at the
end
>> of my
>> >     previous mail), is the precedent this sets for what gets
included in
>> >     tree.  Essentially, we have a subset of our community that
chose to
>> >     go a
>> >     different direction from upstream, as always there are costs
and
>> >     benefits with this decision.  The question for the community
is do
>> we
>> >     want to help or encourage this in the future by removing some
of the
>> >     costs of these decisions and allowing alternative
implementations to
>> >     live in tree.
>> >
>> >     Maybe for build systems this is OK, and for other things this
is
>> not,
>> >     I don't know.  But if we are going to be setting a
precedent, to me,
>> >     the
>> >     best way to do this is through the pitch process.
>> >
>> >
>> > Why are you considering this "setting a precedent" while
there is
>> > already GN in tree?
>> >
>>
>> You are right we are not really setting a precedent here, because GN is
>> already in tree.  However, I don't think we should now just allow
any
>> build system to be added to the tree just because GN is there.  We need
>> to have some kind of process and criteria for deciding what gets added
>> and what doesn't.  I think a pitch will help accomplish this.
>>
>> I'll be honest, I don't really understand why there is so much
push back
>> on turning this into a pitch.  Is it really that much extra work?
>>
>> -Tom
>>
>> > --
>> > Mehdi
>> >
>> >
>> >
>> >
>> >     -Tom
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >     [1]
>> >
>>
https://github.com/llvm/llvm-www/blob/master/proposals/LP0001-LLVMDecisionMaking.md
>> >
>> >      > On Mon, Nov 16, 2020 at 9:41 PM Tom Stellard <
>> tstellar at redhat.com
>> >     <mailto:tstellar at redhat.com>
>> >      > <mailto:tstellar at redhat.com <mailto:tstellar at
redhat.com>>>
>> wrote:
>> >      >
>> >      >      > This should have approximately the same impact
on the
>> >     community
>> >      >     as the
>> >      >      > current GN build in `llvm/utils/gn` does
today. That is,
>> it
>> >      >     should not
>> >      >      > affect anyone who doesn't care.
>> >      >      >
>> >      >
>> >      >     I want to push back on this a little bit, because
having the
>> >     code in
>> >      >     tree does impact everyone, even people who don't
care about
>> >     it.  It
>> >      >     increases disk usage, commit traffic, checkout
times,
>> >     bugzilla / issue
>> >      >     traffic, and CI builds to name a few things.  There
are costs
>> >     to having
>> >      >     this in tree, the question (as always) is do the
benefits
>> >     outweigh the
>> >      >     costs?
>> >      >
>> >      > Yes my apologies that this was poorly phrased. I was
aiming for a
>> >     pithy
>> >      > summary and a clear statement that our goal here is not
to
>> >     significantly
>> >      > impact contributors uninterested in Bazel. My impression
is that
>> >     the GN
>> >      > build has achieved that goal. I definitely agree that
any
>> >     addition to
>> >      > the monorepo should have a clear weighing of costs vs
benefits
>> >     and that
>> >      > the costs are never actually zero. I do think the costs
here are
>> >     really
>> >      > quite low however. I am happy to address your concerns
and also
>> >     think
>> >      > that it is important to note that if additional issues
arise we
>> are
>> >      > still agreeing to be on the hook for addressing them
(e.g. if in
>> >      > practice this causes some unforseen issue with the
release) and
>> >     deleting
>> >      > this contribution if we cannot do so in a timely manner
(`rm -rf
>> >      > utils/bazel` is all it requires).
>> >      >
>> >      >     Personally, I do not think we should have
alternative build
>> >     systems in
>> >      >     tree.  However, I still think you should try to
propose this
>> >     as a pitch.
>> >      >     I would much rather this go through a fair process
and land
>> >     than for it
>> >      >     to be rejected based on a contentious thread.
>> >      >
>> >      >     Here is why I'm not convinced this should be in
tree:
>> >      >
>> >      >     To me it's not clear why having the build files
in-tree is
>> >     better than
>> >      >     having a separate repo with an llvm-project
sub-module.  The
>> >     in tree
>> >      >     bazel files will be broken from time to time, since
most
>> >     developers will
>> >      >     not be updating them, however, with the sub-module
approach
>> >     you can
>> >      >     ensure that the build will always work by pinning
the
>> llvm-bazel
>> >      >     repo to
>> >      >     a known-working commit of llvm-project.  Can you
expand on
>> the
>> >      >     pros/cons
>> >      >     of in-tree vs out-of-tree with sub-modules.
>> >      >
>> >      > Out-of-tree with a submodule is the current approach we
have with
>> >      > https://github.com/google/llvm-bazel. It's certainly
doable, but
>> >      > involves quite a bit of bookkeeping to track which
version
>> >     corresponds
>> >      > to a given version of LLVM such that someone can fetch
the
>> correct
>> >      > configuration (you'll note that the repository has
about 7k tags
>> >     at the
>> >      > moment). To make things somewhat more complicated, the
typical
>> >     way to
>> >      > fetch something for use in Bazel is with an http_archive
>> >      >
>> >     <
>> https://docs.bazel.build/versions/master/repo/http.html#http_archive
>> > which
>> >
>> >      > requires one to specify the archive digest to avoid
refetching on
>> >     each
>> >      > build. This doesn't work particularly well with tags
that change
>> >     which
>> >      > commit they point to. I'm not saying these issues
aren't
>> >     solvable, but
>> >      > they add quite a bit of complexity.
>> >      >
>> >      > The other point is that I think this makes contributing
to the
>> Bazel
>> >      > configuration quite a bit more complex because you have
to apply
>> >     patches
>> >      > across multiple repositories to also be kept in sync.
Given that
>> >     LLVM
>> >      > has a monorepo, it still seems like the logical place
for a build
>> >      > configuration of LLVM used by multiple projects.
>> >      >
>> >      >     Other concerns I have from reviewing the patch:
>> >      >
>> >      > It seems like these are mostly concerns with the
specific
>> >      > implementation. Would you be alright with saving the
specific
>> >     details
>> >      > for an eventual review on the patch if this moves
forward? I've
>> made
>> >      > brief responses below.
>> >      >
>> >      >     * It looks like there is a build configuration for
at least
>> one
>> >      >     external
>> >      >     project (zlib) and possibly another
(vulkan-headers?).  Do we
>> >     really
>> >      >     want to have build configurations for non-LLVM
projects in
>> our
>> >      >     tree?  Is
>> >      >     there any limit to the number of external projects
that can
>> >     and will be
>> >      >     added?
>> >      >
>> >      > These are dependencies of the LLVM Project and LLVM
keeps its
>> >      > dependencies pretty tightly managed AFAIU. These
configurations
>> >     are also
>> >      > pretty trivial, "here are the source files",
type things, so I
>> think
>> >      > it's even a bit generous to call them
configurations: we're just
>> >      > informing Bazel where the files are located.
>> >      >
>> >      >
>> >      >     * There are 3 files (abi-breaking.h.cmake,
config.h.cmake,
>> >      >     llvm-config.h.cmake) that have been copied from the
llvm tree
>> >     into
>> >      >     utils/bazel/, is there some way we can avoid
carrying
>> multiple
>> >      >     copies of
>> >      >     the same file in tree?
>> >      >
>> >      >
>> >      >     * Similarly, there are some files that are normally
generated
>> >     at build
>> >      >     time clang/Config/config.h, llvm/Config/config.h,
>> >      >     llvm/Config/llvm-config.h that have been copied into
>> >     utils/bazel.
>> >      >     Is it
>> >      >     really necessary
>> >      >     to have these in tree?  Especially since some of the
>> >     templates, like
>> >      >     llvm-config.h.cmake, are also in utils/bazel?
>> >      >
>> >      > The copy here is pretty much orthogonal to the actual
build
>> >      > configuration. The intent is to have a literal change
detector
>> >     test for
>> >      > changes to these cmake configurations, since they would
>> invalidate
>> >      > assumptions in the Bazel configuration. Chandler and I
went back
>> and
>> >      > forth on a few different ways to do this. We can
certainly look
>> >     at other
>> >      > options. The issue is that I don't think there's
actually a
>> >     useful way
>> >      > to interpret the .cmake template files since changes to
them are
>> >     also
>> >      > made as changes to the cmake configuration and without
these
>> >     being in
>> >      > sync the files just drift. Happy to discuss other
options for
>> how to
>> >      > handle this. We could, for instance, have some other
process that
>> >     just
>> >      > looks at the git diff/log for these files.
>> >      >
>> >      >     * I still worry about the bazel files causing
merging
>> >     conflicts when
>> >      >     backported to the stable branch.  If these are added
to tree,
>> >     could we
>> >      >     have a rule where commits to utils/bazel/ cannot
include
>> >     changes to
>> >      >     other files?
>> >      >
>> >      > I'd certainly be open to discussing restrictions
that would avoid
>> >      > additional burden on release managers. I think that one
makes
>> >      > contributing to the Bazel configuration more difficult
because
>> you
>> >      > cannot do it as part of a patch that requires a change,
but if
>> it's
>> >      > something that would cause issues with the release then
we can
>> >     avoid it.
>> >      > My intuition is that this wouldn't actually come up
often,
>> >     however. For
>> >      > example, just looking at the gn directory I see several
commits
>> >     in the
>> >      > last week that touch this and other files. Have you
actually run
>> >     into
>> >      > issues? Since this is unsupported the conflicts could
also be
>> >     resolved
>> >      > pretty much however you wanted (e.g. delete the conflict
markers,
>> >     delete
>> >      > the file), so they seem pretty trivial to deal with if
they only
>> >     happen
>> >      > occasionally. My preference would therefore be to see if
this is
>> >      > actually a problem in practice before putting rules in
place.
>> >      >
>> >      >
>> >      > On Tue, Nov 17, 2020 at 2:27 AM Renato Golin
<rengolin at gmail.com
>> >     <mailto:rengolin at gmail.com>
>> >      > <mailto:rengolin at gmail.com <mailto:rengolin at
gmail.com>>> wrote:
>> >      >
>> >      >     Hi Geoffrey,
>> >      >
>> >      >     Thanks for the re-submission.
>> >      >
>> >      >     I have some comments below that may sound negative,
but
>> they're
>> >      >     probably just a reflection of my own ignorance. I
want to
>> >     make sure
>> >      >     the submission is clear, so it can be accepted on
its own
>> right.
>> >      >
>> >      >     On Tue, 17 Nov 2020 at 03:02, Geoffrey Martin-Noble
via
>> llvm-dev
>> >      >     <llvm-dev at lists.llvm.org <mailto:llvm-dev
at lists.llvm.org>
>> >     <mailto:llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>>>
>> >     wrote:
>> >      >
>> >      >           This should not affect development of core
tier
>> components.
>> >      >         One reason we propose adding this to the root
utils/
>> >     directory
>> >      >         instead of under llvm/utils (where GN is
located) is to
>> avoid
>> >      >         unnecessarily sending messages to llvm-commits.
Others
>> have
>> >      >         raised the concern that the existence of an
alternative
>> build
>> >      >         system might lead to lack of maintenance for the
CMake
>> build
>> >      >         system. Given that supporting CMake will remain
a
>> requirement
>> >      >         and maintenance of a Bazel build system will
continue to
>> >     happen
>> >      >         regardless, we do not expect any significant
impact in
>> >     this way.
>> >      >
>> >      >
>> >      >     I was under the impression that "utils"
was actually
>> >     "llvm/utils",
>> >      >     which would be in the same place as GN. I don't
think we
>> should
>> >      >     treat GN and Bazel as different and I really
wouldn't like to
>> >     have a
>> >      >     different quality control (for post commit reviews).
>> >      >
>> >      >     If the Bazel commits are too verbose (for example,
committing
>> >      >     auto-generated code), then we should really clean
that up and
>> >     commit
>> >      >     the script that generates them and make that part of
the
>> build.
>> >      >
>> >      >     I understand the need to move the noise away, but
move it
>> too far
>> >      >     away and it's no better than in a separate repo.
>> >      >
>> >      > I am happy to put this in either location and agree it
should be
>> >     in the
>> >      > same place as GN. If we were to decide that it should go
`utils/`
>> >     then I
>> >      > would also propose we move GN to there as well. I
believe the GN
>> >     files
>> >      > were contributed prior to the existence of the monorepo,
so a
>> >     top-level
>> >      > `utils/` wouldn't have been an option. I think
living under the
>> root
>> >      > `utils/` directory makes more sense because these are
not
>> >     configurations
>> >      > for only the LLVM subproject (we also build MLIR and
Clang with
>> >     perhaps
>> >      > more to come). I believe it was Mehdi's suggestion
that this
>> >     would help
>> >      > mitigate some of the costs to having it in the monorepo
because
>> Tom
>> >      > mentioned commit list traffic as a concern. I don't
think I agree
>> >     that
>> >      > one directory up is akin to a separate repo though :-D
>> >      >
>> >      > That said, this is a really minor point for me. I'm
happy to put
>> >     this
>> >      > wherever people prefer :-)
>> >      >
>> >      >
>> >      >         A number of people raised the question of
"why not a
>> separate
>> >      >         repository". This is indeed possible:
It's what we've
>> >     done with
>> >      > https://github.com/google/llvm-bazel, which is currently
used by
>> >      > https://github.com/google/iree. It is significantly more
>> >      >         infrastructure, coordination, and complexity for
>> >     something that
>> >      >         is specifically a configuration for the LLVM
project
>> >     itself, not
>> >      >         its own dependent or adjacent project.
>> >      >
>> >      >
>> >      >     I was also under the impression that one of the big
reasons
>> >     why we
>> >      >     needed it to be in LLVM is that, like CMake, it
needed files
>> all
>> >      >     over the place. This would indeed be a major
infrastructure
>> >     undertaking.
>> >      >
>> >      >     But given that it's all being hosted in a single
directory,
>> and
>> >      >     outside of the LLVM tree, I really can't see
what's so much
>> >     harder
>> >      >     about an extra checkout in the same tree.
>> >      >
>> >      > Bazel *wants* the build files to be all over the place,
but I've
>> >     tricked
>> >      > it with some repository rule symlinking. That's also
true of the
>> >     LLVM GN
>> >      > configuration, I believe. My assumption is that having
BUILD
>> files
>> >      > actually throughout the repository would be something
that would
>> >     receive
>> >      > quite a bit of pushback and would be confusing for
people who
>> would
>> >      > naturally expect these BUILD files to be maintained as a
>> >     supported build
>> >      > system. I would happily put a BUILD.bazel file at the
root of
>> each
>> >      > subproject and drop the symlinking madness, but I
suspect this
>> >     would not
>> >      > be embraced as a solution ;-P
>> >      >
>> >      >
>> >      >         I believe this contribution will significantly
improve
>> the
>> >      >         situation for downstream users that use Bazel
while
>> having
>> >      >         minimal impact on the community at large.
>> >      >
>> >      >
>> >      >     It's not clear to me yet if LLVM/Bazel is only
used in Google
>> >      >     projects or any other non-Google project. All that
you listed
>> >     so far
>> >      >     seem to be exclusive to Google.
>> >      >
>> >      >     This is not a problem per se, but it does promote
the idea
>> that
>> >      >     Google could common it up internally instead.
>> >      >
>> >      >     The main reasons why it would be upstream are that
it's
>> either a
>> >      >     product by or requirement to the project itself, or
it helps
>> >     unite
>> >      >     cross-industry collaboration that wouldn't be
possible
>> otherwise.
>> >      >
>> >      >     It's clearly not the former (and why it's in
the periphery
>> tier),
>> >      >     but it's also not clear it's in the latter
either.
>> >      >
>> >      > I can really only speak for Google projects. I have also
noticed
>> >     several
>> >      > other Bazel build configurations in the wild, e.g.
PlaidML
>> >      >
>> >     <
>> https://github.com/plaidml/plaidml/blob/master/vendor/llvm/llvm.BUILD
>> > (Intel)
>> >
>> >      > or this bazel_llvm
>> >     <https://github.com/ChrisCummins/bazel_llvm> project
>> >      > that I found after someone contributed a doc fix. I
believe in
>> >     the last
>> >      > thread someone from Facebook mentioned that Bazel build
files
>> >     would also
>> >      > be relatively easily translatable to their internal
Bazel-derived
>> >     build
>> >      > system, Buck. Someone from Lyft also expressed interest
in using
>> >     a Bazel
>> >      > build configuration if it was in-tree. But I can't
really speak
>> >     to the
>> >      > motivations, road maps, etc. for any of these people,
companies,
>> or
>> >      > projects (if you're reading, please chime in ;-P).
>> >      >
>> >
>> >     _______________________________________________
>> >     LLVM Developers mailing list
>> >     llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>> >     https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201204/6581f2a5/attachment.html>

On 12/4/20 8:17 PM, Mehdi AMINI wrote:
> 
> 
> On Fri, Dec 4, 2020 at 8:07 PM Tom Stellard <tstellar at redhat.com 
> <mailto:tstellar at redhat.com>> wrote:
> 
>     On 12/4/20 7:19 PM, Mehdi AMINI wrote:
>      >
>      >
>      > On Fri, Dec 4, 2020 at 6:42 PM Tom Stellard via llvm-dev
>      > <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>     <mailto:llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>>>
>     wrote:
>      >
>      >     On 12/3/20 4:27 PM, Geoffrey Martin-Noble wrote:
>      >      > Apologies for the delayed response here. I was out of
the
>     "office".
>      >      >
>      >      > Thanks for taking another look :-)
>      >      >
>      >      > I want to respond first to the process question of
pitch
>     vs RFC. My
>      >      > impression was that the pitch process should be used in
>     the case
>      >     that an
>      >      > RFC couldn't reach consensus. I asked a few times
in the
>     last thread
>      >      >
>      >   
>       (https://groups.google.com/g/llvm-dev/c/u07o3QREVUg/m/uVlV3pMTBAAJ
and
>      >      >
>     https://groups.google.com/g/llvm-dev/c/u07o3QREVUg/m/wF5mu-dpBAAJ)
>      >      > whether I should move this to a pitch, but feel like
there
>     wasn't a
>      >      > clear response in the context of Renato's support
tiers RFC.
>      >      >
>      >      > It seems like Tom and Renato still disagree about
whether I
>      >     should move
>      >      > this to a pitch. I would appreciate some consensus on
that
>     point at
>      >      > least :-D I do see the appeal of a living document for
>     this sort of
>      >      > thing, so definitely see the appeal there, but also it
>     seems like
>      >     the
>      >      > pitch process is a heavier-weight and more unusual one,
so
>     I was
>      >      > hesitant. My inclination is to continue this as an RFC
>     unless we are
>      >      > unable to reach consensus on the issue as outlined in
the
>     pitch
>      >     process
>      >      > description. It does feel like this is really not quite
as
>     big a
>      >      > decision as you seem to be suggesting. It's also an
easily
>      >     reversible
>      >      > one since there are no build dependencies and
everything is
>      >     contained.
>      >      >
>      >
>      >     I still think this should be a pitch.  The original mailing
list
>      >     discussion was controversial and that's when an RFC
should be
>     escalated
>      >     to a pitch according to: [1].
>      >
>      >
>      > You may have missed it, but in the meantime there has been
>     another RFC
>      > clarifying our policy though:
>     https://llvm.org/docs/SupportPolicy.html
>      > It seems fair to me to revisit this RFC as is in light of the
>     policy change.
>      >
> 
>     I don't think the questions about whether or not this should be
>     included
>     in the project are answered by this new policy. 
> 
> 
> I'll quote the policy:
> 
>  > Section: "What is covered"
>  > The peripheral tier is composed of:
>  > Experimental targets and options that haven’t been enable by default
yet.
>  > Main repository projects that don’t get released or regularly tested.
>  > Legacy tools and scripts that aren’t used in upstream validation.
>  > *Alternative build systems (ex. GN, Bazel) and related
infrastructure.*
> 
> The intent of the policy is to cover exactly this proposal.
> 
My understanding of the policy is that these categories of things still 
need to be approved in order to be added to the tree.  Am I correct, or 
does this policy allow anyone to add an alternative build system as long 
as they can satisfy the support requirements.
>     To me the part about
>     how the bazel build files were going to be supported and what
>     responsibility the community had for maintaining them was always
>     very clear.
> 
>      > I'd actually like to request that the objections are
reiterated and
>      > positioned in terms of the policy before we escalate this.
>      >
> 
>     I don't think it's really fair to ask people to re-object to
the
>     proposal. 
> 
> 
> Why?
> The objections were mostly answered and have been addressed in the 
> policy. I don't quite get what you would put in a "pitch"
while the
> informations are outdated by the policy.
> On the contrary it seems not only fair to me, but necessary.
I don't really agree that all the objections were addressed.  Maybe we 
should directly reach out to people from the original thread and ask them?

I'm not really a fan of having another build system in tree, but I also 
don't want to keep devoting a lot of time to arguing about it.  I was 
hoping that with the pitch process, we could avoid the kind of back and 
forth arguing on the list that typically make these RFCs so tiring.

I still don't quite understand why there is so much push back against 
pitches, but I think everyone knows my perspective now, so I'm going to 
step back and let other people work out what the next steps should be.

-Tom
>     In my opinion, one of the problems with RFCs in the past is
>     that they turn into an endurance test, because there is no process for
>     making a decision.  Either the proposer gets tired of asking and gives
>     up or the objectors get tired of objecting and give up.  We have a
>     decision process now with the pitch process, and I think we should
>     use it
> 
> 
> We have to use it when we can't do otherwise. And again, I disagree
that
> this is a case without having objection formulated in light of the policy.
> 
> 
> 
> 
>      >
>      >     Thank you for responding to my technical concerns, and I
>     agree that
>      >     working out most of those details may be better left for a
>     patch review
>      >     discussion.  But I think at least the presence of build
>     information for
>      >     other projects and the sub-module alternative should be
>     mentioned in
>      >     the
>      >     pitch.
>      >
>      >     If there were only technical or support policy issues like
>     these to
>      >     resolve then I don't think this would be controversial
and
>     require a
>      >     pitch.
>      >
>      >     My main issue with this RFC, (which I tried to address at the
>     end of my
>      >     previous mail), is the precedent this sets for what gets
>     included in
>      >     tree.  Essentially, we have a subset of our community that
>     chose to
>      >     go a
>      >     different direction from upstream, as always there are costs
and
>      >     benefits with this decision.  The question for the community
>     is do we
>      >     want to help or encourage this in the future by removing some
>     of the
>      >     costs of these decisions and allowing alternative
>     implementations to
>      >     live in tree.
>      >
>      >     Maybe for build systems this is OK, and for other things this
>     is not,
>      >     I don't know.  But if we are going to be setting a
precedent,
>     to me,
>      >     the
>      >     best way to do this is through the pitch process.
>      >
>      >
>      > Why are you considering this "setting a precedent"
while there is
>      > already GN in tree?
>      >
> 
>     You are right we are not really setting a precedent here, because GN is
>     already in tree.  However, I don't think we should now just allow
any
>     build system to be added to the tree just because GN is there.  We need
>     to have some kind of process and criteria for deciding what gets added
>     and what doesn't.  I think a pitch will help accomplish this.
> 
>     I'll be honest, I don't really understand why there is so much
push
>     back
>     on turning this into a pitch.  Is it really that much extra work?
> 
>     -Tom
> 
>      > --
>      > Mehdi
>      >
>      >
>      >
>      >
>      >     -Tom
>      >
>      >
>      >
>      >
>      >
>      >
>      >
>      >
>      >
>      >
>      >
>      >
>      >
>      >     [1]
>      >
>    
https://github.com/llvm/llvm-www/blob/master/proposals/LP0001-LLVMDecisionMaking.md
>      >
>      >      > On Mon, Nov 16, 2020 at 9:41 PM Tom Stellard
>     <tstellar at redhat.com <mailto:tstellar at redhat.com>
>      >     <mailto:tstellar at redhat.com <mailto:tstellar at
redhat.com>>
>      >      > <mailto:tstellar at redhat.com <mailto:tstellar
at redhat.com>
>     <mailto:tstellar at redhat.com <mailto:tstellar at
redhat.com>>>> wrote:
>      >      >
>      >      >      > This should have approximately the same
impact on the
>      >     community
>      >      >     as the
>      >      >      > current GN build in `llvm/utils/gn` does
today.
>     That is, it
>      >      >     should not
>      >      >      > affect anyone who doesn't care.
>      >      >      >
>      >      >
>      >      >     I want to push back on this a little bit, because
>     having the
>      >     code in
>      >      >     tree does impact everyone, even people who
don't care
>     about
>      >     it.  It
>      >      >     increases disk usage, commit traffic, checkout
times,
>      >     bugzilla / issue
>      >      >     traffic, and CI builds to name a few things.  There
>     are costs
>      >     to having
>      >      >     this in tree, the question (as always) is do the
benefits
>      >     outweigh the
>      >      >     costs?
>      >      >
>      >      > Yes my apologies that this was poorly phrased. I was
>     aiming for a
>      >     pithy
>      >      > summary and a clear statement that our goal here is not
to
>      >     significantly
>      >      > impact contributors uninterested in Bazel. My
impression
>     is that
>      >     the GN
>      >      > build has achieved that goal. I definitely agree that
any
>      >     addition to
>      >      > the monorepo should have a clear weighing of costs vs
benefits
>      >     and that
>      >      > the costs are never actually zero. I do think the costs
>     here are
>      >     really
>      >      > quite low however. I am happy to address your concerns
and
>     also
>      >     think
>      >      > that it is important to note that if additional issues
>     arise we are
>      >      > still agreeing to be on the hook for addressing them
(e.g.
>     if in
>      >      > practice this causes some unforseen issue with the
>     release) and
>      >     deleting
>      >      > this contribution if we cannot do so in a timely manner
>     (`rm -rf
>      >      > utils/bazel` is all it requires).
>      >      >
>      >      >     Personally, I do not think we should have
alternative
>     build
>      >     systems in
>      >      >     tree.  However, I still think you should try to
>     propose this
>      >     as a pitch.
>      >      >     I would much rather this go through a fair process
and
>     land
>      >     than for it
>      >      >     to be rejected based on a contentious thread.
>      >      >
>      >      >     Here is why I'm not convinced this should be in
tree:
>      >      >
>      >      >     To me it's not clear why having the build files
in-tree is
>      >     better than
>      >      >     having a separate repo with an llvm-project
>     sub-module.  The
>      >     in tree
>      >      >     bazel files will be broken from time to time, since
most
>      >     developers will
>      >      >     not be updating them, however, with the sub-module
>     approach
>      >     you can
>      >      >     ensure that the build will always work by pinning
the
>     llvm-bazel
>      >      >     repo to
>      >      >     a known-working commit of llvm-project.  Can you
>     expand on the
>      >      >     pros/cons
>      >      >     of in-tree vs out-of-tree with sub-modules.
>      >      >
>      >      > Out-of-tree with a submodule is the current approach we
>     have with
>      >      > https://github.com/google/llvm-bazel. It's
certainly
>     doable, but
>      >      > involves quite a bit of bookkeeping to track which
version
>      >     corresponds
>      >      > to a given version of LLVM such that someone can fetch
the
>     correct
>      >      > configuration (you'll note that the repository has
about
>     7k tags
>      >     at the
>      >      > moment). To make things somewhat more complicated, the
typical
>      >     way to
>      >      > fetch something for use in Bazel is with an
http_archive
>      >      >
>      >   
>     
 <https://docs.bazel.build/versions/master/repo/http.html#http_archive> which
>      >
>      >      > requires one to specify the archive digest to avoid
>     refetching on
>      >     each
>      >      > build. This doesn't work particularly well with
tags that
>     change
>      >     which
>      >      > commit they point to. I'm not saying these issues
aren't
>      >     solvable, but
>      >      > they add quite a bit of complexity.
>      >      >
>      >      > The other point is that I think this makes contributing
to
>     the Bazel
>      >      > configuration quite a bit more complex because you have
to
>     apply
>      >     patches
>      >      > across multiple repositories to also be kept in sync.
>     Given that
>      >     LLVM
>      >      > has a monorepo, it still seems like the logical place
for
>     a build
>      >      > configuration of LLVM used by multiple projects.
>      >      >
>      >      >     Other concerns I have from reviewing the patch:
>      >      >
>      >      > It seems like these are mostly concerns with the
specific
>      >      > implementation. Would you be alright with saving the
specific
>      >     details
>      >      > for an eventual review on the patch if this moves
forward?
>     I've made
>      >      > brief responses below.
>      >      >
>      >      >     * It looks like there is a build configuration for
at
>     least one
>      >      >     external
>      >      >     project (zlib) and possibly another
>     (vulkan-headers?).  Do we
>      >     really
>      >      >     want to have build configurations for non-LLVM
>     projects in our
>      >      >     tree?  Is
>      >      >     there any limit to the number of external projects
>     that can
>      >     and will be
>      >      >     added?
>      >      >
>      >      > These are dependencies of the LLVM Project and LLVM
keeps its
>      >      > dependencies pretty tightly managed AFAIU. These
>     configurations
>      >     are also
>      >      > pretty trivial, "here are the source files",
type things,
>     so I think
>      >      > it's even a bit generous to call them
configurations:
>     we're just
>      >      > informing Bazel where the files are located.
>      >      >
>      >      >
>      >      >     * There are 3 files (abi-breaking.h.cmake,
config.h.cmake,
>      >      >     llvm-config.h.cmake) that have been copied from the
>     llvm tree
>      >     into
>      >      >     utils/bazel/, is there some way we can avoid
carrying
>     multiple
>      >      >     copies of
>      >      >     the same file in tree?
>      >      >
>      >      >
>      >      >     * Similarly, there are some files that are normally
>     generated
>      >     at build
>      >      >     time clang/Config/config.h, llvm/Config/config.h,
>      >      >     llvm/Config/llvm-config.h that have been copied
into
>      >     utils/bazel.
>      >      >     Is it
>      >      >     really necessary
>      >      >     to have these in tree?  Especially since some of
the
>      >     templates, like
>      >      >     llvm-config.h.cmake, are also in utils/bazel?
>      >      >
>      >      > The copy here is pretty much orthogonal to the actual
build
>      >      > configuration. The intent is to have a literal change
detector
>      >     test for
>      >      > changes to these cmake configurations, since they would
>     invalidate
>      >      > assumptions in the Bazel configuration. Chandler and I
>     went back and
>      >      > forth on a few different ways to do this. We can
certainly
>     look
>      >     at other
>      >      > options. The issue is that I don't think
there's actually a
>      >     useful way
>      >      > to interpret the .cmake template files since changes to
>     them are
>      >     also
>      >      > made as changes to the cmake configuration and without
these
>      >     being in
>      >      > sync the files just drift. Happy to discuss other
options
>     for how to
>      >      > handle this. We could, for instance, have some other
>     process that
>      >     just
>      >      > looks at the git diff/log for these files.
>      >      >
>      >      >     * I still worry about the bazel files causing
merging
>      >     conflicts when
>      >      >     backported to the stable branch.  If these are
added
>     to tree,
>      >     could we
>      >      >     have a rule where commits to utils/bazel/ cannot
include
>      >     changes to
>      >      >     other files?
>      >      >
>      >      > I'd certainly be open to discussing restrictions
that
>     would avoid
>      >      > additional burden on release managers. I think that one
makes
>      >      > contributing to the Bazel configuration more difficult
>     because you
>      >      > cannot do it as part of a patch that requires a change,
>     but if it's
>      >      > something that would cause issues with the release then
we can
>      >     avoid it.
>      >      > My intuition is that this wouldn't actually come up
often,
>      >     however. For
>      >      > example, just looking at the gn directory I see several
>     commits
>      >     in the
>      >      > last week that touch this and other files. Have you
>     actually run
>      >     into
>      >      > issues? Since this is unsupported the conflicts could
also be
>      >     resolved
>      >      > pretty much however you wanted (e.g. delete the
conflict
>     markers,
>      >     delete
>      >      > the file), so they seem pretty trivial to deal with if
>     they only
>      >     happen
>      >      > occasionally. My preference would therefore be to see
if
>     this is
>      >      > actually a problem in practice before putting rules in
place.
>      >      >
>      >      >
>      >      > On Tue, Nov 17, 2020 at 2:27 AM Renato Golin
>     <rengolin at gmail.com <mailto:rengolin at gmail.com>
>      >     <mailto:rengolin at gmail.com <mailto:rengolin at
gmail.com>>
>      >      > <mailto:rengolin at gmail.com <mailto:rengolin at
gmail.com>
>     <mailto:rengolin at gmail.com <mailto:rengolin at
gmail.com>>>> wrote:
>      >      >
>      >      >     Hi Geoffrey,
>      >      >
>      >      >     Thanks for the re-submission.
>      >      >
>      >      >     I have some comments below that may sound negative,
>     but they're
>      >      >     probably just a reflection of my own ignorance. I
want to
>      >     make sure
>      >      >     the submission is clear, so it can be accepted on
its
>     own right.
>      >      >
>      >      >     On Tue, 17 Nov 2020 at 03:02, Geoffrey Martin-Noble
>     via llvm-dev
>      >      >     <llvm-dev at lists.llvm.org
>     <mailto:llvm-dev at lists.llvm.org> <mailto:llvm-dev at
lists.llvm.org
>     <mailto:llvm-dev at lists.llvm.org>>
>      >     <mailto:llvm-dev at lists.llvm.org
>     <mailto:llvm-dev at lists.llvm.org> <mailto:llvm-dev at
lists.llvm.org
>     <mailto:llvm-dev at lists.llvm.org>>>>
>      >     wrote:
>      >      >
>      >      >           This should not affect development of core
tier
>     components.
>      >      >         One reason we propose adding this to the root
utils/
>      >     directory
>      >      >         instead of under llvm/utils (where GN is
located)
>     is to avoid
>      >      >         unnecessarily sending messages to llvm-commits.
>     Others have
>      >      >         raised the concern that the existence of an
>     alternative build
>      >      >         system might lead to lack of maintenance for
the
>     CMake build
>      >      >         system. Given that supporting CMake will remain
a
>     requirement
>      >      >         and maintenance of a Bazel build system will
>     continue to
>      >     happen
>      >      >         regardless, we do not expect any significant
impact in
>      >     this way.
>      >      >
>      >      >
>      >      >     I was under the impression that "utils"
was actually
>      >     "llvm/utils",
>      >      >     which would be in the same place as GN. I don't
think
>     we should
>      >      >     treat GN and Bazel as different and I really
wouldn't
>     like to
>      >     have a
>      >      >     different quality control (for post commit
reviews).
>      >      >
>      >      >     If the Bazel commits are too verbose (for example,
>     committing
>      >      >     auto-generated code), then we should really clean
that
>     up and
>      >     commit
>      >      >     the script that generates them and make that part
of
>     the build.
>      >      >
>      >      >     I understand the need to move the noise away, but
move
>     it too far
>      >      >     away and it's no better than in a separate
repo.
>      >      >
>      >      > I am happy to put this in either location and agree it
>     should be
>      >     in the
>      >      > same place as GN. If we were to decide that it should
go
>     `utils/`
>      >     then I
>      >      > would also propose we move GN to there as well. I
believe
>     the GN
>      >     files
>      >      > were contributed prior to the existence of the
monorepo, so a
>      >     top-level
>      >      > `utils/` wouldn't have been an option. I think
living
>     under the root
>      >      > `utils/` directory makes more sense because these are
not
>      >     configurations
>      >      > for only the LLVM subproject (we also build MLIR and
Clang
>     with
>      >     perhaps
>      >      > more to come). I believe it was Mehdi's suggestion
that this
>      >     would help
>      >      > mitigate some of the costs to having it in the monorepo
>     because Tom
>      >      > mentioned commit list traffic as a concern. I don't
think
>     I agree
>      >     that
>      >      > one directory up is akin to a separate repo though :-D
>      >      >
>      >      > That said, this is a really minor point for me. I'm
happy
>     to put
>      >     this
>      >      > wherever people prefer :-)
>      >      >
>      >      >
>      >      >         A number of people raised the question of
"why not
>     a separate
>      >      >         repository". This is indeed possible:
It's what we've
>      >     done with
>      >      > https://github.com/google/llvm-bazel, which is
currently
>     used by
>      >      > https://github.com/google/iree. It is significantly
more
>      >      >         infrastructure, coordination, and complexity
for
>      >     something that
>      >      >         is specifically a configuration for the LLVM
project
>      >     itself, not
>      >      >         its own dependent or adjacent project.
>      >      >
>      >      >
>      >      >     I was also under the impression that one of the big
>     reasons
>      >     why we
>      >      >     needed it to be in LLVM is that, like CMake, it
needed
>     files all
>      >      >     over the place. This would indeed be a major
>     infrastructure
>      >     undertaking.
>      >      >
>      >      >     But given that it's all being hosted in a
single
>     directory, and
>      >      >     outside of the LLVM tree, I really can't see
what's so
>     much
>      >     harder
>      >      >     about an extra checkout in the same tree.
>      >      >
>      >      > Bazel *wants* the build files to be all over the place,
>     but I've
>      >     tricked
>      >      > it with some repository rule symlinking. That's
also true
>     of the
>      >     LLVM GN
>      >      > configuration, I believe. My assumption is that having
>     BUILD files
>      >      > actually throughout the repository would be something
that
>     would
>      >     receive
>      >      > quite a bit of pushback and would be confusing for
people
>     who would
>      >      > naturally expect these BUILD files to be maintained as
a
>      >     supported build
>      >      > system. I would happily put a BUILD.bazel file at the
root
>     of each
>      >      > subproject and drop the symlinking madness, but I
suspect this
>      >     would not
>      >      > be embraced as a solution ;-P
>      >      >
>      >      >
>      >      >         I believe this contribution will significantly
>     improve the
>      >      >         situation for downstream users that use Bazel
>     while having
>      >      >         minimal impact on the community at large.
>      >      >
>      >      >
>      >      >     It's not clear to me yet if LLVM/Bazel is only
used in
>     Google
>      >      >     projects or any other non-Google project. All that
you
>     listed
>      >     so far
>      >      >     seem to be exclusive to Google.
>      >      >
>      >      >     This is not a problem per se, but it does
promote the
>     idea that
>      >      >     Google could common it up internally instead.
>      >      >
>      >      >     The main reasons why it would be upstream are that
>     it's either a
>      >      >     product by or requirement to the project itself, or
it
>     helps
>      >     unite
>      >      >     cross-industry collaboration that wouldn't be
possible
>     otherwise.
>      >      >
>      >      >     It's clearly not the former (and why it's
in the
>     periphery tier),
>      >      >     but it's also not clear it's in the latter
either.
>      >      >
>      >      > I can really only speak for Google projects. I have
also
>     noticed
>      >     several
>      >      > other Bazel build configurations in the wild, e.g.
PlaidML
>      >      >
>      >   
>     
 <https://github.com/plaidml/plaidml/blob/master/vendor/llvm/llvm.BUILD> (Intel)
>      >
>      >      > or this bazel_llvm
>      >     <https://github.com/ChrisCummins/bazel_llvm> project
>      >      > that I found after someone contributed a doc fix. I
believe in
>      >     the last
>      >      > thread someone from Facebook mentioned that Bazel build
files
>      >     would also
>      >      > be relatively easily translatable to their internal
>     Bazel-derived
>      >     build
>      >      > system, Buck. Someone from Lyft also expressed interest
in
>     using
>      >     a Bazel
>      >      > build configuration if it was in-tree. But I can't
really
>     speak
>      >     to the
>      >      > motivations, road maps, etc. for any of these people,
>     companies, or
>      >      > projects (if you're reading, please chime in ;-P).
>      >      >
>      >
>      >     _______________________________________________
>      >     LLVM Developers mailing list
>      > llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>     <mailto:llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>>
>      > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>      >
>