Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bazel] MVP for building with bazel #6994

Merged
merged 38 commits into from
Oct 19, 2024

Conversation

pjreiniger
Copy link
Contributor

@pjreiniger pjreiniger commented Aug 23, 2024

This PR introduces the ability to build parts of wpilib with bazel

General principles for the MVP:

  • Build C++ statically only
  • Do not build JNI code
  • Do not run Java tests (because there is no JNI)

Libraries Built:

  • wpiutil / wpinet / ntcore / hal - Java, C++ w/ tests
  • wpimath / cscore / cameraserver / wplibj - Java only

I'm still fighting with a landable solution for the recent protobuf change which is blocking wpimath. Once I get that going we can build up to wpilibc.

Other cool things I left out

  • You can run the pregeneration and check that produces the correct results during a build. Because of the cache it will only get run "once", but this can remove the need for a separate CI step.
  • PMD and checkstyle for Java
  • Building the examples, which can be easily run with bazel run //wpilibjExamples:rapidreact. I use the pregen to generate a build script so I left it out.
  • Building for raspbian / bullseye.

Implementation Notes
For my fork I made the decision to get close to the "one bazel file per directory" structure. This results in a 2-6x number of BUILD.bazel files, but they are more compact and easily understandable. i.e.

wpiutil/src/examples/....
wpiutil/src/dev/java/BUILD.bazel
wpiutil/src/dev/native/BUILD.bazel
wpiutil/src/main/java/BUILD.bazel
wpiutil/src/main/native/BUILD.bazel
wpiutil/src/test/java/BUILD.bazel
wpiutil/src/test/native/BUILD.bazel

This could get all smushed to a single file like what happens with Gradle or Cmake

wpiutil/BUILD.bazel

I also chose to only implement the "legacy" version of dependencies. Bazel has introduced a new scheme that is supposed to mimic the simplicity of maven / pypi called bzlmod. It is easier to pull non-bazel-ified things with the WORKSPACE version instead of the MODULE version if needed

@pjreiniger pjreiniger requested review from PeterJohnson and a team as code owners August 23, 2024 06:32
@pjreiniger
Copy link
Contributor Author

This is a bit heavier than an MVP, the first commit is landable and I can make a smaller PR with that if wanted.

shared/bazel/compiler_flags/windows_arm_flags.rc Outdated Show resolved Hide resolved
fail-fast: false
matrix:
include:
- { name: "Linux (native)", os: ubuntu-22.04, action: "test", config: "--noenable_bzlmod --config=linux -c opt", }
Copy link
Member

@auscompgeek auscompgeek Aug 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of requiring specifying --config=<host_platform> to target the host, let's use --enable_platform_specific_config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that once but didn't figure out how to make it work with cross compilation. i.e. if you are trying to cross compile for roborio on windows, it will add both the windows flags and the roborio flags and not work.

I just pop the config in my user.bazelrc and switch if I want to build athena stuff. In a world where this gets filtered down to teams, the vscode plugin could autodetect which config to run based on context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, hmm. I guess the alternative is to wrap the cc_library rule with one that selects the compiler flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That gets messy if you have third party libraries because you'd have to patch their things to use the same macro you are using for your code if you want everything to be built with the same compiler flags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want, we have all the tool chains sandboxed for Linux with Bazel.

.bazelrc Show resolved Hide resolved
@pjreiniger
Copy link
Contributor Author

If interested in a single-file-per-project version, you can take a look at this. Reduces the file count from 76 to 27, although the 9 projects build files become quite a bit larger

@Gold856
Copy link
Contributor

Gold856 commented Aug 25, 2024

I like the single file per project version. I think that keeping everything together is better than having several short build files scattered throughout the repo, since it makes it easier to track down the references to dependencies.

wpiutil/src/main/java/edu/wpi/first/util/BUILD.bazel Outdated Show resolved Hide resolved
.bazelrc Outdated Show resolved Hide resolved
@pjreiniger
Copy link
Contributor Author

I like the single file per project version. I think that keeping everything together is better than having several short build files scattered throughout the repo, since it makes it easier to track down the references to dependencies.

I think I agree. It more closely follows the ways other two build systems work, which might make it easier to get used to. I don't think I realized how much it ballooned after I started making sure every file had a bazel target attached to it.

I updated the PR to be the amalgamated version.

.github/workflows/bazel.yml Outdated Show resolved Hide resolved
.github/workflows/bazel.yml Show resolved Hide resolved
@spacey-sooty

This comment was marked as outdated.

@PeterJohnson
Copy link
Member

PeterJohnson commented Aug 25, 2024

This PR is just a MVP to demonstrate it and discuss architecture and layout. PJ does have a complete build working. The end state would likely be replacement of Gradle with Bazel, not maintaining 3 build systems indefinitely.

WORKSPACE Outdated
http_archive(
name = "rules_bzlmodrio_toolchains",
sha256 = "cd3ff046427e9c6dbc0c86a458c8cf081b8045fc3fb4265d08c0ebfc17f9cb30",
url = "https://github.com/bzlmodRio/rules_bzlmodRio_toolchains/releases/download/2024-1/rules_bzlmodRio_toolchains-2024-1.tar.gz",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does WPILib want to manage the third-party dependencies being imported here to support toolchains and bazel stuff?

Also, looking at this repo I don't see any license?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the bzlmodRio stuff is mine. I can add whatever license is desirable.

Copy link
Member

@PeterJohnson PeterJohnson Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it that the bzlmodRio stuff (and pulling it in via http_archive) is akin to our current Gradle plugins? We'd want to move/fork the relevant repos into wpilibsuite for long term use. I know this PR is focused on allwpilib, where we can pretty much assume anyone building it is online, but how does this work in the team build case with no internet? Can the http_archive stuff be cached offline / distributed with the installer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting stuff distributed with the installer would be a bit unusual for a bazel project, but should definitely be feasible. Normally the http_archive things being downloaded get saved into a cache on the user's machine in a reasonably standard spot based on the sha256 of the http_archive. I imagine there are a couple of different ways that you can manage offline distribution, with varying levels of hackiness.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there would be two options.:

  • As @jkuszmaul you could place the files in the magic directory that bazel knows about
  • http_archive supports multiple mirrors, and I just tested that you could have a mirror that is a file://. So that could point to the magic directory offline installers put stuff

Copy link
Contributor

@AustinSchuh AustinSchuh Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bazelbuild/bazel#18934 is the official upstream ticket and the corresponding docs for how they expect it to work.

@AustinSchuh
Copy link
Contributor

We are starting to find dependencies we need which are bzlmod only, and it is clear that Bazel and the community is going that way. We are starting to ponder our own migration. I'd highly encourage you to start with bzlmod, especially for a new project like this. The cleanest way to do this long term would be to publish wpilib as a bzlmod module to BCR and either have it depend on toolchains also published to bcr, or provide the MODULE.bazel lines to add the toolchains. That would make the lines required by a new team to get started be quite small.

Also, fair warning, for something like wpilib, including flags (like C++ version, etc) in the .bazelrc files is an anti-pattern. It makes it so that it is very hard to depend on wpilib. That makes it so you have to both depend on the targets in wpilib, but also figure out how to include the flags across the repository boundary, or duplicate them and keep them up to date. The better way to do this is to move the flags into the toolchain.

Thanks for pushing on this! We've wanted upstream bazel support in wpilib for a long time, really exciting to see some momentum. We are very much a linux shop on 971, but happy to review for sure.

If there is interest, I have some contacts in the remote execution world and might be able to get access to resources for CI.

@pjreiniger
Copy link
Contributor Author

pjreiniger commented Aug 26, 2024

We are starting to find dependencies we need which are bzlmod only, and it is clear that Bazel and the community is going that way.

My fork supports bzlmod as do all of the FIRST related dependencies in the bzlmodRio org, published to my fork of the BCR. I thought going the WORKSPACE route would be the best candidate for a minimum viable product as it still has the best documentation, and it is easier to patch in third party libraries that have zero bazel support (usually just with a http_archive and build_file_contents). Another reason to that way with this MVP is that I've had trouble getting the roborio toolchain to cross compile on windows with bzlmod; it works on my machine but not on github CI.

Also, fair warning, for something like wpilib, including flags (like C++ version, etc) in the .bazelrc files is an anti-pattern

I agree. I didn't want to muck around with the auto-discovering native toolchains. I could add the native-tools defaults for the cross compilers, but it felt like a big lift to make custom toolchains for native linux/osx/windows.

I started this as a hobby when I joined a company that used bazel and was super frustrated at how long gradle builds took on my old laptop. I'm sure there is a lot of things that I'm doing stupidly and that you and 971 can help do the right way. If you want, feel free to reach out to me on CD to discuss, one bazel stan to another.

@AustinSchuh
Copy link
Contributor

We are starting to find dependencies we need which are bzlmod only, and it is clear that Bazel and the community is going that way.

My fork supports bzlmod as do all of the FIRST related dependencies in the bzlmodRio org, published to my fork of the BCR. I thought going the WORKSPACE route would be the best candidate for a minimum viable product as it still has the best documentation, and it is easier to patch in third party libraries that have zero bazel support (usually just with a http_archive and build_file_contents). Another reason to that way with this MVP is that I've had trouble getting the roborio toolchain to cross compile on windows with bzlmod; it works on my machine but not on github CI.

Ah, great, I missed that. There's a difference between a MVP and final product. I think you've got a good long term view there.

Do you have a handy list of the dependencies which were missing from bzlmod which you need help with?

Do you have links to the windows toolchain failures to share, and some way to reproduce them? I've fought a toolchain or two in my life...

Also, fair warning, for something like wpilib, including flags (like C++ version, etc) in the .bazelrc files is an anti-pattern

I agree. I didn't want to muck around with the auto-discovering native toolchains. I could add the native-tools defaults for the cross compilers, but it felt like a big lift to make custom toolchains for native linux/osx/windows.

I started this as a hobby when I joined a company that used bazel and was super frustrated at how long gradle builds took on my old laptop. I'm sure there is a lot of things that I'm doing stupidly and that you and 971 can help do the right way. If you want, feel free to reach out to me on CD to discuss, one bazel stan to another.

The context helps. Our story is similar. James has started prodding some folks on 971 (That's why I'm replying now), sounds like we should definately collaborate.

I've brought up Bazel in FRC before to some folks at the Bazel conference in previous years, and gotten more excitement than expected. Are you or anyone else here going to be at this year's conference?

@pjreiniger
Copy link
Contributor Author

Do you have links to the windows toolchain failures to share, and some way to reproduce them

I made an issue in my toolchains repo with the failure. To reproduce you can simply build my allwpilib fork with --config=windows --enable_bzlmod

Do you have a handy list of the dependencies which were missing from bzlmod which you need help with?

I was mostly referring to publishing bzlmod wrappers for all the vendor dep things (i.e. photonvision, ctre, rev, etc). Those all consume the pre built maven artifacts. I did the same for wpi's fork of opencv and apriltag rather than building it from source. I've experimented with all of the open source vendordeps and got them building with bazel, but I think the average team would be fine using the maven artifacts than building everything from source.

Are you or anyone else here going to be at this year's conference?

No bazelcon for me, but someone from my company is a presenter

@pjreiniger
Copy link
Contributor Author

Also fyi, I'm satisfied with protobuf generation approach and have a MVP for everything but gui stuff and java tests ready to go. I could merge it into here but want to avoid this

wpiutil/BUILD.bazel Outdated Show resolved Hide resolved
wpinet/BUILD.bazel Outdated Show resolved Hide resolved
hal/BUILD.bazel Outdated Show resolved Hide resolved
@KangarooKoala
Copy link
Contributor

Just to get things straight, is the reason we're looking into Bazel as a replacement for Gradle because of faster build times? Are there additional reasons?

@pjreiniger
Copy link
Contributor Author

I put some reasons here. To me one of the most compelling things is that it is designed to have first class java / cpp / python support. Gradle is fantastic for java, but requires lots of custom stuff for C++, cmake is fantastic and considered the industry standard for C++, but similarly requires hacks for java.

@TheTripleV
Copy link
Member

Just to get things straight, is the reason we're looking into Bazel as a replacement for Gradle because of faster build times? Are there additional reasons?

Gradle has a bus factor of 1. With bazel, the people on the bus can help with the build system too

@PeterJohnson
Copy link
Member

PeterJohnson commented Aug 29, 2024

A few more things I'd like to see:

  • CI should show how we build all 4 binary variants (debug/release x static/shared)
  • CI Maven artifact publishing of native libs
  • Is a Win32 CI build variant (of ntcoreffi only) possible?

We need to verify this addresses all the windows symbol issues we've had with protobuf et al, and that the resulting libraries are actually usable on Windows.

I think before we merge this we'd also want to fork the key repos (e.g. bzlmod) currently residing outside wpilibsuite into our org and repoint to them.

We also need a README file showing how to do local build/publish, etc, in the same style as the current Gradle and cmake ones. As a community project, it's very important people know how to build it starting from a git clone.

@pjreiniger
Copy link
Contributor Author

  • I can add debug builds after work
  • I'm purposely not building the shared libraries in the MVP, because of the potential problems on Windows. That one will need a thorough inspection by Thad and I didn't want to bog down this pr with that.
  • Once it is added they are automatically built by default, no additional command line necessary
  • there isn't a great option for maven publishing something like this out of the box. The "default" rule probably works great for Java only, but I think it adds dependencies to the pom file, which we currently do not. It also probably doesn't work for cpp things. I can easily make a script though.
  • I'll have to look into ntcoreeffi

@pjreiniger
Copy link
Contributor Author

/format

@PeterJohnson
Copy link
Member

PeterJohnson commented Aug 30, 2024

How does build caching work with bazel? Does it just use ccache/sccache, or is there something more integrated like Gradle build caches? This is critical for our CI to be reasonably speedy on PRs.

@PeterJohnson PeterJohnson merged commit 36e0c9d into wpilibsuite:main Oct 19, 2024
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants