asciidoctor / asciidoctorj

:coffee: Java bindings for Asciidoctor. Asciidoctor on the JVM!
http://asciidoctor.org
Apache License 2.0
617 stars 172 forks source link

Modularization (JPMS) #1036

Open abelsromero opened 3 years ago

abelsromero commented 3 years ago

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.

robertpanzer commented 3 years ago

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)

abelsromero commented 3 years ago

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

robertpanzer commented 3 years ago

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.

osgi.annotation@8.0.0 automatic
requires java.base mandated
contains org.osgi.annotation.bundle
contains org.osgi.annotation.versioning
abelsromero commented 3 years ago

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.

robertpanzer commented 3 years ago

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...