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

Replace javax.annotation.Generated with custom gRPC annotation #9179

Open
ejona86 opened this issue May 17, 2022 · 31 comments
Open

Replace javax.annotation.Generated with custom gRPC annotation #9179

ejona86 opened this issue May 17, 2022 · 31 comments

Comments

@ejona86
Copy link
Member

ejona86 commented May 17, 2022

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.

@mkjensen
Copy link

mkjensen commented Oct 18, 2022

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.

@slinkydeveloper
Copy link

Would it be possible to have an option to skip using javax.annotation as described by #9153?

@hutchig
Copy link

hutchig commented Dec 1, 2022

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 #9153 but it is locked.)

@gkwan-ibm
Copy link

IMO, using eclipse-transformer is a workaround. gRPC should support EE9/EE10 too.

@ejona86
Copy link
Member Author

ejona86 commented Dec 2, 2022

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.

@hutchig
Copy link

hutchig commented Dec 2, 2022

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.

@xfl03
Copy link

xfl03 commented Dec 11, 2022

A workaroud for Gradle, add it to dependencies in build.gradle

implementation 'javax.annotation:javax.annotation-api:1.3.2'

@jorgerod
Copy link

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

@ejona86
Copy link
Member Author

ejona86 commented Jan 10, 2023

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.

@gkwan-ibm
Copy link

gkwan-ibm commented Jan 10, 2023

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

@ejona86
Copy link
Member Author

ejona86 commented Jan 10, 2023

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.

@oburgosm
Copy link

oburgosm commented Mar 7, 2023

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.

@ejona86
Copy link
Member Author

ejona86 commented Mar 20, 2023

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.

@Cypher01
Copy link

Cypher01 commented May 9, 2023

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

@trajano
Copy link

trajano commented Aug 6, 2023

Please ensure that this is not a SOURCE level annotation so that Jacoco can detect it.

@bkorcsmar
Copy link

bkorcsmar commented Nov 17, 2023

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?

alexanderankin added a commit to alexanderankin/grpc-java that referenced this issue Feb 16, 2024
@ejona86 ejona86 modified the milestones: Unscheduled, 1.63 Feb 20, 2024
kou pushed a commit to apache/arrow that referenced this issue Apr 2, 2024
)

### Rationale for this change

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

### What changes are included in this PR?

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

### Are these changes tested?

#40901

### Are there any user-facing changes?

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: #40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
lriggs pushed a commit to lriggs/arrow that referenced this issue Apr 2, 2024
apache#40904)

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: apache#40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@ejona86 ejona86 reopened this Apr 3, 2024
@ejona86 ejona86 modified the milestones: 1.63, Unscheduled Apr 3, 2024
lriggs pushed a commit to lriggs/arrow that referenced this issue Apr 4, 2024
apache#40904)

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: apache#40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@alexanderankin
Copy link
Contributor

alexanderankin commented Apr 19, 2024

examples of how to omit the javax annotation here #10927 (comment)

tolleybot pushed a commit to tmct/arrow that referenced this issue May 2, 2024
apache#40904)

### Rationale for this change

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

### What changes are included in this PR?

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

### Are these changes tested?

apache#40901

### Are there any user-facing changes?

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: apache#40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
tolleybot pushed a commit to tmct/arrow that referenced this issue May 4, 2024
apache#40904)

### Rationale for this change

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

### What changes are included in this PR?

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

### Are these changes tested?

apache#40901

### Are there any user-facing changes?

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: apache#40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
rok pushed a commit to tmct/arrow that referenced this issue May 8, 2024
apache#40904)

### Rationale for this change

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

### What changes are included in this PR?

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

### Are these changes tested?

apache#40901

### Are there any user-facing changes?

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: apache#40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
rok pushed a commit to tmct/arrow that referenced this issue May 8, 2024
apache#40904)

### Rationale for this change

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

### What changes are included in this PR?

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

### Are these changes tested?

apache#40901

### Are there any user-facing changes?

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: apache#40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@Hc747
Copy link

Hc747 commented May 16, 2024

Please consider the solution provided in: #11215

This has been an issue since Java 9 (7 years ago) as per #3633. The above proposal is a pragmatic, optional, opt-in and low-risk solution.

vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
apache#40904)

### Rationale for this change

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

### What changes are included in this PR?

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

### Are these changes tested?

apache#40901

### Are there any user-facing changes?

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: apache#40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@anujpradhaan
Copy link

Summary of solutions for this problem:

  1. Replace the javax annotation with jakarta. Replace javax.annotation.Generated with custom gRPC annotation #9179 (comment) Useful for Spring boot 3 and if you want to keep the generated annotation.
  2. Use the Javax annotation dependency. Replace javax.annotation.Generated with custom gRPC annotation #9179 (comment).
  3. We can ignore the Generated annotation all together. We can pass a configuration parameter to the plugin. Explained implement ability to skip generation of javax annotation #10927 (comment) and implement ability to skip generation of javax annotation #10927 (comment).

We ended up choosing the 3rd solution as we didn't want a javax dependency in our Spring boot 3 project.

<plugin>
        <groupId>org.xolstice.maven.plugins</groupId>
        <artifactId>protobuf-maven-plugin</artifactId>
        <version>0.6.1</version>
        <configuration>
          <protocArtifact>com.google.protobuf:protoc:3.25.3:exe:${os.detected.classifier}</protocArtifact>
          <pluginId>grpc-java</pluginId>
          <pluginArtifact>io.grpc:protoc-gen-grpc-java:1.64.0:exe:${os.detected.classifier}</pluginArtifact>
        </configuration>
        <executions>
          <execution>
            <configuration>
              <pluginParameter>
                @generated=omit
              </pluginParameter>
            </configuration>
            <goals>
              <goal>compile</goal>
              <goal>compile-custom</goal>
            </goals>
          </execution>
        </executions>
      </plugin>

Was our config change in Maven pom.

#9179 is an open issue about replacing the javax, jakarta annotation with some custom one.

@stickfigure
Copy link

I'd just like to respond to the question of "what's the harm of having javax.annotations on the classpath?"

I have a million and a half lines of Java. I have dozens of engineers of various levels of professional development, including interns. If someone autocompletes PostConstruct and gets the wrong import, the method doesn't get called. Maybe it gets caught by tests, maybe it gets caught in review, maybe it introduces a serious bug. Putting nonfunctional javax.annotations on the classpath is dangerous.

@trajano
Copy link

trajano commented Jun 25, 2024

Considering how old this is, I wonder if there's a fork that already the functionality that we all can jump onto.

lriggs pushed a commit to lriggs/arrow that referenced this issue Sep 6, 2024
apache#40904)

Remove runtime dependencies on [Category B](https://apache.org/legal/resolved.html#category-b) dependencies.

- logback: move to test-only
- eclipse: remove dependency, vendor the Netty implementation we originally used

I wanted to remove javax.annotation.Generated but gRPC doesn't yet let us do that (grpc/grpc-java#9179). That's ~okay though since effectively that's a build only dependency.

No.

**This PR contains a "Critical Fix".** License issues do not cause runtime issues but are important as an Apache project.
* GitHub Issue: apache#40896

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests