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

Broken on Java 16 #548

Closed
bmarcaur opened this issue Sep 23, 2021 · 6 comments
Closed

Broken on Java 16 #548

bmarcaur opened this issue Sep 23, 2021 · 6 comments

Comments

@bmarcaur
Copy link
Member

Using JDK 16:

openjdk version "16.0.2" 2021-07-20
OpenJDK Runtime Environment Homebrew (build 16.0.2+0)
OpenJDK 64-Bit Server VM Homebrew (build 16.0.2+0, mixed mode, sharing)

Running task generateMetrics fails with:

> class shadow.com.palantir.goethe.goethe.com.palantir.javaformat.java.JavaInput (in unnamed module @0x1bba56ab) cannot access class com.sun.tools.javac.parser.Tokens$TokenKind (in module jdk.compiler) because module jdk.compiler does not export com.sun.tools.javac.parser to unnamed module @0x1bba56ab

Bumping down to JDK 11 seems to fix the issue.

Errorprone had a similar issue and linked to their install docs: https://errorprone.info/docs/installation#jdk-16

@bmarcaur
Copy link
Member Author

To also confirm, this worked on jdk 15:

openjdk version "15.0.4" 2021-07-20
OpenJDK Runtime Environment Zulu15.34+17-CA (build 15.0.4+5-MTS)
OpenJDK 64-Bit Server VM Zulu15.34+17-CA (build 15.0.4+5-MTS, mixed mode, sharing)

@carterkozak
Copy link
Contributor

This is due to JEP 396: Strongly Encapsulate JDK Internals by Default
We haven't adopted java 16 internally since the supported lifespan is too short for us to roll off in time, however we're beginning to work on Java 17 support.

@iamdanfox
Copy link
Contributor

iamdanfox commented Oct 19, 2021

That JEP dangles some tantalising options:

It will still be possible to use the --add-opens command-line option, or the Add-Opens JAR-file manifest attribute, to open specific packages.

https://openjdk.java.net/jeps/261#Packaging:-Modular-JAR-files - could this be as easy as just embedding a module-info.class file that specifies all the crazy --add-opens things this project needs?

<module name="palantir-java-format" options="--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED" />

EDIT seems like I confused --add-exports with --add-opens - maybe this won't be so easy

@fawind
Copy link
Contributor

fawind commented Oct 20, 2021

Some relevant context from how google-java-format is tackling this: issue, readme.

Basically their solution for running with Java16+ is that you have to run the formatter with --add-exports flags added. Bit unfortunate and we might have to do some hacky stuff to get this working as Gradle integration or with the intellij plugin. One option might be to do something like bootstrapping our own jre when trying to format some files so we can pass through our flags (as mentioned here).

@fawind
Copy link
Contributor

fawind commented Jan 10, 2022

The latest version should work with Java 16+. However depending on how you invoke the formatter you need to add --add-exports flags due to JEP 396: Strongly Encapsulate JDK Internals by Default.

When running the formatter through Gradle, you can for example add the args to your gradle.properties file:

org.gradle.jvmargs=--add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
  --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
  --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
  --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
  --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED

The intellij plugin and integration with palantir/goethe will automatically add these flags for you.

Closing this issue for now. Please re-open if you run into further issues!

@fawind fawind closed this as completed Jan 10, 2022
@raiju
Copy link

raiju commented Jan 30, 2022

Note that I couldn't get goethe to add the flags in modern gradle (filed here: palantir/goethe#91)

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

5 participants