From 01259ff773d943704bcd15ce80cfa44207de2b79 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Sat, 5 Oct 2024 18:35:44 -0400 Subject: [PATCH] Bump Bazel to 7.2.1, rules_java to 7.9.0 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 https://github.com/bazelbuild/bazel/commit/ff1abb29a20c91a3d797842a26fb8a6b6fac48d8 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 https://github.com/bazelbuild/rules_java/commit/30ecf3ff6ee8f30b4df505d9d3bde5bb1c25690b 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 https://github.com/bazelbuild/bazel/pull/23571 https://github.com/bazelbuild/bazel/commit/f92124acf3d0eb99d06570ddc380aca7ab354b90 --- 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 https://github.com/bazelbuild/bazel/issues/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 https://github.com/bazelbuild/bazel/commit/750669078e20f2c6ca7b9241e1c80564a1b8668e --- 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 https://github.com/bazelbuild/bazel/issues/22754 --- .bazelci/presubmit.yml | 12 ++++++------ .bazelversion | 2 +- WORKSPACE | 13 ------------- scala/private/macros/scala_repositories.bzl | 4 ++-- test/shell/test_scala_library.sh | 4 ++-- test/shell/test_strict_dependency.sh | 4 ++-- test/shell/test_unused_dependency.sh | 2 +- 7 files changed, 14 insertions(+), 27 deletions(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 9b895a02b..92c793d08 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -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: @@ -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: diff --git a/.bazelversion b/.bazelversion index a8a188756..b26a34e47 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -7.1.2 +7.2.1 diff --git a/WORKSPACE b/WORKSPACE index 8dbb66def..08eb52097 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -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 diff --git a/scala/private/macros/scala_repositories.bzl b/scala/private/macros/scala_repositories.bzl index e11f199d5..204fd5629 100644 --- a/scala/private/macros/scala_repositories.bzl +++ b/scala/private/macros/scala_repositories.bzl @@ -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"): diff --git a/test/shell/test_scala_library.sh b/test/shell/test_scala_library.sh index 0ae758a46..ec43e1e7d 100755 --- a/test/shell/test_scala_library.sh +++ b/test/shell/test_scala_library.sh @@ -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 diff --git a/test/shell/test_strict_dependency.sh b/test/shell/test_strict_dependency.sh index 2c331c5f1..01a0d4029 100755 --- a/test/shell/test_strict_dependency.sh +++ b/test/shell/test_strict_dependency.sh @@ -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} \ @@ -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} \ diff --git a/test/shell/test_unused_dependency.sh b/test/shell/test_unused_dependency.sh index 4224b42a6..815968fc8 100755 --- a/test/shell/test_unused_dependency.sh +++ b/test/shell/test_unused_dependency.sh @@ -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} \