luben / zstd-jni

JNI binding for Zstd
Other
854 stars 168 forks source link

Support Java Modules #246

Closed aalmiray closed 11 months ago

aalmiray commented 1 year ago

I'd be great if the library supplied a full Java module descriptor. It's possible to keep bytecode baseline compatible with Java 8 while providing a full module descriptor thanks to ModiTect. There are Maven and Gradle plugins. However, it should be possible to wrap moditect-core with an sbt plugin or decorate the resulting JAR with ModiTect to add a moudle-info.class before publication.

luben commented 1 year ago

Sorry for quite late reply. What's the problem using it with the automatic module mechanism? We added Automatic-Module-Name in our manifest so that should be enough to use it as a module.

aalmiray commented 1 year ago

Defining a manifest entry as you did is not enough when the library should be part of a modular Java runtime created with jlink.

jlink only supports full module descriptors, automatic modules are not allowed.

luben commented 1 year ago

I have added it https://github.com/luben/zstd-jni/commit/e2d3bcbbaddd46e06162e82d9e0d8a58695e4365, will look to release soon

Coder-256 commented 11 months ago

I just ran into an issue with this while investigating an issue with a jar compiled from master. It seems that the module-info.java is corrupted:

$ jar -f target/zstd-jni-1.5.5-6.jar -d
java.lang.module.InvalidModuleDescriptorException: The requires table must have an entry for java.base
        at java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1091)
        at java.base/jdk.internal.module.ModuleInfo.readModuleAttribute(ModuleInfo.java:419)
        at java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:252)
        at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:131)
        at jdk.jartool/sun.tools.jar.Main.describeModule(Main.java:1939)
        at jdk.jartool/sun.tools.jar.Main.describeModuleFromEntries(Main.java:1915)
        at jdk.jartool/sun.tools.jar.Main.describeModule(Main.java:1822)
        at jdk.jartool/sun.tools.jar.Main.run(Main.java:423)
        at jdk.jartool/sun.tools.jar.Main.main(Main.java:1680)
$ echo $?
1
$ java --version
openjdk 11.0.20.1 2023-08-24
OpenJDK Runtime Environment (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04)
OpenJDK 64-Bit Server VM (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04, mixed mode, sharing)

I get the same crash when including the jar in a Maven project. I can confirm ModuleInfoPlugin fixes the issue.

I believe this is very likely due to a bug somewhere in sbt-java-module-info... not sure what the best solution is here.

luben commented 11 months ago

@Coder-256 , thanks for bringing that up. It was indeed a bug in sbt-java-module-info. That bug was fixed but new version was not released to Maven so the build will requires first checking out and publishing locally https://github.com/giabao/sbt-java-module-info (until we have fixed version published).

Now it looks it validates:

$ jar -f target/zstd-jni-1.5.5-7.jar -d                                                                                                                                      130 ↵ 
com.github.luben.zstd_jni@1.5.5-7 jar:file:///home/luben/zstd-jni/target/zstd-jni-1.5.5-7.jar/!module-info.class
exports com.github.luben.zstd
exports com.github.luben.zstd.util
requires java.base
luben commented 11 months ago

@aalmiray , @Coder-256 , I have just published zstd-jni-1.5.5-7 with support for java modules. Please verify it works for you.

luben commented 11 months ago

released v1.5.5-8 as there was an issue with the OSGI version, please test that if you have issues with the OSGI manifest

Coder-256 commented 11 months ago

I can confirm v1.5.5-8 works great, and also seems to fix an issue I had with strange newlines in META-INF/MANIFEST.MF. Thank you!!

I'm not using OSGI directly so I can't confirm anything about the OSGI-related manifest headers.