Prettier dies when directory is removed before spotlessApply

[fdw]

I'm using Spotless with Prettier in a Gradle submodule. (Another submodule also uses Spotless, but with Kotlin.)
My build task will clean the /build directory before starting, as the compiled Javascript resources will end up in there. However, it seems that Spotless first creates a directory spotless-node-modules-prettier-format in /build, which will then get cleaned by the build task, and then the actual spotlessCheck fails quite ugly:

Step 'prettier-format' found problem in 'frontend/src/components/App.tsx':
Failed to launch npm command 'npm start --scripts-prepend-node-path=true'.
com.diffplug.spotless.npm.NpmProcess$NpmProcessException: Failed to launch npm command 'npm start --scripts-prepend-node-path=true'.
        at com.diffplug.spotless.npm.NpmProcess.npm(NpmProcess.java:66)
        at com.diffplug.spotless.npm.NpmProcess.start(NpmProcess.java:42)
        at com.diffplug.spotless.npm.NpmFormatterStepStateBase.npmRunServer(NpmFormatterStepStateBase.java:95)
        at com.diffplug.spotless.npm.PrettierFormatterStep$State.createFormatterFunc(PrettierFormatterStep.java:84)
        at com.diffplug.spotless.FormatterStepImpl$Standard.format(FormatterStepImpl.java:76)
        at com.diffplug.spotless.FormatterStep$Strict.format(FormatterStep.java:76)
        at com.diffplug.spotless.Formatter.compute(Formatter.java:230)
        at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:201)
        at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:188)
        at com.diffplug.gradle.spotless.SpotlessTaskImpl.processInputFile(SpotlessTaskImpl.java:71)
        at com.diffplug.gradle.spotless.SpotlessTaskImpl.performAction(SpotlessTaskImpl.java:57)
[...]
Caused by: java.io.IOException: Cannot run program "/usr/bin/npm" (in directory "<snip>/frontend/build/spotless-node-modules-prettier-format"): error=2, File or directory not found
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1128)
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1071)
        at com.diffplug.spotless.npm.NpmProcess.npm(NpmProcess.java:64)
        ... 145 more
Caused by: java.io.IOException: error=2, File or directory not found
        at java.base/java.lang.ProcessImpl.forkAndExec(Native Method)
        at java.base/java.lang.ProcessImpl.<init>(ProcessImpl.java:340)
        at java.base/java.lang.ProcessImpl.start(ProcessImpl.java:271)
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1107)
        ... 147 more

It seems that this only happens when I run build in more than this submodule.

Can Spotless be made to create that directory not in the configuration phase, but only when it actually runs?

Gradle: 7.0, but also observed with earlier versions
Spotless version: 5.12.4
OS: Latest Manjaro

[nedtwigg]

TL;DR this is hard to fix. Implementing #675 would make it easier. One hacky workaround would be to make it possible for prettier() to put its build dependencies somewhere else, like build-prettier/, but that's pretty ugly. I'll describe the full mess below, fixing it is not on my todo list.

If your build looks like this:

spotless {
  typescript {
    prettier()
    ...

Then your task dependency tree looks likes this:

spotlessApply ---> spotlessTypescriptApply -+
                                            |
spotlessCheck ---> spotlessTypescriptCheck -+-> spotlessTypescript

The work happens in spotlessTypescript, where it creates a directory with formatted versions of any badly-formatted files.

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java

Line 83 in 397e36f

Files.createDirectories(parentDir);

Both spotlessTypescriptApply and spotlessTypescriptCheck then use those files to do their work. So at first glance, it seems like this shouldn't be happening.

I think the problem is that the npm step makes a package.json in build/ installs stuff there:

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java

Lines 543 to 553 in 397e36f

	FormatterStep createStep() {
	final Project project = getProject();
	return PrettierFormatterStep.create(
	devDependencies,
	provisioner(),
	project.getBuildDir(),
	new NpmPathResolver(npmFileOrNull(), npmrcFileOrNull(), project.getProjectDir(), project.getRootDir()),
	new com.diffplug.spotless.npm.PrettierConfig(
	this.prettierConfigFile != null ? project.file(this.prettierConfigFile) : null,
	this.prettierConfig));
	}

spotless/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java

Lines 56 to 64 in 397e36f

	protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, File buildDir, File npm) throws IOException {
	this.stepName = requireNonNull(stepName);
	this.npmConfig = requireNonNull(npmConfig);
	this.npmExecutable = npm;
	NodeServerLayout layout = prepareNodeServer(buildDir);
	this.nodeModulesDir = layout.nodeModulesDir();
	this.packageJsonSignature = FileSignature.signAsList(layout.packageJsonFile());
	}

This already happens lazily as part of the up-to-date check.

spotless/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java

Lines 55 to 57 in 397e36f

	return FormatterStep.createLazy(NAME,
	() -> new State(NAME, devDependencies, buildDir, npmPathResolver, prettierConfig),
	State::createFormatterFunc);

Fixing this would require moving the file creation out of the up-to-date check, which would remove checking package.json dependencies from the up-to-date check. Definitely possible to do part in one place and part in another, but tricky.

[fdw]

I'm definitely willing to help, but my knowledge of Gradle internals (or Spotless internals, for that matter) is not that deep, so I'd need help ;)

One dumb question, though: Is it important that the package.json is checked during up-to-date? I would've thought the dependencies in there don't matter, as Spotless installs its own Prettier.

[nedtwigg]

If you change prettier from 2.0.0 to 2.2.1, for example, then the formatted contents of the files might change. If the up-to-date check doesn't take that into account, then you can have stale formatting on your computer compared to say a CI server. If you're using the remote build cache, then this problem becomes even more pervasive.

I just thought of another totally hairbrained workaround. Around this part:

spotless/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java

Lines 88 to 93 in 397e36f

	protected ServerProcessInfo npmRunServer() throws ServerStartException {
	try {
	// The npm process will output the randomly selected port of the http server process to 'server.port' file
	// so in order to be safe, remove such a file if it exists before starting.
	final File serverPortFile = new File(this.nodeModulesDir, "server.port");
	NpmResourceHelper.deleteFileIfExists(serverPortFile);

you could just check to see if the file is missing, and if it is, run prepareNodeServer() all over again. That would be slow, but it would work.

[fdw]

Ah, I wasn't aware that it takes the prettier version from package.json into account. That is neat :)

Your idea sounds wild, but it also sounds like it should still work - then the version check is fine, and the actual formatting, too. Is there a drawback I can't see?

[nedtwigg]

Main drawback is speed and ugly. Tiny chance that the deps resolve differently between runs due to a new transitive dep being published, but I think you can safely ignore that.

[fdw]

Sounds quite good to me. Do you want me to try and implement it?

[nedtwigg]

Yes, if the feature is worth it to you. Spotless does (and has done) everything I need for a very long time. I don't care what gets added, as long as it doesn't break something for somebody else.

[fdw]

As you can see, I opened a PR :)

[nedtwigg]

Fixed in plugin-maven 2.11.1 and plugin-gradle 5.12.5