-
Notifications
You must be signed in to change notification settings - Fork 277
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
Bump Bazel and dependencies to prepare for Bzlmod #1618
Conversation
This begins the Bzlmod compatibility migration by updating Bazel to version 7.3.2 and adding initial `MODULE.bazel` and `WORKSPACE.bzlmod` files. Part of: bazelbuild#1482 Though Bzlmod remains disabled, updating to Bazel 7.3.2 requred updating or adding the following packages to maintain `WORKSPACE` compatibility. In `rules_scala_setup()`: - bazel_skylib: 1.4.1 => 1.7.1 - rules_cc: 0.0.6 => 0.0.10 - rules_java: 5.4.1 => 7.9.0 - rules_proto: 5.3.0-21.7 => 6.0.2 Dev dependencies in `WORKSPACE`: - com_google_protobuf: 28.2 - rules_pkg: 1.0.1 - rules_jvm_external: 6.4 - com_google_absl: abseil-cpp-20240722.0 - zlib: 1.3.1 Of all of the new, explicit dev dependencies, only `com_google_protobuf` will be necessary to include in `MODULE.bazel`. The Bzlmod mechanism will discover these other transitive dev dependencies automatically. Also removed the `rules_java_extra` repo from `WORKSPACE`, which appeared unused. --- Though the current `rules_java` version is 7.12.1, and largely works with this repo, it requires a few temporary workarounds. Rather than commit the workarounds, upgrading only to 7.9.0 now seems less crufty. What follows is a very detailed explanation of what happens with 7.12.1 with Bazel 7.3.2, just to have it on the record. --- The workaround is to change a few toolchain and macro file targets from `@bazel_tools//tools/jdk:` to `@rules_java//toolchains:`. This isn't a terribly bad or invasive workaround, but `@bazel_tools//tools/jdk:` is clearly the canonical path. Best to keep it that way, lest we build up technical debt. Without the workaround, these targets would fail: - //test/src/main/resources/java_sources:CompiledWithJava11 - //test/src/main/resources/java_sources:CompiledWithJava8 - //test/toolchains:java21_toolchain - //test:JunitRuntimePlatform - //test:JunitRuntimePlatform_test_runner - //test:scala_binary_jdk_11 with this error: ```txt ERROR: .../external/rules_java_builtin/toolchains/BUILD:254:14: While resolving toolchains for target @@rules_java_builtin//toolchains:platformclasspath (096dcc8): No matching toolchains found for types @@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type. ``` This appears to be a consequence of both upgrading the Bazel version from 6.3.0 to 7.3.2 and updating `rules_java` to 7.12.1. The `rules_java_builtin` repo is part of the `WORKSPACE` prefix that adds implicit dependencies: - https://bazel.build/external/migration#builtin-default-deps This repo was added to 7.0.0-pre.20231011.2 in the following change, mapped to `@rules_java` within the scope of the `@bazel_tools` repo: - bazelbuild/bazel: Add rules_java_builtin to the users of Java modules bazelbuild/bazel@ff1abb2 This change tried to ensure `rules_java` remained compatible with earlier Bazel versions. However, it changed all instances of `@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type` to `//toolchains:bootstrap_runtime_toolchain_type`: - bazelbuild/rules_java: Make rules_java backwards compatible with Bazel 6.3.0 bazelbuild/rules_java@30ecf3f Bazel has bumped `rules_java` in its `workspace_deps.bzl` from 7.9.0 to 7.11.0, but it's only available as of 8.0.0-pre.20240911.1. - bazelbuild/bazel: Update rules_java 7.11.1 / java_tools 13.8 bazelbuild/bazel#23571 bazelbuild/bazel@f92124a --- What I believe is happening is, under Bazel 7.3.2 and `rules_java` 7.12.1: - Bazel creates `rules_java` 7.9.0 as `@rules_java_builtin` in the `WORKSPACE` prefix. - `@bazel_tools` has `@rules_java` mapped to `@rules_java_builtin` when initialized during the `WORKSPACE` prefix, during which `@bazel_tools//tools/jdk` registers `alias()` targets to `@rules_java` toolchain targets. These aliased toolchains specify `@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type` in their `toolchains` attribute. - `WORKSPACE` loads `@rules_java` 7.12.1 and registers all its toolchains with type `@rules_java//toolchains:bootstrap_runtime_toolchain_type`. - Some `@rules_java` rules explicitly specifying toolchains from `@bazel_tools//tools/jdk` can't find them, because the `@bazel_tools//tools/jdk` toolchain aliases expect toolchains of type `@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type`. This has broken other projects in the same way: - bazelbuild/bazel: [Bazel CI] Downstream project broken by rules_java upgrade #23619 bazelbuild/bazel#23619 These problems don't appear under Bzlmod, and `@rules_java_builtin` was never required. This is because `WORKSPACE` executes its statements sequentially, while Bzlmod builds the module dependency graph _before_ instantiating repositories (within module extensions). It seems a fix is on the way that removes `@rules_java_builtin` from the `WORKSPACE` prefix, and adds `@rules_java` to the suffix. At this moment, though, it's not even in a prerelase: - bazelbuild/bazel: Remove rules_java_builtin in WORKSPACE prefix bazelbuild/bazel@7506690 --- Note that the error message matches that from the following resolved issue, but that issue was for non-Bzlmod child repos when `WORKSPACE` was disabled. - bazelbuild/bazel: Undefined @@rules_java_builtin repository with --noenable_workspace option bazelbuild/bazel#22754
This fixes the following error: ```txt Error in create_header_compilation_action: Rule 'thrift_library' in 'rules_scala/test/src/main/scala/scalarules/test/twitter_scrooge/thrift/ thrift_with_compiler_args/BUILD:3:15' must declare '@@bazel_tools//tools/jdk:toolchain_type' toolchain in order to use java_common. See bazelbuild/bazel#18970. ``` Interestingly, adding the toolchain type to `thrift_library` isn't enough; I had to add it to the twitter_scrooge aspects as well. Until I did, it produced the same error message pointing at `thrift_library`, after first reporting: ```txt ERROR: rules_scala/test/src/main/scala/scalarules/test/twitter_scrooge/ thrift/thrift_with_compiler_args/BUILD:3:15: in //twitter_scrooge:twitter_scrooge.bzl%scrooge_java_aspect aspect on thrift_library rule //test/src/main/scala/scalarules/test/twitter_scrooge/thrift/ thrift_with_compiler_args:thrift5: Traceback (most recent call last): File "rules_scala/twitter_scrooge/twitter_scrooge.bzl", line 420, column 49, in _scrooge_java_aspect_impl return _common_scrooge_aspect_implementation(target, ctx, "java", _compile_generated_java) [ ...snip... ] ```
Whoa, that's a lot of CI breakages on missing |
Didn't notice this until a bunch of builds failed on bazelbuild#1618.
OK, didn't see The |
Didn't notice this until a bunch of builds failed on bazelbuild#1618.
Hoping this will fix the `bazel //tools:lint_check` failures under BuildKite for bazelbuild#1618. Also ran `bazel run //tools:lint_fix` to fix `WORKSPACE` formatting.
Updated the repo information for `bazel_skylib` and `rules_proto` directly, and copied the `rules_proto` initialization calls from `WORKSPACE`. Implemented the `append_additional_dev_dependencies()` scheme to avoid duplicating the setup for `protobuf` and its dependencies (`rules_pkg`, `rules_java_external`, `com_google_absl`, `zlib`).
Copied the `build --noenable_bzlmod --enable_workspace` flags into the `.bazelrc` files. Updated the `rules_proto` initialization stanzas.
I solved the rest of the build failures from bazelbuild#1618. Now multiple jobs are failing due to hanging scalac worker jobs as described in the PR description. - Proto Toolchainisation Design Doc https://docs.google.com/document/d/1CE6wJHNfKbUPBr7-mmk_0Yo3a4TaqcTPE0OWNuQkhPs/edit - bazelbuild/bazel: Protobuf repo recompilation sensitivity bazelbuild/bazel#7095 - bazelbuild/rules_proto: Implement proto toolchainisation bazelbuild/rules_proto#179 - rules_proto 6.0.0 release notes mentioning Protobuf Toolchainisation https://github.com/bazelbuild/rules_proto/releases/tag/6.0.0 It occurred to me to try adopting Proto Toolchainisation to see if that might resolve the issue. I've got it successfully generating the `@io_bazel_rules_scala_protoc` repo and registering toolchains for `@rules_proto//proto:toolchain_type`. However, it's currently dying because there's no toolchain registered for '@rules_java//java/proto:toolchain_type'. ```txt bazel build //src/... ERROR: .../src/protobuf/io/bazel/rules_scala/BUILD:4:14: in @@_builtins//:common/java/proto/java_proto_library.bzl%bazel_java_proto_aspect aspect on proto_library rule //src/protobuf/io/bazel/rules_scala:diagnostics_proto: Traceback (most recent call last): File "/virtual_builtins_bzl/common/java/proto/java_proto_library.bzl", line 53, column 53, in _bazel_java_proto_aspect_impl File "/virtual_builtins_bzl/common/proto/proto_common.bzl", line 364, column 17, in _find_toolchain Error in fail: No toolchains registered for '@rules_java//java/proto:toolchain_type'. ERROR: Analysis of target '//src/protobuf/io/bazel/rules_scala:diagnostics_proto' failed ERROR: Analysis of target '//src/java/io/bazel/rulesscala/scalac:scalac' failed; build aborted: Analysis failed ```
I solved the rest of the build failures from bazelbuild#1618. Now multiple jobs are failing due to hanging scalac worker jobs as described in the PR description. - Proto Toolchainisation Design Doc https://docs.google.com/document/d/1CE6wJHNfKbUPBr7-mmk_0Yo3a4TaqcTPE0OWNuQkhPs/edit - bazelbuild/bazel: Protobuf repo recompilation sensitivity bazelbuild/bazel#7095 - bazelbuild/rules_proto: Implement proto toolchainisation bazelbuild/rules_proto#179 - rules_proto 6.0.0 release notes mentioning Protobuf Toolchainisation https://github.com/bazelbuild/rules_proto/releases/tag/6.0.0 It occurred to me to try adopting Proto Toolchainisation to see if that might resolve the issue. I've got it successfully generating the `@io_bazel_rules_scala_protoc` repo and registering toolchains for `@rules_proto//proto:toolchain_type`. However, it's currently dying because there's no toolchain registered for '@rules_java//java/proto:toolchain_type'. ```txt bazel build //src/... ERROR: .../src/protobuf/io/bazel/rules_scala/BUILD:4:14: in @@_builtins//:common/java/proto/java_proto_library.bzl%bazel_java_proto_aspect aspect on proto_library rule //src/protobuf/io/bazel/rules_scala:diagnostics_proto: Traceback (most recent call last): File "/virtual_builtins_bzl/common/java/proto/java_proto_library.bzl", line 53, column 53, in _bazel_java_proto_aspect_impl File "/virtual_builtins_bzl/common/proto/proto_common.bzl", line 364, column 17, in _find_toolchain Error in fail: No toolchains registered for '@rules_java//java/proto:toolchain_type'. ERROR: Analysis of target '//src/protobuf/io/bazel/rules_scala:diagnostics_proto' failed ERROR: Analysis of target '//src/java/io/bazel/rulesscala/scalac:scalac' failed; build aborted: Analysis failed ```
Updated the `WORKSPACE` to import fewer protobuf deps, which required importing an up to date `@platforms` and `@rules_pkg` earlier in the file. Copied these setup stanzas to all the testing example projects that need it. `./test_examples.sh` now passes completely. `bazel build //test/...` and `./test_version.sh` still hang, but the latter doesn't break on its `WORKSPACE` configuration. --- My working theory is that this breakage from `bazel build //test/proto3:all` provides a hint as to why the other protoc jobs hang: ```txt ERROR: .../test/proto3/BUILD:14:14: ProtoScalaPBRule test/proto3/generated-proto-lib_jvm_extra_protobuf_generator_scalapb.srcjar failed: (Exit 1): scalapb_worker failed: error executing ProtoScalaPBRule command (from target //test/proto3:generated-proto-lib) bazel-out/darwin_arm64-opt-exec-ST-a828a81199fe/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) Could not find file in descriptor database: bazel-out/darwin_arm64-fastbuild/bin/test/proto3/generated.proto: No such file or directory java.lang.RuntimeException: Exit with code 1 at scala.sys.package$.error(package.scala:30) at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44) at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96) at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49) at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39) at scripts.ScalaPBWorker.main(ScalaPBWorker.scala) Use --verbose_failures to see the command lines of failed build steps. ERROR: .../test/proto3/BUILD:14:14 Building source jar bazel-out/darwin_arm64-fastbuild/bin/test/proto3/generated-proto-lib_scalapb-src.jar failed: (Exit 1): scalapb_worker failed: error executing ProtoScalaPBRule command (from target //test/proto3:generated-proto-lib) bazel-out/darwin_arm64-opt-exec-ST-a828a81199fe/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) ``` If we can fix this, I think we might fix the hanging jobs, too. --- I'm focusing on this problem because migrating to Bzlmod will require using these newer versions of `@rules_proto` and `@com_google_protobuf`. It's the same problem I saw after successfully building the rest of the repo under Bzlmod, and it keeps these dependencies revlocked even under `WORKSPACE`. It's a problem that we need to solve one way or the other. I also tried to revert everything else to master but `@rules_proto` and `@com_google_protobuf`, but that didn't work. I wound up having to creep up the versions of other packages until I'd updated most of them again. I can try to update Bazel and all the dependencies _other_ than `@rules_proto` and `@com_google_protobuf` and get those merged first, if that's possible. Then I can try to create a pull request focused only on the protobuf problem. --- On another note: Now I know why `WORKSPACE` must be destroyed. I tried creating macros for these protobuf dependency stanzas, but was stymied by repo ordering errors. Duplicating all these stanzas has made me feel rather filthy. Bzlmod is definitely a superior experience once it's implemented. All of this config will just live in the top level rules_scala module. These example projects won't have to duplicate anything but the statements required to import it.
Forgot to run `bazel run //tools:lint_fix` first.
I've been poking at this pretty hard the past couple of days, and fixed all the non-proto related breakages in CI. I've come to realize is that the protobuf breakage starts happening when updating from Bazel 6.5.0 to 7.0.0, while holding rules_scala dependencies the same. I'm going to convert this to a draft for now, and open a new PR to upgrade Bazel to 6.5.0, with a couple of other small compatible updates to fix other Bazel 7 breakages. This will shrink this PR somewhat, while providing a baseline to isolate and investigate the protobuf problem. When that's fixed, I'll reopen this one, which should be substantially smaller and more focused. |
Related to bazelbuild#1482, bazelbuild#1618, and bazelbuild#1619. Results from the investigation documented at: - bazelbuild#1619 (comment) Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources` values between Bazel 6 and Bazel 7. Without this change, `_import_paths()` emits incorrect values under Bazel 7, causing targets containing generated `.proto` inputs to fail, e.g. `//test/proto3:test_generated_proto`. See also: - Fix paths for sibling repository setup and generated .proto files bazelbuild/bazel@6c6c196 - The docstring for `ProtoInfo.proto_source_root` in the Bazel sources: https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172 - Remove incompatible_generated_protos_in_virtual_imports bazelbuild/bazel@3efaa32 - Comment from: Generated Protos are no longer considered as virtual_imports in Bazel 7 bazelbuild/bazel#21075 (comment) --- I cherrypicked this commit into bazelbuild#1618. While it fixed the `//test/proto3` build failure, it does _not_ fix the hanging scalapb_workers from the ProtoScalaPBRule aspect. I'll have to investiate further whether than hang is related to Bazel, rules_proto, com_google_protobuf, or some mixture thereof. Still, progress!
Related to bazelbuild#1482, bazelbuild#1618, and bazelbuild#1619. Results from the investigation documented at: - bazelbuild#1619 (comment) Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources` values between Bazel 6 and Bazel 7. Without this change, `_import_paths()` emits incorrect values under Bazel 7, causing targets containing generated `.proto` inputs to fail, e.g. `//test/proto3:test_generated_proto`. See also: - Fix paths for sibling repository setup and generated .proto files bazelbuild/bazel@6c6c196 - The docstring for `ProtoInfo.proto_source_root` in the Bazel sources: https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172 - Remove incompatible_generated_protos_in_virtual_imports bazelbuild/bazel@3efaa32 - Comment from: Generated Protos are no longer considered as virtual_imports in Bazel 7 bazelbuild/bazel#21075 (comment) --- I cherrypicked this commit into bazelbuild#1618. While it fixed the `//test/proto3` build failure, it does _not_ fix the hanging scalapb_workers from the ProtoScalaPBRule aspect. I'll have to investiate further whether than hang is related to Bazel, rules_proto, com_google_protobuf, or some mixture thereof. Still, progress!
Related to bazelbuild#1482, bazelbuild#1618, and bazelbuild#1619. Results from the investigation documented at: - bazelbuild#1619 (comment) Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources` values between Bazel 6 and Bazel 7. Without this change, `_import_paths()` emits incorrect values under Bazel 7, causing targets containing generated `.proto` inputs to fail, e.g. `//test/proto3:test_generated_proto`. See also: - Fix paths for sibling repository setup and generated .proto files bazelbuild/bazel@6c6c196 - The docstring for `ProtoInfo.proto_source_root` in the Bazel sources: https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172 - Remove incompatible_generated_protos_in_virtual_imports bazelbuild/bazel@3efaa32 - Comment from: Generated Protos are no longer considered as virtual_imports in Bazel 7 bazelbuild/bazel#21075 (comment) --- I cherrypicked this commit into bazelbuild#1618. While it fixed the `//test/proto3` build failure, it does _not_ fix the hanging scalapb_workers from the ProtoScalaPBRule aspect. I'll have to investiate further whether than hang is related to Bazel, rules_proto, com_google_protobuf, or some mixture thereof. Still, progress!
I've opened #1619 to fix most Bazel 7 compatibility issues, and #1620 to fix the protobuf related build breakage. In the process, I've learned a few things and decided not to bump all the dependencies to their latest versions. So I'm going to close this pull request, and once #1619 and #1620 are in, I'll begin the actual Bzlmod refactoring. Here's what I learned, after including the changes from both #1619 and #1620 in a new experimental branch. It's possible to build with rules_proto 6.0.2 and protobuf v21.7 under both Bazel 6.5.0 and Bazel 7.3.2. It's also possible to build with protobuf v25.5 under Bazel 7, but not Bazel 6, because protobuf v22 dropped support for C++14:
I haven't figured out why protobuf v28.2 caused the scalapb_worker jobs to hang—but I also realized that I don't need to update all the dependencies before implementing Bzlmod. There are already Bazel Central Registry modules available for rules_proto-5.3.0-21.7 and all our other dependencies at the versions we're using. (Except bazelbuild/buildtools, but we can use So updating to rules_proto 6.0.2 can be its own separate change, and even then, rules_scala can stay on protobuf v21.7 until it's ready to drop Bazel 6 support. After that, someone can look into updating protobuf to v28.2 or later and resolving the scalapb_worker hanging issue. I'll now carry on with the Bzlmod migration without blocking on rules_proto or protobuf updates. |
Related to bazelbuild#1482, bazelbuild#1618, and bazelbuild#1619. Results from the investigation documented at: - bazelbuild#1619 (comment) Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources` values between Bazel 6 and Bazel 7. Without this change, `_import_paths()` emits incorrect values under Bazel 7, causing targets containing generated `.proto` inputs to fail, e.g. `//test/proto3:test_generated_proto`. See also: - Fix paths for sibling repository setup and generated .proto files bazelbuild/bazel@6c6c196 - The docstring for `ProtoInfo.proto_source_root` in the Bazel sources: https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172 - Remove incompatible_generated_protos_in_virtual_imports bazelbuild/bazel@3efaa32 - Comment from: Generated Protos are no longer considered as virtual_imports in Bazel 7 bazelbuild/bazel#21075 (comment) --- I cherrypicked this commit into bazelbuild#1618. While it fixed the `//test/proto3` build failure, it does _not_ fix the hanging scalapb_workers from the ProtoScalaPBRule aspect. I'll have to investiate further whether than hang is related to Bazel, rules_proto, com_google_protobuf, or some mixture thereof. Still, progress!
Related to #1482, #1618, and #1619. Results from the investigation documented at: - #1619 (comment) Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources` values between Bazel 6 and Bazel 7. Without this change, `_import_paths()` emits incorrect values under Bazel 7, causing targets containing generated `.proto` inputs to fail, e.g. `//test/proto3:test_generated_proto`. See also: - Fix paths for sibling repository setup and generated .proto files bazelbuild/bazel@6c6c196 - The docstring for `ProtoInfo.proto_source_root` in the Bazel sources: https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172 - Remove incompatible_generated_protos_in_virtual_imports bazelbuild/bazel@3efaa32 - Comment from: Generated Protos are no longer considered as virtual_imports in Bazel 7 bazelbuild/bazel#21075 (comment) --- I cherrypicked this commit into #1618. While it fixed the `//test/proto3` build failure, it does _not_ fix the hanging scalapb_workers from the ProtoScalaPBRule aspect. I'll have to investiate further whether than hang is related to Bazel, rules_proto, com_google_protobuf, or some mixture thereof. Still, progress!
Description
Bumps Bazel and dependencies to prepare for Bzlmod, while maintaining
WORKSPACE
compatibility.This begins the Bzlmod compatibility migration by updating Bazel to version 7.3.2 and adding initial
MODULE.bazel
andWORKSPACE.bzlmod
files. This required updating a few dependency versions and a few existing targets. The commit messages contain extensive detail on what was updated and why.Motivation
I'd like to help unblock users wishing to migrate their projects entirely to Bzlmod and to keep up with the latest Bazel releases, while ensuring
WORKSPACE
users aren't broken. Part of: #1482Outstanding protoc-bridge issue and request for help
Note: There are problems with protoc-bridge targets under //test that I've yet to resolve (and could use help resolving). I'd appreciate any feedback on this change independently of that problem, but obviously don't plan to merge until it's fixed. I may peel off other independent changes from the bzlmod branch in my fork in the meanwhile.
Currently, on my macOS 15.0 + M3 Max machine, the following fails:
Beyond that,
bazel test //test/...
will eventually hang on jobs launched by//src/scala/scripts:scalapb_worker
.cc: @BillyAutrey @jayconrod @benjaminp @TheGrizzlyDev