Skip to content

Commit

Permalink
Bump Bazel to 7.2.1, rules_java to 7.9.0
Browse files Browse the repository at this point in the history
Also removed the seemingly unused `rules_java_extra` stanza from
`WORKSPACE`.

`test_all.sh` and `test_lint.sh` continue to pass, after changing the
`@@repo_name` prefixes set when upgrading to Bazel 7.0.0 back to
`@repo_name`.

The `@rules_java` update fixes 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
```

---

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
  • Loading branch information
mbland committed Oct 5, 2024
1 parent d4b2364 commit 01259ff
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 27 deletions.
12 changes: 6 additions & 6 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,16 @@ tasks:
platform: windows
shell_commands:
- "bash test_rules_scala.sh"
test_coverage_linux_7_1_2:
test_coverage_linux_7_2_1:
name: "./test_coverage"
platform: ubuntu2004
bazel: 7.1.2
bazel: 7.2.1
shell_commands:
- "./test_coverage.sh"
test_coverage_macos_7.1.2:
test_coverage_macos_7.2.1:
name: "./test_coverage"
platform: macos
bazel: 7.1.2
bazel: 7.2.1
shell_commands:
- "./test_coverage.sh"
test_reproducibility_linux:
Expand All @@ -83,13 +83,13 @@ tasks:
examples_linux:
name: "./test_examples"
platform: ubuntu2004
bazel: 7.1.2
bazel: 7.2.1
shell_commands:
- "./test_examples.sh"
cross_build_linux:
name: "./test_cross_build"
platform: ubuntu2004
bazel: 7.1.2
bazel: 7.2.1
shell_commands:
- "./test_cross_build.sh"
lint_linux:
Expand Down
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7.1.2
7.2.1
13 changes: 0 additions & 13 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -155,19 +155,6 @@ go_rules_dependencies()

go_register_toolchains(version = "1.19.5")

# Explicitly pull in a different (newer) version of rules_java for remote jdks
rules_java_extra_version = "5.1.0"

rules_java_extra_sha = "d974a2d6e1a534856d1b60ad6a15e57f3970d8596fbb0bb17b9ee26ca209332a"

rules_java_extra_url = "https://github.com/bazelbuild/rules_java/releases/download/{}/rules_java-{}.tar.gz".format(rules_java_extra_version, rules_java_extra_version)

http_archive(
name = "rules_java_extra",
sha256 = rules_java_extra_sha,
url = rules_java_extra_url,
)

load("@rules_java//java:repositories.bzl", "remote_jdk8_repos")

# We need to select based on platform when we use these
Expand Down
4 changes: 2 additions & 2 deletions scala/private/macros/scala_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ def rules_scala_setup(scala_compiler_srcjar = None):
http_archive(
name = "rules_java",
urls = [
"https://github.com/bazelbuild/rules_java/releases/download/5.4.1/rules_java-5.4.1.tar.gz",
"https://github.com/bazelbuild/rules_java/releases/download/7.9.0/rules_java-7.9.0.tar.gz",
],
sha256 = "a1f82b730b9c6395d3653032bd7e3a660f9d5ddb1099f427c1e1fe768f92e395",
sha256 = "41131de4417de70b9597e6ebd515168ed0ba843a325dc54a81b92d7af9a7b3ea",
)

if not native.existing_rule("rules_proto"):
Expand Down
4 changes: 2 additions & 2 deletions test/shell/test_scala_library.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ test_scala_library_expect_failure_on_missing_direct_internal_deps() {
}

test_scala_library_expect_failure_on_missing_direct_external_deps_jar() {
dependenecy_target='@@com_google_guava_guava_21_0//:com_google_guava_guava_21_0'
dependenecy_target='@com_google_guava_guava_21_0//:com_google_guava_guava_21_0'
test_target='test_expect_failure/missing_direct_deps/external_deps:transitive_external_dependency_user'

test_scala_library_expect_failure_on_missing_direct_deps $dependenecy_target $test_target
}

test_scala_library_expect_failure_on_missing_direct_external_deps_file_group() {
dependenecy_target='@@com_google_guava_guava_21_0_with_file//:com_google_guava_guava_21_0_with_file'
dependenecy_target='@com_google_guava_guava_21_0_with_file//:com_google_guava_guava_21_0_with_file'
test_target='test_expect_failure/missing_direct_deps/external_deps:transitive_external_dependency_user_file_group'

test_scala_library_expect_failure_on_missing_direct_deps $dependenecy_target $test_target
Expand Down
4 changes: 2 additions & 2 deletions test/shell/test_strict_dependency.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ test_plus_one_ast_analyzer_strict_deps() {

test_stamped_target_label_loading() {
local test_target="//test_expect_failure/missing_direct_deps/external_deps:java_lib_with_a_transitive_external_dep"
local expected_message="buildozer 'add deps @@io_bazel_rules_scala_guava//:io_bazel_rules_scala_guava' ${test_target}"
local expected_message="buildozer 'add deps @io_bazel_rules_scala_guava//:io_bazel_rules_scala_guava' ${test_target}"

test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message \
"${expected_message}" ${test_target} \
Expand All @@ -59,7 +59,7 @@ test_strict_deps_filter_excluded_target() {

test_strict_deps_filter_included_target() {
local test_target="//test_expect_failure/missing_direct_deps/filtering:b"
local expected_message="buildozer 'add deps @@com_google_guava_guava_21_0//:com_google_guava_guava_21_0' ${test_target}"
local expected_message="buildozer 'add deps @com_google_guava_guava_21_0//:com_google_guava_guava_21_0' ${test_target}"

test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message \
"${expected_message}" ${test_target} \
Expand Down
2 changes: 1 addition & 1 deletion test/shell/test_unused_dependency.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ test_unused_deps_filter_excluded_target() {

test_unused_deps_filter_included_target() {
local test_target="//test_expect_failure/unused_dependency_checker/filtering:b"
local expected_message="buildozer 'remove deps @@com_google_guava_guava_21_0//:com_google_guava_guava_21_0' ${test_target}"
local expected_message="buildozer 'remove deps @com_google_guava_guava_21_0//:com_google_guava_guava_21_0' ${test_target}"

test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message \
"${expected_message}" ${test_target} \
Expand Down

0 comments on commit 01259ff

Please sign in to comment.