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

Breaks with JDK 16 #99

Closed
bitzl opened this issue Mar 18, 2021 · 14 comments
Closed

Breaks with JDK 16 #99

bitzl opened this issue Mar 18, 2021 · 14 comments

Comments

@bitzl
Copy link

bitzl commented Mar 18, 2021

Describe the bug
Using this plugin with JDK16 fails because com.sun.tools.javac.parser.Tokens$TokenKind is not accessible anymore:

Error:  Failed to execute goal com.coveo:fmt-maven-plugin:2.10:format (default) on project framework: Execution default of goal com.coveo:fmt-maven-plugin:2.10:format failed: An API incompatibility was encountered while executing com.coveo:fmt-maven-plugin:2.10:format: java.lang.IllegalAccessError: class com.google.googlejavaformat.java.JavaInput (in unnamed module @0x47c7a9e5) 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 @0x47c7a9e5

To Reproduce
Run a Maven build using this plugin with OpenJDK 16.

Expected behavior
The code get formatted without an error.

Additional context
Builds with OpenJDK 15 work fine. Cause for this error are likely changes for JEP 396: Strongly Encapsulate JDK Internals by Default.

Corresponding Ticket for Google Java Format: google/google-java-format#538

@famaridon
Copy link

famaridon commented May 4, 2021

#100 will fix it just need a release.

and add documentation because https://github.com/google/google-java-format/releases/tag/v1.10.0 need more java flags due to JEP 396: Strongly Encapsulate JDK Internals by Default

export MAVEN_OPTS=" --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 "

@klaraward
Copy link
Contributor

klaraward commented Jun 14, 2021

#100 is merged and version 2.11 is released, but the problem with the options still prevail.

Assuming that forking and setting jvmargs for this plugin is a lot of work @malaporte ?

@msche
Copy link

msche commented Aug 14, 2021

Upgrading google-java-format to version 1.11.0 might fix this issue as [https://github.com/google/google-java-format/issues/538 ](Running on Java 16 fails) seems to be fixed in that release.

@klaraward
Copy link
Contributor

Upgrading google-java-format to version 1.11.0 might fix this issue as [https://github.com/google/google-java-format/issues/538 ](Running on Java 16 fails) seems to be fixed in that release.

Is it 1.10.0 - #100
or 1.11.0 - #108
that is needed?

@klaraward
Copy link
Contributor

I would say that 1.11.0 of the google-java-format does not help with the required commandline options, but it has other improvments.

@msche
Copy link

msche commented Aug 19, 2021

I was referring to 1.11.0

@malaporte
Copy link
Contributor

I just release 2.12 which uses the latest Google Java Format version, but as mentioned above you still need to pass all the annoying options to Maven for it to work.

I started a branch to experiment with running the formatter in a child process, but as of now I have yet to figure out how to extract the right class path from the class loader that Maven is using (using the "all deps" version of Google Java Format would fix the problem, but it's not available on Maven meh).

@Stephan202
Copy link
Contributor

@malaporte if you're successful with the fork approach, can you please make this an optional feature? Forking does have some performance impact, and some users will have instead solved the problem by adding the aforementioned flags to .mvn/jvm.config.

@malaporte
Copy link
Contributor

Oh definitively - I was intending on doing that only for Java >= 16, and if possible check for the presence of the required flag to see if running in-process is an option.

@klaraward
Copy link
Contributor

Any update on this @malaporte ?

veithen added a commit to veithen/ws-axiom that referenced this issue Nov 27, 2021
@mattnelson
Copy link
Contributor

mattnelson commented Dec 23, 2021

using the "all deps" version of Google Java Format would fix the problem, but it's not available on Maven

I'm seeing it there now.
https://repo1.maven.org/maven2/com/google/googlejavaformat/google-java-format/1.13.0/

@klaraward
Copy link
Contributor

@malaporte Have you looked more into this?

@stanislavb
Copy link

I solved this by using instructions in README: https://github.com/spotify/fmt-maven-plugin#using-with-java-16-and-maven

Added file .mvn/jvm.config to my repository with contents:

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

@klaraward
Copy link
Contributor

Readme updated in #117

We intend to also add forking support to the plugin, so you can specify these flags in a better way, or preferably include them in the plugin itself. (If it's not fixed in google-java-format before then)

rusher added a commit to mariadb-corporation/mariadb-connector-j that referenced this issue Nov 4, 2022
rusher added a commit to mariadb-corporation/mariadb-connector-j that referenced this issue Nov 4, 2022
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 a pull request may close this issue.

8 participants