-
Notifications
You must be signed in to change notification settings - Fork 36
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add support for Java 16 and update ASM to version 9.1
- Loading branch information
1 parent
b131231
commit ac754c4
Showing
7 changed files
with
651 additions
and
5 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
ac754c4
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.
With JDK17 almost ready, could we have new FA release with JDK16 signatures, please?
ac754c4
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.
Sorry, yes. I had no time and because of some initial issues with JDK 17, the whole thing delayed. Also due to much work it was impossible to release in time.
Generally, you won't need a separate release for JDK 16, as there were no changes. Forbiddenapis works fine with JDK 16, you only can't directly refer to signatures specific to this version. Pass JDK 15 and all is fine.
I am planning to make a change to forbiddenapis, to allow releases > the maximum supported and automatically fake signatures files, so passing source/target/relaese won't break builds.
ac754c4
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.
Thank you for checking and the tip.
I didn't think on setting java version for FA, was using auto-detection (probably from maven.compiler.release) only.
ac754c4
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.
In genrral you should always set source/target/release, because if you don't, Maven detects the current runtime Java version as "default". And if that is used with forbiddenapis, it would cause issues if you compile code with later versions.
For reproducible builds maven.compiler.release/target/source should always be set and not autodetection used.
In case you write code specifically for JDK 16 or JDK 17, of course you must set release=16/17, but then it's a simple workaround to specifiy an older version in the FA maven plugin.
ac754c4
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 meant that I have maven.compiler.release set explicitly (to say 15).
Using newer JDK to compile and check for forbidden APIs works then perfectly fine.
However, as I'm using
<bundledSignature>jdk-deprecated</bundledSignature>
- I get problem once maven.compiler.release is updated to 16.Per
forbidden-apis/src/main/java/de/thetaphi/forbiddenapis/maven/AbstractCheckMojo.java
Line 191 in df06185
but I confirm that configuring
<releaseVersion>15</releaseVersion>
in plugin works fine as well.ac754c4
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.
All fine. I was not aware that you wanted to explicitely compile against JDK 16, which is quite uncommon! Most people having this issue just forget to use maven.compiler.release at all (which is bad as it is an unreproducible build) and then when using a recent JDK it breaks due to autoused defaults.