-
Notifications
You must be signed in to change notification settings - Fork 15.5k
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
Bazel: Support building with Java 9 #4257
Conversation
Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure. |
1 similar comment
Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure. |
Unfortunately, it still doesn't work:
Any clue, why passing: |
I wonder, if sun.misc.Unsafe can be replaced with https://github.com/jnr/jffi/blob/master/src/main/java/com/kenai/jffi/MemoryIO.java, to not mess around with internal/unsupported JDK API altogether. Another resource: https://blog.dripstat.com/removal-of-sun-misc-unsafe-a-disaster-in-the-making . |
Never mind, I missed that
I've updated the commit message correspondingly. |
BUILD
Outdated
@@ -608,7 +619,10 @@ java_library( | |||
]) + [ | |||
":gen_well_known_protos_java", | |||
], | |||
javacopts = ["-source 7", "-target 7"], | |||
javacopts = select({ | |||
"//:jdk9": ["--add-modules=jdk.unsupported", "-target 9"], |
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.
Can you add the jdk.unsupported flag but keep "-source 7" and "-target 7"? We don't want to introduce Java 8 or Java 9 only features regardless which javac is used.
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 removed explicit "-target 9" version, but --add-modules
only works on java 9 release. That why to work on java 9, the Bazel mus be invoked like this:
$ bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk build --javacopt='--release 9' --java_toolchain=@bazel_tools//tools/jdk:toolchain_jdk9 :protobuf_java
I added the command to the commit message.
But the good news is: this patch is backwards compatible. It still works, on Java 7,8,9 and uses "-source 7", "-target 7"
option, if the user doesn't switch to java 9 toolcain.
Thanks! I'll merge after the tests pass. |
Thanks. Would you mind to create a new release in the next days? |
Fixes: protocolbuffers#4256. Bazel@HEAD supports Java 9. The current code has one single issue with Java 9 compliance: the usage of sun.misc package. We add jdk.unsupported module with --add-modules compiler option for now. Long term, the usage of non public API should be avoided. To build with Java 9, build custom bazel version and issue: $ bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk build \ --javacopt='--release 9' \ --java_toolchain=@bazel_tools//tools/jdk:toolchain_jdk9 \ :protobuf_java
I only improved the commit message a bit in the latest push. |
Bazel: Support building with Java 9
Fixes: #4256.
Bazel@HEAD supports Java 9.
The current code has one single issue with Java 9 compliance: the usage
of sun.misc package. We add sun.misc module with --add-modules compiler
option for now. Long term, the usage of non poblic API should be
avoided.
To build (or test) with Java 9, build custom bazel version and issue:
$ bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk build
--javacopt='--release 9'
--java_toolchain=@bazel_tools//tools/jdk:toolchain_jdk9
:protobuf_java