[PROPOSAL] Add Bazel Build Configuration to the LLVM Monorepo

Hello LLVM community,

The review of “Add Bazel Build Configuration to the LLVM Monorepo” begins now and runs through 2021-02-23 (two weeks). The proposal is available online.

Feedback is an important part of the LLVM Proposal process. All review feedback should be either on this thread or, if you would like to keep your feedback private, directly to one of the review managers.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of LLVM. When writing your response, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal? What positive or negative implications would accepting this have?
  • Do you have experience from other communities that relates to this issue and is important to consider?
  • How involved have you been in the LLVM project? Frequent contributor, occasional contributor, user of LLVM libraries, user of LLVM-based tools, or other?
  • Self Evaluation: How much effort did you put into your review and how knowledgeable are you about this area? For example, a quick reading or an in-depth study?

In addition to your opinion and thoughts, please include any additional framing that may be useful.

Thank you,

Geoffrey

Review Manager

Hi Geoffrey!

The review of "Add Bazel Build Configuration to the LLVM Monorepo" begins
now and runs through 2021-02-23 (two weeks). The proposal is available
online
<https://github.com/llvm/llvm-www/blob/main/proposals/LP0002-BazelBuildConfiguration.md>

Bazel currently does not build on any 32-bit architecture and has issues even
on some 64-bit architectures like MIPS or RISC-V:

https://buildd.debian.org/status/package.php?p=bazel-bootstrap&suite=sid

Is there any chance this can get fixed before accepting Bazel support into LLVM?

Adrian

Why would this be a limiting factor? It just wouldn’t work on those platforms?

-eric

Hi Geoffrey!

The review of “Add Bazel Build Configuration to the LLVM Monorepo” begins
now and runs through 2021-02-23 (two weeks). The proposal is available
online
<https://github.com/llvm/llvm-www/blob/main/proposals/LP0002-BazelBuildConfiguration.md>

Bazel currently does not build on any 32-bit architecture and has issues even
on some 64-bit architectures like MIPS or RISC-V:

https://buildd.debian.org/status/package.php?p=bazel-bootstrap&suite=sid

Is there any chance this can get fixed before accepting Bazel support into LLVM?

Why would this be a limiting factor? It just wouldn’t work on those platforms?

To expand a bit on Eric’s response, the intent here is not to make Bazel a supported build system for LLVM or to replace CMake (which I believe the proposal makes clear), but rather to enable Bazel usage and shared configuration for people and projects that already use it. I do not expect that Bazel will cover all the use cases currently supported by LLVM CMake any time soon (ever?).I don’t work on Bazel itself, so have no insight on the support plan for those architectures. Only developers interested in working with Bazel would be expected to use or update the configuration, so lack of support for specific architectures should not affect things, I think.

Because the portability of the build system shouldn't be limiting the portability
of the project I'm trying to build.

Out of curiosity: Is there any key factor where Bazel beats other build systems?

Adrian

Is there any chance this can get fixed before accepting Bazel support into
LLVM?

Why would this be a limiting factor? It just wouldn’t work on those
platforms?

Because the portability of the build system shouldn’t be limiting the portability
of the project I’m trying to build.

I thought this might be the case :slight_smile: I’ll point to Geoffrey’s much more detailed response then.

Out of curiosity: Is there any key factor where Bazel beats other build systems?

Dependency analysis is quite strong in Bazel, but in this case it’s largely for the other projects that use it as a dependency and not to replace anything :slight_smile:

Thanks!

-eric

To expand a bit on Eric’s response, the intent here is not to make Bazel a supported build system for LLVM or to replace CMake (which I believe the proposal makes clear), but rather to enable Bazel usage and shared configuration for people and projects that already use it. I do not expect that Bazel will cover all the use cases currently supported by LLVM CMake any time soon (ever?).I don’t work on Bazel itself, so have no insight on the support plan for those architectures. Only developers interested in working with Bazel would be expected to use or update the configuration, so lack of support for specific architectures should not affect things, I think.

My views exactly. Bazel will not be a “supported” build system and doesn’t need to build on all platforms and environments LLVM builds. It should only concern people that actually use Bazel and be completely transparent to the rest who don’t.

A reminder that the review period for this ends 2021-02-23, this coming Tuesday. Rest assured that if you expressed opinions in the previous RFC threads then review managers will also consider those points when discussing. We’re not going to skip some point just because it wasn’t posted in the correct thread :smiley:

Best,
Geoffrey

Hello LLVM-Dev,

Last week the review managers met to discuss this proposal. I’ve updated the proposal document with a summary of the meeting. You can find the proposal online here.

The TL;DR is that the review managers agreed the proposal should be approved.

Thank you everyone who participated in the conversations around this proposal, and especially Geoffrey for putting the proposal together and shepherding it along.

-Chris

Thanks for the update Chris - could you summarize what this means for the proposal/what stage in the proposal process this is? Does this represent approval, and the patch should now be submitted without further high level design review (that is covered by the proposal review)? Or are there further steps?

(does the approval indicate where these files should live? Next to the gn files? A new top level location? or is that still up to further community review)

This is an approval of the proposal (patch to actually indicate that in the status field). The next step is landing the Bazel build files, which will be subject to the normal patch review process. Chris added notes from our discussion about the issues discussed, which includes the location of the build files. We agreed these should be in the root utils/ directory and we also think the gn build should move there (it’s current location predates the monorepo). I was going to start a separate thread, but I’ll just +Nico Weber. Nico can you take a look at moving the gn files? Hopefully this should be pretty trivial?

Congratulations Geoffrey and everyone else involved on getting this done! It was a long journey, but we set a lot of important legal precedent so hopefully similar issues go more smoothly in the future.

Thanks for the summary & other work with this proposal, Geoffrey!

Hi David,

This is the first time we’ve really gone through the full proposal process, so this is a bit new for everyone. The decision making process was documented in the first proposal here, and the relevant bits are steps 8 & 9:

  1. When the discussion concludes, Chris and the review managers have a video chat to review the outcome of the discussion. The goal of this private discussion is to achieve consensus on an outcome between the review managers and Chris, but if that isn’t possible, then Chris will tie break. The outcome may be Approve, Deny, Approve with Changes, or to kick it back to the pitch phase for more discussion.

  2. A review manager writes up a summary of the outcome and shares that with the community on the llvm-dev. The outcome is added to the proposal in github to build a history of proposals and their outcomes.

With that completed, the intent is that the decision of the review managers is the final decision.

I was probably overly brief in my on-list summary of the review managers decision. It is all captured now in the proposal, but the summary is:

The review managers met to discuss the technical and community aspects of the proposal. It was the consensus of the review managers in attendance that the proposal, if approved, would impose little to no burden on the community, and provided material benefit to contributors and downstream users.

The committee also agreed that the Bazel build files should be included under the top-level github.com/llvm/llvm-project/utils directory as they relate to multiple subprojects in LLVM.

In the discussion notes we did discuss source location as it relates to gn as well:

In the interest of this proposal resulting in the correct decision, we agreed the Bazel files should live under the utils directory at the root of the llvm-project repository, and we think GN should move there as well.

Hope this answers all your questions.

-Chris

Yes, moving gn shouldn’t be a big problem. It needs some minor bot and docs wrangling. Let me know once this is in and I’ll work on moving the GN files. Should be doable in a week or two.

Thanks NIco :slight_smile: I think it shouldn’t be necessary for this to happen in any particular order, so you can start moving gn now, if you’d like (but also not a huge rush).

Thanks Nico!

To expand a bit on Eric's response, the intent here is *not* to make Bazel
a supported build system for LLVM or to replace CMake (which I believe the
proposal makes clear), but rather to enable Bazel usage and shared
configuration for people and projects that already use it. I do not expect
that Bazel will cover all the use cases currently supported by LLVM CMake
any time soon (ever?).I don't work on Bazel itself, so have no insight on
the support plan for those architectures. Only developers interested in
working with Bazel would be expected to use or update the configuration, so
lack of support for specific architectures should not affect things, I
think.

Looking at the amount of copy-and-paste code in Bazel [1], I'm not really convinced
that the code quality of Bazel speaks for itself.

Also, my personal experience with sending patches to Google projects so far has been
rather underwhelming. Usually, Google projects did not accept any patches for use
cases that are not of Google's own interested such as better support for big-endian
targets [2]. On the other hand, Google engineers expect upstream projects to add
support for technology Google uses internally.

I wish it would be more balanced and Google would allow patches in Chromium or V8
to support more architectures if - on the other hand - they ask other upstream
projects to carry support for their usecases.

Adrian

(full disclosure, I am a Google employee)

I don’t think this is appropriate content, communication, or tone for the LLVM community.

To expand a bit on Eric’s response, the intent here is not to make Bazel
a supported build system for LLVM or to replace CMake (which I believe the
proposal makes clear), but rather to enable Bazel usage and shared
configuration for people and projects that already use it. I do not expect
that Bazel will cover all the use cases currently supported by LLVM CMake
any time soon (ever?).I don’t work on Bazel itself, so have no insight on
the support plan for those architectures. Only developers interested in
working with Bazel would be expected to use or update the configuration, so
lack of support for specific architectures should not affect things, I
think.

Looking at the amount of copy-and-paste code in Bazel [1], I’m not really convinced
that the code quality of Bazel speaks for itself.

This patch doesn’t seem to me to be reflective of “good” or “bad” code, nor has anyone made any claim about the code quality of Bazel. It isn’t relevant to this discussion.

Also, my personal experience with sending patches to Google projects so far has been
rather underwhelming. Usually, Google projects did not accept any patches for use
cases that are not of Google’s own interested such as better support for big-endian
targets [2]. On the other hand, Google engineers expect upstream projects to add
support for technology Google uses internally.

I wish it would be more balanced and Google would allow patches in Chromium or V8
to support more architectures if - on the other hand - they ask other upstream
projects to carry support for their usecases.

[1] https://github.com/bazelbuild/bazel/commit/1c29dff364fd34d7e6158c812cfd6d96f66be747

[2] https://catfox.life/2021/03/21/really-leaving-the-linux-desktop-behind/

These seem like unhelpful ad-hominem criticisms that aren’t relevant to the matter being discussed. This proposal has been specifically designed to be minimally impactful to the community (should only be “there are some more commits to the project/more commit list emails” - and if gn is anything to go by, not many (<0.1% I’d wager, at a rough guess)).

  • Dave

(full disclosure, I am NOT a Google employee nor I particularly like Bazel)