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

infra: should use release 11 for compilation #14485

Closed
Bananeweizen opened this issue Feb 14, 2024 · 11 comments
Closed

infra: should use release 11 for compilation #14485

Bananeweizen opened this issue Feb 14, 2024 · 11 comments

Comments

@Bananeweizen
Copy link
Contributor

Bananeweizen commented Feb 14, 2024

The checkstyle build has warnings, because it configures only source and target for the Java compiler. That was fine with old Java versions, where that clarified the source code level and the binary code level. Meanwhile the compiler knows that it's important to also check the API of the Java runtime, because it changes between Java versions. Therefore "release" is meanwhile the recommended setting, if the source level, binary level and API level are all identical.
grafik

it is only for jdk that is not 11 that we use as default.
https://dev.azure.com/romanivanovjr/romanivanovjr/_build/results?buildId=19781&view=logs&j=30eb8b1d-aec7-5534-8285-cea9e43390b9&t=9b4e45b1-0f32-5d25-c850-57a5aa9f241d&l=80

[INFO] 
[INFO] --- compiler:3.12.1:compile (default-compile) @ checkstyle ---
[INFO] Recompiling the module because of changed source code.
[INFO] Compiling 448 source files with javac [debug target 11] to target/classes
[WARNING] system modules path not set in conjunction with -source 11
[INFO] /Users/runner/work/1/s/src/main/java/com/puppycrawl/tools/checkstyle/DefaultConfiguration.java: Some input files use or override a deprecated API.
[INFO] /Users/runner/work/1/s/src/main/java/com/puppycrawl/tools/checkstyle/DefaultConfiguration.java: Recompile with -Xlint:deprecation for details.
[INFO] /Users/runner/work/1/s/src/main/java/com/puppycrawl/tools/checkstyle/AbstractAutomaticBean.java: /Users/runner/work/1/s/src/main/java/com/puppycrawl/tools/checkstyle/AbstractAutomaticBean.java uses unchecked or unsafe operations.
[INFO] /Users/runner/work/1/s/src/main/java/com/puppycrawl/tools/checkstyle/AbstractAutomaticBean.java: Recompile with -Xlint:unchecked for details.
[INFO] 

attention to [WARNING] system modules path not set in conjunction with -source 11

@romani
Copy link
Member

romani commented Feb 14, 2024

Can you share example of maven config to set this 3 items?

If update is simple, you might be able to make PR from web site editor.

@MANISH-K-07
Copy link
Contributor

MANISH-K-07 commented Feb 15, 2024

Can you share example of maven config to set this 3 items?

@romani ,
Firstly, sorry for getting involved and please do correct me if I am wrong.

Based on my observations from PR #14397 ,
Any other mvn release used other than the current version is raising an API incompatibility error.
I had to find a workaround to keep the version intact and so added a new config to the plugin which did the trick.
I could elaborate a bit more on this if you insist. Please review PR #14397 to see the changes I have made.

Bananeweizen added a commit to Bananeweizen/checkstyle that referenced this issue Feb 16, 2024
@rnveach
Copy link
Member

rnveach commented Feb 16, 2024

@romani PR has been made for this issue. I showed it fixes the warning.

@romani
Copy link
Member

romani commented Feb 17, 2024

I can not find such warning at windows builds https://ci.appveyor.com/project/Checkstyle/checkstyle/builds/49208331, strange.

@rnveach
Copy link
Member

rnveach commented Feb 17, 2024

@romani See PR, CI only shows at JRE 17

the compiler knows that it's important to also check the API of the Java runtime, because it changes between Java versions.

@romani
Copy link
Member

romani commented Feb 17, 2024

Ok , so it is warming only for java version above 11 ?

@rnveach
Copy link
Member

rnveach commented Feb 17, 2024

I can only assume it is for java versions that do not match our POM.

@github-actions github-actions bot added this to the 10.14.0 milestone Feb 17, 2024
@Bananeweizen
Copy link
Contributor Author

@rnveach not sure how you typically process issues, should this one be closed already? the PR is merged, and since that is infrastructure only, there is nothing further to document.

@romani
Copy link
Member

romani commented Feb 20, 2024

Yes, this should be closed, we lost track of it

@romani romani closed this as completed Feb 20, 2024
@rnveach
Copy link
Member

rnveach commented Mar 5, 2024

9240ad3#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R225
This line fails backport.

[INFO] �[1m--- �[0;32mcompiler:3.12.1:compile�[m �[1m(default-compile)�[m @ �[36mcheckstyle-backport-jre8�[0;1m ---�[m
[INFO] Recompiling the module because of �[1mchanged source code�[m.
[INFO] Compiling 448 source files with javac [debug release 1.8] to target\classes
...
Caused by: java.lang.IllegalArgumentException: invalid flag: --release
	at com.sun.tools.javac.api.JavacTool.processOptions(JavacTool.java:206)
	at com.sun.tools.javac.api.JavacTool.getTask(JavacTool.java:156)
	at com.sun.tools.javac.api.JavacTool.getTask(JavacTool.java:107)
	at com.sun.tools.javac.api.JavacTool.getTask(JavacTool.java:64)
	at org.codehaus.plexus.compiler.javac.JavaxToolsCompiler.compileInProcess(JavaxToolsCompiler.java:125)
	... 30 more

This was fixed in JDK 9.
https://bugs.openjdk.org/browse/JDK-8167965

@Bananeweizen
Copy link
Contributor Author

@rnveach you can put the definition of the release property into a profile which is only enabled on JDK9 or higher, see https://maven.apache.org/plugins/maven-compiler-plugin/examples/set-compiler-release.html

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

4 participants