diffplug / spotless

Keep your code spotless
Apache License 2.0
4.55k stars 456 forks source link

Support `java { toolchain { languageVersion` #724

Open anuraaga opened 4 years ago

anuraaga commented 4 years ago

Currently, the spotless invocation is inlined into the task execution itself.

https://github.com/diffplug/spotless/blob/main/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java#L50

Is it possible to instead run a separate main file in JavaExec? Gradle has introduced toolchain feature to allow build to control the Java version used for compile and execution which works very well to allow any user's Java version to run the build with a specific one. But unfortunately, this currently doesn't affect Spotless which runs in Gradle itself - for example, this means that a build must become unbuildable on Java 8 to upgrade to the latest googleJavaFormat since it requires higher Java version. It would be nice if this could be avoided.

nedtwigg commented 4 years ago

I think it's a great idea to support the toolchain feature. For now, it would be very difficult, and is not likely to happen soon.

tbroyer commented 3 years ago

I wonder if that's not going to be required (forking, not necessarily using toolchains) to support JDK 16 for Google Java Format: https://github.com/google/google-java-format/releases/tag/v1.10.0

nedtwigg commented 3 years ago

I wonder if Gradle might be setting some of those same flags for itself already

tbroyer commented 3 years ago

It doesn't:

BTW, to support Java Toolchains, I believe you'd have to start by using the Worker API, with process isolation (possibly conditionally) so you can setExecutable() to the toolchain's executable (https://docs.gradle.org/current/userguide/toolchains.html#sec:plugins). Using the Worker API, with process isolation (for forking), even without specifying an executable, would be a required first step for those Google Java Format --add-exports JVM args.

nedtwigg commented 2 months ago

@aciccarello I'm still happy to merge your proposal, but it might be worth reading this before investing too much time: https://jakewharton.com/gradle-toolchains-are-rarely-a-good-idea/

aciccarello commented 2 months ago

Did you mean to ping @anuraaga?

Regarding that article, it brings up some good points about java compatibility and managing version but I don't completely agree with the conclusions. I think toolchains are still a helpful way to select an installed JDK. I also thought that this section (under "Not all bad") was relevant.

Some tools that dip into JDK internals regularly break on newer versions of the compiler because they rely on unstable APIs. I’m thinking about things like Google Java Format or Error-Prone. No need to hold the rest of your project from enjoying the latest JDK, if those tools are run via a JavaExec task you can use a toolchain to keep them on an older JDK until a newer version is available.

anuraaga commented 2 months ago

I've been out of the Java world for some time so don't have much stake in this anymore. But do still remember those tough issues with Java versions and tools.

I agree completely with Jake, using toolchains to pick an old version of Java only for generating old bytecode is a bad idea, indeed javadoc is an obvious reason. I would personally still use toolchains to compile code in my projects to ensure contributors use a consistent version - but that would be the latest (maybe LTS) Java.

But as @aciccarello quotes nicely, when running code that requires an old version of Java, toolchains can make unrunnable, runnable and are very convenient for that.