Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make npmInstallCache more robust #2096

Merged
merged 4 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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");
}
Expand All @@ -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) {
Expand Down
22 changes: 14 additions & 8 deletions lib/src/main/java/com/diffplug/spotless/npm/ShadowCopy.java
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -39,13 +40,18 @@ class ShadowCopy {

private static final Logger logger = LoggerFactory.getLogger(ShadowCopy.class);

private final File shadowCopyRoot;
private final Supplier<File> shadowCopyRootSupplier;

public ShadowCopy(@Nonnull File shadowCopyRoot) {
this.shadowCopyRoot = shadowCopyRoot;
public ShadowCopy(@Nonnull Supplier<File> 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) {
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions plugin-maven/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
Loading