diffplug / spotless

Keep your code spotless
Apache License 2.0
4.49k stars 455 forks source link

Support Gradle configuration cache on multiple JVMs #1274

Open nedtwigg opened 2 years ago

nedtwigg commented 2 years ago

Gradle configuration cache requires task state to be round-trip serializable. As outlined at #987, this is a signficant constraint on plugin developers and it slows down the end-user experience of change-test-change-test on a single JVM daemon. We built a workaround (JvmLocalCache) that complies with Gradle's requirement by making our tasks round-trip serializable without requiring much work from plugin developers or the end user's machine, but it only works within a single JVM.

To work on multiple JVMs, we need to

eskatos commented 2 years ago

FYI Gradle 8 will do a full roundtrip on the first invocation with CC enabled. See work in progress here https://github.com/gradle/gradle/issues/21985#issuecomment-1260018610 The goal is for this to be the default in 8.0.

nedtwigg commented 2 years ago

Full roundtrip will work great on the same JVM, so Spotless will still work and not work under the same conditions it does currently. EDIT: Compatibility with configuration cache within a single JVM on Gradle 8 is confirmed.

nedtwigg commented 10 months ago

I think we now have a way to incrementally build this until we can eventually remove our workaround. See:

In this comment, I'm going to maintain a list of remaining TODOs before we will fully support Gradle's configuration cache model without any workarounds. If you'd like to help: comment in this issue when you start on a given step, follow-up with a PR (draft is fine) and I will update the checklist below

lehmanju commented 5 months ago

I checked npm.EslintFormatterStep, npm.PrettierFormatterStep, npm.TsFmtFormatterStep and sql.DBeaverSQLFormatterStep. They all use StepHarness for testing which has roundtrip serialization testing enabled by default. Since their tests do not fail, it should mean they are already serializable, am I correct?

For NativeCmdStep I created a PR #2108 which adds a test using StepHarness.

nedtwigg commented 4 months ago

Thanks mostly to @Goooler's tireless dedication, we have finally shipped:

These contain a bazillion fixes. Please test and let us know if you encounter any issues!

marcphilipp commented 4 months ago

JUnit build updated

bddckr commented 4 months ago

All working fine for me as well, except I used

removeUnusedImports("cleanthat-javaparser-unnecessaryimport")

as seen in the docs. That one seems to not support the config cache yet:

Could not load the value of field `steps` of task `:spotlessInternalRegisterDependencies` of type `com.diffplug.gradle.spotless.RegisterDependenciesTask`.
> Could not load the value of field `excluded` of `com.diffplug.spotless.java.CleanthatJavaStep` bean found in field `roundtripStateInternal` of `com.diffplug.spotless.FormatterStepSerializationRoundtrip` bean found in field `steps` of task `:spotlessInternalRegisterDependencies` of type `com.diffplug.gradle.spotless.RegisterDependenciesTask`.

I just removed it as it looks like googleJavaFormat already takes care of that.

Magotchi commented 3 months ago

I get a similar error to @bddckr if I use any prettier steps, and only while using the Gradle Configuration Cache, like:

Could not load the value of field `steps` of task `:spotlessJava` of type `com.diffplug.gradle.spotless.SpotlessTaskImpl`.
> Could not load the value of field `additionalNpmrcLocations` of `com.diffplug.spotless.npm.NpmPathResolver` bean found in field `resolver` of `com.diffplug.spotless.npm.NpmFormatterStepLocations` bean found in field `locations` of `com.diffplug.spotless.npm.PrettierFormatterStep$State` bean found in field `roundtripStateInternal` of `com.diffplug.spotless.FormatterStepSerializationRoundtrip` bean found in field `steps` of task `:spotlessJava` of type `com.diffplug.gradle.spotless.SpotlessTaskImpl`.

or:

Could not load the value of field `steps` of task `:spotlessPrettierGeneric` of type `com.diffplug.gradle.spotless.SpotlessTaskImpl`.
> Could not load the value of field `additionalNpmrcLocations` of `com.diffplug.spotless.npm.NpmPathResolver` bean found in field `resolver` of `com.diffplug.spotless.npm.NpmFormatterStepLocations` bean found in field `locations` of `com.diffplug.spotless.npm.PrettierFormatterStep$State` bean found in field `roundtripStateInternal` of `com.diffplug.spotless.FormatterStepSerializationRoundtrip` bean found in field `steps` of task `:spotlessPrettierGeneric` of type `com.diffplug.gradle.spotless.SpotlessTaskImpl`.

I've worked around it by declaring Spotless incompatible with the configuration cache, like:

tasks.withType<SpotlessTask>().configureEach {
  notCompatibleWithConfigurationCache(
      "Spotless 7.0.0.BETA1 with prettier steps does not support the configuration cache.")
}
davidburstrom commented 2 weeks ago

FWIW, I've upgraded to 7.0.0.BETA2 in all my repositories and everything seems to be in order! Great job!