KtLint 0.43.0 is missing requisite `--add-opens` to work in JDK 16+

ZacSweers commented 3 years ago

Expected Behavior

I expect KtLint's fat jar on CLI to add requisite opens to run

Observed Behavior

It crashes when using JDK 16 or later

Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private transient java.lang.Object java.lang.Throwable.backtrace accessible: module java.base does not "opens java.lang" to unnamed module @654b5005
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357)
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
        at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:177)
        at java.base/java.lang.reflect.Field.setAccessible(Field.java:171)
        at org.jetbrains.kotlin.com.intellij.util.ReflectionUtil.findFieldInHierarchy(ReflectionUtil.java:153)
        at org.jetbrains.kotlin.com.intellij.util.ReflectionUtil.getDeclaredField(ReflectionUtil.java:278)
        at org.jetbrains.kotlin.com.intellij.openapi.util.objectTree.ThrowableInterner.<clinit>(ThrowableInterner.java:81)
        ... 31 more

You can see this warning on older JDKs as well

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jetbrains.kotlin.com.intellij.util.ReflectionUtil (file:/Users/zsweers/dev/slack/restructure/slack-android-ng/config/bin/ktlint) to field java.lang.Throwable.backtrace
WARNING: Please consider reporting this to the maintainers of org.jetbrains.kotlin.com.intellij.util.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

Steps to Reproduce

Run the KtLint CLI binary with JDK 16

Your Environment

Version of ktlint used: 0.43.0
Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): CLI
Version of Gradle used (if applicable): N/A
Operating System and version: N/A
Link to your project (if it's a public repository): N/A

ZacSweers commented 3 years ago

The way we handle this with google-java-format is to add it to the exec prefix like so

private val EXEC_PREFIX =
  """#!/bin/sh

     exec java \
       -Xmx512m \
       --add-opens java.base/java.util=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
       --add-opens jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
       -jar "${'$'}0" "${'$'}@"

  """.trimIndent()

You could update this line to do the same with the requisite args https://github.com/pinterest/ktlint/blob/94f42e0514d7b919986c1293372943bcc1adf2ac/ktlint/build.gradle#L57

One down side is it makes the CLI jar unusable on JDK versions <9. Another solution would be to publish two CLI jars - one with the exec prefix and one without (where folks that need to add extra commands can use the latter and repackage it themselves, like we do with GJF)

paul-dingemans commented 3 years ago

Thx for sharing the solution above. I would not be surprised if this relates to #1271 in which it is reported that ktlint no longer works on jdk8.

TwoClocks commented 2 years ago

You don't need to add all those opens. You just need to add one. I altered the build.gradle like so :

it.write "#!/bin/sh\n\nexec java --add-opens java.base/java.lang=ALL-UNNAMED -Xmx512m -jar \"\$0\" \"\$@\"\n\n".bytes

And it works fine on Java 17.

I'd put in a PR, but I see in this thread that this would break Java 8? The --add-opens is required for JDK 16+, and that cmd is unknown to JDK 8. You could either remove the reflection (seems unlikely), or have the shell detect the JDK version and use the appropriate cmd-line, or create / release two bins.

@paul-dingemans I do not think it is related to #1271

nahguam commented 2 years ago

For maven I've added <jvmarg line="--add-opens java.base/java.lang=ALL-UNNAMED"/> to my maven-antrun-plugin configuration to get it working with Java 17.

romtsn commented 2 years ago

Let's track this in #1195

dk185173 commented 10 months ago

@nahguam

For maven I've added <jvmarg line="--add-opens java.base/java.lang=ALL-UNNAMED"/> to my maven-antrun-plugin configuration to get it working with Java 17. Can you please help with the complete excerpt of the antrun plugin?

paul-dingemans commented 10 months ago

See ktlint documentation.

pinterest / ktlint