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

Set java_runtime on java_toolchains. #12333

Closed

Conversation

comius
Copy link
Contributor

@comius comius commented Oct 22, 2020

Toolchains that don't have java_runtime in a remote repository (toolchain_hostjdk8, legacy_toolchain, ...) were removed. They are better defined using java_toolchain_default, next to the rules defining local JDK.

Toolchains that don't have java_runtime in a remote repository (toolchain_hostjdk8, legacy_toolchain, ...) were removed. They are better defined using java_toolchain_default, next to the rules defining local JDK.
@google-cla google-cla bot added the cla: yes label Oct 22, 2020
@comius comius self-assigned this Oct 22, 2020
@comius comius requested a review from katre October 22, 2020 14:00
@comius
Copy link
Contributor Author

comius commented Oct 22, 2020

@@ -648,7 +656,7 @@ JAVA_VERSIONS = ("11",)
name = "create_java_tools_build_java" + java_version,
srcs = ["//tools/jdk:BUILD.java_tools"],
outs = ["remote_java_tools_java" + java_version + "/BUILD"],
cmd = "sed 's/JAVA_LANGUAGE_LEVEL/" + java_version + "/g' $< > $@",
cmd = "sed 's/JAVA_LANGUAGE_LEVEL/" + java_version + "/g; s/PLATFORM/" + _PLATFORM + "/g' $< > $@",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My read of this is that only the remote JDK that matches the host platform will be generated. Is this the intended use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the comment.

@comius
Copy link
Contributor Author

comius commented Oct 22, 2020 via email

@katre
Copy link
Member

katre commented Oct 22, 2020

That makes sense to me, but it's worth calling out the behavior in a comment.

@bazel-io bazel-io closed this in f421934 Oct 22, 2020
bazel-io pushed a commit that referenced this pull request Oct 27, 2020
*** Reason for rollback ***

Setting java_runtime attribute on java_toolchain before bazel is released doesn't work, because @remote_java_tools_linux_for_testing (and others) are loaded with old bazel release.

*** Original change description ***

Set java_runtime on java_toolchains.

Toolchains that don't have java_runtime in a remote repository (toolchain_hostjdk8, legacy_toolchain, ...) were removed. They are better defined using java_toolchain_default, next to the rules defining local JDK.

Closes #12333.

PiperOrigin-RevId: 339218931
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants