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

Support Bazel 8 #2378

Closed
5 tasks done
rickeylev opened this issue Nov 6, 2024 · 3 comments · Fixed by #2492
Closed
5 tasks done

Support Bazel 8 #2378

rickeylev opened this issue Nov 6, 2024 · 3 comments · Fixed by #2492
Assignees
Milestone

Comments

@rickeylev
Copy link
Collaborator

rickeylev commented Nov 6, 2024

I was trying to run our test with Bazel 8 and have been encountering a variety of problems.

The overall impression I'm getting is that the the core code is working, but various dev dependencies aren't.

Debugging this has been rather tough. Usually this is just a matter of updating some dependencies, but I've yet to find a combination of dependencies that works with both Bazel 7 and Bazel 8.

Issues I've seen thus far:

  • WORKSPACE.bzlmod is non-empty. It needs to become empty as WORKSPACE files become no-ops
  • gazelle has native.sh_binary references: native.sh_binary reference in def.bzl blocks Bazel 8 integration bazel-contrib/bazel-gazelle#1959
  • We need to update rules_bazel_integration_test version to fix some native references.
  • rules_cc shenanigans -- it seems that between 0.0.9 and 0.0.14 there have been various changes to how to reference bzl files. This ends up breaking doc building and the bzl_library deps. What we'll probably want to do is stop using cc:defs.bzl and start using the more specific loads instead.
  • Some issue with stardoc. Upgrading to 0.7.1 seems to fix it, but the release notes for stardoc say 0.7 requires Bazel 7. This will probably be OK; stardoc is a dev dependency, and only otherwise used via sphinxdocs, which is use-at-your-own-risk.

Various notes:
rules_cc centric:

  • rules_cc 0.0.9 can't be used with bazel 8: it lacks some fixes
  • rules_cc 0.0.10 can't be used with bazel 6: it has a dep on stardoc 7, which requires (in its MODULE definition) bazel 7+
  • rules_cc 0.0.11, 0.0.12, and 0.0.13 can't be used with Bazel 6: it tries to use extension_metadata(reproducible=True)
  • rules_cc 0.0.14 might be usable with Bazel 6 (mod graph passes) and relnotes say they restored bazel 6 compatibility
  • rules_cc 0.0.14 with protobuf 24.4 doesn't work -- protobuf loads @rules_cc//:defs.bzl#cc_proto_library, which was removed in rules_cc 0.0.14`. This was fixed in protobuf 27.0.
  • So basically, we must use rules_cc 0.0.14 and protobuf 27.0 or higher together.

stardoc centric:

  • Bazel 6 with bzlmod and stardoc 0.7 is a problem: Bazel will fail during module phase because stardoc requires (in its MODULE) Bazel 7.
  • The stardoc 0.7 requiring bazel 7 comes from rules_jvm_external: rules_jvm_external has a use_repo_rule call, which is Bazel 7 only.

rules_java centric:

  • rules_java before 8.2.0 and using the remote jdsk can't be used with rules_cc 0.0.14 because of the rules_cc cc_proto_library removal.
  • Usage of the remotejdk only appears to be done by our examples with the coverage command.
  • rules_java 8.2.0+ fixes the rules_cc issue, but can't be used because of a bug defining an android rule which fails due to some private API being used.
  • rules_java 8.2.0 requires Bazel 7.3.2 or higher (MODULE checks bazel version)
  • rules_java 8.3.0 works, but:
  • rules_java 8.3.0 doesn't support Bazel 6 (MODULE checks bazel version)

proto centric

  • protobuf 24 can't be used because of the rules_cc 0.0.14 removal of cc_proto_library
  • protobuf 28 is sort of the minimum, but our CI is unhappy with that version because protobuf set -Werror (treat warnings as errors), and the debian jobs then fail due to various warnings when compiling protobuf. Other OSes are OK
  • protobuf 29 fixes the -Werror problem, but that version isn't fully released. So 29.0-rc1 is the next available version.
  • Alternatively, we could use toolchains_protoc in our dev builds, which should avoid having to compile protoc entirely.
github-merge-queue bot pushed a commit that referenced this issue Nov 8, 2024
The WORKSPACE.bzlmod file will eventually be ignored by later Bazel
versions. To support
setup of the extra repos we rely on it for, create a dev-only module
extension that
invokes the necessary repo rules.

