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

compiler: add option @generated=jakarta #11215

Closed
wants to merge 2 commits into from
Closed

Conversation

Hc747
Copy link

@Hc747 Hc747 commented May 16, 2024

Add (optional) support for @jakarta.annotation.Generated while retaining existing functionality.

An extension of #11086

Copy link

linux-foundation-easycla bot commented May 16, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@ejona86
Copy link
Member

ejona86 commented May 16, 2024

As mentioned in #9179, we don't want a dependency on jakarta. The dependency was originally part of the JDK itself. Disable the annotation or take a look at #9179 (comment) to see about creating a new annotation in gRPC itself.

@Hc747
Copy link
Author

Hc747 commented May 16, 2024

@ejona86 this is an opt-in feature that would be at the discretion of users / consumers of the tool, right? The default behaviour (javax) is unchanged, as is the ability to opt-out of javax/jakarta entirely, and no direct dependency on jakarta is added to grpc-java itself.

We're using JDK 22, and the framework we use only supports jakarta, so prior to #11086, we had to add a dependency on javax in order to build our application.

@ejona86
Copy link
Member

ejona86 commented May 16, 2024

In what way does @generated=omit not work for you?

we had to add a dependency on javax in order to build our application.

That's literally what everyone else had to do since Java 9.

@Hc747
Copy link
Author

Hc747 commented May 16, 2024

In what way does @generated=omit not work for you?

@ejona86 In what way does @generated=jakarta or it's inclusion not work for you? I don't understand the zealotry towards this change.

That's literally what everyone else had to do since Java 9.

Great. Now why don't we modernise (as other frameworks and tooling have done) rather than perpetuating this indefinitely?

@ejona86
Copy link
Member

ejona86 commented May 16, 2024

Now why don't we modernise (as other frameworks and tooling have done) rather than perpetuating this indefinitely?

We were never trying to depend on J2EE. We were depending on the JRE. But then the JRE moved stuff into J2EE. And then J2EE was replaced with a new J2EE. We want to solve this by stopping depending on J2EE, not replace it with the new J2EE.

@Hc747
Copy link
Author

Hc747 commented May 16, 2024

We were never trying to depend on J2EE. We were depending on the JRE. But then the JRE moved stuff into J2EE. And then J2EE was replaced with a new J2EE. We want to solve this by stopping depending on J2EE, not replace it with the new J2EE.

The reality of the situation is that J2EE has been adopted as the defacto standard and is supported by major frameworks (Micronaut, Spring, Quarkus, etc). This pull request doesn't add a direct dependency on J2EE to grpc-java itself, but allows users to opt-in to generating code that supports it. It is a really straightforward and relatively harmless change, that is clearly desired by users.

https://www.eclipse.org/community/eclipse_newsletter/2019/may/javax.php

@panchenko
Copy link
Contributor

Is there a reason for keeping the @Deprecated annotation at all? Does it serve any purpose?
The protobuf-generated messages don't have any annotation.
Might be just change default of this option?

@ejona86
Copy link
Member

ejona86 commented May 17, 2024

allows users to opt-in to generating code that supports it.

"supports it"? How does this benefit J2EE users?

Is there a reason for keeping the @Deprecated annotation at all?

To my knowledge nothing is marked @Deprecated. It's just that the project is end-of-life. The point was tools looking for @Generated and changing their behavior. But tools exactly logic was the heart of #9179. See also #9179 (comment) , as I referenced above.

@Hc747
Copy link
Author

Hc747 commented May 18, 2024

@ejona86 Because it makes use of a framework that has long been considered the defacto standard (and is used as such). I don't understand the opposition to this pragmatic change - it adds no maintenance or support overhead and is beneficial to real-world use-cases and users. The funny/sad thing is that this has been an open issue since 2017/2022 and nothing has been done to remedy it. I don't understand why something so simple has been neglected for such a long time, as there doesn't seem to be a compelling reason for doing so. This feels like a scenario in which the pursuit of perfection is being used as the enemy of good enough.

@ejona86
Copy link
Member

ejona86 commented May 20, 2024

Because it makes use of a framework that has long been considered the defacto standard

In what way does this do that?

This feels like a scenario in which the pursuit of perfection is being used as the enemy of good enough.

I still don't understand why omitting the annotation is not "good enough" for you.

@Hc747
Copy link
Author

Hc747 commented May 20, 2024

I still don't understand why omitting the annotation is not "good enough" for you.

Because we want an annotation to be emitted.

@ejona86
Copy link
Member

ejona86 commented May 20, 2024

Because we want an annotation to be emitted.

That is circular. What behavior are you wanting from the annotation?

@Hc747
Copy link
Author

Hc747 commented May 21, 2024

That is circular. What behavior are you wanting from the annotation?

To denote that code was automatically generated and to be treated as such.

@ejona86
Copy link
Member

ejona86 commented May 21, 2024

To denote that code was automatically generated and to be treated as such.

But why this specific annotation? You can communicate it was generated with a comment. That sounds fine. And #9179 is to create a grpc-specific annotation. What's wrong with that?

@Hc747
Copy link
Author

Hc747 commented May 21, 2024

To denote that code was automatically generated and to be treated as such.

But why this specific annotation? You can communicate it was generated with a comment. That sounds fine. And #9179 is to create a grpc-specific annotation. What's wrong with that?

Because Jakarta is the drop-in replacement for Javax and the defacto standard.
#9179 has been open for over two years.

@ejona86
Copy link
Member

ejona86 commented May 21, 2024

We have no desire to depend on jakarta.

@ejona86 ejona86 closed this May 21, 2024
@Hc747
Copy link
Author

Hc747 commented May 21, 2024

We have no desire to depend on jakarta.

You realise this adds no dependency on Jakarta right? It just adds optional support for users who wish to use it.

@ejona86
Copy link
Member

ejona86 commented May 21, 2024

An optional dependency is a dependency. And the need here is zero.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants