asciidoctor / asciidoctorj

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

(v2.5.x) Remove open access warnings from cli #1159

Closed abelsromero closed 1 year ago

abelsromero commented 1 year ago

Kind of change

Description

What is the goal of this pull request?

Remove Native subprocess... warning when running the CLI in Java > 8.

023-04-01T22:03:19.247+02:00 [main] WARN FilenoUtil : Native subprocess control requires open access to the JDK IO subsystem
Pass '--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED' to enable.

How does it achieve that?

Modifies CLI script to check the Java version running, in case of Java >8 adds required arguments to remove warning.

Are there any alternative ways to implement this?

Leave the warning and document it. However, given the majority of users should be on Java > 8, leaving the warning because we still support it, seems unfair and lazy.

Are there any implications of this pull request? Anything a user must know?

The script changes will add some delay, but it's not appreciable, especially when one takes into account the startup time for the jvm and Ruby scripts. This does not fix the other warning that appear in newer versions.

OpenJDK 64-Bit Server VM warning: Options -Xverify:none and -noverify were deprecated in JDK 13 and will likely be removed in a future release

Issue

Fixes #1155

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

robertpanzer commented 1 year ago

I wonder if this really hurts so much that it is worth starting the Java command twice? Wouldn't it suffice to document that users that want to avoid this warning can run asciidoctorj like this?

ASCIIDOCTORJ_OPTS="--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED" asciidoctorj test.adoc
abelsromero commented 1 year ago

Wouldn't it suffice to document that users that want to avoid this warning can run asciidoctorj like this?

To me, it's a matter of user experience. Since it doesn't seem jRuby is going to get a fix soon (if it can even be resolved), and we know most users are going to be affected (no one should be in Java8 by now). To me, it doesn't make much sense we force users to a) endure warnings or b) do additional configurations in their machines*. If we can fix it, even with some bash hacking, let's do it and prevent any possible doubt, especially for new users who might wonder if they are doing something wrong or don't know about/care about Java.

* However, that's going to be my approach for the maven plugin 😢

Btw, there's also the matter of -Xverify:none and -noverify were deprecated messages which I also wanted to fix by extending the Bash checks. If we consider the way should be documenting it, it can be done too, but in that case means instructing users to copy the current VARS and remove elements, not nice imho.

robertpanzer commented 1 year ago

OK, let's go for it :)