search

spotify / fmt-maven-plugin

Opinionated Maven Plugin that formats your Java code.
MIT License
335 stars 65 forks source link

run formatter in fork - run on java 16+ without --add-exports flags #140

Closed danielnorberg closed 2 years ago

danielnorberg commented 2 years ago

Remove the need to add a .mvn/jvm.config with --add-exports flags when using Java 16+

This is achieved by running the formatter in a java subprocess with the necessary --add-exports specified. The forking logic was adapted from https://github.com/spotify/flo/blob/master/flo-runner%2Fsrc%2Fmain%2Fjava%2Fcom%2Fspotify%2Fflo%2Fcontext%2FForkingExecutor.java. We could also consider putting the forking logic in a separate library for easier reuse. Presumably we might encounter the same problem again.

Alternative to https://github.com/spotify/fmt-maven-plugin/pull/139

klaraward commented 2 years ago

In the light of #141, do you want to make any changes to this @danielnorberg ?

klaraward commented 2 years ago

Also, suggest we add a config to enable forking, setting it to false by default (and possibly to true for JDK 16+ ?)

danielnorberg commented 2 years ago

Yeah, I think the way this sets up the classpath when forking will not actually work when running as a maven plugin.

danielnorberg commented 2 years ago

I think I've now managed to resolve the classpath problem, but would want integration tests that run the plugin as a real maven plugin.

danielnorberg commented 2 years ago

Fwiw I've tested a snapshot build of this PR locally and it seems to work as expected.

danielnorberg commented 2 years ago

fwiw the integration tests pass locally with corretto 11 & 17 (x86) when rebasing this on https://github.com/spotify/fmt-maven-plugin/pull/145

danielnorberg commented 2 years ago

Perhaps we should also test this on windows. Does GitHub actions support that?

klaraward commented 2 years ago

Perhaps we should also test this on windows. Does GitHub actions support that?

Have no idea, but hopefully? https://blog.devgenius.io/write-your-github-actions-workflow-for-build-windows-application-94e5a989f477

danielnorberg commented 2 years ago

Re windows, here goes nothing https://github.com/spotify/fmt-maven-plugin/pull/146

danielnorberg commented 2 years ago

It blows my mind a bit that this just works on windows on the first try. Java is amazing.