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 16 + gradle 7.0 compatibility #520

Merged
merged 4 commits into from Nov 24, 2021

Conversation

popfalushi
Copy link
Contributor

Hello. Tried to build examples on gradle 6.8.3 and java 16 and build process failed for 3 reasons:
0. Gradle 6.8.3 failed with Could not compile initialization script 'C:\Users\popfa\AppData\Local\Temp\ijmapper.gradle'. Updated gradle to 7.0.

  1. Lombok plugin needed update to be compatible with java 16. Changed plugin to io.freefair.lombok because it is recommended by Gradle.
  2. Spotless plugin needed additional arguments in gradle.properties. For more information look here: google-java-format (and removeUnusedImports) broken on JDK 16+ (has workaround) diffplug/spotless#834 . Plugin version got updated as well just because at first I thought that it will resolve the problem, but afterwards found issue on github. Didn't reverse plugin update after that.

After all changes local examples build and run ok, overall project also, javadoc is generated with getters/setters methods (checked on GrpcCodecDefinition class javadoc).

@popfalushi
Copy link
Contributor Author

As far as I understand, java 11 will pass the CI test and java 8 will be eternally failed because java 8 does not support --add-opens. Don't really know what to do with it. If java8 is essential than I suppose PR can be closed. Maybe in the future spotless plugin will do its job without need for add-opens, but at the same time lombok most likely will require it.

@ST-DDT
Copy link
Collaborator

ST-DDT commented Apr 16, 2021

1. Changed plugin to io.freefair.lombok because it is recommended by Gradle.

Do you have a link for that? Or at least a comparison between these two?

@ST-DDT
Copy link
Collaborator

ST-DDT commented Apr 16, 2021

As far as I understand, java 11 will pass the CI test and java 8 will be eternally failed because java 8 does not support --add-opens. Don't really know what to do with it. If java8 is essential than I suppose PR can be closed. Maybe in the future spotless plugin will do its job without need for add-opens, but at the same time lombok most likely will require it.

Thats a though one. I'll have a look this weekend.

@ST-DDT ST-DDT self-assigned this Apr 16, 2021
@ST-DDT ST-DDT added the enhancement A feature request or improvement label Apr 16, 2021
@popfalushi
Copy link
Contributor Author

1. Changed plugin to io.freefair.lombok because it is recommended by Gradle.

Do you have a link for that? Or at least a comparison between these two?

My mistake - it is lombok project that recommends this plugin - gradle itself does not recommend anything. This plugin is referenced on official lombok page: https://projectlombok.org/setup/gradle and it is also under active development - it has milestone and stable version from April, the former plugin didn't get an update from last May.

FYI: Lombok right now is in the tough spot: projectlombok/lombok#2681 . There were many discussions about it on reddit and hackernews.

@ST-DDT
Copy link
Collaborator

ST-DDT commented Apr 17, 2021

IMO making our project compilable in the latest version is a good thing.
However, we should ensure that we generate Java 8 compatible jars (ClassVersion + Java Api), as there might still be developers out there that use that version for compatibility reasons. (We can think about dropping support once it is EOL).
Could you adjust the gradle file to always generate J8 compatible classes?

@yidongnan What do you think?

@ST-DDT
Copy link
Collaborator

ST-DDT commented Apr 17, 2021

I included the gradle update in another PR that only updates dependencies #521, so that we can focus or changes here entirely to the J16 changes.

Copy link
Collaborator

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please update your PR for the latest changes?

@popfalushi
Copy link
Contributor Author

in a week or two - right now I'm on holiday. But what about compatibility with java 8? My PR was incompatible with java 8 and it will stay this way in the future. If compatibity with java 8 is a must than what's the point?

If you are ready to ditch java 8 - I'll rebase this branch on current state, no problem.

@ST-DDT
Copy link
Collaborator

ST-DDT commented May 3, 2021

If you are ready to ditch java 8 - I'll rebase this branch on current state, no problem.

I'm not sure about that yet. Enjoy your holidays, after that there is still plenty of time to decide on this.

Conflicts:
	build.gradle
	gradle/wrapper/gradle-wrapper.properties
@ST-DDT ST-DDT added this to the 2.13.0 milestone Nov 22, 2021
@ST-DDT
Copy link
Collaborator

ST-DDT commented Nov 22, 2021

Fixes #604

@ST-DDT ST-DDT linked an issue Nov 22, 2021 that may be closed by this pull request
Copy link
Collaborator

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long wait. Lets get this merged.

gradle.properties Outdated Show resolved Hide resolved
@yidongnan yidongnan merged commit 6563287 into grpc-ecosystem:master Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature request or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update lombok plugin
3 participants