Skip to content

fix: allow concurrent prettier setups #2462

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
### Changed
* Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447))
* Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448))
* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462))

## [3.1.0] - 2025-02-20
### Added
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright 2025 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.spotless.npm;

import java.io.File;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

import javax.annotation.Nonnull;

import com.diffplug.spotless.ThrowingEx;

interface ExclusiveFolderAccess {

static ExclusiveFolderAccess forFolder(@Nonnull File folder) {
return forFolder(folder.getAbsolutePath());
}

static ExclusiveFolderAccess forFolder(@Nonnull String path) {
return new ExclusiveFolderAccessSharedMutex(Objects.requireNonNull(path));
}

void runExclusively(ThrowingEx.Runnable runnable);

class ExclusiveFolderAccessSharedMutex implements ExclusiveFolderAccess {

private static final ConcurrentHashMap<String, Lock> mutexes = new ConcurrentHashMap<>();

private final String path;

private ExclusiveFolderAccessSharedMutex(@Nonnull String path) {
this.path = Objects.requireNonNull(path);
}

private Lock getMutex() {
return mutexes.computeIfAbsent(path, k -> new ReentrantLock());
}

@Override
public void runExclusively(ThrowingEx.Runnable runnable) {
final Lock lock = getMutex();
try {
lock.lock();
runnable.run();
} catch (Exception e) {
throw ThrowingEx.asRuntime(e);
} finally {
lock.unlock();
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023-2024 DiffPlug
* Copyright 2023-2025 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,6 +18,7 @@
import java.io.File;
import java.util.List;
import java.util.Objects;
import java.util.UUID;

import javax.annotation.Nonnull;

Expand Down Expand Up @@ -65,8 +66,8 @@ public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, Npm
}

@Override
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) {
return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations);
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) {
return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations, nodeServerInstanceId);
}

private class CachingNmpInstall implements NpmProcess {
Expand Down
8 changes: 5 additions & 3 deletions lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-2025 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,6 +15,8 @@
*/
package com.diffplug.spotless.npm;

import java.util.UUID;

import javax.annotation.Nonnull;

import org.slf4j.Logger;
Expand All @@ -32,9 +34,9 @@ public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConf
super(nodeServerLayout, npmConfig, formatterStepLocations);
}

ProcessRunner.LongRunningProcess startNpmServeProcess() {
ProcessRunner.LongRunningProcess startNpmServeProcess(UUID nodeServerInstanceId) {
return timedLogger.withInfo("Starting npm based server in {} with {}.", this.nodeServerLayout.nodeModulesDir(), this.npmProcessFactory.describe())
.call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations).start());
.call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations, nodeServerInstanceId).start());
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2024 DiffPlug
* Copyright 2016-2025 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -25,13 +25,15 @@
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.diffplug.spotless.FormatterFunc;
import com.diffplug.spotless.ProcessRunner;
import com.diffplug.spotless.ProcessRunner.LongRunningProcess;
import com.diffplug.spotless.ThrowingEx;

Expand Down Expand Up @@ -87,14 +89,17 @@ protected void prepareNodeServer() throws IOException {
}

protected void assertNodeServerDirReady() throws IOException {
if (needsPrepareNodeServerLayout()) {
// reinstall if missing
prepareNodeServerLayout();
}
if (needsPrepareNodeServer()) {
// run npm install if node_modules is missing
prepareNodeServer();
}
ExclusiveFolderAccess.forFolder(nodeServerLayout.nodeModulesDir())
.runExclusively(() -> {
if (needsPrepareNodeServerLayout()) {
// reinstall if missing
prepareNodeServerLayout();
}
if (needsPrepareNodeServer()) {
// run npm install if node_modules is missing
prepareNodeServer();
}
});
}

protected boolean needsPrepareNodeServer() {
Expand All @@ -109,12 +114,13 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept
assertNodeServerDirReady();
LongRunningProcess server = null;
try {
// The npm process will output the randomly selected port of the http server process to 'server.port' file
final UUID nodeServerInstanceId = UUID.randomUUID();
// The npm process will output the randomly selected port of the http server process to 'server-<id>.port' file
// so in order to be safe, remove such a file if it exists before starting.
final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), "server.port");
final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), String.format("server-%s.port", nodeServerInstanceId));
NpmResourceHelper.deleteFileIfExists(serverPortFile);
// start the http server in node
server = nodeServeApp.startNpmServeProcess();
server = nodeServeApp.startNpmServeProcess(nodeServerInstanceId);

