fix: allow concurrent prettier setups

[simschla]

One of my colleagues had strange issues in his project when using prettier - issues, I'd never seen in our project setups. The NPM-based steps behaved erratically. Sometimes, they did not work, sometimes they did. The behavior was different if he was using configuration cache then when he did not. It also seemed to make a difference which spotlessXYCheck task he started - or if he started the root task spotlessCheck.
We did a lot of debugging and finally found the culprit: They were making heavy use of prettier for their formatting. They used the same prettier setup for several formatters, which turned out to be a problem. A setup like the following triggers the issue:

spotless {
    format 'mytypescript', {
        target 'test.ts'
        prettier().config(prettierConfig1)
    }
    format 'json', {
        target 'test.json'
        prettier().config(prettierConfig2)
    }
    javascript {
        target 'test.js'
        prettier().config(prettierConfig3)
    }
}

All these prettier()-setups are resolving internally to the same package.json which in turn resolved to the same path for the node_modules directory to use. So each of these steps tried to install its node_modules into the same directory and start a server from there. Sometimes concurrently, sometimes not (depending on the files involved, the scheduling done by Gradle, race conditions, ...).

This PR fixes this by making sure that only one formatter modifies the node environment (package.json / node_modules via npm install). Additionally, we add explicit support for launching multiple node-formatter instances in the same node_dir.

[nedtwigg]

I really don't like the plumbing changes that this requires. A FormatterStep should not need to know about the Formatter it came from.

If multiple steps all resolve to the same configuration, that should be a blessing, right? We only need to resolve the dependencies once now, instead of three indepdendent times, if we can manage the concurrency.

I would try again with either a static mutex or a lockfile.

[simschla]

I really don't like the plumbing changes that this requires. A FormatterStep should not need to know about the Formatter it came from.

If multiple steps all resolve to the same configuration, that should be a blessing, right? We only need to resolve the dependencies once now, instead of three indepdendent times, if we can manage the concurrency.

I would try again with either a static mutex or a lockfile.

Never thought of it that way — but if we can make it work using the same on-disk resources, that would be totally fine too 😄

There are three requirements we'd need to satisfy to go down that route:

1. Avoid multiple installations
   We need to ensure the npm dependencies are installed only once.

2. Distinguish between server instances
   If multiple servers are launched from the same node_modules directory, we need a way to identify which server was started for which Java formatter instance.

3. Support concurrent usage
   The npm modules must allow concurrent access.

Regarding point 3, I believe we're safe — since we don’t mutate any shared state on disk once the npm servers are installed and running.

For 1, we could use Java semaphores to guard the install step. Lock files might work too — I’ll need to investigate further.

2 is currently handled by the npm server writing a file with its assigned port when it starts. To make this work concurrently, we need a reliable way to map Java formatter instances to the correct npm server ports. Two options come to mind:

• Pass a unique identifier from the formatter to the npm process on launch, so we can name the port file accordingly. This would allow each formatter to read its own port file reliably. (My preference.)
• Use stdout/stderr to pass the port from the npm process back to the Java side. (Not ideal — I'd rather avoid parsing output streams if we can help it.)

Alternatively — and this just occurred to me — what if we don’t run the npm module as a long-running server at all? Instead, we could call it as a CLI tool per format call. This would simplify things in terms of lifecycle and coordination, though of course it could have performance tradeoffs. On the flip side, it would increase robustness and reduce the coupling between formatter and process.

Curious to hear your thoughts on all of the above — especially the CLI-per-call idea!

[nedtwigg]

Sorry for the slow review, this second attempt looks great, and is good to merge imo.

If I undertstand correctly, because of the ExclusiveFolderAccess, I would have guessed that we didn't need the UUID. But it's not a problem to have the UUID, so I'm fine keeping it.

If you're ready for this to get merged and released lemme know and I'll pull the trigger :)

[nedtwigg]

Seems like we don't need the UUID because we have the ExclusiveFolderAccess?

[simschla]

I'm using the ExclusiveFolderAccess to make sure that npm install is not run concurrently. Launching the server in parallel, however, is now explicitly supported and thus we use the UUID to give each java thread its corresponding running npm server (differentiated via the UUID passed on launch). Hope that clears things up.

[simschla]

Sorry for the slow review, this second attempt looks great, and is good to merge imo.

If I undertstand correctly, because of the ExclusiveFolderAccess, I would have guessed that we didn't need the UUID. But it's not a problem to have the UUID, so I'm fine keeping it.

If you're ready for this to get merged and released lemme know and I'll pull the trigger :)

From my end, the current solution fixes the root cause, and it is an even better approach than my initial suggestion 👍 Thanks for the constructive feedback on this. Merge away if you're fine with the solution, too.