-
Notifications
You must be signed in to change notification settings - Fork 323
Housekeeping: allow more jdks to be used, fixes #1314 #1315
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
base: main
Are you sure you want to change the base?
Conversation
Kehrlann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. Left a few notes and questions.
Will run on my machine with JDK 25 and let you know how that goes.
| <source>${java.version}</source> | ||
| <target>${java.version}</target> | ||
| <source>${java.specification.version}</source> | ||
| <target>${java.specification.version}</target> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you build with JDK 11, the jar will have the same name but a different target than with 1.8 ; we should probably pin to 1.8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, everything before JDK 16 uses the same jar file name. I can either use different names, as in the later JDKs or I can pin everything before 16 to 1.8. The effort should (hopefully) be the same. I will check tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now in all JDK versions the version is part of the jar name. That's more consistent. Hopefully the changed jar name does not create problems in other tools.
|
Question: have you run the integration tests with your changes? |
Usually I execute this integration-test: org.cloudfoundry.client.v2.InfoTest, but I did not try all JDKs. Will try tomorrow... |
|
@Lokowandtg I don't think we should support non-LTS JDKs. So 17, 21 and 25 are the current targets. Since there are bigger issues with JDK 25, we'll have to work on that for 6.0.0 |
|
Good that you asked for running the tests. Now the v2.infoTest runs in all tested JDKs: 8, 11, 17, 21, 22, 24, 25. And for my local CF instance I needed to cherry-pick the UAA rate limiting code: #1313 |
| <wire.version>4.9.1</wire.version> | ||
| <wire.plugin.version>3.0.2</wire.plugin.version> | ||
| <wire.suffix>-jvm</wire.suffix> | ||
| <dependencies.version>3.5.8</dependencies.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because the whole project is not compatible with JDK 25, which requires Boot 3.5 and up. But Boot 3.5 is not compatible with JDK 8 and 11.
I don't want to support both 2.x and 3.x lines in a single release. This needs to go in a separate branch, that supports JDK17+ and Boot 3.5.x ; which will go into a 6.0 release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will that mean double maintenance in 5 and 6 or will 5 and therefor JDK8 and 11 then phase out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think 6.x will be the new primary, with the intent to phase out support for 5.x .
We'll keep a 5.x branch for critical CVEs, but we've somewhat maxed out the versions that we can upgrade on that branch anyway.
JDK 25 is a new LTS-version, so we might add it to the java matix.
Changes in java module system require some more explicit dependencies, some of which break build in older JDKs. So the dependencies are in their own profile.
Also the "annotationProcessorPaths" needs to be defined explicitly for later JDKs.
Rules for javadoc generation are now stricter, resulting in errors where previously only warnings where shown.
In "maven-compiler-plugin, "source" and "target" are allways defined as the major java version.
${java.specification.version}
${java.specification.version}
We might consider to set the source version always to "1.8" to avoid breaking builds in Java 8 when a developer accidentially uses features from later java versions. The target should remain the currently used JDK.