GH-39000: [Java] Build arrow-memory-netty as a JPMS module #39012
Conversation
This file contains hidden or 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
|
|
jduo
force-pushed
the
netty-modularization
branch
2 times, most recently
from
9076c9d to
4b71fd9
Compare
Note that some configuration from the arrow root POM is copied to the maven plugins module because it cannot be inherited without creating a cyclic depdendency.
Reports a used dependency as unused only on Windows 2022 CI runs
It is not correctly getting skipped.
This won't build with the JDK 8 target
Newer versions of the checkstyle plugin do support module-info.java files but require rewriting the checkstyle rules file.
Do not add tests in org.apache.arrow.util because that is an exported package for arrow-memory-core and causes module conflicts
Having an implicit dependency from arrow-memory-core does not put immutables on the module-path when running tests and causes module issues.
Export public package for arrow-vector module. Allow jackson to use arrow-vector pojo classes reflectively for serialization.
Needed because Vector classes use MemoryUtil.UNSAFE.
Based on apache#13072 - Avoid having multiple memory modules contribute to the same package - Introduce memory-netty-buffer-patch module for patching classes into Netty modules - Avoid using BaseAllocator in tests and use RootAllocator instead - Move TestBaseAllocator#testMemoryUsage() to a new test in memory-netty TestNettyAllocator because TestBaseAllocator is now in memory-core, but that specific test has Netty dependencies.
Also update arrow-vector to depend on memory-core. Netty in this PR omitted due to the complexity of needing to use patch-module on distributed JARs at run time.
This test fails because we no longer use Netty's allocation manager factory since the test has moved into memory-core. To be fixed.
Log what message was reported if the message didn't contain the correct information.
The tests themselves (specifically testEnableHistoricalLog) use reflection on java.lang.reflect so surefire needs to enable this permission.
jduo
force-pushed
the
netty-modularization
branch
from
4b71fd9 to
bcf8ce6
Compare
Due to using stream() on a log appender while the appender is still running and possibly adding more logs.
Need to patch Netty at runtime and enable reflection
Move tests outside of io.netty.buffer package because that causes a conflict with Netty's modules. Change functions marked as protected that are used in tests to be public to facilitate this refactoring.
jduo
force-pushed
the
netty-modularization
branch
from
bcf8ce6 to
7a9ba6f
Compare
vibhatha
reviewed
Dec 13, 2023
| </build> | ||
|
|
||
|
|
||
| </project> No newline at end of file |
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This depends on #39011 .
Rationale for this change
Allows arrow-memory-netty to work with JPMS tools such as jlink.
What changes are included in this PR?
Are these changes tested?
Yes, with existing tests.
Are there any user-facing changes?
Users can now use arrow-memory-netty as a module. Users will need to add --add-opens java.base/jdk.internal.misc=io.netty.common --patch-module=io.netty.buffer=${project.basedir}/../memory-netty-buffer-patch/target/arrow-memory-netty-buffer-patch-${project.version}.jar --add-opens=java.base/java.nio=org.apache.arrow.memory.core,io.netty.common,ALL-UNNAMED to the command line (depending on where the arrow-memory-netty-buffer-patch JAR is located).
This PR includes breaking changes to public APIs.
Users will need to modify their java commands as described above.