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.

Sign up for GitHub

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

Jump to bottom

Bazel 7 compatibility updates #1619

Merged
merged 10 commits into from
Oct 8, 2024
Merged

Bazel 7 compatibility updates #1619

merged 10 commits into from
Oct 8, 2024
+220 −38

Conversation

mbland
Copy link
Contributor

@mbland mbland commented Oct 4, 2024 •
edited
Loading

Description

Resolves all but one Bazel 7 compatibility issue, while maintaining Bazel 6 and WORKSPACE compatibility.

The changes here are orthogonal to one another and can be broken out into separate pull requests if desired. See the list of commits and their corresponding messages for more details.

Updates the build to use Bazel 6.5.0, the last in the Bazel 6 line. The one Bazel 7 problem not addressed is the protobuf problem described below, which is resolved by #1620. With the change from that pull request, the changes from this pull request also successfully build under the latest mainline release, Bazel 7.3.2.

Motivation

I'm attempting to resolve #1482 to add Bzlmod support, based on the bzlmod branch in my fork. I'd like to ensure rules_scala builds cleanly under both Bazel 6 and Bazel 7 before beginning the Bzlmod refactoring.

With these changes, ./test_{lint,all,examples,coverage}.sh all succeed under Bazel 6.5.0. Then, when changing nothing else but updating to Bazel 7.0.0, protobuf generation fails as seen below. Applying the change from #1620 resolves the problem, while maintaining Bazel 6 compatibility.

$ bazel build //test/proto3:all

