Respect rules_java as the source of truth for java toolchains#18423
Closed
meteorcloudy wants to merge 26 commits intobazelbuild:masterfrom
Closed
Respect rules_java as the source of truth for java toolchains#18423meteorcloudy wants to merge 26 commits intobazelbuild:masterfrom
meteorcloudy wants to merge 26 commits intobazelbuild:masterfrom
Conversation
affcfdb to
8a153d3
Compare
Member
Author
|
/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
Member
Author
meteorcloudy
commented
May 25, 2023
| ]) | ||
|
|
||
| # Used to distinguish toolchains used for Java development, ie the JavaToolchainProvider. | ||
| toolchain_type(name = "toolchain_type") |
Member
Author
There was a problem hiding this comment.
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?
Member
Author
|
Please review the internal CL instead! |
Member
Author
|
This PR is breaking many downstream projects: bazelbuild/rules_java#110 (comment) |
copybara-service bot
pushed a commit
to protocolbuffers/protobuf
that referenced
this pull request
Jun 1, 2023
…el@HEAD Context bazelbuild/bazel#18423 PiperOrigin-RevId: 536961555
This was referenced Jun 1, 2023
Closed
copybara-service bot
pushed a commit
that referenced
this pull request
Jun 7, 2023
… 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
amishra-u
pushed a commit
to amishra-u/bazel
that referenced
this pull request
Jun 7, 2023
… 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.tmplto load java tools and JDKs from rules_java as WORKSPACE suffix.jdk_http_archiveshack 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_toolsof the Bazel binary, now we use the java toolchains from therules_javadefined in distdir_deps.bzl. This enables us to build Bazel itself with the latest java toolchain, therefore can detect issues earlier._for_testingsuffixes 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_fileshack forworkspace_resolved_test.sh.Breaking changes:
.bzlfiles under@bazel_tools//tools/jdkin WORKSPACE now requires you to definerules_javain advance, therules_javadefinition in WORKSPACE suffix cannot guarantee@rules_javaexist because it's appended after the content of the user WORKSPACE file.