-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add support for toolchain java 14 #11514
Conversation
cc @lberki |
Thanks for the month of patience! We should really have reviewed this earlier but... things came up. |
Dear @davido, sorry for the waiting. I did a quick general review of your pull request and at first sight it seems this is a work in progress. Did you plan to do additional work on it? Do you need some support with it? Also may I ask what was the reasoning to drop Java 12 support? Thanks again for your effort. |
Right. It depends on my Error Prone 2.4.0 PR. Because currently used EP doesn't support Java 14 yet.
Yes, I will resume the work on this PR, once EP upgrade is landed.
In fact we could preserve Java 12, but what is the point? Java 12 and 13 are dead.
One thing so solve is to conditionally remove these two lines in toolchain definition for java toolchans >= 14: # override the javac in the JDK.
#"--patch-module=java.compiler=$(location :java_compiler_jar)",
#"--patch-module=jdk.compiler=$(location :jdk_compiler_jar)", Do you have a suggestion how to do it: preserve those lines for Java 8, 11 and 12 toolchains, but remove them for 14, 15, 17, ...? One idea I had, is to duplicate this toolchain, and name it differently, say: |
Well one thing I can think of is, there might be people out there with projects depending on Java 12. Having the option and not being forced into switch might help them.
You can use inline
How to actually properly write the condition for java version or if it is even possible I don't know. If you cannot write it then yes, your best option is to split into two toolchains. |
f091346
to
f9ae943
Compare
Cc @aiuto @philwo @meteorcloudy Some tests are failing, e.g.: because download doesn't work:
Can you help to upload the required artifact to bazel mirror? This test is passing as expected here:
|
When trying to build Gerrit with java_tools produced by this PR the log is flooded with:
This is because of: https://bugs.openjdk.java.net/browse/JDK-8214731. Should |
https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu14.28.21-ca-jdk14.0.1-linux_x64.tar.gz should be available now. |
Thanks for the quick upload. Could you also upload the tars for Darwin and Wndows? Thanks! |
a39aa88
to
8696269
Compare
And on Windows:
It's trying to use wrong
|
On RBE
|
The (12:50:27) INFO: From Executing genrule //:bazel-distfile-tar:
./combine_distfiles_to_tar.sh: 44: [: Linux: unexpected operator
(13:00:04) ERROR: /workdir/src/main/java/com/google/devtools/build/lib/BUILD:521:8: Executing genrule //src/main/java/com/google/devtools/build/lib:gen_skylarklibrary failed (Timeout): bash failed: error executing command
(cd /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/execroot/io_bazel && \
exec env - \
PATH=/bin:/usr/bin:/usr/local/bin \
/bin/bash -c 'source external/bazel_tools/tools/genrule/genrule-setup.sh; mkdir -p bazel-out/k8-fastbuild/bin/src/main/java/com/google/devtools/build/lib/skylark-lib &&bazel-out/host/bin/src/main/java/com/google/devtools/build/docgen/skydoc_bin bazel-out/k8-fastbuild/bin/src/main/java/com/google/devtools/build/lib/skylark-lib && zip -qj bazel-out/k8-fastbuild/bin/src/main/java/com/google/devtools/build/lib/skylark-library.zip bazel-out/k8-fastbuild/bin/src/main/java/com/google/devtools/build/lib/skylark-lib/*')
Execution platform: //:rbe_ubuntu1604_java8_platform (failed due to timeout.)
Generating Starlark documentation... |
@davido We've been seeing some test timeouts on RBE, not sure of the cause yet. If you believe it's unrelated, then there's a good chance that it is. |
Thanks, @philwo for confirming. Can someone help to kick off the pipeline for releasing I'm not sure, how @iirina did it for JDK12, but there is "the chicken or the egg" problem. To release https://github.com/bazelbuild/java_tools/releases only has releases for The workaround I used for now is to duplicate JAVA_VERSIONS = ("11", "14")
# TODO(davido): Remove this once remote_java_tools_javac14 is released
JAVA_TOOLS_VERSIONS = ("11",) and skip the # Test java coverage with the java_toolchain in the released java_tools versions.
[
sh_test(
name = "bazel_coverage_java_jdk" + java_version + "toolchain_released_test",
srcs = ["bazel_coverage_java_test.sh"],
args = select({
"//src/conditions:darwin": ["@remote_java_tools_javac" + java_version + "_test_darwin//:toolchain"],
"//src/conditions:darwin_x86_64": ["@remote_java_tools_javac" + java_version + "_test_darwin//:toolchain"],
"//src/conditions:windows": ["@remote_java_tools_javac" + java_version + "_test_windows//:toolchain"],
"//src/conditions:linux_x86_64": ["@remote_java_tools_javac" + java_version + "_test_linux//:toolchain"],
}) + [
# java_tools zip to test
"released",
# --javabase value
] + select({
"//src/conditions:darwin": ["@openjdk" + java_version + "_darwin_archive//:runtime"],
"//src/conditions:darwin_x86_64": ["@openjdk" + java_version + "_darwin_archive//:runtime"],
"//src/conditions:windows": ["@openjdk" + java_version + "_windows_archive//:runtime"],
"//src/conditions:linux_x86_64": ["@openjdk" + java_version + "_linux_archive//:runtime"],
}),
data = [
":test-deps",
"//src/test/shell/bazel/testdata:jdk_http_archives_filegroup",
],
tags = [
"no_windows",
],
)
# for java_version in JAVA_VERSIONS
for java_version in JAVA_TOOLS_VERSIONS
] |
Can we move forward with the review here to make sure toolchain for JDK14 is included in upcoming Bazel releases? Of course, we could wait for general availbility of JDK 15 on 2020/09/15, see: [1], but if we always wait, we would never release anything new 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.
Hi @davido,
thanks for your amazing effort, and sorry for the delay. It took me some time to thoroughly review your PR. Those should now be the last changes.
WORKSPACE
Outdated
@@ -701,6 +701,16 @@ http_archive( | |||
], | |||
) | |||
|
|||
|
|||
# TODO(davido): This is not needed, because jdk compiler module patching is disabled for JDK 14 | |||
http_archive( |
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.
This is potentially using a different version of Java compiler. Remove it.
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.
1dfb45a
to
f4cf7c5
Compare
I addressed your comments. PTAL. |
45ab7db
to
97b2bd9
Compare
Closes bazelbuild#11017. Test Plan: 1. Create java_tools from this commit: $ bazel build src:java_tools_java14.zip 2. Switch to using the java_tools from the above step in WORKSPACE file: load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "remote_java_tools_linux", urls = [ "file:///<path to the java_tools_java14.zip>", ], ) 3. Add Zulu OpenJDK14 to use as javabase in WORKSPACE file: http_archive( name = "openjdk14_linux_archive", build_file_content = """ java_runtime(name = 'runtime', srcs = glob(['**']), visibility = ['//visibility:public']) exports_files(["WORKSPACE"], visibility = ["//visibility:public"]) """, sha256 = "48bb8947034cd079ad1ef83335e7634db4b12a26743a0dc314b6b861480777aa", strip_prefix = "zulu14.28.21-ca-jdk14.0.1-linux_x64", urls = ["https://cdn.azul.com/zulu/bin/zulu14.28.21-ca-jdk14.0.1-linux_x64.tar.gz"], ) 4. Activate custom java toolchain and javabase in .bazelrc file: build --java_toolchain=@remote_java_tools_linux//:toolchain_jdk_14 build --host_java_toolchain=@remote_java_tools_linux//:toolchain_jdk_14 build --javabase=@openjdk14_linux_archive//:runtime build --host_javabase=@openjdk14_linux_archive//:runtime 5. Create Java 14 example class file: public class Javac14Example { record Point(int x, int y) {} public static void main(String[] args) { Point point = new Point(0, 1); System.out.println(point.x); } } 6. Add Bazel file to build Java 14 syntax class with activated preview features: java_binary( name = "Javac14Example", srcs = ["Javac14Example.java"], javacopts = ["--enable-preview"], jvm_flags = ["--enable-preview"], main_class = "Javac14Example", ) 7. Test that it works as expected: $ bazel run java:Javac14Example INFO: Analyzed target //java:Javac14Example (1 packages loaded, 2 targets configured). INFO: Found 1 target... INFO: From Building java/Javac14Example.jar (1 source file): Note: java/Javac14Example.java uses preview language features. Note: Recompile with -Xlint:preview for details. Target //java:Javac14Example up-to-date: bazel-bin/java/Javac14Example.jar bazel-bin/java/Javac14Example INFO: Elapsed time: 1.502s, Critical Path: 1.30s INFO: 1 process: 1 worker. INFO: Build completed successfully, 2 total actions INFO: Build completed successfully, 2 total actions 0
97b2bd9
to
a36d987
Compare
I tried that, but the problem is that the default toolchain is still using them: java_toolchain(
name = "toolchain",
bootclasspath = ["@bazel_tools//tools/jdk:platformclasspath"],
forcibly_disable_header_compilation = 0,
genclass = [":GenClass"],
header_compiler = [":Turbine"],
header_compiler_direct = [":TurbineDirect"],
ijar = [":ijar"],
jacocorunner = ":jacoco_coverage_runner_filegroup",
javabuilder = [":JavaBuilder"],
javac = [":javac_jar"],
[...] So that it's still needed. I added a comment, that until default toolchain still relies on it, custom javac compiler should still be included into Of course, when we abandon Java 8 and Java 11 (in 5 years?) we could remove custom javac from |
Can we move forward here, and kick off For that I filed this issue in |
@jin (or somebody else) Could you please remove "WIP" label, as this PR is ready now. |
Closes bazelbuild#11017. This is the follow-up for PR: [1]. Because of the chicken and the egg problem, it has to be done on three steps: 1. add support for java toolchange JDK 14, done in: [1] 2. publish new java_tools_javac14, kicked off by the pipeline in java_tools repository 3. add unit test against released java_tools_javac4 published in step 2, this change Test Plan: $ bazel test //src/test/shell/bazel:bazel_coverage_java_jdk14toolchain_released_test [1] bazelbuild#11514
Closes bazelbuild#11017. This is the follow-up for PR: [1]. Because of the chicken and the egg problem, it has to be done in three steps: 1. add support for java toolchange JDK 14, done in: [1] 2. publish new java_tools_javac14, kicked off by the pipeline in java_tools repository, in context of: [2] 3. add unit test against released java_tools_javac14 published in step 2, this change Test Plan: $ bazel test //src/test/shell/bazel:bazel_coverage_java_jdk14toolchain_released_test [1] bazelbuild#11514 [2] bazelbuild/java_tools#28
Closes #11017. This is the follow-up for PR: [1]. Because of the chicken and the egg problem, it has to be done in three steps: 1. add support for java toolchange JDK 14, done in: [1] 2. publish new java_tools_javac14, kicked off by the pipeline in java_tools repository, in context of: [2] 3. add unit test against released java_tools_javac14 published in step 2, this change Test Plan: $ bazel test //src/test/shell/bazel:bazel_coverage_java_jdk14toolchain_released_test [1] #11514 [2] bazelbuild/java_tools#28 Closes #11828. PiperOrigin-RevId: 324358602
Closes bazelbuild#11017. Test Plan: 1. Create java_tools from this commit: ```bash $ bazel build src:java_tools_java14.zip ``` 2. Switch to using the java_tools from the above step in WORKSPACE file: ```python load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "remote_java_tools_linux", urls = [ "file:///<path to the java_tools_java14.zip>", ], ) ``` 3. Add Zulu OpenJDK14 to use as javabase in WORKSPACE file: ```python http_archive( name = "openjdk14_linux_archive", build_file_content = """ java_runtime(name = 'runtime', srcs = glob(['**']), visibility = ['//visibility:public']) exports_files(["WORKSPACE"], visibility = ["//visibility:public"]) """, sha256 = "48bb8947034cd079ad1ef83335e7634db4b12a26743a0dc314b6b861480777aa", strip_prefix = "zulu14.28.21-ca-jdk14.0.1-linux_x64", urls = ["https://cdn.azul.com/zulu/bin/zulu14.28.21-ca-jdk14.0.1-linux_x64.tar.gz"], ) ``` 4. Activate custom java toolchain and javabase in .bazelrc file: ```bash build --java_toolchain=@remote_java_tools_linux//:toolchain_jdk_14 build --host_java_toolchain=@remote_java_tools_linux//:toolchain_jdk_14 build --javabase=@openjdk14_linux_archive//:runtime build --host_javabase=@openjdk14_linux_archive//:runtime ``` 5. Create Java 14 example class file: ```java public class Javac14Example { record Point(int x, int y) {} public static void main(String[] args) { Point point = new Point(0, 1); System.out.println(point.x); } } ``` 6. Add Bazel file to build Java 14 syntax class with activated preview features: ```python java_binary( name = "Javac14Example", srcs = ["Javac14Example.java"], javacopts = ["--enable-preview"], jvm_flags = ["--enable-preview"], main_class = "Javac14Example", ) ``` 7. Test that it works as expected: ``` $ bazel run java:Javac14Example INFO: Analyzed target //java:Javac14Example (1 packages loaded, 2 targets configured). INFO: Found 1 target... INFO: From Building java/Javac14Example.jar (1 source file): Note: java/Javac14Example.java uses preview language features. Note: Recompile with -Xlint:preview for details. Target //java:Javac14Example up-to-date: bazel-bin/java/Javac14Example.jar bazel-bin/java/Javac14Example INFO: Elapsed time: 1.502s, Critical Path: 1.30s INFO: 1 process: 1 worker. INFO: Build completed successfully, 2 total actions INFO: Build completed successfully, 2 total actions 0 ``` Closes bazelbuild#11514. PiperOrigin-RevId: 322759522
Closes bazelbuild#11017. This is the follow-up for PR: [1]. Because of the chicken and the egg problem, it has to be done in three steps: 1. add support for java toolchange JDK 14, done in: [1] 2. publish new java_tools_javac14, kicked off by the pipeline in java_tools repository, in context of: [2] 3. add unit test against released java_tools_javac14 published in step 2, this change Test Plan: $ bazel test //src/test/shell/bazel:bazel_coverage_java_jdk14toolchain_released_test [1] bazelbuild#11514 [2] bazelbuild/java_tools#28 Closes bazelbuild#11828. PiperOrigin-RevId: 324358602
Closes #11871 I modelled this on @davido's addition of a java 14 toolchain, here:[https://github.com/bazelbuild/bazel/pull/11514](url). 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: ``` load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "remote_java_tools_linux", urls = [ "file:///<path to the java_tools_java11.zip>", ], ) ``` Add Zulu OpenJDK15 to use as javabase in WORKSPACE file: ``` http_archive( name = "openjdk15_linux_archive", build_file_content = """ java_runtime(name = 'runtime', srcs = glob(['**']), visibility = ['//visibility:public']) exports_files(["WORKSPACE"], visibility = ["//visibility:public"]) """, strip_prefix = "zulu15.27.17-ca-jdk15.0.0-linux_x64", urls = ["https://cdn.azul.com/zulu/bin/zulu15.27.17-ca-jdk15.0.0-linux_x64.tar.gz"], ) ``` Activate custom java toolchain and javabase in .bazelrc file: ``` build --java_toolchain=@remote_java_tools_linux//:toolchain_jdk_15 build --host_java_toolchain=@remote_java_tools_linux//:toolchain_jdk_15 build --javabase=@openjdk15_linux_archive//:runtime build --host_javabase=@openjdk15_linux_archive//:runtime ``` Create Java 15 example class file: ``` public class Javac15Example { static String textBlock = """ Hello, World """; public static void main(String[] args) { System.out.println(textBlock); } } ``` Add Bazel file to build Java 15 syntax class: ``` java_binary( name = "Javac15Example", srcs = ["Javac15Example.java"], main_class = "Javac15Example", ) ``` Test that it works as expected: ``` $ bazel run java:Javac15Example INFO: Analyzed target //java:Javac15Example (1 packages loaded, 2 targets configured). INFO: Found 1 target... INFO: From Building java/Javac15Example.jar (1 source file): Target //java:Javac15Example up-to-date: bazel-bin/java/Javac15Example.jar bazel-bin/java/Javac15Example INFO: Elapsed time: 1.502s, Critical Path: 1.30s INFO: 1 process: 1 worker. INFO: Build completed successfully, 2 total actions INFO: Build completed successfully, 2 total actions Hello, World ``` Closes #12168. PiperOrigin-RevId: 335931179
Closes #11017.
Test Plan:
features: