From c35888a27e095661162572e288a6b67e0a53ddd9 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Thu, 11 Apr 2024 20:37:12 +0200 Subject: [PATCH 1/3] chore: try reproducing issue 1984 --- .../spotless/PrettierIntegrationTest.java | 50 ++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) 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 { From f3dc4801b4288dd0f97a6fe5c284e59ff3304910 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Thu, 11 Apr 2024 20:53:18 +0200 Subject: [PATCH 2/3] fix: increase robustness by recreating cache dir --- .../NodeModulesCachingNpmProcessFactory.java | 9 ++++---- .../com/diffplug/spotless/npm/ShadowCopy.java | 22 ++++++++++++------- .../diffplug/spotless/npm/ShadowCopyTest.java | 5 +++-- 3 files changed, 22 insertions(+), 14 deletions(-) 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/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 From dda3a8e2cde34c1b4a3d903a4a4972d674fde3bc Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Thu, 11 Apr 2024 20:53:28 +0200 Subject: [PATCH 3/3] chore: add changelog --- CHANGES.md | 1 + plugin-gradle/CHANGES.md | 1 + plugin-maven/CHANGES.md | 1 + 3 files changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 22f14f2137..aab29376e3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Bump default `shfmt` version to latest `3.7.0` -> `3.8.0`. ([#2050](https://github.com/diffplug/spotless/pull/2050)) * Bump default `ktlint` version to latest `1.1.1` -> `1.2.1`. ([#2057](https://github.com/diffplug/spotless/pull/2057)) * Bump default `sortpom` version to latest `3.4.0` -> `3.4.1`. ([#2078](https://github.com/diffplug/spotless/pull/2078)) +* 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)) ### Removed * **BREAKING** Remove `JarState.getMavenCoordinate(String prefix)`. ([#1945](https://github.com/diffplug/spotless/pull/1945)) * **BREAKING** Replace `PipeStepPair` with `FenceStep`. ([#1954](https://github.com/diffplug/spotless/pull/1954)) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index ffaaa932dc..5865966320 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -14,6 +14,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Bump default `shfmt` version to latest `3.7.0` -> `3.8.0`. ([#2050](https://github.com/diffplug/spotless/pull/2050)) * Bump default `ktlint` version to latest `1.1.1` -> `1.2.1`. ([#2057](https://github.com/diffplug/spotless/pull/2057)) * Bump default `sortpom` version to latest `3.4.0` -> `3.4.1`. ([#2078](https://github.com/diffplug/spotless/pull/2078)) +* 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)) ### Added * Respect `.editorconfig` settings for formatting shell via `shfmt` ([#2031](https://github.com/diffplug/spotless/pull/2031)) * Add support for formatting and sorting Maven POMs ([#2082](https://github.com/diffplug/spotless/issues/2082)) diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index af419b1ad3..e2858b1095 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -12,6 +12,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Bump default `shfmt` version to latest `3.7.0` -> `3.8.0`. ([#2050](https://github.com/diffplug/spotless/pull/2050)) * Bump default `ktlint` version to latest `1.1.1` -> `1.2.1`. ([#2057](https://github.com/diffplug/spotless/pull/2057)) * Bump default `sortpom` version to latest `3.4.0` -> `3.4.1`. ([#2078](https://github.com/diffplug/spotless/pull/2078)) +* 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)) ### Added * Respect `.editorconfig` settings for formatting shell via `shfmt` ([#2031](https://github.com/diffplug/spotless/pull/2031)) * Skip execution in M2E (incremental) builds by default ([#1814](https://github.com/diffplug/spotless/issues/1814), [#2037](https://github.com/diffplug/spotless/issues/2037))