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
Absence of Pack200 in JDK 14 should not cause failure of JaCoCo build #984
Conversation
To avoid compilation errors with JDK 14, where Pack200 not available.
Instrumenter and Analyzer provide location information when Pack200 streams throw IOException on error.
To avoid compilation errors with JDK 14, where Pack200 not available.
To avoid compilation errors with JDK 14, where Pack200 not available.
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 fix @Godin! We now have some warnings that should be fixed or suppressed. Also I wonder whether there should be a change log entry making clear that Pack200 will no longer be supported by JaCoCo when executed on the latest JREs.
I was wondering the same - on one side we're making change, on the other side real change is in JDK and Pack200 doesn't work with and without our change - see behaviours in description of this PR. Maybe something like Support for Pack200 was removed in JDK 14, so instead of as non-functional change? Also now after preparation of this change, I see that there are actually more than 2 options that we discussed:
In all these cases we have change of behavior, with conditional in 1st case. 2nd case unconditional, explicit and behaves the same way as 1st on JDKs >=14. And unfortunately we don't provide way to specify exclusions for contents of archives, so both 1st and 2nd cases might be problematic in case of nested archives. 3rd case is unconditional, but for consumers might be hard to spot reason of change in behavior. So I'm wondering whether you had in mind 2nd or 3rd by "remove Pack200 support"? 😉 |
As many users will not upgrade to Java 14 quickly I wouldn't kill that feature right now and stick with your proposed PR. For the change log I would rephrase it a bit (as non-functional change):
|
@marchof once merged I'll continue updates of version of JDK 14 Early Access builds that is used for our builds in Travis CI. |
Pack200 was deprecated in JDK 13 (https://openjdk.java.net/jeps/336)
with planned removal in JDK 14 (https://openjdk.java.net/jeps/367)
which happened recently (http://hg.openjdk.java.net/jdk/jdk/rev/f236fd5d0c2c)
and supposed to be part of upcoming JDK 14 Early Access Build 27.
This doesn't cause troubles with released JaCoCo 0.8.5 for cases without processing of Pack200, and in the last case it fails as following
However this causes compilation failure during JaCoCo build with JDK 14:
This PR prevents compilation failure and makes support of Pack200 optional by using reflection for JDKs below 14.
Also note that after this change error message will contain location: