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

Bump Bazel and dependencies to prepare for Bzlmod #1618

Closed
wants to merge 10 commits into from

Conversation

mbland
Copy link
Contributor

@mbland mbland commented Oct 3, 2024

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 and WORKSPACE.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: #1482

Outstanding 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:

$ bazel build //test/proto3:all

ERROR: .../test/proto3/BUILD:14:14: ProtoScalaPBRule test/proto3/generated-proto-lib_jvm_extra_protobuf_generator_scalapb.sr
cjar 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/darwi
n_arm64-opt-exec-ST-a828a81199fe/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped)

Beyond that, bazel test //test/... will eventually hang on jobs launched by //src/scala/scripts:scalapb_worker.

cc: @BillyAutrey @jayconrod @benjaminp @TheGrizzlyDev

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... ]
```
@mbland
Copy link
Contributor Author

mbland commented Oct 3, 2024

Whoa, that's a lot of CI breakages on missing @rules_java and @rules_python repos that I didn't see at all locally. Investigating...

Didn't notice this until a bunch of builds failed on bazelbuild#1618.
@mbland
Copy link
Contributor Author

mbland commented Oct 3, 2024

OK, didn't see tools/bazel.rc.buildkite until now. Updated that.

The test_coverage jobs are still using 6.3.0 for some reason. Investigating...

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.
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 4, 2024
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
```
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 4, 2024
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.
@mbland
Copy link
Contributor Author

mbland commented Oct 4, 2024

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.

@mbland mbland marked this pull request as draft October 4, 2024 22:28
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 5, 2024
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!
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 5, 2024
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!
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 6, 2024
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!
@mbland
Copy link
Contributor Author

mbland commented Oct 6, 2024

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 archive_override to import it as a module.)

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.

@mbland mbland closed this Oct 6, 2024
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 7, 2024
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!
liucijus pushed a commit that referenced this pull request Oct 8, 2024
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!
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.

1 participant