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

Support Gradle configuration cache on multiple JVMs #1274

Closed
nedtwigg opened this issue Aug 11, 2022 · 11 comments
Closed

Support Gradle configuration cache on multiple JVMs #1274

nedtwigg opened this issue Aug 11, 2022 · 11 comments

Comments

@nedtwigg
Copy link
Member

nedtwigg commented Aug 11, 2022

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

  • make every step round-trip serializable
  • remove JvmLocalCache
@eskatos
Copy link
Contributor

eskatos commented Sep 28, 2022

FYI Gradle 8 will do a full roundtrip on the first invocation with CC enabled.
See work in progress here gradle/gradle#21985 (comment)
The goal is for this to be the default in 8.0.

@nedtwigg
Copy link
Member Author

nedtwigg commented Sep 28, 2022

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
Copy link
Member Author

nedtwigg commented Dec 4, 2023

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

lehmanju commented May 6, 2024

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
Copy link
Member Author

nedtwigg commented Jun 4, 2024

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

  • plugin-gradle 3.0.0.BETA1
  • plugin-maven 2.44.0.BETA1

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

@marcphilipp
Copy link

JUnit build updated

@bddckr
Copy link

bddckr commented Jun 5, 2024

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

Magotchi commented Jun 13, 2024

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
Copy link
Contributor

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

@nedtwigg
Copy link
Member Author

With 7.0.0.BETA4 I think this is now just about fully solved. Last chance for feedback before these APIs all get baked!

@ZacSweers
Copy link
Contributor

Just filed a regression on #2318

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants