diff --git a/CHANGES.md b/CHANGES.md index 0e44b999e2..3ad46fb24c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,6 +26,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Fixed * Ignore system git config when running tests ([#1990](https://github.com/diffplug/spotless/issues/1990)) * Correctly provide EditorConfig property types for Ktlint ([#2052](https://github.com/diffplug/spotless/issues/2052)) +* Made ShadowCopy (`npmInstallCache`) more robust by re-creating the cache dir if it goes missing ([#1984](https://github.com/diffplug/spotless/issues/1984),[2096](https://github.com/diffplug/spotless/pull/2096)) ## [2.45.0] - 2024-01-23 ### Added diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java index 8a3af5fdf2..2f1addf2af 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -38,11 +38,11 @@ public class NodeModulesCachingNpmProcessFactory implements NpmProcessFactory { private NodeModulesCachingNpmProcessFactory(@Nonnull File cacheDir) { this.cacheDir = Objects.requireNonNull(cacheDir); - assertDir(cacheDir); - this.shadowCopy = new ShadowCopy(cacheDir); + assertDir(); // throws if cacheDir is not a directory + this.shadowCopy = new ShadowCopy(this::assertDir); } - private void assertDir(File cacheDir) { + private synchronized File assertDir() { if (cacheDir.exists() && !cacheDir.isDirectory()) { throw new IllegalArgumentException("Cache dir must be a directory"); } @@ -51,6 +51,7 @@ private void assertDir(File cacheDir) { throw new IllegalArgumentException("Cache dir could not be created."); } } + return cacheDir; } public static NodeModulesCachingNpmProcessFactory create(@Nonnull File cacheDir) { diff --git a/lib/src/main/java/com/diffplug/spotless/npm/ShadowCopy.java b/lib/src/main/java/com/diffplug/spotless/npm/ShadowCopy.java index 044b5d70ea..0241d0cbcc 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/ShadowCopy.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/ShadowCopy.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ import java.nio.file.attribute.BasicFileAttributes; import java.time.Duration; import java.util.concurrent.TimeoutException; +import java.util.function.Supplier; import javax.annotation.Nonnull; @@ -39,13 +40,18 @@ class ShadowCopy { private static final Logger logger = LoggerFactory.getLogger(ShadowCopy.class); - private final File shadowCopyRoot; + private final Supplier shadowCopyRootSupplier; - public ShadowCopy(@Nonnull File shadowCopyRoot) { - this.shadowCopyRoot = shadowCopyRoot; + public ShadowCopy(@Nonnull Supplier shadowCopyRootSupplier) { + this.shadowCopyRootSupplier = shadowCopyRootSupplier; + } + + private File shadowCopyRoot() { + File shadowCopyRoot = shadowCopyRootSupplier.get(); if (!shadowCopyRoot.isDirectory()) { - throw new IllegalArgumentException("Shadow copy root must be a directory: " + shadowCopyRoot); + throw new IllegalStateException("Shadow copy root must be a directory: " + shadowCopyRoot); } + return shadowCopyRoot; } public void addEntry(String key, File orig) { @@ -86,17 +92,17 @@ private void cleanupReservation(String key) { } private Path markerFilePath(String key) { - return Paths.get(shadowCopyRoot.getAbsolutePath(), key + ".marker"); + return Paths.get(shadowCopyRoot().getAbsolutePath(), key + ".marker"); } private File entry(String key, String origName) { - return Paths.get(shadowCopyRoot.getAbsolutePath(), key, origName).toFile(); + return Paths.get(shadowCopyRoot().getAbsolutePath(), key, origName).toFile(); } private boolean reserveSubFolder(String key) { // put a marker file named "key".marker in "shadowCopyRoot" to make sure no other process is using it or return false if it already exists try { - Files.createFile(Paths.get(shadowCopyRoot.getAbsolutePath(), key + ".marker")); + Files.createFile(Paths.get(shadowCopyRoot().getAbsolutePath(), key + ".marker")); return true; } catch (FileAlreadyExistsException e) { return false; diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 9ff689d273..fe543d8014 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -8,6 +8,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Ignore system git config when running tests ([#1990](https://github.com/diffplug/spotless/issues/1990)) * Correctly provide EditorConfig property types for Ktlint ([#2052](https://github.com/diffplug/spotless/issues/2052)) * Fixed memory leak introduced in 6.21.0 ([#2067](https://github.com/diffplug/spotless/issues/2067)) +* Made ShadowCopy (`npmInstallCache`) more robust by re-creating the cache dir if it goes missing ([#1984](https://github.com/diffplug/spotless/issues/1984),[2096](https://github.com/diffplug/spotless/pull/2096)) ### Changes * Bump default `ktfmt` version to latest `0.46` -> `0.47`. ([#2045](https://github.com/diffplug/spotless/pull/2045)) * Bump default `sortpom` version to latest `3.2.1` -> `3.4.0`. ([#2049](https://github.com/diffplug/spotless/pull/2049)) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java index 74b6db9167..9799fbcf15 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2023 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -358,6 +358,54 @@ void autodetectNpmrcFileConfig(String prettierVersion) throws IOException { Assertions.assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); } + @ParameterizedTest(name = "{index}: verifyCleanAndSpotlessWorks with prettier {0}") + @ValueSource(strings = {PRETTIER_VERSION_2, PRETTIER_VERSION_3}) + void verifyCleanAndSpotlessWorks(String prettierVersion) throws IOException { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "repositories { mavenCentral() }", + "def prettierConfig = [:]", + "prettierConfig['printWidth'] = 20", + "prettierConfig['parser'] = 'typescript'", + "spotless {", + " format 'mytypescript', {", + " target 'test.ts'", + " prettier('" + prettierVersion + "').config(prettierConfig)", + " }", + "}"); + setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); + final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + final BuildResult spotlessApply2 = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); + Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); + } + + @ParameterizedTest(name = "{index}: verifyCleanAndSpotlessWithNpmInstallCacheWorks with prettier {0}") + @ValueSource(strings = {PRETTIER_VERSION_2, PRETTIER_VERSION_3}) + void verifyCleanAndSpotlessWithNpmInstallCacheWorks(String prettierVersion) throws IOException { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "repositories { mavenCentral() }", + "def prettierConfig = [:]", + "prettierConfig['printWidth'] = 20", + "prettierConfig['parser'] = 'typescript'", + "spotless {", + " format 'mytypescript', {", + " target 'test.ts'", + " prettier('" + prettierVersion + "').npmInstallCache().config(prettierConfig)", + " }", + "}"); + setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); + final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + final BuildResult spotlessApply2 = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); + Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); + } + @ParameterizedTest(name = "{index}: autodetectNpmrcFileConfig with prettier {0}") @ValueSource(strings = {PRETTIER_VERSION_2, PRETTIER_VERSION_3}) void pickupNpmrcFileConfig(String prettierVersion) throws IOException { diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index a759f923b9..bd0b6ca7c5 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -6,6 +6,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Fixed * Ignore system git config when running tests ([#1990](https://github.com/diffplug/spotless/issues/1990)) * Correctly provide EditorConfig property types for Ktlint ([#2052](https://github.com/diffplug/spotless/issues/2052)) +* Made ShadowCopy (`npmInstallCache`) more robust by re-creating the cache dir if it goes missing ([#1984](https://github.com/diffplug/spotless/issues/1984),[2096](https://github.com/diffplug/spotless/pull/2096)) ### Changes * Bump default `ktfmt` version to latest `0.46` -> `0.47`. ([#2045](https://github.com/diffplug/spotless/pull/2045)) * Bump default `sortpom` version to latest `3.2.1` -> `3.4.0`. ([#2049](https://github.com/diffplug/spotless/pull/2049)) diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/ShadowCopyTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/ShadowCopyTest.java index 1ef1b77caa..297fa1d5bd 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/ShadowCopyTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/ShadowCopyTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import com.diffplug.common.base.Suppliers; import com.diffplug.spotless.ResourceHarness; class ShadowCopyTest extends ResourceHarness { @@ -43,7 +44,7 @@ class ShadowCopyTest extends ResourceHarness { @BeforeEach void setUp() throws IOException { shadowCopyRoot = newFolder("shadowCopyRoot"); - shadowCopy = new ShadowCopy(shadowCopyRoot); + shadowCopy = new ShadowCopy(Suppliers.ofInstance(shadowCopyRoot)); } @Test