Revive java module related changes #184
Merged
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
ikhoon
approved these changes
Jul 24, 2024
ikhoon
pushed a commit
to line/armeria
that referenced
this pull request
Jul 26, 2024
…5825) Motivation: Recently we noticed that java modules don't work well with Armeria. The reason was that `META-INF/services/java.security.Provider` is included in our SPI, but `org.bouncycastle.jce.provider.BouncyCastleProvider` doesn't exist in the JAR causing an exception in the following [location](https://github.com/openjdk/jdk/blob/476d2ae69d6f67fdf9e2a9353f224141318690f2/src/java.base/share/classes/jdk/internal/module/ModulePath.java#L554). Since we shade our custom `BouncyCastleProvider` and use it sparingly, we probably don't need to also export the SPI implementations. https://github.com/line/armeria/blob/6dec8498457d7032cbbc192e4070ca2bc5658535/core/src/main/java/com/linecorp/armeria/internal/common/util/MinifiedBouncyCastleProvider.java#L93-L98 Note that this problem can happen if a shaded module provides SPI implementations and is not necessarily unique to `BouncyCastle`. From a quick check I don't think (`guava`, `jctools`, `reflections`, `fastutil`, `futures`) have a similar issue though. A working version with java 9 modules with the built snapshot version can be found in the following repository: https://github.com/jrhee17/my-java-module. Note that this change also requires line/gradle-scripts#184. Also, did a local test to ensure there are no unintentional changes ``` user@AL02437565 shaded-diff % tar -zxf /Users/user/Projects/armeria/core/build/libs/armeria-shaded-1.30.0-SNAPSHOT.jar -C new user@AL02437565 shaded-diff % tar -zxf /Users/user/Projects/upstream-armeria/core/build/libs/armeria-shaded-1.30.0-SNAPSHOT.jar -C base user@AL02437565 shaded-diff % diff -rq new base Files new/META-INF/com.linecorp.armeria.versions.properties and base/META-INF/com.linecorp.armeria.versions.properties differ Only in base/META-INF/services: java.security.Provider user@AL02437565 shaded-diff % ``` Modifications: - Added a `shadowExclusions` property which excludes files from the shadowed JAR Result: - Armeria has more compatibility with Java 9 modules. <!-- Visit this URL to learn more about how to write a pull request description: https://armeria.dev/community/developer-guide#how-to-write-pull-request-description -->
ikhoon
added a commit
to ikhoon/armeria
that referenced
this pull request
Jul 26, 2024
Reapply unexpectedly overwitten changes for Java module Related: line/gradle-scripts#184 git subrepo pull --force gradle/scripts subrepo: subdir: "gradle/scripts" merged: "107037098" upstream: origin: "https://github.com/line/gradle-scripts" branch: "main" commit: "107037098" git-subrepo: version: "0.4.5" origin: "https://github.com/ingydotnet/git-subrepo" commit: "dbb99be"
ikhoon
added a commit
to line/armeria
that referenced
this pull request
Jul 26, 2024
Reapply unexpectedly overwritten changes for Java module Related: line/gradle-scripts#184 git subrepo pull --force gradle/scripts subrepo: subdir: "gradle/scripts" merged: "107037098" upstream: origin: "https://github.com/line/gradle-scripts" branch: "main" commit: "107037098" git-subrepo: version: "0.4.5" origin: "https://github.com/ingydotnet/git-subrepo" commit: "dbb99be"
ikhoon
added a commit
to line/armeria
that referenced
this pull request
Jul 26, 2024
Reapply unexpectedly overwritten changes for Java module Related: line/gradle-scripts#184 git subrepo pull --force gradle/scripts subrepo: subdir: "gradle/scripts" merged: "107037098" upstream: origin: "https://github.com/line/gradle-scripts" branch: "main" commit: "107037098" git-subrepo: version: "0.4.5" origin: "https://github.com/ingydotnet/git-subrepo" commit: "dbb99be"
ikhoon
pushed a commit
to line/armeria
that referenced
this pull request
Jul 26, 2024
…5825) Motivation: Recently we noticed that java modules don't work well with Armeria. The reason was that `META-INF/services/java.security.Provider` is included in our SPI, but `org.bouncycastle.jce.provider.BouncyCastleProvider` doesn't exist in the JAR causing an exception in the following [location](https://github.com/openjdk/jdk/blob/476d2ae69d6f67fdf9e2a9353f224141318690f2/src/java.base/share/classes/jdk/internal/module/ModulePath.java#L554). Since we shade our custom `BouncyCastleProvider` and use it sparingly, we probably don't need to also export the SPI implementations. https://github.com/line/armeria/blob/6dec8498457d7032cbbc192e4070ca2bc5658535/core/src/main/java/com/linecorp/armeria/internal/common/util/MinifiedBouncyCastleProvider.java#L93-L98 Note that this problem can happen if a shaded module provides SPI implementations and is not necessarily unique to `BouncyCastle`. From a quick check I don't think (`guava`, `jctools`, `reflections`, `fastutil`, `futures`) have a similar issue though. A working version with java 9 modules with the built snapshot version can be found in the following repository: https://github.com/jrhee17/my-java-module. Note that this change also requires line/gradle-scripts#184. Also, did a local test to ensure there are no unintentional changes ``` user@AL02437565 shaded-diff % tar -zxf /Users/user/Projects/armeria/core/build/libs/armeria-shaded-1.30.0-SNAPSHOT.jar -C new user@AL02437565 shaded-diff % tar -zxf /Users/user/Projects/upstream-armeria/core/build/libs/armeria-shaded-1.30.0-SNAPSHOT.jar -C base user@AL02437565 shaded-diff % diff -rq new base Files new/META-INF/com.linecorp.armeria.versions.properties and base/META-INF/com.linecorp.armeria.versions.properties differ Only in base/META-INF/services: java.security.Provider user@AL02437565 shaded-diff % ``` Modifications: - Added a `shadowExclusions` property which excludes files from the shadowed JAR Result: - Armeria has more compatibility with Java 9 modules. <!-- Visit this URL to learn more about how to write a pull request description: https://armeria.dev/community/developer-guide#how-to-write-pull-request-description -->
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.
Motivation
It seems like changes made in the following PR weren't applied in this repo and were overwritten.
https://github.com/line/armeria/pull/5543/files
ref: https://discord.com/channels/1087271586832318494/1210154982784110673/1264906811266568205