From 9c03d4dbd1551490b02bbe2f3b180b52a797417c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 00:30:09 -0800 Subject: [PATCH 01/15] Create a `spotlessSetup` plugin, and repurpose `RegisterDependenciesTask` so that it stores a value which can be injected into the SpotlessTaskService on every build. --- plugin-gradle/build.gradle | 9 +++++ .../spotless/RegisterDependenciesTask.java | 27 +++++--------- .../gradle/spotless/SpotlessSetup.java | 37 +++++++++++++++++++ .../gradle/spotless/SpotlessSetupPlugin.java | 30 +++++++++++++++ .../gradle/spotless/UpToDateTest.java | 4 +- 5 files changed, 88 insertions(+), 19 deletions(-) create mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java create mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetupPlugin.java diff --git a/plugin-gradle/build.gradle b/plugin-gradle/build.gradle index 3a5afd2ba8..80fc8d6a79 100644 --- a/plugin-gradle/build.gradle +++ b/plugin-gradle/build.gradle @@ -45,6 +45,12 @@ gradlePlugin { displayName = 'Spotless formatting plugin' description = project.description } + spotlessSetupPlugin { + id = 'com.diffplug.spotless-setup' + implementationClass = 'com.diffplug.gradle.spotless.SpotlessSetupPlugin' + displayName = 'Spotless formatting plugin setup' + description = project.description + } spotlessPluginLegacy { id = 'com.diffplug.gradle.spotless' implementationClass = 'com.diffplug.gradle.spotless.SpotlessPluginRedirect' @@ -78,6 +84,9 @@ if (version.endsWith('-SNAPSHOT')) { 'clang-format' ] plugins { + spotlessSetupPlugin { + id = 'com.diffplug.spotless-setup' + } spotlessPlugin { id = 'com.diffplug.spotless' } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java index 6ba228e0b5..3fc8e39620 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java @@ -15,22 +15,17 @@ */ package com.diffplug.gradle.spotless; -import java.io.File; -import java.io.IOException; -import java.nio.charset.StandardCharsets; - import javax.inject.Inject; import org.gradle.api.DefaultTask; import org.gradle.api.provider.Property; import org.gradle.api.services.BuildServiceRegistry; +import org.gradle.api.tasks.Input; import org.gradle.api.tasks.Internal; -import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.TaskAction; import org.gradle.build.event.BuildEventsListenerRegistry; import com.diffplug.common.base.Preconditions; -import com.diffplug.common.io.Files; /** * NOT AN END-USER TASK, DO NOT USE FOR ANYTHING! @@ -59,26 +54,24 @@ void hookSubprojectTask(SpotlessTask task) { task.dependsOn(this); } - File unitOutput; - - @OutputFile - public File getUnitOutput() { - return unitOutput; - } - void setup() { Preconditions.checkArgument(getProject().getRootProject() == getProject(), "Can only be used on the root project"); - unitOutput = new File(getProject().getBuildDir(), "tmp/spotless-register-dependencies"); BuildServiceRegistry buildServices = getProject().getGradle().getSharedServices(); getTaskService().set(buildServices.registerIfAbsent("SpotlessTaskService", SpotlessTaskService.class, spec -> {})); getBuildEventsListenerRegistry().onTaskCompletion(getTaskService()); } + boolean enableConfigCacheDaemonLocal; + + @Input + public boolean getEnableConfigCacheDaemonLocal() { + return enableConfigCacheDaemonLocal; + } + @TaskAction - public void trivialFunction() throws IOException { - Files.createParentDirs(unitOutput); - Files.write(Integer.toString(1), unitOutput, StandardCharsets.UTF_8); + public void trivialFunction() { + getTaskService().get().registerDependenciesTask(this); } @Internal diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java new file mode 100644 index 0000000000..a2b0a3f373 --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java @@ -0,0 +1,37 @@ +/* + * Copyright 2021 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import org.gradle.api.Project; +import org.gradle.api.tasks.TaskProvider; + +public class SpotlessSetup { + static final String NAME = "spotlessSetup"; + + private final TaskProvider task; + + public SpotlessSetup(Project project) { + task = (TaskProvider) (Object) project.getTasks().named(RegisterDependenciesTask.TASK_NAME); + } + + public boolean isEnableConfigCacheDaemonLocal() { + return task.get().getEnableConfigCacheDaemonLocal(); + } + + public boolean setEnableConfigCacheDaemonLocal(boolean enabled) { + return task.get().enableConfigCacheDaemonLocal = enabled; + } +} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetupPlugin.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetupPlugin.java new file mode 100644 index 0000000000..38aeadf5d7 --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetupPlugin.java @@ -0,0 +1,30 @@ +/* + * Copyright 2021 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import org.gradle.api.Plugin; +import org.gradle.api.Project; + +import com.diffplug.common.base.Preconditions; + +public class SpotlessSetupPlugin implements Plugin { + @Override + public void apply(Project rootProject) { + Preconditions.checkArgument(rootProject.getProject() == rootProject.getRootProject(), "com.diffplug.spotless-setup must be applied to only the root project"); + rootProject.getPlugins().apply(SpotlessPlugin.class); + rootProject.getExtensions().create(SpotlessSetup.NAME, SpotlessSetup.class, rootProject); + } +} diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/UpToDateTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/UpToDateTest.java index 9b7eccdfff..a0026d2625 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/UpToDateTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/UpToDateTest.java @@ -88,8 +88,8 @@ void testPathologicalCase() throws IOException { // the format task is UP-TO-DATE (same inputs), but the apply tasks will run again pauseForFilesystem(); BuildResult buildResult = gradleRunner().withArguments("spotlessApply").build(); - Assertions.assertThat(buildResult.taskPaths(TaskOutcome.UP_TO_DATE)).containsExactly(":spotlessInternalRegisterDependencies", ":spotlessMisc"); - Assertions.assertThat(buildResult.taskPaths(TaskOutcome.SUCCESS)).containsExactly(":spotlessMiscApply", ":spotlessApply"); + Assertions.assertThat(buildResult.taskPaths(TaskOutcome.UP_TO_DATE)).containsExactly(":spotlessMisc"); + Assertions.assertThat(buildResult.taskPaths(TaskOutcome.SUCCESS)).containsExactly(":spotlessInternalRegisterDependencies", ":spotlessMiscApply", ":spotlessApply"); assertFile("README.md").hasContent("abc"); // and it'll take two more runs to get to fully UP-TO-DATE From 0316ef7a2f37c8c6b51e518257c75e3f1f54e4c1 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 00:31:11 -0800 Subject: [PATCH 02/15] It's easier to make GitRatchet cache-friendly if `setupRatchet` always gets called, so that we can differentiate `not-hydrated` from `ratchet inactive` --- .../java/com/diffplug/gradle/spotless/FormatExtension.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 77e0e4f7f7..c340a5551a 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -734,9 +734,7 @@ protected void setupTask(SpotlessTask task) { task.setSteps(steps); task.setLineEndingsPolicy(getLineEndings().createPolicy(getProject().getProjectDir(), () -> totalTarget)); spotless.getRegisterDependenciesTask().hookSubprojectTask(task); - if (getRatchetFrom() != null) { - task.setupRatchet(getRatchetFrom()); - } + task.setupRatchet(getRatchetFrom() != null ? getRatchetFrom() : ""); } /** Returns the project that this extension is attached to. */ From b384f5a38b0541ab37f955d960376ab39d22797d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 00:35:58 -0800 Subject: [PATCH 03/15] Introduce a class to store all the transient fields of a task. --- .../gradle/spotless/SpotlessTaskImpl.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index e33e7f3ec6..2ec5a668e9 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -20,6 +20,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.util.List; +import java.util.Objects; import javax.inject.Inject; @@ -36,6 +38,8 @@ import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.FormatterStep; +import com.diffplug.spotless.LineEnding; import com.diffplug.spotless.PaddedCell; @CacheableTask @@ -130,4 +134,26 @@ private File getOutputFile(File input) { } return new File(outputDirectory, outputFileName); } + + static boolean isHydrated(SpotlessTask task) { + return task.lineEndingsPolicy != null; + } + + static class LiveCache { + LineEnding.Policy lineEndingsPolicy; + List steps; + String ratchetFrom; + + LiveCache(SpotlessTask task) { + lineEndingsPolicy = Objects.requireNonNull(task.lineEndingsPolicy); + steps = Objects.requireNonNull(task.steps); + ratchetFrom = Objects.requireNonNull(task.ratchetFrom); + } + + void hydrate(SpotlessTask task) { + task.lineEndingsPolicy = lineEndingsPolicy; + task.steps = steps; + task.setupRatchet(ratchetFrom); + } + } } From b92b256b0f10f8a2a0fb689a5e25036c5deef1c2 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 00:37:37 -0800 Subject: [PATCH 04/15] Whenever a task runs, we store its transient state. --- .../gradle/spotless/SpotlessTaskService.java | 53 ++++++++++++++++--- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index de016ae6d2..dca0eb3e36 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -23,6 +23,7 @@ import javax.inject.Inject; import org.gradle.api.DefaultTask; +import org.gradle.api.GradleException; import org.gradle.api.Project; import org.gradle.api.file.DirectoryProperty; import org.gradle.api.model.ObjectFactory; @@ -35,6 +36,7 @@ import com.diffplug.common.base.Preconditions; import com.diffplug.common.base.Unhandled; +import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.Provisioner; /** @@ -44,26 +46,60 @@ * apply already did). */ public abstract class SpotlessTaskService implements BuildService, AutoCloseable, OperationCompletionListener { - private final Map apply = Collections.synchronizedMap(new HashMap<>()); - private final Map source = Collections.synchronizedMap(new HashMap<>()); - private final Map provisioner = Collections.synchronizedMap(new HashMap<>()); + private boolean enableConfigCacheDaemonLocal; + private Map apply = Collections.synchronizedMap(new HashMap<>()); + private Map source = Collections.synchronizedMap(new HashMap<>()); + private Map provisioner = Collections.synchronizedMap(new HashMap<>()); - Provisioner provisionerFor(Project project) { - return provisioner.computeIfAbsent(project.getPath(), unused -> { - return GradleProvisioner.newDedupingProvisioner(project); - }); + void registerDependenciesTask(RegisterDependenciesTask task) { + enableConfigCacheDaemonLocal = task.getEnableConfigCacheDaemonLocal(); } void registerSourceAlreadyRan(SpotlessTask task) { source.put(task.getPath(), task); + if (enableConfigCacheDaemonLocal) { + storeOrHydrate(task); + } + } + + void hydrate(SpotlessTask task) { + storeOrHydrate(task); + } + + private void storeOrHydrate(SpotlessTask task) { + if (!enableConfigCacheDaemonLocal) { + return; + } + String cacheKey = task.getProjectDir().getAsFile().get().getAbsolutePath() + ">" + task.getPath(); + if (SpotlessTaskImpl.isHydrated(task)) { + daemonLocalMap.put(cacheKey, new SpotlessTaskImpl.LiveCache(task)); + } else { + SpotlessTaskImpl.LiveCache cached = daemonLocalMap.get(cacheKey); + if (cached == null) { + throw new GradleException("Spotless daemon-local cache is stale. Regenerate the cache with\n" + + " " + (FileSignature.machineIsWin() ? "rmdir /q /s" : "rm -rf") + " .gradle/configuration-cache\n" + + "For more information see #123"); + } else { + cached.hydrate(task); + } + } + } + + private static final Map daemonLocalMap = Collections.synchronizedMap(new HashMap<>()); + + Provisioner provisionerFor(Project project) { + return provisioner.computeIfAbsent(project.getPath(), unused -> { + return GradleProvisioner.newDedupingProvisioner(project); + }); } void registerApplyAlreadyRan(SpotlessApply task) { apply.put(task.sourceTaskPath(), task); } + ///////////////// // - private final GitRatchetGradle ratchet = new GitRatchetGradle(); + private GitRatchetGradle ratchet = new GitRatchetGradle(); GitRatchetGradle getRatchet() { return ratchet; @@ -79,6 +115,7 @@ public void close() throws Exception { ratchet.close(); } // + ////////////////// static String INDEPENDENT_HELPER = "Helper"; From 00646fe63c6120fb4b590b81f58f50b86d7b1f80 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 00:38:03 -0800 Subject: [PATCH 05/15] And we guard every transient getter of SpotlessTask with `hydrateIfNull`. --- .../gradle/spotless/SpotlessTask.java | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index adf69b5e1d..ae9c286561 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -48,6 +48,12 @@ public abstract class SpotlessTask extends DefaultTask { @Internal abstract Property getTaskService(); + private void hydrateIfNull(Object o) { + if (o == null) { + getTaskService().get().hydrate(this); + } + } + // set by SpotlessExtension, but possibly overridden by FormatExtension protected String encoding = "UTF-8"; @@ -64,6 +70,7 @@ public void setEncoding(String encoding) { @Input public LineEnding.Policy getLineEndingsPolicy() { + hydrateIfNull(lineEndingsPolicy); return lineEndingsPolicy; } @@ -82,12 +89,19 @@ public void setLineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { * compared to using the project root. */ private transient ObjectId subtreeSha = ObjectId.zeroId(); + /** Stored so that the configuration cache can recreate the GitRatchetGradle state. */ + protected transient String ratchetFrom; public void setupRatchet(String ratchetFrom) { - ratchet = getTaskService().get().getRatchet(); - File projectDir = getProjectDir().get().getAsFile(); - rootTreeSha = ratchet.rootTreeShaOf(projectDir, ratchetFrom); - subtreeSha = ratchet.subtreeShaOf(projectDir, rootTreeSha); + this.ratchetFrom = ratchetFrom; + if (!ratchetFrom.isEmpty()) { + ratchet = getTaskService().get().getRatchet(); + File projectDir = getProjectDir().get().getAsFile(); + rootTreeSha = ratchet.rootTreeShaOf(projectDir, ratchetFrom); + subtreeSha = ratchet.subtreeShaOf(projectDir, rootTreeSha); + } else { + subtreeSha = ObjectId.zeroId(); + } } @Internal @@ -105,6 +119,7 @@ ObjectId getRootTreeSha() { @Input public ObjectId getRatchetSha() { + hydrateIfNull(subtreeSha); return subtreeSha; } @@ -147,6 +162,7 @@ public File getOutputDirectory() { @Input public List getSteps() { + hydrateIfNull(steps); return Collections.unmodifiableList(steps); } From 1cd0de0ad9a20691aa6d9652829a0d95a846e723 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 02:17:52 -0800 Subject: [PATCH 06/15] Consolidate our hydrate exceptions into one place. --- .../gradle/spotless/SpotlessTaskImpl.java | 36 ++++++++++++------- .../gradle/spotless/SpotlessTaskService.java | 7 ++-- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index 2ec5a668e9..7e56fcc435 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -37,6 +37,7 @@ import org.gradle.work.InputChanges; import com.diffplug.common.base.StringPrinter; +import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -68,22 +69,18 @@ public void performAction(InputChanges inputs) throws Exception { Files.createDirectories(outputDirectory.toPath()); } - if (lineEndingsPolicy != null) { - try (Formatter formatter = buildFormatter()) { - for (FileChange fileChange : inputs.getFileChanges(target)) { - File input = fileChange.getFile(); - if (fileChange.getChangeType() == ChangeType.REMOVED) { - deletePreviousResult(input); - } else { - if (input.isFile()) { - processInputFile(formatter, input); - } + assertHydrated(this); + try (Formatter formatter = buildFormatter()) { + for (FileChange fileChange : inputs.getFileChanges(target)) { + File input = fileChange.getFile(); + if (fileChange.getChangeType() == ChangeType.REMOVED) { + deletePreviousResult(input); + } else { + if (input.isFile()) { + processInputFile(formatter, input); } } } - } else { - throw new GradleException("Spotless doesn't support configuration cache yet.\n" + - "Rerun with --no-configuration-cache"); } } @@ -139,6 +136,19 @@ static boolean isHydrated(SpotlessTask task) { return task.lineEndingsPolicy != null; } + static void assertHydrated(SpotlessTask task) { + if (!isHydrated(task)) { + throw new GradleException("Spotless doesn't support configuration cache yet.\n" + + "Rerun with --no-configuration-cache"); + } + } + + static GradleException cacheIsStale() { + return new GradleException("Spotless daemon-local cache is stale. Regenerate the cache with\n" + + " " + (FileSignature.machineIsWin() ? "rmdir /q /s" : "rm -rf") + " .gradle/configuration-cache\n" + + "For more information see #123"); + } + static class LiveCache { LineEnding.Policy lineEndingsPolicy; List steps; diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index dca0eb3e36..a8a79ef828 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -23,7 +23,6 @@ import javax.inject.Inject; import org.gradle.api.DefaultTask; -import org.gradle.api.GradleException; import org.gradle.api.Project; import org.gradle.api.file.DirectoryProperty; import org.gradle.api.model.ObjectFactory; @@ -36,7 +35,6 @@ import com.diffplug.common.base.Preconditions; import com.diffplug.common.base.Unhandled; -import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.Provisioner; /** @@ -68,6 +66,7 @@ void hydrate(SpotlessTask task) { private void storeOrHydrate(SpotlessTask task) { if (!enableConfigCacheDaemonLocal) { + SpotlessTaskImpl.assertHydrated(task); return; } String cacheKey = task.getProjectDir().getAsFile().get().getAbsolutePath() + ">" + task.getPath(); @@ -76,9 +75,7 @@ private void storeOrHydrate(SpotlessTask task) { } else { SpotlessTaskImpl.LiveCache cached = daemonLocalMap.get(cacheKey); if (cached == null) { - throw new GradleException("Spotless daemon-local cache is stale. Regenerate the cache with\n" + - " " + (FileSignature.machineIsWin() ? "rmdir /q /s" : "rm -rf") + " .gradle/configuration-cache\n" + - "For more information see #123"); + throw SpotlessTaskImpl.cacheIsStale(); } else { cached.hydrate(task); } From 5f4a0b5adba891466ba2ca545ba884bf69b14c19 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 00:38:13 -0800 Subject: [PATCH 07/15] And a test to demonstrate that it works. --- .../ConfigurationCacheDaemonLocalTest.java | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheDaemonLocalTest.java diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheDaemonLocalTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheDaemonLocalTest.java new file mode 100644 index 0000000000..5ad4c8b5a7 --- /dev/null +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheDaemonLocalTest.java @@ -0,0 +1,69 @@ +/* + * Copyright 2020-2021 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import java.io.IOException; + +import org.gradle.testkit.runner.BuildResult; +import org.gradle.testkit.runner.GradleRunner; +import org.junit.jupiter.api.Test; + +public class ConfigurationCacheDaemonLocalTest extends GradleIntegrationHarness { + @Override + protected GradleRunner gradleRunner() throws IOException { + setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true"); + return super.gradleRunner().withGradleVersion(GradleVersionSupport.CONFIGURATION_CACHE.version); + } + + @Test + public void gjf() throws IOException { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.spotless-setup'", + " id 'com.diffplug.spotless'", + "}", + "repositories { mavenCentral() }", + "spotlessSetup {", + " enableConfigCacheDaemonLocal = true", + "}", + "spotless {", + " java {", + " target file('test.java')", + " googleJavaFormat('1.2')", + " }", + "}"); + + // first run works + setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + gradleRunner().withArguments("spotlessApply").build(); + assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); + + // and it keeps working! + gradleRunner().withArguments("spotlessApply", "--stacktrace").build(); + gradleRunner().withArguments("spotlessApply").build(); + gradleRunner().withArguments("spotlessApply").build(); + + setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + gradleRunner().withArguments("spotlessCheck").buildAndFail(); + gradleRunner().withArguments("spotlessApply").build(); + assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); + + BuildResult failure = gradleRunner().withDebug(true).withArguments("spotlessApply", "--stacktrace").buildAndFail(); + failure.getOutput().contains("Spotless daemon-local cache is stale. Regenerate the cache with\n" + + " rm -rf .gradle/configuration-cache\n" + + "For more information see #123\n"); + } +} From 642ac61303c80009d2f50ef25792066279af4e27 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 02:18:37 -0800 Subject: [PATCH 08/15] It works for git ratchet too. Note: we removed the test for an empty project folder (deleted L186-187) because it triggered a bug in Gradle https://github.com/gradle/gradle/issues/18897. Hard to imagine how it could actually happen in the wild, so no sense banging our head against it. --- .../gradle/spotless/GitRatchetGradleTest.java | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java index ca9324b123..62fa954572 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java @@ -31,8 +31,10 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.treewalk.TreeWalk; import org.gradle.testkit.runner.BuildResult; +import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; class GitRatchetGradleTest extends GradleIntegrationHarness { private static final String TEST_PATH = "src/markdown/test.md"; @@ -46,11 +48,24 @@ private Git initRepo() throws IllegalStateException, GitAPIException, IOExceptio return git; } - @Test - void singleProjectExhaustive() throws Exception { + @Override + protected GradleRunner gradleRunner() throws IOException { + return super.gradleRunner().withGradleVersion(GradleVersionSupport.CONFIGURATION_CACHE.version); + } + + @ParameterizedTest + @ValueSource(ints = {0, 1}) + void singleProjectExhaustive(int useConfigCache) throws Exception { try (Git git = initRepo()) { + if (useConfigCache == 1) { + setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true"); + } setFile("build.gradle").toLines( - "plugins { id 'com.diffplug.spotless' }", + "plugins {", + " id 'com.diffplug.spotless'", + " id 'com.diffplug.spotless-setup'", + "}", + "spotlessSetup { enableConfigCacheDaemonLocal = true }", "spotless {", " ratchetFrom 'baseline'", " format 'misc', {", @@ -135,13 +150,17 @@ private BuildResultAssertion assertFail(String... tasks) throws Exception { return new BuildResultAssertion(gradleRunner().withArguments(tasks).buildAndFail()); } - private static final String BASELINE_ROOT = "cf049829afeba064f27cd67911dc36e585c9d869"; + private static final String BASELINE_ROOT = "a5efb43a0da929853e596a85c1f225a60ae0acfd"; private static final String BASELINE_CLEAN = "65fdd75c1ae00c0646f6487d68c44ddca51f0841"; private static final String BASELINE_DIRTY = "4cfc3358ccbf186738b82a60276b1e5306bc3870"; - @Test - void multiProject() throws Exception { + @ParameterizedTest + @ValueSource(ints = {0, 1}) + void multiProject(int useConfigCache) throws Exception { try (Git git = initRepo()) { + if (useConfigCache == 1) { + setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true"); + } setFile("settings.gradle").toLines( "plugins {", " id 'com.diffplug.spotless' apply false", @@ -159,14 +178,21 @@ void multiProject() throws Exception { " bumpThisNumberIfACustomStepChanges(1)", " }", "}"); - setFile(".gitignore").toContent("build/\n.gradle\n"); - setFile("build.gradle").toContent("apply from: rootProject.file('spotless.gradle') // root"); + setFile(".gitignore").toContent("build/\n.gradle\n*.properties\n"); + setFile("build.gradle").toLines( + "apply plugin: 'com.diffplug.spotless-setup'", + "spotlessSetup { enableConfigCacheDaemonLocal = true }", + "apply from: rootProject.file('spotless.gradle') // root"); setFile(TEST_PATH).toContent("HELLO"); setFile("clean/build.gradle").toContent("apply from: rootProject.file('spotless.gradle') // clean"); setFile("clean/" + TEST_PATH).toContent("HELLO"); setFile("dirty/build.gradle").toContent("apply from: rootProject.file('spotless.gradle') // dirty"); setFile("dirty/" + TEST_PATH).toContent("HELLO"); + setFile("added/build.gradle").toContent("apply from: rootProject.file('spotless.gradle') // added"); RevCommit baseline = addAndCommit(git); + if (useConfigCache == 1) { + setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true"); + } ObjectId cleanFolder = TreeWalk.forPath(git.getRepository(), "clean", baseline.getTree()).getObjectId(0); ObjectId dirtyFolder = TreeWalk.forPath(git.getRepository(), "dirty", baseline.getTree()).getObjectId(0); @@ -180,12 +206,7 @@ void multiProject() throws Exception { .outcome(":clean:spotlessCheck", TaskOutcome.SUCCESS) .outcome(":dirty:spotlessCheck", TaskOutcome.SUCCESS); - setFile("added/build.gradle").toContent("apply from: rootProject.file('spotless.gradle') // added"); setFile("added/" + TEST_PATH).toContent("HELLO"); - - TreeWalk isNull = TreeWalk.forPath(git.getRepository(), "added", baseline.getTree()); - Assertions.assertThat(isNull).isNull(); - assertPass("spotlessMisc") .outcome(":spotlessMisc", TaskOutcome.UP_TO_DATE) .outcome(":clean:spotlessMisc", TaskOutcome.UP_TO_DATE) From 276899614e0d53639154a6b764c3e57f4a53d7d3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 14:57:31 -0800 Subject: [PATCH 09/15] `enableConfigCacheDaemonLocal` is too long, `jvmLocalCache` is better. --- .../spotless/RegisterDependenciesTask.java | 17 +++++++---------- .../diffplug/gradle/spotless/SpotlessSetup.java | 8 ++++---- .../gradle/spotless/SpotlessTaskService.java | 2 +- ...java => ConfigurationCacheJvmLocalTest.java} | 4 ++-- .../gradle/spotless/GitRatchetGradleTest.java | 6 +++--- 5 files changed, 17 insertions(+), 20 deletions(-) rename plugin-gradle/src/test/java/com/diffplug/gradle/spotless/{ConfigurationCacheDaemonLocalTest.java => ConfigurationCacheJvmLocalTest.java} (95%) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java index 3fc8e39620..b450cf6c1c 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java @@ -41,16 +41,13 @@ public abstract class RegisterDependenciesTask extends DefaultTask { static final String TASK_NAME = "spotlessInternalRegisterDependencies"; void hookSubprojectTask(SpotlessTask task) { - // TODO: in the future, we might use this hook to add an optional perf improvement - // spotlessRoot { + // TODO: in the future, we might use this hook to implement #984 + // spotlessSetup { // java { googleJavaFormat('1.2') } // ...etc // } - // The point would be to reuse configurations from the root project, - // with the restriction that you have to declare every formatter in - // the root, and you'd get an error if you used a formatter somewhere - // which you didn't declare in the root. That's a problem for the future - // though, not today! + // it's also needed to make sure that jvmLocalCache gets set + // in the SpotlessTaskService before any spotless tasks run task.dependsOn(this); } @@ -62,11 +59,11 @@ void setup() { getBuildEventsListenerRegistry().onTaskCompletion(getTaskService()); } - boolean enableConfigCacheDaemonLocal; + boolean jvmLocalCache; @Input - public boolean getEnableConfigCacheDaemonLocal() { - return enableConfigCacheDaemonLocal; + public boolean getJvmLocalCache() { + return jvmLocalCache; } @TaskAction diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java index a2b0a3f373..b4f4394a82 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java @@ -27,11 +27,11 @@ public SpotlessSetup(Project project) { task = (TaskProvider) (Object) project.getTasks().named(RegisterDependenciesTask.TASK_NAME); } - public boolean isEnableConfigCacheDaemonLocal() { - return task.get().getEnableConfigCacheDaemonLocal(); + public boolean isJvmLocalCache() { + return task.get().getJvmLocalCache(); } - public boolean setEnableConfigCacheDaemonLocal(boolean enabled) { - return task.get().enableConfigCacheDaemonLocal = enabled; + public boolean setJvmLocalCache(boolean enabled) { + return task.get().jvmLocalCache = enabled; } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index a8a79ef828..143b611916 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -50,7 +50,7 @@ public abstract class SpotlessTaskService implements BuildService provisioner = Collections.synchronizedMap(new HashMap<>()); void registerDependenciesTask(RegisterDependenciesTask task) { - enableConfigCacheDaemonLocal = task.getEnableConfigCacheDaemonLocal(); + enableConfigCacheDaemonLocal = task.getJvmLocalCache(); } void registerSourceAlreadyRan(SpotlessTask task) { diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheDaemonLocalTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java similarity index 95% rename from plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheDaemonLocalTest.java rename to plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java index 5ad4c8b5a7..b9bc003f8d 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheDaemonLocalTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java @@ -21,7 +21,7 @@ import org.gradle.testkit.runner.GradleRunner; import org.junit.jupiter.api.Test; -public class ConfigurationCacheDaemonLocalTest extends GradleIntegrationHarness { +public class ConfigurationCacheJvmLocalTest extends GradleIntegrationHarness { @Override protected GradleRunner gradleRunner() throws IOException { setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true"); @@ -37,7 +37,7 @@ public void gjf() throws IOException { "}", "repositories { mavenCentral() }", "spotlessSetup {", - " enableConfigCacheDaemonLocal = true", + " jvmLocalCache = true", "}", "spotless {", " java {", diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java index 62fa954572..bad66378ad 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java @@ -65,7 +65,7 @@ void singleProjectExhaustive(int useConfigCache) throws Exception { " id 'com.diffplug.spotless'", " id 'com.diffplug.spotless-setup'", "}", - "spotlessSetup { enableConfigCacheDaemonLocal = true }", + "spotlessSetup { jvmLocalCache = true }", "spotless {", " ratchetFrom 'baseline'", " format 'misc', {", @@ -150,7 +150,7 @@ private BuildResultAssertion assertFail(String... tasks) throws Exception { return new BuildResultAssertion(gradleRunner().withArguments(tasks).buildAndFail()); } - private static final String BASELINE_ROOT = "a5efb43a0da929853e596a85c1f225a60ae0acfd"; + private static final String BASELINE_ROOT = "71a2671ed452d52625245e5101cbf8467e905865"; private static final String BASELINE_CLEAN = "65fdd75c1ae00c0646f6487d68c44ddca51f0841"; private static final String BASELINE_DIRTY = "4cfc3358ccbf186738b82a60276b1e5306bc3870"; @@ -181,7 +181,7 @@ void multiProject(int useConfigCache) throws Exception { setFile(".gitignore").toContent("build/\n.gradle\n*.properties\n"); setFile("build.gradle").toLines( "apply plugin: 'com.diffplug.spotless-setup'", - "spotlessSetup { enableConfigCacheDaemonLocal = true }", + "spotlessSetup { jvmLocalCache = true }", "apply from: rootProject.file('spotless.gradle') // root"); setFile(TEST_PATH).toContent("HELLO"); setFile("clean/build.gradle").toContent("apply from: rootProject.file('spotless.gradle') // clean"); From 3307b82d0b41ec584f276f31b4e5a999c965eb6a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 15:10:51 -0800 Subject: [PATCH 10/15] Change our error messages to reference the workaround, and also to point to our explanation issue. --- .../diffplug/gradle/spotless/SpotlessTaskImpl.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index 7e56fcc435..c42228bc8a 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -138,15 +138,18 @@ static boolean isHydrated(SpotlessTask task) { static void assertHydrated(SpotlessTask task) { if (!isHydrated(task)) { - throw new GradleException("Spotless doesn't support configuration cache yet.\n" + - "Rerun with --no-configuration-cache"); + throw new GradleException("Spotless needs a workaround to support configuration cache:\n" + + " (in your root build.gradle)\n" + + " apply plugin: 'com.diffplug.spotless-setup\n" + + " spotlessSetup { jvmLocalCache = true }\n" + + "To make this workaround obsolete, please upvote https://github.com/diffplug/spotless/issues/987"); } } static GradleException cacheIsStale() { - return new GradleException("Spotless daemon-local cache is stale. Regenerate the cache with\n" + + return new GradleException("Spotless JVM-local cache is stale. Regenerate the cache with\n" + " " + (FileSignature.machineIsWin() ? "rmdir /q /s" : "rm -rf") + " .gradle/configuration-cache\n" + - "For more information see #123"); + "To make this workaround obsolete, please upvote https://github.com/diffplug/spotless/issues/987"); } static class LiveCache { From 166ab79060165af911a446a6cbf99469e85237a1 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 16:14:20 -0800 Subject: [PATCH 11/15] Migrate from our one-off to the design we'd like Gradle to adopt. --- .../gradle/spotless/JvmLocalCache.java | 105 ++++++++++++++++++ .../gradle/spotless/SpotlessTask.java | 37 +++--- .../gradle/spotless/SpotlessTaskImpl.java | 44 -------- .../gradle/spotless/SpotlessTaskService.java | 32 ------ 4 files changed, 125 insertions(+), 93 deletions(-) create mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java new file mode 100644 index 0000000000..edc52afdac --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java @@ -0,0 +1,105 @@ +/* + * Copyright 2021 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import java.io.File; +import java.io.Serializable; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import javax.annotation.Nullable; + +import org.gradle.api.GradleException; +import org.gradle.api.Task; + +import com.diffplug.spotless.FileSignature; + +class JvmLocalCache { + private static GradleException cacheIsStale() { + return new GradleException("Spotless JVM-local cache is stale. Regenerate the cache with\n" + + " " + (FileSignature.machineIsWin() ? "rmdir /q /s" : "rm -rf") + " .gradle/configuration-cache\n" + + "To make this workaround obsolete, please upvote https://github.com/diffplug/spotless/issues/987"); + } + + interface CacheKey { + T get(); + + void set(T value); + } + + static CacheKey createCacheKey(Task task, String keyName, @Nullable T initialValue) { + CacheKeyImpl key = new CacheKeyImpl(new InternalCacheKey(task.getProject().getProjectDir(), task.getPath(), keyName)); + if (initialValue != null) { + key.set(initialValue); + } + return key; + } + + static class CacheKeyImpl implements CacheKey, Serializable { + InternalCacheKey internalKey; + + CacheKeyImpl(InternalCacheKey internalKey) { + this.internalKey = internalKey; + } + + @Override + public void set(T value) { + daemonState.put(internalKey, value); + } + + @Override + public T get() { + Object value = daemonState.get(internalKey); + if (value == null) { + // TODO: throw TriggerConfigurationException(); (see https://github.com/diffplug/spotless/issues/987) + throw cacheIsStale(); + } else { + return (T) value; + } + } + } + + private static Map daemonState = Collections.synchronizedMap(new HashMap<>()); + + private static class InternalCacheKey implements Serializable { + private File projectDir; + private String taskPath; + private String keyName; + + InternalCacheKey(File projectDir, String taskPath, String keyName) { + this.projectDir = projectDir; + this.taskPath = taskPath; + this.keyName = keyName; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + InternalCacheKey that = (InternalCacheKey) o; + return projectDir.equals(that.projectDir) && taskPath.equals(that.taskPath) && keyName.equals(that.keyName); + } + + @Override + public int hashCode() { + return Objects.hash(projectDir, taskPath, keyName); + } + } +} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index ae9c286561..1b4662a9a7 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -38,6 +38,7 @@ import org.gradle.api.tasks.PathSensitivity; import org.gradle.work.Incremental; +import com.diffplug.gradle.spotless.JvmLocalCache.CacheKey; import com.diffplug.spotless.FormatExceptionPolicy; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; @@ -48,10 +49,12 @@ public abstract class SpotlessTask extends DefaultTask { @Internal abstract Property getTaskService(); - private void hydrateIfNull(Object o) { - if (o == null) { - getTaskService().get().hydrate(this); - } + protected CacheKey createCacheKey(String keyName) { + return createCacheKey(keyName, null); + } + + protected CacheKey createCacheKey(String keyName, @Nullable T initialValue) { + return JvmLocalCache.createCacheKey(this, keyName, initialValue); } // set by SpotlessExtension, but possibly overridden by FormatExtension @@ -66,16 +69,15 @@ public void setEncoding(String encoding) { this.encoding = Objects.requireNonNull(encoding); } - protected transient LineEnding.Policy lineEndingsPolicy; + protected final CacheKey lineEndingsPolicy = createCacheKey("lineEndingsPolicy"); @Input public LineEnding.Policy getLineEndingsPolicy() { - hydrateIfNull(lineEndingsPolicy); - return lineEndingsPolicy; + return lineEndingsPolicy.get(); } public void setLineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { - this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy); + this.lineEndingsPolicy.set(lineEndingsPolicy); } /*** API which performs git up-to-date tasks. */ @@ -90,7 +92,7 @@ public void setLineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { */ private transient ObjectId subtreeSha = ObjectId.zeroId(); /** Stored so that the configuration cache can recreate the GitRatchetGradle state. */ - protected transient String ratchetFrom; + protected String ratchetFrom; public void setupRatchet(String ratchetFrom) { this.ratchetFrom = ratchetFrom; @@ -119,7 +121,9 @@ ObjectId getRootTreeSha() { @Input public ObjectId getRatchetSha() { - hydrateIfNull(subtreeSha); + if (subtreeSha == null) { + setupRatchet(ratchetFrom); + } return subtreeSha; } @@ -158,20 +162,19 @@ public File getOutputDirectory() { return outputDirectory; } - protected transient List steps = new ArrayList<>(); + protected final CacheKey> steps = createCacheKey("steps", new ArrayList()); @Input public List getSteps() { - hydrateIfNull(steps); - return Collections.unmodifiableList(steps); + return Collections.unmodifiableList(steps.get()); } public void setSteps(List steps) { - this.steps = PluginGradlePreconditions.requireElementsNonNull(steps); + this.steps.set(PluginGradlePreconditions.requireElementsNonNull(steps)); } public boolean addStep(FormatterStep step) { - return this.steps.add(Objects.requireNonNull(step)); + return this.steps.get().add(Objects.requireNonNull(step)); } /** Returns the name of this format. */ @@ -186,10 +189,10 @@ String formatName() { Formatter buildFormatter() { return Formatter.builder() - .lineEndingsPolicy(lineEndingsPolicy) + .lineEndingsPolicy(lineEndingsPolicy.get()) .encoding(Charset.forName(encoding)) .rootDir(getProjectDir().get().getAsFile().toPath()) - .steps(steps) + .steps(steps.get()) .exceptionPolicy(exceptionPolicy) .build(); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index c42228bc8a..a9bf428caa 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -20,8 +20,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.util.List; -import java.util.Objects; import javax.inject.Inject; @@ -37,10 +35,7 @@ import org.gradle.work.InputChanges; import com.diffplug.common.base.StringPrinter; -import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.FormatterStep; -import com.diffplug.spotless.LineEnding; import com.diffplug.spotless.PaddedCell; @CacheableTask @@ -69,7 +64,6 @@ public void performAction(InputChanges inputs) throws Exception { Files.createDirectories(outputDirectory.toPath()); } - assertHydrated(this); try (Formatter formatter = buildFormatter()) { for (FileChange fileChange : inputs.getFileChanges(target)) { File input = fileChange.getFile(); @@ -131,42 +125,4 @@ private File getOutputFile(File input) { } return new File(outputDirectory, outputFileName); } - - static boolean isHydrated(SpotlessTask task) { - return task.lineEndingsPolicy != null; - } - - static void assertHydrated(SpotlessTask task) { - if (!isHydrated(task)) { - throw new GradleException("Spotless needs a workaround to support configuration cache:\n" + - " (in your root build.gradle)\n" + - " apply plugin: 'com.diffplug.spotless-setup\n" + - " spotlessSetup { jvmLocalCache = true }\n" + - "To make this workaround obsolete, please upvote https://github.com/diffplug/spotless/issues/987"); - } - } - - static GradleException cacheIsStale() { - return new GradleException("Spotless JVM-local cache is stale. Regenerate the cache with\n" + - " " + (FileSignature.machineIsWin() ? "rmdir /q /s" : "rm -rf") + " .gradle/configuration-cache\n" + - "To make this workaround obsolete, please upvote https://github.com/diffplug/spotless/issues/987"); - } - - static class LiveCache { - LineEnding.Policy lineEndingsPolicy; - List steps; - String ratchetFrom; - - LiveCache(SpotlessTask task) { - lineEndingsPolicy = Objects.requireNonNull(task.lineEndingsPolicy); - steps = Objects.requireNonNull(task.steps); - ratchetFrom = Objects.requireNonNull(task.ratchetFrom); - } - - void hydrate(SpotlessTask task) { - task.lineEndingsPolicy = lineEndingsPolicy; - task.steps = steps; - task.setupRatchet(ratchetFrom); - } - } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index 143b611916..0b8af7b713 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -44,46 +44,14 @@ * apply already did). */ public abstract class SpotlessTaskService implements BuildService, AutoCloseable, OperationCompletionListener { - private boolean enableConfigCacheDaemonLocal; private Map apply = Collections.synchronizedMap(new HashMap<>()); private Map source = Collections.synchronizedMap(new HashMap<>()); private Map provisioner = Collections.synchronizedMap(new HashMap<>()); - void registerDependenciesTask(RegisterDependenciesTask task) { - enableConfigCacheDaemonLocal = task.getJvmLocalCache(); - } - void registerSourceAlreadyRan(SpotlessTask task) { source.put(task.getPath(), task); - if (enableConfigCacheDaemonLocal) { - storeOrHydrate(task); - } - } - - void hydrate(SpotlessTask task) { - storeOrHydrate(task); - } - - private void storeOrHydrate(SpotlessTask task) { - if (!enableConfigCacheDaemonLocal) { - SpotlessTaskImpl.assertHydrated(task); - return; - } - String cacheKey = task.getProjectDir().getAsFile().get().getAbsolutePath() + ">" + task.getPath(); - if (SpotlessTaskImpl.isHydrated(task)) { - daemonLocalMap.put(cacheKey, new SpotlessTaskImpl.LiveCache(task)); - } else { - SpotlessTaskImpl.LiveCache cached = daemonLocalMap.get(cacheKey); - if (cached == null) { - throw SpotlessTaskImpl.cacheIsStale(); - } else { - cached.hydrate(task); - } - } } - private static final Map daemonLocalMap = Collections.synchronizedMap(new HashMap<>()); - Provisioner provisionerFor(Project project) { return provisioner.computeIfAbsent(project.getPath(), unused -> { return GradleProvisioner.newDedupingProvisioner(project); From ee5aaf821ec750017c443a97635e83d0d04d8040 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 16:21:18 -0800 Subject: [PATCH 12/15] It's hard to turn off JvmLocalCache, and there isn't a good reason to, so we don't have any need for the SpotlessSetup plugin anymore. --- plugin-gradle/build.gradle | 9 ---- .../spotless/RegisterDependenciesTask.java | 10 +---- .../gradle/spotless/SpotlessSetup.java | 37 ---------------- .../gradle/spotless/SpotlessSetupPlugin.java | 30 ------------- .../ConfigurationCacheJvmLocalTest.java | 4 -- .../spotless/ConfigurationCacheTest.java | 44 ------------------- .../gradle/spotless/GitRatchetGradleTest.java | 9 +--- 7 files changed, 3 insertions(+), 140 deletions(-) delete mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java delete mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetupPlugin.java diff --git a/plugin-gradle/build.gradle b/plugin-gradle/build.gradle index 80fc8d6a79..3a5afd2ba8 100644 --- a/plugin-gradle/build.gradle +++ b/plugin-gradle/build.gradle @@ -45,12 +45,6 @@ gradlePlugin { displayName = 'Spotless formatting plugin' description = project.description } - spotlessSetupPlugin { - id = 'com.diffplug.spotless-setup' - implementationClass = 'com.diffplug.gradle.spotless.SpotlessSetupPlugin' - displayName = 'Spotless formatting plugin setup' - description = project.description - } spotlessPluginLegacy { id = 'com.diffplug.gradle.spotless' implementationClass = 'com.diffplug.gradle.spotless.SpotlessPluginRedirect' @@ -84,9 +78,6 @@ if (version.endsWith('-SNAPSHOT')) { 'clang-format' ] plugins { - spotlessSetupPlugin { - id = 'com.diffplug.spotless-setup' - } spotlessPlugin { id = 'com.diffplug.spotless' } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java index b450cf6c1c..db74c04781 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java @@ -20,7 +20,6 @@ import org.gradle.api.DefaultTask; import org.gradle.api.provider.Property; import org.gradle.api.services.BuildServiceRegistry; -import org.gradle.api.tasks.Input; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; import org.gradle.build.event.BuildEventsListenerRegistry; @@ -59,16 +58,9 @@ void setup() { getBuildEventsListenerRegistry().onTaskCompletion(getTaskService()); } - boolean jvmLocalCache; - - @Input - public boolean getJvmLocalCache() { - return jvmLocalCache; - } - @TaskAction public void trivialFunction() { - getTaskService().get().registerDependenciesTask(this); + // nothing to do :) } @Internal diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java deleted file mode 100644 index b4f4394a82..0000000000 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetup.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2021 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.diffplug.gradle.spotless; - -import org.gradle.api.Project; -import org.gradle.api.tasks.TaskProvider; - -public class SpotlessSetup { - static final String NAME = "spotlessSetup"; - - private final TaskProvider task; - - public SpotlessSetup(Project project) { - task = (TaskProvider) (Object) project.getTasks().named(RegisterDependenciesTask.TASK_NAME); - } - - public boolean isJvmLocalCache() { - return task.get().getJvmLocalCache(); - } - - public boolean setJvmLocalCache(boolean enabled) { - return task.get().jvmLocalCache = enabled; - } -} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetupPlugin.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetupPlugin.java deleted file mode 100644 index 38aeadf5d7..0000000000 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessSetupPlugin.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright 2021 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.diffplug.gradle.spotless; - -import org.gradle.api.Plugin; -import org.gradle.api.Project; - -import com.diffplug.common.base.Preconditions; - -public class SpotlessSetupPlugin implements Plugin { - @Override - public void apply(Project rootProject) { - Preconditions.checkArgument(rootProject.getProject() == rootProject.getRootProject(), "com.diffplug.spotless-setup must be applied to only the root project"); - rootProject.getPlugins().apply(SpotlessPlugin.class); - rootProject.getExtensions().create(SpotlessSetup.NAME, SpotlessSetup.class, rootProject); - } -} diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java index b9bc003f8d..a1b2caaa3f 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java @@ -32,13 +32,9 @@ protected GradleRunner gradleRunner() throws IOException { public void gjf() throws IOException { setFile("build.gradle").toLines( "plugins {", - " id 'com.diffplug.spotless-setup'", " id 'com.diffplug.spotless'", "}", "repositories { mavenCentral() }", - "spotlessSetup {", - " jvmLocalCache = true", - "}", "spotless {", " java {", " target file('test.java')", diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java index ae90930417..ddc316d3d8 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java @@ -15,13 +15,8 @@ */ package com.diffplug.gradle.spotless; -import java.io.File; import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Comparator; -import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.GradleRunner; import org.junit.jupiter.api.Test; @@ -64,43 +59,4 @@ public void helpConfiguresIfTasksAreCreated() throws IOException { "tasks.named('spotlessJavaApply').get()"); gradleRunner().withArguments("help").build(); } - - @Test - public void gjf() throws IOException { - setFile("build.gradle").toLines( - "plugins {", - " id 'com.diffplug.spotless'", - "}", - "repositories { mavenCentral() }", - "apply plugin: 'java'", - "spotless {", - " java {", - " target file('test.java')", - " googleJavaFormat('1.2')", - " }", - "}"); - - // first run works - setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); - gradleRunner().withArguments("spotlessApply").build(); - assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); - - // but the second fails - BuildResult failure = gradleRunner().withArguments("spotlessApply").buildAndFail(); - failure.getOutput().contains("> Spotless doesn't support configuration cache yet"); - - // and it will keep failing forever - gradleRunner().withArguments("spotlessApply").buildAndFail(); - - // until you delete the .gradlle/configuration-cache folder - File configCache = new File(super.rootFolder(), ".gradle/configuration-cache"); - Files.walk(configCache.toPath()) - .sorted(Comparator.reverseOrder()) - .map(Path::toFile) - .forEach(File::delete); - - // then it will work again (but only once) - gradleRunner().withArguments("spotlessApply").build(); - gradleRunner().withArguments("spotlessApply").buildAndFail(); - } } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java index bad66378ad..51f4808259 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java @@ -63,9 +63,7 @@ void singleProjectExhaustive(int useConfigCache) throws Exception { setFile("build.gradle").toLines( "plugins {", " id 'com.diffplug.spotless'", - " id 'com.diffplug.spotless-setup'", "}", - "spotlessSetup { jvmLocalCache = true }", "spotless {", " ratchetFrom 'baseline'", " format 'misc', {", @@ -150,7 +148,7 @@ private BuildResultAssertion assertFail(String... tasks) throws Exception { return new BuildResultAssertion(gradleRunner().withArguments(tasks).buildAndFail()); } - private static final String BASELINE_ROOT = "71a2671ed452d52625245e5101cbf8467e905865"; + private static final String BASELINE_ROOT = "fdc3ca3c850cee44d95d32c64cda30afbb29323c"; private static final String BASELINE_CLEAN = "65fdd75c1ae00c0646f6487d68c44ddca51f0841"; private static final String BASELINE_DIRTY = "4cfc3358ccbf186738b82a60276b1e5306bc3870"; @@ -179,10 +177,7 @@ void multiProject(int useConfigCache) throws Exception { " }", "}"); setFile(".gitignore").toContent("build/\n.gradle\n*.properties\n"); - setFile("build.gradle").toLines( - "apply plugin: 'com.diffplug.spotless-setup'", - "spotlessSetup { jvmLocalCache = true }", - "apply from: rootProject.file('spotless.gradle') // root"); + setFile("build.gradle").toContent("apply from: rootProject.file('spotless.gradle') // root"); setFile(TEST_PATH).toContent("HELLO"); setFile("clean/build.gradle").toContent("apply from: rootProject.file('spotless.gradle') // clean"); setFile("clean/" + TEST_PATH).toContent("HELLO"); From cfc82911b2e0961ee0927d92ab1ca5f519628b75 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 16:30:13 -0800 Subject: [PATCH 13/15] Make the diff a little smaller. --- .../gradle/spotless/SpotlessTaskService.java | 18 +++-- .../ConfigurationCacheJvmLocalTest.java | 65 ------------------- .../spotless/ConfigurationCacheTest.java | 36 ++++++++++ 3 files changed, 44 insertions(+), 75 deletions(-) delete mode 100644 plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index 0b8af7b713..de016ae6d2 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -44,13 +44,9 @@ * apply already did). */ public abstract class SpotlessTaskService implements BuildService, AutoCloseable, OperationCompletionListener { - private Map apply = Collections.synchronizedMap(new HashMap<>()); - private Map source = Collections.synchronizedMap(new HashMap<>()); - private Map provisioner = Collections.synchronizedMap(new HashMap<>()); - - void registerSourceAlreadyRan(SpotlessTask task) { - source.put(task.getPath(), task); - } + private final Map apply = Collections.synchronizedMap(new HashMap<>()); + private final Map source = Collections.synchronizedMap(new HashMap<>()); + private final Map provisioner = Collections.synchronizedMap(new HashMap<>()); Provisioner provisionerFor(Project project) { return provisioner.computeIfAbsent(project.getPath(), unused -> { @@ -58,13 +54,16 @@ Provisioner provisionerFor(Project project) { }); } + void registerSourceAlreadyRan(SpotlessTask task) { + source.put(task.getPath(), task); + } + void registerApplyAlreadyRan(SpotlessApply task) { apply.put(task.sourceTaskPath(), task); } - ///////////////// // - private GitRatchetGradle ratchet = new GitRatchetGradle(); + private final GitRatchetGradle ratchet = new GitRatchetGradle(); GitRatchetGradle getRatchet() { return ratchet; @@ -80,7 +79,6 @@ public void close() throws Exception { ratchet.close(); } // - ////////////////// static String INDEPENDENT_HELPER = "Helper"; diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java deleted file mode 100644 index a1b2caaa3f..0000000000 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheJvmLocalTest.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright 2020-2021 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.diffplug.gradle.spotless; - -import java.io.IOException; - -import org.gradle.testkit.runner.BuildResult; -import org.gradle.testkit.runner.GradleRunner; -import org.junit.jupiter.api.Test; - -public class ConfigurationCacheJvmLocalTest extends GradleIntegrationHarness { - @Override - protected GradleRunner gradleRunner() throws IOException { - setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true"); - return super.gradleRunner().withGradleVersion(GradleVersionSupport.CONFIGURATION_CACHE.version); - } - - @Test - public void gjf() throws IOException { - setFile("build.gradle").toLines( - "plugins {", - " id 'com.diffplug.spotless'", - "}", - "repositories { mavenCentral() }", - "spotless {", - " java {", - " target file('test.java')", - " googleJavaFormat('1.2')", - " }", - "}"); - - // first run works - setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); - gradleRunner().withArguments("spotlessApply").build(); - assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); - - // and it keeps working! - gradleRunner().withArguments("spotlessApply", "--stacktrace").build(); - gradleRunner().withArguments("spotlessApply").build(); - gradleRunner().withArguments("spotlessApply").build(); - - setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); - gradleRunner().withArguments("spotlessCheck").buildAndFail(); - gradleRunner().withArguments("spotlessApply").build(); - assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); - - BuildResult failure = gradleRunner().withDebug(true).withArguments("spotlessApply", "--stacktrace").buildAndFail(); - failure.getOutput().contains("Spotless daemon-local cache is stale. Regenerate the cache with\n" + - " rm -rf .gradle/configuration-cache\n" + - "For more information see #123\n"); - } -} diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java index ddc316d3d8..14b6de7702 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java @@ -17,6 +17,7 @@ import java.io.IOException; +import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.GradleRunner; import org.junit.jupiter.api.Test; @@ -59,4 +60,39 @@ public void helpConfiguresIfTasksAreCreated() throws IOException { "tasks.named('spotlessJavaApply').get()"); gradleRunner().withArguments("help").build(); } + + @Test + public void jvmLocalCache() throws IOException { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "repositories { mavenCentral() }", + "spotless {", + " java {", + " target file('test.java')", + " googleJavaFormat('1.2')", + " }", + "}"); + + // first run works + setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + gradleRunner().withArguments("spotlessApply").build(); + assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); + + // and it keeps working! + gradleRunner().withArguments("spotlessApply", "--stacktrace").build(); + gradleRunner().withArguments("spotlessApply").build(); + gradleRunner().withArguments("spotlessApply").build(); + + setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + gradleRunner().withArguments("spotlessCheck").buildAndFail(); + gradleRunner().withArguments("spotlessApply").build(); + assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); + + BuildResult failure = gradleRunner().withDebug(true).withArguments("spotlessApply", "--stacktrace").buildAndFail(); + failure.getOutput().contains("Spotless daemon-local cache is stale. Regenerate the cache with\n" + + " rm -rf .gradle/configuration-cache\n" + + "For more information see #123\n"); + } } From ba490439f27cacc9e188bb39f4a0a4173f83b136 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 16:42:32 -0800 Subject: [PATCH 14/15] Better names for JvmLocalCache, and initial values were a bad idea. --- .../gradle/spotless/JvmLocalCache.java | 24 +++++++------------ .../gradle/spotless/SpotlessTask.java | 17 +++++++------ 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java index edc52afdac..9478e9e66f 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java @@ -22,8 +22,6 @@ import java.util.Map; import java.util.Objects; -import javax.annotation.Nullable; - import org.gradle.api.GradleException; import org.gradle.api.Task; @@ -36,24 +34,20 @@ private static GradleException cacheIsStale() { "To make this workaround obsolete, please upvote https://github.com/diffplug/spotless/issues/987"); } - interface CacheKey { + interface LiveCache { T get(); void set(T value); } - static CacheKey createCacheKey(Task task, String keyName, @Nullable T initialValue) { - CacheKeyImpl key = new CacheKeyImpl(new InternalCacheKey(task.getProject().getProjectDir(), task.getPath(), keyName)); - if (initialValue != null) { - key.set(initialValue); - } - return key; + static LiveCache createLive(Task task, String propertyName) { + return new LiveCacheKeyImpl(new InternalCacheKey(task.getProject().getProjectDir(), task.getPath(), propertyName)); } - static class CacheKeyImpl implements CacheKey, Serializable { + static class LiveCacheKeyImpl implements LiveCache, Serializable { InternalCacheKey internalKey; - CacheKeyImpl(InternalCacheKey internalKey) { + LiveCacheKeyImpl(InternalCacheKey internalKey) { this.internalKey = internalKey; } @@ -79,12 +73,12 @@ public T get() { private static class InternalCacheKey implements Serializable { private File projectDir; private String taskPath; - private String keyName; + private String propertyName; InternalCacheKey(File projectDir, String taskPath, String keyName) { this.projectDir = projectDir; this.taskPath = taskPath; - this.keyName = keyName; + this.propertyName = keyName; } @Override @@ -94,12 +88,12 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; InternalCacheKey that = (InternalCacheKey) o; - return projectDir.equals(that.projectDir) && taskPath.equals(that.taskPath) && keyName.equals(that.keyName); + return projectDir.equals(that.projectDir) && taskPath.equals(that.taskPath) && propertyName.equals(that.propertyName); } @Override public int hashCode() { - return Objects.hash(projectDir, taskPath, keyName); + return Objects.hash(projectDir, taskPath, propertyName); } } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index 1b4662a9a7..fe5628bb83 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -38,7 +38,7 @@ import org.gradle.api.tasks.PathSensitivity; import org.gradle.work.Incremental; -import com.diffplug.gradle.spotless.JvmLocalCache.CacheKey; +import com.diffplug.gradle.spotless.JvmLocalCache.LiveCache; import com.diffplug.spotless.FormatExceptionPolicy; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; @@ -49,12 +49,8 @@ public abstract class SpotlessTask extends DefaultTask { @Internal abstract Property getTaskService(); - protected CacheKey createCacheKey(String keyName) { - return createCacheKey(keyName, null); - } - - protected CacheKey createCacheKey(String keyName, @Nullable T initialValue) { - return JvmLocalCache.createCacheKey(this, keyName, initialValue); + protected LiveCache createLive(String keyName) { + return JvmLocalCache.createLive(this, keyName); } // set by SpotlessExtension, but possibly overridden by FormatExtension @@ -69,7 +65,7 @@ public void setEncoding(String encoding) { this.encoding = Objects.requireNonNull(encoding); } - protected final CacheKey lineEndingsPolicy = createCacheKey("lineEndingsPolicy"); + protected final LiveCache lineEndingsPolicy = createLive("lineEndingsPolicy"); @Input public LineEnding.Policy getLineEndingsPolicy() { @@ -162,7 +158,10 @@ public File getOutputDirectory() { return outputDirectory; } - protected final CacheKey> steps = createCacheKey("steps", new ArrayList()); + protected final LiveCache> steps = createLive("steps"); + { + steps.set(new ArrayList()); + } @Input public List getSteps() { From ca3560e95c99c9fb0d572db6467c4b4270d4e2eb Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 16:57:12 -0800 Subject: [PATCH 15/15] Update changelog. --- plugin-gradle/CHANGES.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 8aeccf8b88..325b5edfcb 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,8 +3,17 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`). ## [Unreleased] +### Added +* Support for Gradle Configuration Cache* ([#982](https://github.com/diffplug/spotless/pull/982), [#986](https://github.com/diffplug/spotless/pull/986)) + * *Spotless must run on the same daemon that wrote the configuration cache. If it isn't, you'll get this error message: + * ``` + Spotless JVM-local cache is stale. Regenerate the cache with + rm -rf .gradle/configuration-cache + ``` + * To make this daemon-restriction obsolete, please see and upvote [#987](https://github.com/diffplug/spotless/issues/987). ### Changed -* **BREAKING** Previously, many projects required `buildscript { repositories { mavenCentral() }}` at the top of their root project, because Spotless resolved its dependencies using the buildscript repositories. Spotless now resolves its dependencies from the normal project repositories of each project with a `spotless {...}` block. This means that you can remove the `buildscript {}` block, but you still need a `repositories { mavenCentral() }` (or similar) in each project which is using Spotless. +* **BREAKING** Previously, many projects required `buildscript { repositories { mavenCentral() }}` at the top of their root project, because Spotless resolved its dependencies using the buildscript repositories. Spotless now resolves its dependencies from the normal project repositories of each project with a `spotless {...}` block. This means that you can remove the `buildscript {}` block, but you still need a `repositories { mavenCentral() }` (or similar) in each project which is using Spotless. ([#980](https://github.com/diffplug/spotless/pull/980), [#983](https://github.com/diffplug/spotless/pull/983)) + * If you prefer the old behavior, we are open to adding that back as a new feature, see [#984 predeclared dependencies](https://github.com/diffplug/spotless/issues/984). * **BREAKING** `createIndepentApplyTask(String taskName)` now requires that `taskName` does not end with `Apply` * Bump minimum required Gradle from `6.1` to `6.1.1`.