Work towards #2378
github-merge-queue bot pushed a commit that referenced this issue Nov 8, 2024
…ed by Bazel 8 (#2383)

This upgrades some module dependencies to versions that will be needed
in order to
support Bazel 8. More upgrades will be necessary, but these are some
easy ones
that don't cause compatibility issues.


* skylib 1.7.1
* (dev) rules_shell 0.3.0
* (dev) rules_bazel_integration_test 0.26.1


Work towards #2378
@aignas
Copy link
Collaborator

aignas commented Nov 11, 2024

I think we should release 1.0 with bazel 8 support and I think that means that we will have to drop bazel 6, which is reasonable since rules_python 1.0 will have to support the min bazel version until we bump to the next major version.

github-merge-queue bot pushed a commit that referenced this issue Nov 12, 2024
Various changes to support Bazel 8. An important note is dependencies
have forced
us to change the versions of Bazel we support.

Summary of changes:
* rules_cc 0.0.14: Releases after 0.0.9 have some Bazel 8 fixes, but
also broke
  some things. Things seemed to have settled by 0.0.14.
* protobuf 29.0-rc1: Technically 28.0 works, however:
  1. 29.0-rc1 is coming via a transitive dependency anyways, and
2. In protobuf 28.0, compile warnings are treated as errors, which our
Debian CI
     respects (and thus fails), while other platforms ignore.
* stardoc 0.7.1: Fixes an issue with Bazel 8 and stardoc using empty
globs.
* Bazel 7.4 is now the minimum supported Bazel version. This
requirements comes via
  dependencies.
* Drop Bazel 6 bzlmod support. This requirement comes via dependencies.
* Add a presubmit job for `last_rc` Bazel (currently the 8.x RC).
* Use a local patch so Gazelle works with Bazel 8. This can be removed
once bazel-contrib/bazel-gazelle#1959 is fixed
and released.
* Fix a `$(rpathlocation)` call in bootstrap tests.
* Update bzl_library deps after upgrading deps: the set of targets that
provide
  bzl sources changed in rules_cc and protobuf in these newer versions.

Sorting this all out and finding the right combination of dependency
versions was
fairly involved. The details of that are in
#2378.

Work towards #2378,
#2387
github-merge-queue bot pushed a commit that referenced this issue Nov 12, 2024
Referring to @rules_cc//cc:defs.bzl, refers to
@protobuf//bazel:cc_proto_library.bzl, which fetches protobuf
repository.
Referring directly to what's needed limits the fetches just to rules_cc.

Fix reference to bzl libs in rules_cc that are needed for docs
generation.

This requires rules_cc 0.0.13 or higher.

Work towards #2387,
#2378

---------

Co-authored-by: Richard Levasseur <rlevasseur@google.com>
@rickeylev
Copy link
Collaborator Author

We have basic Bazel 8 coverage in CI now. However, I ran across a couple minor issues in the bzlmod example itself. Fix for that is in #2395

github-merge-queue bot pushed a commit that referenced this issue Nov 12, 2024
…patibility (#2395)

In Bazel 8, the singular `$(rootpath)` expansions require that the
target expands to a
single file. The py rules have an unfortunate legacy behavior where
their default outputs
are the executable and the main py file, thus causing an error.

To fix, use the plural `$(rootpaths)`, then post-process the
space-separated string to get
just the executable portion of it.

Along the way...
* Add tests/integration/py_cc_toolchain_registered/bazel-* symlink to
Bazel ignore.
This avoids an infinite symlink expansions error and performance issues
when those
  symlinks exist.

Work towards #2378

---------

Co-authored-by: Ivo List <ilist@google.com>
rickeylev added a commit to rickeylev/rules_python that referenced this issue Nov 13, 2024
This allows us to drop the patch for removing native.sh_binary

Work towards bazelbuild#2378
github-merge-queue bot pushed a commit that referenced this issue Nov 14, 2024
This allows us to drop the patch for removing native.sh_binary

Work towards #2378
@aignas aignas added this to the v1.0.0 milestone Nov 14, 2024
@rickeylev
Copy link
Collaborator Author

Ok, I think we have everything passing with Bazel 8.

The one exception is the tests that BCR itself runs -- when I had BCR run our example with Bazel 8, it failed. But it passed in our CI. I'm not sure what to make of that, as the two looked otherwise identical. Anyways, I think we're covered on our end.

github-merge-queue bot pushed a commit that referenced this issue Dec 11, 2024
Summary:
* Remove the old `7.x` config from all `.bazelrc`.
* Drop bazel 6 from example testing/support. The `.bazelrc`
  for enabling `WORKSPACE` cannot work across bazel 6,7,8.
* Add missing `BUILD.bazel` file for integration tests.
* Remove an integration test runner for `bazel 6.x`.
* RBE test for bazel 8 is still not working. 
* bump `rules_java` for internal WORKSPACE dependencies to
  a version that supports `8.x`.
* start running bazel-in-bazel integration tests using bazel `8.x`.
* until bazel-contrib/rules_bazel_integration_test#414 is merged,
  we need to use bazel 7 for the delete_packagese pre-commit hook,
  not sure about how to track this as the `cgrindel/bazel-lib` needs
  a new version.

Fixes #2378

---------

Co-authored-by: Richard Levasseur <richardlev@gmail.com>
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 a pull request may close this issue.

2 participants