grpc-java
grpc-java copied to clipboard
Replace javax.annotation.Generated with custom gRPC annotation
Since Java 9 dropped javax.annotation.Generated users have had to explicitly depend on a dep (typically Tomcat's annotation API) to get the annotation. It'd be nice not to need that the extra dep.
But even more important is that the removal of Generated from Java 9 upset the ecosystem as a whole and fragmented it so badly that I believe many tools are no longer assuming they can predict which annotation will be used and are heuristics like "is the annotation named 'Generated'" to determine whether they should it as generated code.
If we do an investigation and find that indeed all the tools we may care about (linters, static analyzers, IDEs) are observing Generated annotations in any package, then we can make our own io.grpc.Generated. Unfortunately, I expect the io.grpc.GrpcGenerated annotation may not suffice because its name is not exactly "Generated." We'll also need to figure out what retention it needs.
Tools to investigate (off the top of my head): Error Prone, IntelliJ, Eclipse, Android linter, Find Bugs, Checkstyle. The tools to investigate should be those that may be used by gRPC users, not just those directly used by gRPC maintainers.
javax.annotation.processing.Generated is not a relevant replacement; see #3633. I highly doubt jakarta.annotation.Generated would ever be appropriate, even with it being the new home for the annotation; it'd only have an advantage if Nullable goes that way as well, which seems unlikely. But that'd also take investigation of Kotlin and other null-caring tools.
FYI, I just tried upgrading a project from Spring Boot 2.7.4 to 3.0.0-M5.
One of the first issues I run into is:
[ERROR] <snip>/target/generated-sources/protobuf/grpc-java/<snip>ServiceGrpc.java:[12,17] error: cannot find symbol
[ERROR] symbol: class Generated
[ERROR] location: package javax.annotation
Spring Boot 3 (Spring Framework 6) is moving to Jakarta EE 9 APIs (jakarta.) instead of EE 8 (javax.).
Depending on javax.annotation:javax.annotation-api:1.3.2 seems to be a workaround.
Would it be possible to have an option to skip using javax.annotation as described by #9153?
Our gRPC users in OpenLiberty are affected by this too when using EE9/EE10. The recommended solution is not to bundle an extra compile time [ADDED gh] dependency (why would users want to use Tomcat for this when they are used to OpenLiberty) but to use https://openliberty.io/blog/2021/03/17/eclipse-transformer.html (I would have added this to https://github.com/grpc/grpc-java/issues/9153 but it is locked.)
IMO, using eclipse-transformer is a workaround. gRPC should support EE9/EE10 too.
The recommended solution is not to bundle an extra dependency
Nobody needs to bundle an extra dependency. annotations-api is a compile-only dependency.
why would users want to use Tomcat for this when they are used to OpenLiberty
Nobody is depending on Tomcat. It is the annotations-api, which has no actual logic inside.
I was aware of the retention policy of the annotation, I was referring to #9153 where you say "Our README recommends a compile-time dependency on org.apache.tomcat:annotations-api:6.0.53", by users I meant developers who would need something like the above reference in their build, apologies for my loose language causing any loss of meaning.
A workaroud for Gradle, add it to dependencies in build.gradle:
implementation 'javax.annotation:javax.annotation-api:1.3.2'
Hello @ejona86
Is there any news on this issue? With the upgrade to Spring Boot 3, this is something blocking and will affect a lot of projects.
It would be nice if this annotation could be configurable or even to be able to skip this annotation.
Thank you very much
I don't really see any new problem here. "Add the dependency like everyone else." You simply were able to side-step the dependency before. I do not recommend bundling; a project that compiles generated code is the one that should have the compile-only dependency.
Nothing is broken. Everyone can build. Nobody is blocked.
Nothing has changed in my original issue description: before we make any change we need to understand what tools are able to consume. Anyone is free to audit parts of the tooling ecosystem and reported their findings.
As an application developer, I am developing a JavaEE 9 application and using gRPC. I used this maven project
pom.xml to generate and build a gRPC library by install goal but I got following error:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project systemproto: Compilation failure
[ERROR] /Users/gkwan/tasks/.../systemproto/target/generated-sources/protobuf/grpc-java/.../systemproto/SystemServiceGrpc.java:[10,18] cannot find symbol
[ERROR] symbol: class Generated
[ERROR] location: package javax.annotation
It is not a solution to use JEE 8 to build this library because it will not be compatible to my JEE9 application.
That's why I made this comment
It is not a solution to use JEE 8 to build this library because it will not be compatible to my JEE9 application.
Compatible? If you add the snippet from the README:
<dependency> <!-- necessary for Java 9+ -->
<groupId>org.apache.tomcat</groupId>
<artifactId>annotations-api</artifactId>
<version>6.0.53</version>
<scope>provided</scope>
</dependency>
What error do you get? My understanding is that should work without issue.
It is not a solution to use JEE 8 to build this library because it will not be compatible to my JEE9 application.
Compatible? If you add the snippet from the README:
<dependency> <!-- necessary for Java 9+ --> <groupId>org.apache.tomcat</groupId> <artifactId>annotations-api</artifactId> <version>6.0.53</version> <scope>provided</scope> </dependency>What error do you get? My understanding is that should work without issue.
I don't understand why I need to add an artifact from 2017, that was renamed, to get my java-grpc application working. Furthermore when javax no longer exists. If you don't have inconvenience, we can contribute to solve this issue.
It appears ErrorProne accepts any annotation, as long as the name is "Generated": https://github.com/google/error-prone/blob/47c2b05c6a422c2fc0488dddfcfad56513c9bc04/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java#L1557
@oburgosm, we'd be happy for you to contribute. But that contribution would need to at least start with investigating what annotations the tooling ecosystem would accept, as mentioned when this issue was opened.
I use the following workaround to not require to include the useless javax.annotation-api dependency.
My project is based on Spring Boot 3.
I use Maven as my build system.
I use the protobuf-maven-plugin to generate Java code from protobuf
The code generation runs during the generate-sources phase. Before the code is compiled (compile phase), I edit the generated code, replacing javax by jakarta. The process-sources phase is the perfect fit for this task.
For replacement, I use the find-and-replace-maven-plugin, which does exactly what I need.
For all Maven users, simply add this to your plugins and you are good to go.
<plugin>
<groupId>io.github.floverfelt</groupId>
<artifactId>find-and-replace-maven-plugin</artifactId>
<executions>
<execution>
<id>exec</id>
<phase>process-sources</phase>
<goals>
<goal>find-and-replace</goal>
</goals>
<configuration>
<replacementType>file-contents</replacementType>
<baseDir>target/generated-sources/protobuf/</baseDir>
<findRegex>javax</findRegex>
<replaceValue>jakarta</replaceValue>
<recursive>true</recursive>
<fileMask>.java</fileMask>
</configuration>
</execution>
</executions>
</plugin>
If you want to use the Tomcat annotation instead, adapt the <findRegex> and <replaceValue> configurations to your needs.
hth
Please ensure that this is not a SOURCE level annotation so that Jacoco can detect it.
Lombok is an example of a widely used library in java. In the latest version 1.18.30 the user can configure which Generated annotation should be added, or if it should not be added at all. The default behavior is no Generated annotation is added.
Key : lombok.addJakartaGeneratedAnnotation Type: boolean
Generate @jakarta.annotation.Generated on all generated code (default: false).
Examples: clear lombok.addJakartaGeneratedAnnotation lombok.addJakartaGeneratedAnnotation = [false | true]
Key : lombok.addJavaxGeneratedAnnotation Type: boolean
Generate @javax.annotation.Generated on all generated code (default: follow lombok.addGeneratedAnnotation).
Examples: clear lombok.addJavaxGeneratedAnnotation lombok.addJavaxGeneratedAnnotation = [false | true]
@ejona86 as the developer could you possible consider to listen to the request of the users of the library that you are developing and make the Generated annotation configurable similar to Lombok?
@BalintKorcsmar, this is an open source project and I think I've already laid out what can be done here. If you are interested, please contribute.
The problem is "I don't want to add a compile-only dependency." There's little actual harm here; it is a minor inconvenience which non-servlet users have already been doing. I agree it'd be nicer not to need the dependency. The approach I've outlined benefits all users without them doing anything, not just jakarta users who would have to read extra docs to set an option.
This is all it should take - surprised there are no PRs for this issue?
diff which seems to allow for customization of the protoc generated service code
diff --git a/compiler/src/java_plugin/cpp/java_generator.cpp b/compiler/src/java_plugin/cpp/java_generator.cpp
index cf8445035..68cea8eb7 100644
--- a/compiler/src/java_plugin/cpp/java_generator.cpp
+++ b/compiler/src/java_plugin/cpp/java_generator.cpp
@@ -1217,7 +1217,8 @@ void PrintImports(Printer* p) {
void GenerateService(const ServiceDescriptor* service,
protobuf::io::ZeroCopyOutputStream* out,
ProtoFlavor flavor,
- bool disable_version) {
+ bool disable_version,
+ bool jakarta_over_javax) {
// All non-generated classes must be referred by fully qualified names to
// avoid collision with generated classes.
std::map<std::string, std::string> vars;
@@ -1249,7 +1250,9 @@ void GenerateService(const ServiceDescriptor* service,
vars["MethodDescriptor"] = "io.grpc.MethodDescriptor";
vars["StreamObserver"] = "io.grpc.stub.StreamObserver";
vars["Iterator"] = "java.util.Iterator";
- vars["Generated"] = "javax.annotation.Generated";
+ vars["Generated"] = jakarta_over_javax
+ ? "javax.annotation.Generated"
+ : "jakarta.annotation.Generated";
vars["GrpcGenerated"] = "io.grpc.stub.annotations.GrpcGenerated";
vars["ListenableFuture"] =
"com.google.common.util.concurrent.ListenableFuture";
diff --git a/compiler/src/java_plugin/cpp/java_generator.h b/compiler/src/java_plugin/cpp/java_generator.h
index f3ec49f8e..4e1f78c9b 100644
--- a/compiler/src/java_plugin/cpp/java_generator.h
+++ b/compiler/src/java_plugin/cpp/java_generator.h
@@ -68,7 +68,8 @@ std::string ServiceClassName(const impl::protobuf::ServiceDescriptor* service);
void GenerateService(const impl::protobuf::ServiceDescriptor* service,
impl::protobuf::io::ZeroCopyOutputStream* out,
ProtoFlavor flavor,
- bool disable_version);
+ bool disable_version,
+ bool jakarta_over_javax);
} // namespace java_grpc_generator
diff --git a/compiler/src/java_plugin/cpp/java_plugin.cpp b/compiler/src/java_plugin/cpp/java_plugin.cpp
index 2eed9d260..9e9a73022 100644
--- a/compiler/src/java_plugin/cpp/java_plugin.cpp
+++ b/compiler/src/java_plugin/cpp/java_plugin.cpp
@@ -59,12 +59,15 @@ class JavaGrpcGenerator : public protobuf::compiler::CodeGenerator {
java_grpc_generator::ProtoFlavor flavor =
java_grpc_generator::ProtoFlavor::NORMAL;
+ bool jakarta_over_javax = false;
bool disable_version = false;
for (size_t i = 0; i < options.size(); i++) {
if (options[i].first == "lite") {
flavor = java_grpc_generator::ProtoFlavor::LITE;
} else if (options[i].first == "noversion") {
disable_version = true;
+ } else if (options[i].first == "jakarta") {
+ jakarta_over_javax = true;
}
}
@@ -77,7 +80,7 @@ class JavaGrpcGenerator : public protobuf::compiler::CodeGenerator {
std::unique_ptr<protobuf::io::ZeroCopyOutputStream> output(
context->Open(filename));
java_grpc_generator::GenerateService(
- service, output.get(), flavor, disable_version);
+ service, output.get(), flavor, disable_version, jakarta_over_javax);
}
return true;
}
When using the following replacement in gradle:
dependencies {
modules {
module("javax.annotation:javax.annotation-api") {
replacedBy("jakarta.annotation:jakarta.annotation-api", "Javax is replaced by Jakarta")
}
}
}
Defining a compile-only dependency like compileOnly("javax.annotation:javax.annotation-api:1.3.2") simply DOES NOT work.
This rule is defined by our company plugin, so I can't just remove it easily.
I am also surprised that three years after the renaming was decided and two years after it was carried out, there is still no solution in place for such an important project.
I hope that #10786 will be merged soon; until then, it seems we are blocked.
"I highly doubt jakarta.annotation.Generated would ever be appropriate, even with it being the new home for the annotation" is simply the wrong point of view, the solution is to reconsider for the sake of the pain your users are going through, like with empathy or something, maybe.
@tristanlins, that would be a problem. It seems completely broken because the two packages are completely unrelated (yes, they are mirrors of each other, but from the JVM's perspective they have no relationship). Is there other code rewriting combined with that such that the replacement makes sense? Or is that simply a hack to disallow javax.annotation:javax.annotation-api?
@alexanderankin, @tristanlins is the first person here to describe a problem caused as far as I can tell. The most common complaint is a variation of "I don't like it."
Ok I am glad that we are making progress on communicating the issue at least then. I think this generally is how it progresses, first there are complaints, then there are blockers.
On Thu, Feb 15, 2024, 10:58 AM Eric Anderson @.***> wrote:
@tristanlins https://github.com/tristanlins, that would be a problem. It seems completely broken because the two packages are completely unrelated (yes, they are mirrors of each other, but from the JVM's perspective they have no relationship). Is there other code rewriting combined with that such that the replacement makes sense? Or is that simply a hack to disallow javax.annotation:javax.annotation-api?
@alexanderankin https://github.com/alexanderankin, @tristanlins https://github.com/tristanlins is the first person here to describe a problem caused as far as I can tell. The most common complaint is a variation of "I don't like it."
— Reply to this email directly, view it on GitHub https://github.com/grpc/grpc-java/issues/9179#issuecomment-1946396073, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACECGJGYFRE74PPZVUFU4FTYTYWD7AVCNFSM5WFIC3NKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJUGYZTSNRQG4ZQ . You are receiving this because you were mentioned.Message ID: @.***>
It seems completely broken because the two packages are completely unrelated (yes, they are mirrors of each other, but from the JVM's perspective they have no relationship)
This is not correct, it only applies when comparing javax.annotation-api version 1 and jakarta.annotation-api version 2. However, there is a jakarta.annotation-api version 1, which is an "identical" mirror of javax.annotation-api version 1. The issue arises when you have javax.annotation-api and jakarta.annotation-api, both in version 1.3, (as transitive dependencies) in the classpath. This can lead to sporadic ClassCastExceptions with the message "Cannot cast X to X," especially during annotation scanning.
The problem is that javax.annotation-api is not a compile-only dependency. Some annotations have runtime retention, which requires the class to be found in the classpath at runtime. Examples of such annotations are @Priority, @PostConstruct, and @PreDestroy.
We have not determined exactly why the JVM (or in our case, Tomcat) loaded the same class from the javax.annotation-api JAR sometimes and from the jakarta.annotation-api JAR other times. We only know that this situation can occur.
@ejona86 I asked a colleague who analyzed it back then. The ClassCastExceptions occur primarily when the javax.annotation-api JAR is present in the Tomcat Common Loader, but a web app has the jakarta.annotation-api JAR in its dependencies. In this scenario, the javax.annotation-api class is loaded by the Common ClassLoader, and the jakarta.annotation-api class is loaded by the Webapp ClassLoader.
This led to sporadic ClassCastExceptions during Tomcat startup and Tomcat's annotation scanning. Although the classes have the same names, they are NOT compatible with each other.
Our only option was to ensure that only one of the two JARs is used. By the way, our Gradle plugin also enforces that the javax.activation-api and jakarta.activation-api are always set to "provided" for web apps, and thus NOT included in the web app WAR.
This is not correct, it only applies when comparing javax.annotation-api version 1 and jakarta.annotation-api version 2.
Ah, so this is because "Jakarta completely changed the API without changing the Maven package name." It's that jakarta 1 and 2 actually had no relation.
This can lead to sporadic ClassCastExceptions with the message "Cannot cast X to X,"
Yeah, you need only one in the classpath at a time. The provided jakarta and compileOnly javax would interact with each other just fine. But jakarta 1 vs 2 means you can't easily prevent issues.
This issue is still around mostly because nobody's done investigation into the Generated-observing tools that mostly disable themselves for generated code. Instead of providing an option to "use jakarta" how about an option to "disable the generated annotation"? Then (if/)as tools cause problems there is a concrete thing to look into.
Although I just looked into:
- ErrorProne. It checks the simple name is exactly "Generated". So
GrpcGeneratedis not sufficient. But we could make a new annotation - Checkstyle. Looks like there isn't a prebuilt filter based on annotations at all. Looks like for gRPC we are filtering based on generated file name since the path on disk of generated files is different than normal source. This makes some sense, because it is purely textual processing.
- Intellij/Eclipse. They might not care about generated vs not. A quick search didn't show anything, but there's a lot of noise about having them generate code. I'd have imagined it'd be mentioned on the doc about managing inspections
- Android Lint. Has
isCheckGeneratedSourcesoption. Looks to be file-location based - FindBugs. I see filtering by file location. I don't see any specific annotation-based rules
Maybe ErrorProne is the only thing that cares. It isn't an annotation processor and instead integrates itself into the javac processing directly. Few tools do that. Protobuf isn't using any annotation that I can tell, at least going back to 3.8.0.
examples of how to omit the javax annotation here https://github.com/grpc/grpc-java/pull/10927#issuecomment-2066632469
Please consider the solution provided in: https://github.com/grpc/grpc-java/pull/11215
This has been an issue since Java 9 (7 years ago) as per https://github.com/grpc/grpc-java/issues/3633. The above proposal is a pragmatic, optional, opt-in and low-risk solution.