-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Bazel: Fix compilation in Java 9 #4325
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
Fixes: grpc#3633. Test Plan: On most recent Bazel version run: $ bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk \ build --javacopt='--release 9' \ --java_toolchain=@bazel_tools//tools/jdk:toolchain_jdk9 \ examples:helloworld_java_grpc
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.
Just need to tweak the version of javax.annotation-api
repositories.bzl
Outdated
native.http_file( | ||
name = "javax_annotation_api", | ||
sha256 = "e04ba5195bcd555dc95650f7cc614d151e4bcd52d29a10b8aa2197f3ab89ab9b", | ||
urls = ["http://central.maven.org/maven2/javax/annotation/javax.annotation-api/1.3.2/javax.annotation-api-1.3.2.jar"], |
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.
We use version 1.2 in build.gradle and like to keep the versions in sync. 1.3.2 also doesn't support Java 7 (although I honestly don't know if Bazel supports Java 7). Let's change this to version 1.2 (unless there's some problem with doing so).
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.
core/BUILD.bazel
Outdated
# javax.annotation.Generated is not included in the default root modules in 9, | ||
# see: http://openjdk.java.net/jeps/320. | ||
java_import( | ||
name = "javax_annotation", |
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.
Technically, having this in core seems a bad idea, but we don't have a third_party/ yet, so this seems good enough for now.
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.
Ack.
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.
Maybe in stub
is more appropriate than core
, although not as good as third_party
.
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 moved the new javax_annotation
rule to stub package instead of core. Done.
Thank you for this! |
Use the same version as in gradle build.
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.
LGTM
Fixes: #3633.
Test Plan:
On most recent Bazel version run: