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`. 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. */ 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..9478e9e66f --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java @@ -0,0 +1,99 @@ +/* + * 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 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 LiveCache { + T get(); + + void set(T value); + } + + static LiveCache createLive(Task task, String propertyName) { + return new LiveCacheKeyImpl(new InternalCacheKey(task.getProject().getProjectDir(), task.getPath(), propertyName)); + } + + static class LiveCacheKeyImpl implements LiveCache, Serializable { + InternalCacheKey internalKey; + + LiveCacheKeyImpl(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 propertyName; + + InternalCacheKey(File projectDir, String taskPath, String keyName) { + this.projectDir = projectDir; + this.taskPath = taskPath; + this.propertyName = 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) && propertyName.equals(that.propertyName); + } + + @Override + public int hashCode() { + return Objects.hash(projectDir, taskPath, propertyName); + } + } +} 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..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 @@ -15,22 +15,16 @@ */ 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.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! @@ -46,29 +40,18 @@ 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); } - 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 -> {})); @@ -76,9 +59,8 @@ void setup() { } @TaskAction - public void trivialFunction() throws IOException { - Files.createParentDirs(unitOutput); - Files.write(Integer.toString(1), unitOutput, StandardCharsets.UTF_8); + public void trivialFunction() { + // nothing to do :) } @Internal 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..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,6 +38,7 @@ import org.gradle.api.tasks.PathSensitivity; import org.gradle.work.Incremental; +import com.diffplug.gradle.spotless.JvmLocalCache.LiveCache; import com.diffplug.spotless.FormatExceptionPolicy; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; @@ -48,6 +49,10 @@ public abstract class SpotlessTask extends DefaultTask { @Internal abstract Property getTaskService(); + protected LiveCache createLive(String keyName) { + return JvmLocalCache.createLive(this, keyName); + } + // set by SpotlessExtension, but possibly overridden by FormatExtension protected String encoding = "UTF-8"; @@ -60,15 +65,15 @@ public void setEncoding(String encoding) { this.encoding = Objects.requireNonNull(encoding); } - protected transient LineEnding.Policy lineEndingsPolicy; + protected final LiveCache lineEndingsPolicy = createLive("lineEndingsPolicy"); @Input public LineEnding.Policy getLineEndingsPolicy() { - 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. */ @@ -82,12 +87,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 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 +117,9 @@ ObjectId getRootTreeSha() { @Input public ObjectId getRatchetSha() { + if (subtreeSha == null) { + setupRatchet(ratchetFrom); + } return subtreeSha; } @@ -143,19 +158,22 @@ public File getOutputDirectory() { return outputDirectory; } - protected transient List steps = new ArrayList<>(); + protected final LiveCache> steps = createLive("steps"); + { + steps.set(new ArrayList()); + } @Input public List getSteps() { - 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. */ @@ -170,10 +188,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 e33e7f3ec6..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 @@ -64,22 +64,17 @@ 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); - } + 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"); } } 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..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 @@ -15,11 +15,7 @@ */ 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; @@ -66,13 +62,12 @@ public void helpConfiguresIfTasksAreCreated() throws IOException { } @Test - public void gjf() throws IOException { + public void jvmLocalCache() throws IOException { setFile("build.gradle").toLines( "plugins {", " id 'com.diffplug.spotless'", "}", "repositories { mavenCentral() }", - "apply plugin: 'java'", "spotless {", " java {", " target file('test.java')", @@ -85,22 +80,19 @@ public void gjf() throws IOException { 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); + // and it keeps working! + gradleRunner().withArguments("spotlessApply", "--stacktrace").build(); + gradleRunner().withArguments("spotlessApply").build(); + gradleRunner().withArguments("spotlessApply").build(); - // then it will work again (but only once) + setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + gradleRunner().withArguments("spotlessCheck").buildAndFail(); gradleRunner().withArguments("spotlessApply").build(); - gradleRunner().withArguments("spotlessApply").buildAndFail(); + 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/GitRatchetGradleTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java index ca9324b123..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 @@ -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,22 @@ 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'", + "}", "spotless {", " ratchetFrom 'baseline'", " format 'misc', {", @@ -135,13 +148,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 = "fdc3ca3c850cee44d95d32c64cda30afbb29323c"; 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 +176,18 @@ void multiProject() throws Exception { " bumpThisNumberIfACustomStepChanges(1)", " }", "}"); - setFile(".gitignore").toContent("build/\n.gradle\n"); + setFile(".gitignore").toContent("build/\n.gradle\n*.properties\n"); 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"); 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 +201,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) 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