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

Incompatible change - Rules which use java_common need to register java toolchain #18970

Open
kotlaja opened this issue Jul 18, 2023 · 5 comments
Labels
breaking-change-7.0 Incompatible flags to be flipped in Bazel 7.0 incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules type: support / not a bug (process)

Comments

@kotlaja
Copy link
Contributor

kotlaja commented Jul 18, 2023

Rules which use java_common need to register @bazel_tools//tools/jdk:toolchain_type.

This is a breaking change for rules which are using java_common and not registering @bazel_tools//tools/jdk:toolchain_type.
The error you might see if you're affected by this change is:
Rule 'custom_rule' in 'location' must declare '@bazel_tools//tools/jdk:toolchain_type' toolchain in order to use java_common.

How to fix this error?
Just register @bazel_tools//tools/jdk:toolchain_type inside rule's toolchains in the rule's definition:

 custom_rule = rule(
    impl = _impl, 
    ...,
    toolchains = ['@bazel_tools//tools/jdk:toolchain_type']
)
@kotlaja kotlaja added team-Rules-Java Issues for Java rules incompatible-change Incompatible/breaking change labels Jul 18, 2023
@meteorcloudy meteorcloudy added the breaking-change-7.0 Incompatible flags to be flipped in Bazel 7.0 label Jul 18, 2023
@meteorcloudy
Copy link
Member

The change has been made in d0e2958, this issue is only for documenting this breaking change.

copybara-service bot pushed a commit that referenced this issue Jul 18, 2023
…e affected

#18970

RELNOTE[INC]: Rules which use java_common should register java toolchain. See #18970

PiperOrigin-RevId: 548986304
Change-Id: I32378c66ed045dee2e0169bdb73fafc27a881e88
@jayconrod
Copy link
Contributor

As a rule user, how can I work around this incompatible change when a rule set I depend on triggers the error pointing here?

I'm seeing this Bazel 7.0.0-pre.20230724.1 and rules_scala 6.0.0 (the latest release version). I don't really want to go patching rules_scala. Is there an --incompatible flag I can turn off?

@meteorcloudy
Copy link
Member

rules_scala has been fixed at bazelbuild/rules_scala@a42f009, I think the right approach is to ask for a new release and update your rules_scala version.

@meteorcloudy
Copy link
Member

@hvadehra hvadehra added type: support / not a bug (process) P2 We'll consider working on this in future. (Assignee optional) labels Sep 13, 2023
gergelyfabian added a commit to gergelyfabian/rules_scala that referenced this issue Nov 3, 2023
Added `@bazel_tools//tools/jdk:toolchain_type` toolchain scala_junit_test:
bazelbuild/bazel#18970

Changed deprecated transitive_runtime_deps to transitive_runtime_jars in
jmh.bzl:
https://bazel.build/versions/6.4.0/rules/lib/JavaInfo#transitive_runtime_deps
simuons pushed a commit to bazelbuild/rules_scala that referenced this issue Nov 6, 2023
Added `@bazel_tools//tools/jdk:toolchain_type` toolchain scala_junit_test:
bazelbuild/bazel#18970

Changed deprecated transitive_runtime_deps to transitive_runtime_jars in
jmh.bzl:
https://bazel.build/versions/6.4.0/rules/lib/JavaInfo#transitive_runtime_deps
@vorburger
Copy link
Contributor

vorburger commented Dec 31, 2023

@meteorcloudy I'm running into grpc/grpc-java#10787 after adding that toolchains = ['@bazel_tools//tools/jdk:toolchain_type'] to a java_grpc_library in enola-dev/enola#390.

mbland added a commit to mbland/rules_scala that referenced this issue Oct 1, 2024
Adds `Label("@bazel_tools//tools/jdk:toolchain_type")` to thrift_library
and scala_repl per bazelbuild/bazel#18970.

The error before:

```txt
Error in create_header_compilation_action: Rule 'thrift_library' in
'.../rules_scala/test/src/main/scala/scalarules/test/twitter_scrooge/
  thrift/thrift2/thrift3/BUILD.bazel:4:15'
must declare '@@bazel_tools//tools/jdk:toolchain_type' toolchain in
order to use java_common. See
bazelbuild/bazel#18970.
```

Also removed a couple more unnecessary toolchain JAR deps.
mbland added a commit to mbland/rules_scala that referenced this issue Oct 3, 2024
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 added a commit to mbland/rules_scala that referenced this issue Oct 4, 2024
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... ]
```
liucijus pushed a commit to bazelbuild/rules_scala that referenced this issue Oct 8, 2024
* Add toolchain_type to thrift_lib, scrooge aspects

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... ]
```

* Fix proto_cross_repo_boundary build_file attribute

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
```

* Add rules_python stanza to WORKSPACE files

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'
```

* Set --noenable_bzlmod to prevent MODULE.bazel

Makes sure Bazel doesn't autogenerate MODULE.bazel and MODULE.bazel.lock
files for now.

* Update to Bazel 6.5.0

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)
```

* Fix WORKSPACE lint errors

One of these days I'll remember to run `bazel run //tools:lint_fix`
before opening a pull request.

* Set --enable_workspace on last_green CI build

Learning more about BuildKite every day.

* Allow test_all.sh to pass under Bazel 7

`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.

* Bump to rules_go 0.39.1 for Bazel 7 compatibility

`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
```

* Bump to rules_java 7.9.0 for Bazel 7 compatibility

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change-7.0 Incompatible flags to be flipped in Bazel 7.0 incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules type: support / not a bug (process)
Projects
None yet
Development

No branches or pull requests

5 participants