-
Notifications
You must be signed in to change notification settings - Fork 284
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 8 + rules_java
8 compatibility updates
#1710
base: master
Are you sure you want to change the base?
Conversation
Bumps dependencies to versions that are compatible with both Bazel 7.5.0 and 8.0.0. Part of bazelbuild#1482. Closes bazelbuild#1652. - ScalaPB jars: 0.11.17 => 1.0.0-alpha.1 - `rules_python`: 0.38.0 => 1.2.0 - `rules_cc`: 0.0.9 => 0.1.1 - `abseil-cpp`: 20220623.1 => 20250127.0 - `rules_java`: 7.12.4 => 8.9.0 - `protobuf`: 21.7 => 29.3 - `rules_proto`: 6.0.2 => 7.1.0 - `google-common-protos`: 2.52.0 => 2.53.0 This precipitated the following updates also included in this commit: - Loads `java_proto_library` from `com_google_protobuf`. - Bumps `.bazelversion` to 7.5.0. - Sets `.bazelci/presubmit.yml` to use Bazel 7.5.0 instead of 6.5.0, and `last_rc` in place of `7.x`. - Sets `common --enable_workspace --noenable_bzlmod` in `.bazelrc` and `tools/bazel\.rc\.buildkite`. - Adds `allow_empty = True` to a `glob` expression in `//test/semanticdb:lib_with_tempsrc`. - Removes Scala 2.11 test cases. Bazel 6 is officially unsupported as of this change and the upcoming `rules_scala` 7.0.0 release. `WORKSPACE` builds succeed under Bazel 6.5.0 (with C++ compiler flags) for the time being, but are not guaranteed to continue working. (Bazel 6 Bzlmod builds would break anyway, because Bazel 6 doesn't provide `use_repo_rule`, required by `rules_jvm_external` 6.3, which is required by `protobuf` v29.) The `README` now documents that `scala_proto` or any rules otherwise depending on `protobuf` are no longer supported out of the box for Scala 2.11. Such users will have to ensure they register their own downgraded versions, at their own risk of future `rules_scala` 7.x incompatibility. Version bump related updates to `.bazelversion`,`WORKSPACE`, and `third_party/repositories` comprise the bulk of this change. Other small, yet important changes (other than those due to the version bumps noted above) include: - Adding a new `examples/overridden_artifacts` repository and the `overridden_artifacts_example` test case in `test/shell/test_examples.sh`. - Making `_validate_scalac_srcjar()` and `dt_patched_compiler_setup()` in `scala/private/macros/scala_repositories.bzl` more tolerant of dictionaries containing keys mapped to `None`. The new `overridden_artifacts_example` test covers this. --- We're no longer planning to support Bazel 6 in the next major release per @simuons's decision in: - bazelbuild#1482 (comment) The plan is now to land the Bazel 7 and 8 compatibility updates first, then land the Bzlmod change. This enables us to make only one new major version release, instead of two (the first supporting with Bazel 6, the second dropping support). Bazel 8 and `rules_java` 8 require `protobuf` >= v29. After the `protobuf` v29 bump, and before the ScalaPB 1.0.0-alpha.1 bump, `scala_proto` targets would fail with the following error: ```txt ERROR: .../external/com_google_protobuf/src/google/protobuf/BUILD.bazel:23:14: ProtoScalaPBRule external/com_google_protobuf/src/google/protobuf/any_proto_jvm_extra_protobuf_generator_scalapb.srcjar failed: (Exit 1): scalapb_worker failed: error executing ProtoScalaPBRule command (from target @@com_google_protobuf//src/google/protobuf:any_proto) bazel-out/.../bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) --jvm_extra_protobuf_generator_out: java.lang.NoSuchMethodError: 'java.lang.Object com.google.protobuf.DescriptorProtos$FieldOptions.getExtension(com.google.protobuf.GeneratedMessage$GeneratedExtension)' at scalapb.compiler.DescriptorImplicits$ExtendedFieldDescriptor.fieldOptions(DescriptorImplicits.scala:329) [ ...snip... ] 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) ERROR: .../external/com_google_protobuf/src/google/protobuf/BUILD.bazel:23:14 Building source jar external/com_google_protobuf/src/google/protobuf/any_proto_scalapb-src.jar failed: (Exit 1): scalapb_worker failed: error executing ProtoScalaPBRule command (from target @@com_google_protobuf//src/google/protobuf:any_proto) bazel-out/darwin_arm64-opt-exec-ST-a828a81199fe/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) ``` Here's why the other changes were necessary in light of the version bumps: - `java_proto_library` from `rules_java` is now officially deprecated, hence loading it from `com_google_protobuf`. - Setting `common --enable_workspace --noenable_bzlmod` (instead of `build`) fixes `test_semanticdb_handles_removed_sourcefiles`. This test relies on `bazel query`, which is also affected by these flags, hence `common` instead of `build`. Bazel 8 defaults to `--enable_bzlmod --noenable_workspace`, causing the `WORKSPACE` run of this test to fail. - `glob` requires an explicit `allow_empty = True` parameter now that `--incompatible_disallow_empty_glob` defaults to `True` in Bazel 8. - ScalaPB 0.9.8, the last version compatible with Scala 2.11, does not support `protobuf` >= 26.0. For this reason, we must remove the Scala 2.11 test cases. We should consider dropping Scala 2.11 support at this point, since there's no ScalaPB release for it that supports later versions of `protobuf`. That, and we could remove some of the special case code added in the following changes, amongst other 2.11 support details: - bazelbuild#1631 - bazelbuild#1648 - bazelbuild#1687 - bazelbuild#1688 Finally, the motivation for the non-version bump changes: - The design bug in the upcoming Bzlmod API for `overridden_artifacts` reported by @dmivankov precipitated the `examples/overriden_artifacts` repo and test. See: bazelbuild#1482 (comment) bazelbuild#1482 (comment) - The `_validate_scalac_srcjar()` update maintains the strict checks for mutually exclusive values, while preventing client code from having to explicitly filter out `None` entries. This pairs with the change in `dt_patched_compiler_setup()` that uses the `compiler_srcjar` dictionary. These changes helps keep the upcoming module extension a bit cleaner.
Hmm, an odd failure on the Windows build: ERROR: C:/tools/msys64/home/b/_bazel_b/xknd5zlq/external/com_google_protobuf/src/google/protobuf/compiler/java/BUILD.bazel:87:11: Compiling src/google/protobuf/compiler/java/java_features.pb.cc [for tool] failed: (Exit 2): cl.exe failed: error executing CppCompile command (from target @@com_google_protobuf//src/google/protobuf/compiler/java:java_features_bootstrap) C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.39.33519\bin\HostX64\x64\cl.exe ... (remaining 1 argument skipped)
--
| external/com_google_protobuf/src/google/protobuf/compiler/java/java_features.pb.cc(6): fatal error C1083: Cannot open include file: 'google/protobuf/compiler/java/java_features.pb.h': No such file or directory src/google/protobuf/compiler/java/java_features.pb.h most definitely exists in protobuf v29.3. All the other builds seem to pass. If they do, I'll kick this with an empty commit to see if we can get Windows to play along, in case it was some weird transient error. |
Got an odd failure on the Windows build: - https://buildkite.com/bazel/rules-scala-scala/builds/5394#01954e5a-8f5b-4880-befa-1bce0d21d512/75-179 ```txt ERROR: C:/tools/msys64/home/b/_bazel_b/xknd5zlq/external/com_google_protobuf/src/google/protobuf/compiler/java/BUILD.bazel:87:11: Compiling src/google/protobuf/compiler/java/java_features.pb.cc [for tool] failed: (Exit 2): cl.exe failed: error executing CppCompile command (from target @@com_google_protobuf//src/google/protobuf/compiler/java:java_features_bootstrap) C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.39.33519\bin\HostX64\x64\cl.exe ... (remaining 1 argument skipped) -- | external/com_google_protobuf/src/google/protobuf/compiler/java/java_features.pb.cc(6): fatal error C1083: Cannot open include file: 'google/protobuf/compiler/java/java_features.pb.h': No such file or directory ``` `src/google/protobuf/compiler/java/java_features.pb.h` most definitely exists in protobuf v29.3: - https://github.com/protocolbuffers/protobuf/blob/v29.3/src/google/protobuf/compiler/java/java_features.pb.h All the other builds passed. Kicking the pull requeest branch with this empty commit to see if the Windows build will pass, in case it was some weird transient error.
Yep, just windows. And my empty commit bump just failed in the same way. Hmm... why can't it find the |
I looked at the windows issue... the issue is that the full include path for that file is over the char limit (260 chars). Also, I don't remember having to compile those protobuf tools locally before... I feel like they came precompiled before? |
OK, I found the related issue here: protocolbuffers/protobuf#12947. (I'm going to look into it more cause I use MSVC all the time and haven't run into these issues.) I wonder if we can go back to using the prebuilt protobuf libraries, then we don't need to require the c++ compiler in order to use rules_scala. |
@crt-31 Heh, you beat me to it...I snuck a look on my phone while out this evening and found the same issue. Got online to post it, but was too late. 😛 What's more, my next Bzlmod blog post is going to talk about how I had to patch And I saw the paragraph that you just responded to:
The linked announcement makes reference to protocolbuffers/protobuf#20085: "Breaking Change: Dropping support for Bazel+MSVC". But to your point above:
I did experiment with using prebuilt Protobuf toolchains in my proto-toolchainization branch in mbland/rules_scala back in October, when I was first experimenting with Bzlmodification. I think I may dust it off and play with it again in light of this development; it may also provide a performance win, since |
Keeps the `README` guidance in sync with what we're actually using in `WORKSPACE` for consistency's sake. @crt-31 and I found that the Windows build failure for bazelbuild#1710 mentioned in the earlier commit is related to the Windows/MSVC file path length limit. `src/google/protobuf/compiler/java/java_features.pb.h`, the path specified in the error message, doesn't exist until `protobuf` v25.0. - protocolbuffers/protobuf#12947 Furthermore, the Protobuf team currently plans to just drop MSVC support: - https://protobuf.dev/news/v30/#poison-msvc--bazel - protocolbuffers/protobuf#20085 I plan to experiment again with "Protobuf Toolchainization", which I'd tried in October when beginning the Bzlmod experiment. Here are some interesting background resources before I dig in on that: - bazelbuild/rules_proto#213 - bazelbuild/rules_proto#179 - https://github.com/bazelbuild/rules_proto/releases/tag/6.0.0 - https://github.com/aspect-build/toolchains_protoc/ - protocolbuffers/protobuf#20182 - protocolbuffers/protobuf#19679 - protocolbuffers/protobuf#19558
Registers a precompiled protocol compiler toolchain when `--incompatible_enable_proto_toolchain_resolution` is `True`. Part of bazelbuild#1482 and bazelbuild#1652. Stops `protoc` recompilation, and fixes the build breakage in bazelbuild#1710 due to `protobuf` include paths exceeding the Visual Studio path length limit. The updates to `scala_proto/scala_proto_toolchain.bzl` were inspired by: - protocolbuffers/protobuf: bazel: Remove hardcoded dependency on //:protoc from language runtimes #19679 protocolbuffers/protobuf#19679 The `proto_lang_toolchain` call was inspired by the `README` from: - https://github.com/aspect-build/toolchains_protoc/ Adds `scripts/update_protoc_integrity.py` to automatically update `scala/private/protoc/protoc_integrity.bzl`. This should make builds of `rules_scala` much faster all around. Given the fact that this feature depends on recent `protobuf` versions, and the Windows `protobuf` build breaks without it, we have a catch-22. It likely can't be separated from the rest of bazelbuild#1710, though I would prefer that. It also seems likely that we'd eventually need to do this to continue supporting Windows, per: - protocolbuffers/protobuf#12947 - https://protobuf.dev/news/v30/#poison-msvc--bazel - protocolbuffers/protobuf#20085 More background on proto toolchainization: - 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
UPDATE: Please skip this comment and read my new comment about the more recent fully working and polished protoc toolchainization. The good news is, I went ahead and implemented The bad news is, the Windows build is still broken in the same way. def java_proto_library(**kwattrs):
# Only use Starlark rules when they are removed from Bazel
if not hasattr(native, "java_proto_library"):
_java_proto_library(**kwattrs)
else:
native.java_proto_library(**kwattrs) I believe this is what @fmeum is trying to fix in protocolbuffers/protobuf#19679 (which I borrowed from in @simuons Until this gets resolved one way or another, what do you think of making Windows builds optional for now? Then when a fix comes, we can make it required again. Or maybe I can hack something to downgrade the Windows build down to @crt-31 I know you really don't want that to happen, but it's effectively blocking the next @fmeum If you've any better ideas, or my understanding is incorrect in any way, I'm all ears. At any rate, this gives me time to think about maybe adding more tests for this latest |
This is an experiment to see if applying protocolbuffers/protobuf#19679 via a patch and registering `@rules_scala_toolchains//protoc/...:all` will fix Windows MSVC builds. Local experiments suggest that this does indeed enable the toolchainized `protoc` to take precedence and avoid compiling protobuf.
Hold the presses... I just pushed an experiment that fixed the Windows build. I created a patch from protocolbuffers/protobuf#19679, then added a line to Everything else is failing right now (all the tests for all the other |
This required importing `@bazel_features` and setting it up so we could access `@com_google_protobuf//bazel/common/proto_common.bzl`. Began adding documentation. Haven't tried with Bzlmod yet, but we may need a way to create targets for users to call `register_toolchains` on. That, or just expose `@rules_scala_toolchains` as part of the API, which I've been avoiding.
Consolidates all the precompiled `protoc` logic in the new `//protoc` package. Ensures that `register_toolchains(//protoc:all)` toolchains is a no-op without `--incompatible_enable_proto_toolchain_resolution`. This achieves my goal of exporting a stable API for `register_toolchains()` calls that doesn't expose the `@rules_scala_toolchains` repo name. It also avoids special case logic by always generating `@rules_scala_toolchains//protoc` even if its `BUILD` file is empty. Fixes Windows builds, and makes all builds and tests run much faster. At the same time, the previous behavior remains unchanged if the user chooses not to use the precompiled toolchains. Also cleaned up `scripts/update_protoc_integrity.py` and added lots of docstrings.
OK, please ignore my update from yesterday. I've learned a lot, including that the The majority of the changes in the PR are still the boilerplate updates to
Still, this is a lot more than I'd originally planned to include in this pull request. Some of these updates can probably be teased out into separate PRs. And/or, we could go back to the original 7.x and 8.x release plan. This would mean backing out the In either case, Bzlmod could land in 7.0 or 7.1. I'll prepare my Bzlmod branch, but it would remain a relatively light update on top of whatever changes we decide to include, and when. Anywho, as usual, options... @simuons Let me know what you'd prefer to do here. |
Description
Bumps dependencies to versions that are compatible with both Bazel 7.5.0 and 8.0.0. Part of #1482. Closes #1652.
rules_python
: 0.38.0 => 1.2.0rules_cc
: 0.0.9 => 0.1.1abseil-cpp
: 20220623.1 => 20250127.0rules_java
: 7.12.4 => 8.9.0protobuf
: 21.7 => 29.3rules_proto
: 6.0.2 => 7.1.0google-common-protos
: 2.52.0 => 2.53.0This precipitated the following updates also included in this commit:
java_proto_library
fromcom_google_protobuf
..bazelversion
to 7.5.0..bazelci/presubmit.yml
to use Bazel 7.5.0 instead of 6.5.0, andlast_rc
in place of7.x
.common --enable_workspace --noenable_bzlmod
in.bazelrc
andtools/bazel\.rc\.buildkite
.allow_empty = True
to aglob
expression in//test/semanticdb:lib_with_tempsrc
.Bazel 6 is officially unsupported as of this change and the upcoming
rules_scala
7.0.0 release.WORKSPACE
builds succeed under Bazel 6.5.0 (with C++ compiler flags) for the time being, but are not guaranteed to continue working. (Bazel 6 Bzlmod builds would break anyway, because Bazel 6 doesn't provideuse_repo_rule
, required byrules_jvm_external
6.3, which is required byprotobuf
v29.)The
README
now documents thatscala_proto
or any rules otherwise depending onprotobuf
are no longer supported out of the box for Scala 2.11. Such users will have to ensure they register their own downgraded versions, at their own risk of futurerules_scala
7.x incompatibility.Version bump related updates to
.bazelversion
,WORKSPACE
, andthird_party/repositories
comprise the bulk of this change. Other small, yet important changes (other than those due to the version bumps noted above) include:Adding a new
examples/overridden_artifacts
repository and theoverridden_artifacts_example
test case intest/shell/test_examples.sh
.Making
_validate_scalac_srcjar()
anddt_patched_compiler_setup()
inscala/private/macros/scala_repositories.bzl
more tolerant of dictionaries containing keys mapped toNone
. The newoverridden_artifacts_example
test covers this.Motivation
We're no longer planning to support Bazel 6 in the next major release per @simuons's decision in:
The plan is now to land the Bazel 7 and 8 compatibility updates first, then land the Bzlmod change. This enables us to make only one new major version release, instead of two (the first supporting with Bazel 6, the second dropping support).
Bazel 8 and
rules_java
8 requireprotobuf
>= v29. After theprotobuf
v29 bump, and before the ScalaPB 1.0.0-alpha.1 bump,scala_proto
targets would fail with the following error:Here's why the other changes were necessary in light of the version bumps:
java_proto_library
fromrules_java
is now officially deprecated, hence loading it fromcom_google_protobuf
.Setting
common --enable_workspace --noenable_bzlmod
(instead ofbuild
) fixestest_semanticdb_handles_removed_sourcefiles
. This test relies onbazel query
, which is also affected by these flags, hencecommon
instead ofbuild
. Bazel 8 defaults to--enable_bzlmod --noenable_workspace
, causing theWORKSPACE
run of this test to fail.glob
requires an explicitallow_empty = True
parameter now that--incompatible_disallow_empty_glob
defaults toTrue
in Bazel 8.ScalaPB 0.9.8, the last version compatible with Scala 2.11, does not support
protobuf
>= 26.0. For this reason, we must remove the Scala 2.11 test cases.We should consider dropping Scala 2.11 support at this point, since there's no ScalaPB release for it that supports later versions of
protobuf
. That, and we could remove some of the special case code added in the following changes, amongst other 2.11 support details:ProtobufAdapter
s andScalaPBCodeGenerator
wrappers #1648protoc-bridge
to 0.9.8 for Scala >= 2.12 #1688Finally, the motivation for the non-version bump changes:
The design bug in the upcoming Bzlmod API for
overridden_artifacts
reported by @dmivankov precipitated theexamples/overriden_artifacts
repo and test. See: Support Bzlmod and add rules_scala to bazel-central-registry #1482 (comment) Support Bzlmod and add rules_scala to bazel-central-registry #1482 (comment)The
_validate_scalac_srcjar()
update maintains the strict checks for mutually exclusive values, while preventing client code from having to explicitly filter outNone
entries. This pairs with the change indt_patched_compiler_setup()
that uses thecompiler_srcjar
dictionary. These changes helps keep the upcoming module extension a bit cleaner.