Use Multi-Release JAR instead of reflection for decompression #25158
+118
−66
Conversation
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
wendigo
force-pushed
the
serafin/multi-release-jar
branch
3 times, most recently
from
e91abbe to
2ddab37
Compare
losipiuk
reviewed
Feb 26, 2025
|
@losipiuk if JDK is 22+ the |
wendigo
force-pushed
the
serafin/multi-release-jar
branch
2 times, most recently
from
1e663bc to
52a5b0b
Compare
Since we've moved to JDK 11 we can now use multi-release JAR
wendigo
force-pushed
the
serafin/multi-release-jar
branch
2 times, most recently
from
115c875 to
07d3008
Compare
|
How does this interact with IntelliJ? |
|
@martint badly but at least it's not breaking IntelliJ :) |
wendigo
force-pushed
the
serafin/multi-release-jar
branch
from
07d3008 to
d5779eb
Compare
Multi-Release JARs are causing error_prone/javadoc/surefire to use the latest compilation unit while resolving dependencies for other modules.
wendigo
force-pushed
the
serafin/multi-release-jar
branch
from
d5779eb to
518256b
Compare
mosabua
reviewed
Mar 2, 2025
| @@ -82,7 +82,7 @@ jobs: | |||
| - name: Maven Checks | |||
| run: | | |||
| export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}" | |||
| $MAVEN clean verify -B --strict-checksums -V -T 1C -DskipTests -P ci | |||
| $MAVEN clean install verify -B --strict-checksums -V -T 1C -DskipTests -P ci | |||
There was a problem hiding this comment.
verify is redundant now ..
Suggested change
| $MAVEN clean install verify -B --strict-checksums -V -T 1C -DskipTests -P ci | |
| $MAVEN clean install -B --strict-checksums -V -T 1C -DskipTests -P ci |
mosabua
reviewed
Mar 2, 2025
| @@ -197,7 +197,7 @@ jobs: | |||
| run: | | |||
| export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}" | |||
| # Skip checks, these are run in `maven-checks` job and e.g. checkstyle is expensive. | |||
| $MAVEN ${MAVEN_TEST} -T 1C clean compile test-compile -DskipTests -Dair.check.skip-all=true ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \ | |||
| $MAVEN ${MAVEN_TEST} -T 1C clean compile test-compile install -DskipTests -Dair.check.skip-all=true ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \ | |||
There was a problem hiding this comment.
Suggested change
| $MAVEN ${MAVEN_TEST} -T 1C clean compile test-compile install -DskipTests -Dair.check.skip-all=true ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \ | |
| $MAVEN ${MAVEN_TEST} -T 1C clean install -DskipTests -Dair.check.skip-all=true ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \ |
mosabua
reviewed
Mar 2, 2025
| @@ -328,7 +328,7 @@ jobs: | |||
| - name: Maven Tests | |||
| id: tests | |||
| run: | | |||
| $MAVEN test ${MAVEN_TEST} -pl ' | |||
| $MAVEN test install ${MAVEN_TEST} -pl ' | |||
There was a problem hiding this comment.
Suggested change
| $MAVEN test install ${MAVEN_TEST} -pl ' | |
| $MAVEN install ${MAVEN_TEST} -pl ' |
How so? Can you elaborate? If this doesn't work seamlessly with IntelliJ, then it's a non-starter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since we've moved to JDK 11 we can now use multi-release JAR
Description
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: