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

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

Conversation

abelsromero
Copy link
Member

Kind of change

  • Bug fix
  • New non-breaking feature
  • New breaking feature
  • Documentation update
  • Build improvement

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

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

abelsromero commented Apr 5, 2023

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

OK, let's go for it :)

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

Successfully merging this pull request may close these issues.

2 participants