ERROR: .../rules_scala/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-13d3ddad9198/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: .../rules_scala/test/proto3/BUILD:14:14
  scala @//test/proto3:generated-proto-lib
  failed: (Exit 1): scalapb_worker failed:
  error executing ProtoScalaPBRule command
  (from target //test/proto3:generated-proto-lib)
  bazel-out/darwin_arm64-opt-exec-ST-13d3ddad9198/bin/src/scala/scripts/scalapb_worker
    ... (remaining 2 arguments skipped)

cc: @BillyAutrey @jayconrod @benjaminp @TheGrizzlyDev

@mbland
Add toolchain_type to thrift_lib, scrooge aspects
afb3b5d 
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
Fix proto_cross_repo_boundary build_file attribute
2be1f1a 
Bazel 7 enforces that this be a proper target label, not a relative
path.

```txt
ERROR: WORKSPACE:91:37: fetching new_local_repository rule
  //external:proto_cross_repo_boundary:
  .../external/test/proto_cross_repo_boundary/repo/BUILD.repo
  is not a regular file; if you're using a relative or absolute path for
  `build_file` in your `new_local_repository` rule, please switch to
  using a label instead

INFO: Repository remote_jdk8_macos instantiated at:
  WORKSPACE:175:18: in <toplevel>
  .../external/rules_java/java/repositories.bzl:120:10:
    in remote_jdk8_repos
  .../external/bazel_tools/tools/build_defs/repo/utils.bzl:240:18:
    in maybe
  .../external/rules_java/toolchains/remote_java_repository.bzl:48:17:
    in remote_java_repository

Repository rule http_archive defined at:
  .../external/bazel_tools/tools/build_defs/repo/http.bzl:384:31:
  in <toplevel>

ERROR: no such package '@@proto_cross_repo_boundary//':
  .../external/test/proto_cross_repo_boundary/repo/BUILD.repo
  is not a regular file; if you're using a relative or absolute path for
  `build_file` in your `new_local_repository` rule, please switch to
  using a label instead

ERROR: test/proto_cross_repo_boundary/BUILD:3:22:
  //test/proto_cross_repo_boundary:sample_scala_proto depends on
  @@proto_cross_repo_boundary//:sample_proto in repository
  @@proto_cross_repo_boundary which failed to fetch. no such package
  '@@proto_cross_repo_boundary//':
  .../external/test/proto_cross_repo_boundary/repo/BUILD.repo
  is not a regular file; if you're using a relative or absolute path for
  `build_file` in your `new_local_repository` rule, please switch to
  using a label instead

Use --verbose_failures to see the command lines of failed build steps.
ERROR: Analysis of target
  '//test/proto_cross_repo_boundary:sample_scala_proto' failed;
  build aborted: Analysis failed
```
@mbland
Add rules_python stanza to WORKSPACE files
8c1786b 
Fixes the following breakages under Bazel 7:

```txt
$ bazel build //src/...

ERROR: error loading package '@@bazel_tools//src/main/protobuf':
  cannot load '@@rules_python//python:proto.bzl': no such file

ERROR:
  .../src/java/io/bazel/rulesscala/coverage/instrumenter/BUILD:3:12:
  error loading package '@@bazel_tools//src/main/protobuf':
  cannot load '@@rules_python//python:proto.bzl':
  no such file and referenced by
  '//src/java/io/bazel/rulesscala/coverage/instrumenter:instrumenter'
```
@mbland
Set --noenable_bzlmod to prevent MODULE.bazel
a4284a2 
Makes sure Bazel doesn't autogenerate MODULE.bazel and MODULE.bazel.lock
files for now.
@mbland
Update to Bazel 6.5.0
d91bf77 
This is the last release in the Bazel 6 line. With the changes up to
this point, the Bazel 7 build only fails on protobuf generation, e.g.
on `bazel build //test/proto3:all`, as seen below. Bumping to Bazel
6.5.0 on top of all the previous changes isolates the Bazel 7.0.0
changes causing the failure.

```txt
$ bazel build //test/proto3:all

ERROR: .../rules_scala/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-13d3ddad9198/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: .../rules_scala/test/proto3/BUILD:14:14
  scala @//test/proto3:generated-proto-lib
  failed: (Exit 1): scalapb_worker failed:
  error executing ProtoScalaPBRule command
  (from target //test/proto3:generated-proto-lib)
  bazel-out/darwin_arm64-opt-exec-ST-13d3ddad9198/bin/src/scala/scripts/scalapb_worker
    ... (remaining 2 arguments skipped)
```
@mbland
Fix WORKSPACE lint errors
091f487 
One of these days I'll remember to run `bazel run //tools:lint_fix`
before opening a pull request.
@mbland
Set --enable_workspace on last_green CI build
14d3fa6 
Learning more about BuildKite every day.
@mbland
Copy link
Contributor Author

mbland commented Oct 5, 2024

BTW, I've found the smoking gun for the protobuf problem from Bazel 7.0.0, though I'm not yet sure exactly how it went off and what to do about it yet:

I'm posting my notes here, but feel free to ignore them for the purpose of this review.

This Bazel 7.0.0 change is playing havoc with the _import_paths() function from scala_proto/private/scala_proto_aspect.bzl. Inserting a print(source_root) statement as the second line revealed the following values:

  • Bazel 6.5.0: bazel-out/darwin_arm64-fastbuild/bin/test/proto3/_virtual_imports/generated-proto-lib
  • Bazel 7.0.0: .

As a result, _import_paths() strips the source_root properly under 6.5.0 (see the diff below), but doesn't stand a chance under 7.0.0.

The resulting paths are passed as command line arguments to the scalapb_worker launched by _generate_sources in the same file.

I got the hint to start looking here after running bazel build --verbose_failures //test/proto3:all and noticed the final argument, @bazel-out/darwin_arm64-fastbuild/bin/test/proto3/generated-proto-lib_jvm_extra_protobuf_generator_scalapb.srcjar-0.params. I saved off the versions of this .params file for 6.5.0 and 7.0.0, and got this diff:

--- params-6.5.0.txt	2024-10-04 20:28:49
+++ params-7.0.0.txt	2024-10-04 20:29:12
@@ -8,4 +8,4 @@
 grpc,single_line_to_proto_string
 --descriptor_set_in
 bazel-out/darwin_arm64-fastbuild/bin/test/proto3/generated-proto-lib-descriptor-set.proto.bin
-test/proto3/generated.proto
+bazel-out/darwin_arm64-fastbuild/bin/test/proto3/generated.proto

FWIW, the corresponding generated-proto-lib-descriptor-set.proto.bin (the descriptor database referenced by the protobuf command line interface error message) compared the same between both Bazel versions.

Now that I know I'm looking in the right place, I'll see if I can make sense of it and come up with a proper fix. If someone wants to beat me to it, though, that's fine by me.

mbland added a commit to mbland/rules_scala that referenced this pull request Oct 5, 2024
@mbland
Fix Bazel 7 related protobuf build failures
463509d 
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 mbland mentioned this pull request Oct 5, 2024
Merged
@mbland
Copy link
Contributor Author

mbland commented Oct 5, 2024

I figured out the //test/proto3 breakage and created #1620 to fix it. It doesn't fix the hanging scalapb_workers from #1618, but it's still an important fix. Either that PR or this one can go in first, but both must go in before upgrading to Bazel 7.

mbland added a commit to mbland/rules_scala that referenced this pull request Oct 5, 2024
@mbland
Fix Bazel 7 related protobuf build failures
d2c6646 
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
Allow test_all.sh to pass under Bazel 7
7ff6833 
`bash ./test_all.sh` passes after minor updates to the following test
cases to handle slightly different behavior under Bazel 7:

- `test_custom_reporter_class_has_been_set` makes the test output dir
  writable, since it no longer is by default.

- `test_scala_import_expect_failure_on_missing_direct_deps_warn_mode`
  removes preexisting output, since Bazel 7 won't emit warnings if it
  already exists.

---

FYI: Under Bazel 7.0 and Bazel 7.1, buildifier warnings for external
targets in the following test cases changed to begin with `@@repo_name`
instead of `@repo_name`:

- test/shell/test_scala_library.sh:
  `test_scala_library_expect_failure_on_missing_direct_external_deps_jar`
  `test_scala_library_expect_failure_on_missing_direct_external_deps_file_group`

- test/shell/test_strict_dependency.sh:
  `test_stamped_target_label_loading`
  `test_strict_deps_filter_included_target`

- test/shell/test_test_unused_dependency.sh:
  `test_unused_deps_filter_included_target`

However, as of Bazel 7.2, those warnings changed back to `@repo_name`,
so those changes aren't reflected in this commit.
@mbland
Bump to rules_go 0.39.1 for Bazel 7 compatibility
2339bc7 
`bazel run //tools:lint_{check,fix}` required updating rules_go to
0.39.1 to work under Bazel 7.

The previous rules_go version 0.38.1 caused the following error:

```txt
$ ./test_lint.sh
ERROR: .../external/io_bazel_rules_go/BUILD.bazel:71:16:
  in (an implicit dependency) attribute of go_context_data rule
  @@io_bazel_rules_go//:go_context_data:
  in $whitelist_function_transition attribute of go_context_data rule
  @@io_bazel_rules_go//:go_context_data: package group
  '@@bazel_tools//tools/whitelists/function_transition_whitelist:function_transition_whitelist'
  is misplaced here (they are only allowed in the visibility attribute)

ERROR: .../external/io_bazel_rules_go/BUILD.bazel:71:16:
  Analysis of target '@@io_bazel_rules_go//:go_context_data' failed

ERROR: Analysis of target '//tools:lint_check' failed;
  build aborted: Analysis failed
```
@mbland
Bump to rules_java 7.9.0 for Bazel 7 compatibility
6bc8d2f 
Updated rules_java to 7.9.0 to fix errors of the following type under
Bazel 7.2.1:

```txt
ERROR: test/BUILD:640:10:
  While resolving toolchains for target //test:jar_lister (096dcc8):
  invalid registered toolchain
  '@remote_jdk8_linux_aarch64_toolchain_config_repo//:bootstrap_runtime_toolchain':
  no such target
  '@@remote_jdk8_linux_aarch64_toolchain_config_repo//:bootstrap_runtime_toolchain':
  target 'bootstrap_runtime_toolchain' not declared in package '' defined by
  .../external/remote_jdk8_linux_aarch64_toolchain_config_repo/BUILD.bazel

ERROR: Analysis of target '//test:jar_lister' failed; build aborted
```

Also removed the seemingly unused `rules_java_extra` stanza from
`WORKSPACE`.

---

Though `rules_java` version 7.12.1 is available, and largely works with
this repo when using Bazel 7.3.2, it requires a few temporary
workarounds. (As I write this, 8.0.0 came out just a few hours ago and I
haven't tried it.) Rather than commit the workarounds, upgrading only to
7.9.0 now seems less crufty.

Though this commit only updates Bazel to 7.2.1, I suspect it's probably
the same basic problem at play.

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 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, whereby `@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 prerelease:

- 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
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 6, 2024
@mbland
Fix Bazel 7 related protobuf build failures
4e549df 
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 mbland changed the title Prepare for Bazel 7 while isolating the protobuf related failure Bazel 7 compatibility updates Oct 6, 2024
@mbland mbland mentioned this pull request Oct 6, 2024
Closed
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 7, 2024
@mbland
Fix Bazel 7 related protobuf build failures
2a387c8 
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
@mbland
Fix Bazel 7 related protobuf build failures (#1620)
7d3f022 
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!
simuons
simuons approved these changes Oct 8, 2024
View reviewed changes
Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

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

Thank you @mbland

@liucijus liucijus merged commit cd22d88 into bazelbuild:master Oct 8, 2024
2 checks passed
@mbland mbland deleted the bzlmod-prepare-for-bazel-7 branch October 8, 2024 13:15
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 8, 2024
@mbland
Merge branch 'master' into bzlmod
0909f42 
Incorporates:

- bazelbuild#1619
- bazelbuild#1620
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 8, 2024
@mbland
Merge branch 'master' into bzlmod
ffdf8ce 
Incorporates:

- bazelbuild#1619
- bazelbuild#1620

Removes --noenable_bzlmod from .bazelrc
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 8, 2024
@mbland
Merge branch 'master' into bzlmod
d332ddd 
Incorporates:

- bazelbuild#1619
- bazelbuild#1620

Removes --noenable_bzlmod from .bazelrc
@mbland mbland mentioned this pull request Oct 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simuons simuons simuons approved these changes

@liucijus liucijus liucijus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Bzlmod and add rules_scala to bazel-central-registry
3 participants
@mbland @simuons @liucijus