-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Respect rules_java as the source of truth for java toolchains #18423
Conversation
affcfdb
to
8a153d3
Compare
/cc @zhengwei143 since you are working on JDK 20 migration. This PR still keeps the embedded JDK definitions in https://github.com/bazelbuild/bazel/blob/master/repositories.bzl so it should not impact the migration too much. |
2542349
to
c45edc4
Compare
c45edc4
to
9c8e612
Compare
exports_files([ | ||
"BUILD.java_tools", | ||
"java_stub_template.txt", | ||
]) | ||
|
||
# Used to distinguish toolchains used for Java development, ie the JavaToolchainProvider. | ||
toolchain_type(name = "toolchain_type") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't move the definition of those two targets because I'm not sure if it's going to be a major breaking change. Would an alias work if I move them to rules_java?
Please review the internal CL instead! |
This PR is breaking many downstream projects: bazelbuild/rules_java#110 (comment) |
…el@HEAD Context bazelbuild/bazel#18423 PiperOrigin-RevId: 536961555
… try) Rolling forward 975866a (which was rollbacked at d51c75f) with fixes. - Introduce a `rules_java_builtin` repo in WORKSPACE prefix to avoid conflict with user defined rules_java. - `@bazel_tools//tools/jdk/*.bzl` loads from `rules_java_builtin` through repo-mappings. - `@local_jdk` was overridden in `jdk.WORKSPACE` to add repo_mapping for `rules_java`. - `jdk.WORKSPACE` explicitly loads from `rules_java_builtin` for JDK definitions and java toolchain definitions. - Allow using `__SKIP_WORKSPACE_PREFIX__` and `__SKIP_WORKSPACE_SUFFIX__` in WORKSPACE comment. - Fixed many tests by adjusting the WORKSPACE file content. - Re-export more symbols from `@bazel_tools` to be backward-compatible. Fixes #18551 Related: - #18373 - bazelbuild/rules_java#110 - #18423 Closes #18558. PiperOrigin-RevId: 538483417 Change-Id: I5223eec2c4b10131fc8c5b342237385ff2f56413
… try) Rolling forward bazelbuild@975866a (which was rollbacked at bazelbuild@d51c75f) with fixes. - Introduce a `rules_java_builtin` repo in WORKSPACE prefix to avoid conflict with user defined rules_java. - `@bazel_tools//tools/jdk/*.bzl` loads from `rules_java_builtin` through repo-mappings. - `@local_jdk` was overridden in `jdk.WORKSPACE` to add repo_mapping for `rules_java`. - `jdk.WORKSPACE` explicitly loads from `rules_java_builtin` for JDK definitions and java toolchain definitions. - Allow using `__SKIP_WORKSPACE_PREFIX__` and `__SKIP_WORKSPACE_SUFFIX__` in WORKSPACE comment. - Fixed many tests by adjusting the WORKSPACE file content. - Re-export more symbols from `@bazel_tools` to be backward-compatible. Fixes bazelbuild#18551 Related: - bazelbuild#18373 - bazelbuild/rules_java#110 - bazelbuild#18423 Closes bazelbuild#18558. PiperOrigin-RevId: 538483417 Change-Id: I5223eec2c4b10131fc8c5b342237385ff2f56413
Context: #18373
Currently the definitions of java tools, remote JDKs, and Java toolchains are duplicated in both Bazel sources and rules_java, and they go out of sync quite often. By default, the default toolchains shipped with the Bazel binary uses the one in Bazel sources except when Bzlmod is enabled.
bazelbuild/rules_java#110 syncs java toolchain related files from Bazel sources to rules_java and this PR removes most of the definitions in Bazel source and uses rules_java as the source of truth.
Significant changes in this PR:
jdk.WORKSPACE.tmpl
to load java tools and JDKs from rules_java as WORKSPACE suffix.jdk_http_archives
hack for some shell tests. That dates back to Test java coverage with java_tools released and built at head. #8361, I don't think it's needed since we have the WORKSPACE suffix by default.@bazel_tools
of the Bazel binary, now we use the java toolchains from therules_java
defined in distdir_deps.bzl. This enables us to build Bazel itself with the latest java toolchain, therefore can detect issues earlier._for_testing
suffixes for the shared repo hack for Bazel CI since the java tools and remote JDKs versions we load to build Bazel are the same as the ones shipped in the Bazel binary we built (viajdk.WORKSPACE.tmpl
) and used in the integration tests. They all come from the rules_java version in distdir_deps.bzl.test_WORKSPACE_files
hack forworkspace_resolved_test.sh
.Breaking changes:
.bzl
files under@bazel_tools//tools/jdk
in WORKSPACE now requires you to definerules_java
in advance, therules_java
definition in WORKSPACE suffix cannot guarantee@rules_java
exist because it's appended after the content of the user WORKSPACE file.