// await the readiness of the http server - wait for at most 60 seconds
try {
Expand All @@ -124,10 +130,12 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept
try {
if (server.isAlive()) {
server.destroyForcibly();
server.waitFor();
ProcessRunner.Result result = server.result();
logger.info("Launching npm server process failed. Process result:\n{}", result);
}
} catch (Throwable t) {
// ignore
ProcessRunner.Result result = ThrowingEx.get(server::result);
logger.debug("Unable to forcibly end the server process. Process result:\n{}", result, t);
}
throw timeoutException;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-2025 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,6 +15,8 @@
*/
package com.diffplug.spotless.npm;

import java.util.UUID;

public interface NpmProcessFactory {

enum OnlinePreferrence {
Expand All @@ -33,7 +35,7 @@ public String option() {

NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, OnlinePreferrence onlinePreferrence);

NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations);
NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId);

default String describe() {
return getClass().getSimpleName();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-2025 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.ExecutionException;

import com.diffplug.spotless.ProcessRunner;
Expand All @@ -37,8 +38,8 @@ public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, Npm
}

@Override
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) {
return new NpmServe(nodeServerLayout.nodeModulesDir(), formatterStepLocations);
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) {
return new NpmServe(nodeServerLayout.nodeModulesDir(), formatterStepLocations, nodeServerInstanceId);
}

private static abstract class AbstractStandardNpmProcess {
Expand Down Expand Up @@ -119,16 +120,21 @@ public ProcessRunner.Result waitFor() {

private static class NpmServe extends AbstractStandardNpmProcess implements NpmLongRunningProcess {

public NpmServe(File workingDir, NpmFormatterStepLocations formatterStepLocations) {
private final UUID nodeServerInstanceId;

public NpmServe(File workingDir, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) {
super(workingDir, formatterStepLocations);
this.nodeServerInstanceId = nodeServerInstanceId;
}

@Override
protected List<String> commandLine() {
return List.of(
npmExecutable(),
"start",
"--scripts-prepend-node-path=true");
"--scripts-prepend-node-path=true",
"--",
"--node-server-instance-id=" + nodeServerInstanceId);
}

@Override
Expand Down
30 changes: 25 additions & 5 deletions lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const GracefulShutdownManager = require("@moebius/http-graceful-shutdown").Grace
const express = require("express");
const app = express();

app.use(express.json({ limit: "50mb" }));
app.use(express.json({limit: "50mb"}));

const fs = require("fs");

Expand All @@ -14,13 +14,33 @@ function debugLog() {
}
}

function getInstanceId() {
const args = process.argv.slice(2);

// Look for the --node-server-instance-id option
let instanceId;

args.forEach(arg => {
if (arg.startsWith('--node-server-instance-id=')) {
instanceId = arg.split('=')[1];
}
});

// throw if instanceId is not set
if (!instanceId) {
throw new Error("Missing --node-server-instance-id argument");
}
return instanceId;
}

var listener = app.listen(0, "127.0.0.1", () => {
debugLog("Server running on port " + listener.address().port);
fs.writeFile("server.port.tmp", "" + listener.address().port, function(err) {
const instanceId = getInstanceId();
debugLog("Server running on port " + listener.address().port + " for instance " + instanceId);
fs.writeFile("server.port.tmp", "" + listener.address().port, function (err) {
if (err) {
return console.log(err);
} else {
fs.rename("server.port.tmp", "server.port", function(err) {
fs.rename("server.port.tmp", `server-${instanceId}.port`, function (err) {
if (err) {
return console.log(err);
}
Expand All @@ -32,7 +52,7 @@ const shutdownManager = new GracefulShutdownManager(listener);

app.post("/shutdown", (req, res) => {
res.status(200).send("Shutting down");
setTimeout(function() {
setTimeout(function () {
shutdownManager.terminate(() => debugLog("graceful shutdown finished."));
}, 200);
});
Expand Down
1 change: 1 addition & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
* Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447))
* Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448))
* Apply Gradle's strict plugin types validation to the Spotless plugin. ([#2454](https://github.com/diffplug/spotless/pull/2454))
* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462))

## [7.0.2] - 2025-01-14
### Fixed
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 (
### Changed
* Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447))
* Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448))
* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462))

## [2.44.3] - 2025-02-20
* Support for `clang-format` ([#2406](https://github.com/diffplug/spotless/pull/2406))
Expand Down
Loading