Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modularization (JPMS) #1036

Open
abelsromero opened this issue May 7, 2021 · 5 comments
Open

Modularization (JPMS) #1036

abelsromero opened this issue May 7, 2021 · 5 comments

Comments

@abelsromero
Copy link
Member

abelsromero commented May 7, 2021

The need for proper modules support is starting to be nessary, at least to avoid warning (unless I am wrong #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 🤕 .
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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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

@abelsromero
Copy link
Member Author

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants