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

Java 9: cannot find symbol javax.annotation.Generated #3633

Closed
fcnn opened this issue Oct 30, 2017 · 24 comments
Closed

Java 9: cannot find symbol javax.annotation.Generated #3633

fcnn opened this issue Oct 30, 2017 · 24 comments
Milestone

Comments

@fcnn
Copy link

fcnn commented Oct 30, 2017

/root/grpc-kotlin-hello/proto/src/main/java/io/grpc/examples/helloworld/GreeterGrpc.java:23: error: cannot find symbol
@javax.annotation.Generated(
^
symbol: class Generated
location: package javax.annotation
1 error

FAILURE: Build failed with an exception.

@ejona86
Copy link
Member

ejona86 commented Oct 30, 2017

That class exists in Java 9: https://docs.oracle.com/javase/8/docs/api/javax/annotation/Generated.html . It seems like the problem is with an example that isn't maintained here.

@ejona86 ejona86 changed the title java 9 not supported yet? Java 9: cannot find symbol javax.annotation.Generated Oct 30, 2017
ejona86 added a commit to ejona86/grpc-java that referenced this issue Oct 30, 2017
While this fixes a Gradle-caused failure on Java 9, it is still failing
due to Generated annotations as seen in grpc#3633.

Fixes grpc#3632
@ejona86
Copy link
Member

ejona86 commented Oct 30, 2017

Okay... I've now run into this as well for our example after #3637.

@ejona86 ejona86 added this to the Next milestone Oct 30, 2017
@ejona86
Copy link
Member

ejona86 commented Oct 30, 2017

For gRPC's build, after #3633 I don't see a problem with @Generated for :compileJava. But I do see it for :javadoc. But in the example @Generated is broken in :compileJava, even if depending on a grpc dep that built fine (e.g., grpc-testing-proto, but installed using Java 8). :-/

@ejona86
Copy link
Member

ejona86 commented Oct 30, 2017

@onesea, I'd suggest adding a dependency on javax.annotation:javax.annotation-api for the moment. It's possible to pass --add-modules java.xml.ws.annotation, but it seems it might cause more problems than it solves.

ejona86 added a commit that referenced this issue Oct 30, 2017
While this fixes a Gradle-caused failure on Java 9, it is still failing
due to Generated annotations as seen in #3633.

Fixes #3632
@davido
Copy link
Contributor

davido commented Mar 8, 2018

I run into the same problem while working on Bazel build with Java 9: bazelbuild/bazel#4709.

@davido
Copy link
Contributor

davido commented Mar 8, 2018

The way it was fixed in other java based libraries, like auto-common, auto-value and dagger, is to dynamically select the right annotation to use:

  • Java 9: javax.annotation.processing.Generated
  • Java 8: javax.annotation.Generated

See e.g.: google/auto@f04406c, google/dagger#882.

@ejona86
Copy link
Member

ejona86 commented Mar 8, 2018

@davido, dynamic selection doesn't really seem to be a solution except for annotation processors.

@davido
Copy link
Contributor

davido commented Mar 8, 2018

@ejona86 The other two options would be to detect the Java runtime or pass explicit option to gRpc compiler, a lá: --release 9 or similar?

@davido
Copy link
Contributor

davido commented Mar 8, 2018

I was able to patch gRpc compiler in Bazel tree, with this one liner:

diff --git a/third_party/grpc/compiler/src/java_plugin/cpp/java_generator.cpp b/third_party/grpc/compiler/src/java_plugin/cpp/java_generator.cpp
index 7cb01238ac..3264f15ecf 100644
--- a/third_party/grpc/compiler/src/java_plugin/cpp/java_generator.cpp
+++ b/third_party/grpc/compiler/src/java_plugin/cpp/java_generator.cpp
@@ -1203,7 +1203,7 @@ void GenerateService(const ServiceDescriptor* service,
   vars["NanoUtils"] = "io.grpc.protobuf.nano.NanoUtils";
   vars["StreamObserver"] = "io.grpc.stub.StreamObserver";
   vars["Iterator"] = "java.util.Iterator";
-  vars["Generated"] = "javax.annotation.Generated";
+  vars["Generated"] = "javax.annotation.processing.Generated";
   vars["ListenableFuture"] =
       "com.google.common.util.concurrent.ListenableFuture";
   vars["ExperimentalApi"] = "io.grpc.ExperimentalApi";

With this, the generated source is:

/**
 */
@javax.annotation.processing.Generated(
    value = "by gRPC proto compiler",
    comments = "Source: src/main/protobuf/command_server.proto")
public final class CommandServerGrpc {

  private CommandServerGrpc() {}
[...]

And the Bazel build on Java 9 just works:

  $ bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk build --javacopt='--release 9' --java_toolchain=@bazel_tools//tools/jdk:toolchain_jdk9 src:bazel
[...]
INFO: From Compiling Java headers src/main/java/com/google/devtools/build/lib/worker/libworker-hjar.jar (16 files):
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.google.protobuf.UnsafeUtil (file:/home/davido/.cache/bazel/_bazel_davido/install/5906941a530f40a3f4749e5cfd148312/_embedded_binaries/embedded_tools/tools/jdk/turbine_deploy.jar) to field java.nio.Buffer.address
WARNING: Please consider reporting this to the maintainers of com.google.protobuf.UnsafeUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
INFO: From Building src/main/java/com/google/devtools/build/lib/libbazel-main-class.jar (4 source files) and running annotation processors (OptionProcessor):
warning: Supported source version 'RELEASE_8' from annotation processor 'com.google.devtools.common.options.processor.OptionProcessor' less than -source '1.9'
Target //src:bazel up-to-date:
  bazel-bin/src/bazel
INFO: Elapsed time: 149.130s, Critical Path: 111.80s
INFO: Build completed successfully, 1986 total actions

davido added a commit to davido/bazel that referenced this issue Mar 8, 2018
Fixes: bazelbuild#3410, bazelbuild#4709.

To build bazel with Java 9 issue:

  $ bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk build \
  --javacopt='--release 9' \
  --java_toolchain=@bazel_tools//tools/jdk:toolchain_jdk9 \
  src:bazel

This still doesn't work due to gRpc dependency on Java 8 specific
@javax.annotation.Generated annotation: [1].

[1] grpc/grpc-java#3633

Change-Id: Ic529e222344dc894e9d6b853a9551c7a19a52697
davido added a commit to davido/bazel that referenced this issue Mar 8, 2018
This is a workaround for: [1].

[1] grpc/grpc-java#3633

Change-Id: I07ebacaa400022f6ff3e2396cf236e93bef12554
@ejona86
Copy link
Member

ejona86 commented Mar 8, 2018

@davido, as I said in an earlier comment, I recommend you to depend on javax.annotation:javax.annotation-api if on Java 9.

@davido
Copy link
Contributor

davido commented Mar 8, 2018

@ejona86 Thanks, I noticed your commentm and I did it indeed for gerrit (before auto-common was fixed) and it worked. Still I wonder, if we could do some dynamic @Generated annotation selection in Bazel, based on Java runtime, e.g.: [1]. So basically, it could even be added upstream. I know that gRpc also supports other build tool chains, but we could start fixing Java 9 out of the box support for Bazel first?

[1] http://paste.openstack.org/show/695786

@ejona86
Copy link
Member

ejona86 commented Mar 9, 2018

@davido, I don't think the two annotations are interchangeable. At the very least javax.annotation.processing.Generated is in the java.compiler module and the package is explicitly for use in annotation processors. That doesn't sound like a good fit; there's no annotation processor involved here. I'd be more prone to remove the annotation completely.

javax.annotation.Generated is the smaller dependency and the one that other tools (like ErrorProne) are observing. It's just that its location has moved.

davido added a commit to davido/bazel that referenced this issue Mar 9, 2018
Fixes: [1].

[1] grpc/grpc-java#3633

Change-Id: I07ebacaa400022f6ff3e2396cf236e93bef12554
davido added a commit to davido/grpc-java that referenced this issue Mar 9, 2018
@zyxist
Copy link

zyxist commented Mar 9, 2018

Technically speaking, there is no statement in JDK API docs that java.compiler is a module solely for annotation processors. It rather contains stuff for every program that wants an access to the compiler.

At the same time, when I look at it right now, I see that this might be a design issue in Java API. From the usage perspective, the annotation for marking generated code has little to do with accessing compiler API.

@davido
Copy link
Contributor

davido commented Mar 9, 2018

@zyxist To move forward for now, I've uploaded a RFC PC and removed the problematic annotation completely, as was suggested by @ejona86.

@davido
Copy link
Contributor

davido commented Mar 9, 2018

One disadvantage to remove the @Generated annotation entirely, is that the code sanity tools, like Error Prone, would assume that the code was hand written and miss that it was generated.

@cushon, @eamonnmcmanus, @ronshapiro: Do you have any comments on this discussion as what would be the right course of action here?

@ronshapiro
Copy link

Adding the dependency to javax.annotation.Generated is likely to cause someone issues with Java 9's JPMS because the of the way the javax.annotation package was split across modules.

While it's true that the "new" version is in the "annotation processing" package, it is the canonical Generated annotation in Java 9. I think you could argue that javax.annotation.Generated itself was meant for to be "annotation processing" as well.

But taking a step back - both annotations are source level.

@cushon
Copy link
Contributor

cushon commented Mar 9, 2018

it is the canonical Generated annotation in Java 9

+1

The docs for @javax.annotation.processing.Generated say "The Generated annotation is used to mark source code that has been generated." It does not say it's only for use by annotation processors. Error Prone and any other tools that recognize the annotation should recognize both versions.

I do not recommend using javax.annotation:javax.annotation-api with Java 9. javax.annotation is part of the java.xml.ws.annotation module in 9, which is deprecated for removal. Using classes in that package is a source of trouble.

@ejona86
Copy link
Member

ejona86 commented Mar 9, 2018

The first-line description of package javax.annotation.processing's Javadoc:

Facilities for declaring annotation processors and for allowing annotation processors to communicate with an annotation processing tool environment.

While I do agree that it could be used for other things, pulling in a dependency on all of java.compiler seems an overkill for our use. For annotation processors, it wouldn't be any problem, because they'd be depending on java.compiler already.

javax.annotation is part of the java.xml.ws.annotation module in 9, which is deprecated for removal.

@cushon, everything I see for it isn't "deprecated and stop using it" it is "being removed; get it externally from the JDK". The Migration Documentation says it was removed because it was part of J2EE, not because it was bad to use.

It also explicitly says in the "upgrade paths" (none of which are "migrate to another api"):

Deploy the standalone version of the API (and implementation if needed) on the class path. Each of the Java EE APIs are standalone technologies with projects published in Maven Central.

Using classes in that package is a source of trouble.

Why do you say that? In my mind, a much bigger source of trouble is thinking that we must use the new annotation with Java 9+ and we must use the old annotation with Java pre-9. That's simply unacceptable.

@cushon
Copy link
Contributor

cushon commented Mar 9, 2018

pulling in a dependency on all of java.compiler seems an overkill

java.compiler is in the set of default root modules, everything that uses java.se already depends on it.

a much bigger source of trouble is thinking that we must use the new annotation with Java 9+ and we must use the old annotation with Java pre-9. That's simply unacceptable.

Why? That's the approach we've been using for other code generators, so far successfully. If you see specific problems with it I'm interested to know what they are?

@ejona86
Copy link
Member

ejona86 commented Mar 9, 2018

java.compiler is in the set of default root modules, everything that uses java.se already depends on it.

Everything outside of java.se is going away, isn't it? It seems someone using jlink would notice, and it seems Oracle is pushing jlink pretty hard, since they're trying to discontinue the self-contained JRE. Although I guess the user can make that a compile-only dependency.

That's the approach we've been using for other code generators

Aren't all the generators you're talking about annotation processors? Those are integrating with the JDK anyway. The new annotation seems like a convenience for annotation processors (as it can remove a dependency for them; and I would expect it helps prevent processors from stepping on each other's toes) and easy for such a processor to choose at runtime (it already has all the information it needs), transparent to the user.

In our case we don't know the Java version that will be compiling the code. We could add an option, but that seems no easier for the user than an additional dependency (which is still a pain) and I reject the idea that we should need to know and generate different code (but I don't reject that it may be easier for our users if we do so).

We've had a very strong policy of needing a "canonical" version of the generated code. There are no options. This was based on years of learning the folly of providing options with protobuf (and there's still lots of work to remove such options). Now, it's really only important that the class files are canonical. But at the same time I've not noticed any mention of a problem that is solved by moving to javax.annotation.processing.Generated (without introducing an equivalent problem).


I have found a parenthetical in JEP 320 mentioning migrating to javax.annotation.processing.Generated. With that in mind, I could understand potentially migrating to the Java 9 annotation after we require Java 9. But that would be years in the future.

Right now I feel I'm more likely to remove the Generated annotation entirely than swap to the Java 9 version. To date, I don't think it's gained us anything.

@cushon
Copy link
Contributor

cushon commented Mar 9, 2018

I guess the user can make that a compile-only dependency.

Right, since it's a source retention annotation it is not needed at runtime. Some build systems make this easier to configure than others. Realistically some users will probably end up including the dependency at runtime.

I reject the idea that we should need to know and generate different code

I agree that this change problematic. The decision to move the canonical @Generated annotation to a different package is a breaking API change, and the JDK has historically tried to avoid making breaking API changes. That said, I think the principled options are (1) stop using the API (i.e. stop emitting @Generated), or (2) find a way to detect which version the compilation is targeting and emit the appropriate version of the annotation. Finding a way to keep using the Java EE @Generated is possible, but it's not the one upstream is recommending.

But at the same time I've not noticed any mention of a problem that is solved by moving to javax.annotation.processing.Generated (without introducing an equivalent problem).

There are two parts to this: whether emitting @Generated at all solves problems, and whether using the javax.annotation.processing annotation vs. the javax.annotation annotation solves problems.

For the first part, I don't have a strong opinion. We've seen people rely on @Generated to avoid running static analysis on generated code. I don't remember other uses of it.

For the second part, relying on the java.xml.ws.annotation module for javax.annotation.Generated creates split-package issues with other non-modularized uses of javax.annotation like jsr305.jar. E.g.:

@javax.annotation.Nullable
@javax.annotation.Generated("")
class X {}

Compiling with java.xml.ws.annotation on the module path on jsr305.jar on the class path doesn't work:

javac --add-modules=java.xml.ws.annotation -cp jsr305.jar X.java
X.java:1: error: cannot find symbol
@javax.annotation.Nullable
                 ^
  symbol:   class Nullable
  location: package javax.annotation
1 error

Using -cp javax.annotation-api-1.3.2.jar instead of --add-modules=java.xml.ws.annotation works, but it may be hard to ensure that everyone compiling the generated code avoids that flag. Hacking jsr305.jar onto the module path also works (but I do not recommend it).

$ javac -cp javax.annotation-api-1.3.2.jar:jsr30
5.jar X.java
... ok
$ javac --add-modules=java.xml.ws.annotation --patch-module=java.xml.ws.annotation=jsr305.jar X.java
... ok

davido added a commit to davido/bazel that referenced this issue Mar 10, 2018
Fixes: [1].

[1] grpc/grpc-java#3633

Change-Id: I07ebacaa400022f6ff3e2396cf236e93bef12554
cushon added a commit to cushon/bazel that referenced this issue Mar 29, 2018
This library provides the @generated annotation, which is not available
by default in JDK 9 (see http://openjdk.java.net/jeps/320).

This will be used to work around grpc/grpc-java#3633.

Change-Id: I7d6d9a6d6c44fe23818e093c6ecc97f557dd5a3e
cushon added a commit to cushon/bazel that referenced this issue Apr 9, 2018
This library provides the @generated annotation, which is not available
by default in JDK 9 (see http://openjdk.java.net/jeps/320).

This will be used to work around grpc/grpc-java#3633.

Change-Id: I7d6d9a6d6c44fe23818e093c6ecc97f557dd5a3e
@davido
Copy link
Contributor

davido commented Apr 9, 2018

@ejona86 As suggested, we added dependency to bazel on javax.annotation and added this as dependency to gRPC in Bazel tool chain, in: [1]. However, it was pointed out in the review, that this problem is going to be fixed upstream? Just wondering, as you said in comment to this issue, that you rather prefer that gRPC users will fix that problem?

If you are planning to address that problem upstream, do you have any time frame? I'm asking because this issue is very last issue, for full JDK 9 support in Bazel.

@ejona86
Copy link
Member

ejona86 commented Apr 9, 2018

I'd have no problem adding the dependency to java_grpc_library. It's been done for Blaze internally, for instance. I simply didn't think about it when I fixed up gradle+Java 9.

What's the easiest way to test out Bazel+Java 9? It should be as easy as adding a line to added_deps. @davido, if you have a working Bazel+Java 9 environment, I'd be happy to accept a PR.

@davido
Copy link
Contributor

davido commented Apr 9, 2018

The easiest way to reproduce the breakage on Bazel+Java 9, is to run this command in grpc-java:

  $ 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
 [...]
Target //examples:helloworld_java_grpc up-to-date:
  bazel-bin/examples/libhelloworld_java_grpc.jar
INFO: Elapsed time: 8.263s, Critical Path: 2.72s
INFO: 217 processes: 212 remote cache hit, 1 linux-sandbox, 3 local, 1 worker.
INFO: Build completed successfully, 243 total actions

I'm working on a fix and will upload a PR in a moment.

davido added a commit to davido/grpc-java that referenced this issue Apr 9, 2018
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
bazel-io pushed a commit to bazelbuild/bazel that referenced this issue Apr 13, 2018
This library provides the @generated annotation, which is not available
by default in JDK 9 (see http://openjdk.java.net/jeps/320).

This will be used to work around grpc/grpc-java#3633.

Change-Id: I7d6d9a6d6c44fe23818e093c6ecc97f557dd5a3e
cushon added a commit to cushon/bazel that referenced this issue Apr 13, 2018
to work around grpc/grpc-java#3633.

Change-Id: I7623a3d2a26f91cd37cad3c3446c37ce6fbd0706
bazel-io pushed a commit to bazelbuild/bazel that referenced this issue Apr 16, 2018
to work around grpc/grpc-java#3633.

Change-Id: I7623a3d2a26f91cd37cad3c3446c37ce6fbd0706

Closes #5017.
ejona86 pushed a commit that referenced this issue Apr 26, 2018
Fixes: #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
@ejona86 ejona86 modified the milestones: Next, 1.13 May 15, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Sep 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants