asciidoctorj icon indicating copy to clipboard operation
asciidoctorj copied to clipboard

Modularization (JPMS)

Open abelsromero opened this issue 4 years ago • 5 comments

The need for proper modules support is starting to be nessary, at least to avoid warning (unless I am wrong https://github.com/asciidoctor/asciidoctorj/issues/1035). I started working on it but got stuck when dealing with import org.osgi.annotation.bundle.Export; in the package-info-java.

We don't really need those since this is for osgi, but the compiler complains anyway. Also Intellij suggests requiring osgi.annotations, but the compiler still complains :face_with_head_bandage: . Maybe the solution would be to explore some other configuration methd (no osgie expert myself) based on other descriptors that do not require code compilation.

abelsromero avatar May 07 '21 21:05 abelsromero

Shouldn't we already be fine? All classes in asciidoctorj-api and asciidoctorj are in disjoint packages. Also for me AsciidoctorJ runs with no warnings with this command:

java -p <path to asciidoctorj/lib> \
  --add-opens java.base/sun.nio.ch=org.jruby.complete \
  --add-opens java.base/java.io=org.jruby.complete \
  --add-modules jdk.unsupported \
  -m asciidoctorj/org.asciidoctor.jruby.cli.AsciidoctorInvoker -b pdf -r asciidoctor-diagram test.adoc

As JRuby loads classes like sun.nio.ch.NativeThread JRuby should probably add this to their module descriptor (which they don't seem to have yet and also rely on the auto-module mechanism)

robertpanzer avatar May 22 '21 13:05 robertpanzer

Even if not necessary it would stil be nice to have. If only for those internal packages. Tbh I was expecting to be simple, but the osgi thing did not make it possible, the mudule configuration was simple enough https://github.com/asciidoctor/asciidoctorj/compare/main...abelsromero:poc/modules?expand=1

abelsromero avatar May 22 '21 15:05 abelsromero

I see. This part of the Gradle documentation suggests that it is not possible for a module to depend on a non-modularised library: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules

I guess I don't understand JPMS good enough yet, I thought that you could use almost every jar as an automatic module and java also gives it a name:

# jar --file=/Users/robertpanzer/Downloads/osgi.annotation-8.0.0.jar --describe-module
No module descriptor found. Derived automatic module.

[email protected] automatic
requires java.base mandated
contains org.osgi.annotation.bundle
contains org.osgi.annotation.versioning

robertpanzer avatar May 23 '21 09:05 robertpanzer

I guess I don't understand JPMS good enough yet, I thought that you could use almost every jar as an automatic module and java also gives it a name:

Tbh, I am longer sure of the path and I also need to read the docs further. Modularization feels the correct thing to do, but seems to cause more issues than features. I guess we can leave this in stand-by for now.

abelsromero avatar May 23 '21 12:05 abelsromero

Funny enough I have no problem compiling asciidoctorj-api with this simple command and the module-info.java that you have provided:

~/dev/asciidoctorj/asciidoctorj-api$ javac --source-path src/main/java -d classes --module-path modules $SRCS
src/main/java/org/asciidoctor/OptionsBuilder.java:83: warning: [dep-ann] deprecated item is not annotated with @Deprecated
    public OptionsBuilder templateDir(File templateDir) {
                          ^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: src/main/java/org/asciidoctor/Options.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
1 warning

The modules directory only contains the file osgi.annotation-8.0.0.jar with that exact name.

And after jar'ing the contents of the classes directory I get this:

jar --file asciidoctorj-api.jar --describe-module
org.asciidoctorj.api jar:file:///Users/robertpanzer/dev/asciidoctorj/asciidoctorj-api/asciidoctorj-api.jar/!module-info.class
exports org.asciidoctor
exports org.asciidoctor.ast
exports org.asciidoctor.converter
exports org.asciidoctor.extension
exports org.asciidoctor.log
exports org.asciidoctor.syntaxhighlighter
requires java.base mandated
requires osgi.annotation static

I really wish this isn't Gradle this time...

robertpanzer avatar May 23 '21 12:05 robertpanzer