-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Add support for toolchain java 15 #12168
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
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
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.
Thanks for the PR. Can you check why CI is failing: [1]? The error is:
(09:53:10) ERROR: /workdir/src/BUILD:744:10: no such package '@remote_java_tools_javac15_test_windows//': The repository '@remote_java_tools_javac15_test_windows' could not be resolved and referenced by '//src:test_repos'.
WORKSPACE
Outdated
@@ -81,7 +81,7 @@ list_source_repository(name = "local_bazel_source_list") | |||
# android_sdk_repository(name = "androidsdk") | |||
# android_ndk_repository(name = "androidndk") | |||
|
|||
# In order to run //src/test/shell/bazel:maven_skylark_test, follow the | |||
# In order to run //src/test/shell/bazel:maven_starlark_test, follow the |
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 see some unrelated white space changes. I would rather revert them all, and only include additions related to java_toolchain_jdk15. Clean up changes should be done in their own PR. Just guessing: you have run buildifier on this files, that also changed the white spaces and shuffled around the attributes.
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.
Done
I fixed the specific error you highlighted, @davido , but I'm quite lost on the current set - I can reproduce the failure locally but the error message gives me no hints... |
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.
Thanks @Jonpez2 for your effort! It seems that for one time we will catch up with Java releases.
I uploaded zulu15 to bazel mirror. |
771d953
to
44b806a
Compare
40708bd
to
e8e4925
Compare
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.
After this fix all the tests should pass.
src/test/shell/bazel/BUILD
Outdated
# --java_toolchain and --host_java_toolchain values | ||
"@local_java_tools//:toolchain_jdk_15", | ||
# java_tools zip to test | ||
"src/java_tools_java15.zip", |
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.
src/java_tools_java11.zip
here
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.
Nice thank you, done.
src/test/shell/bazel/BUILD
Outdated
}), | ||
data = [ | ||
":test-deps", | ||
"//src:java_tools_java15_zip", |
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.
and "//src:java_tools_java11_zip"
here...
tools 11 will have java 14 and java 15
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.
Nice thank you, done.
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.
Thanks!
Closes #11871
I modelled this on @davido's addition of a java 14 toolchain, here:https://github.com/bazelbuild/bazel/pull/11514.
The test plan has the same form:
Create java_tools from this commit:
$ bazel build src:java_tools_java11.zip
Switch to using the java_tools from the above step in WORKSPACE file:
Add Zulu OpenJDK15 to use as javabase in WORKSPACE file:
Activate custom java toolchain and javabase in .bazelrc file:
Create Java 15 example class file:
Add Bazel file to build Java 15 syntax class:
Test that it works as expected: