maven IDE hook for `jj fix`

[victornoel]

Hi,

I am trying to setup jujutsu so that it runs spotless (configured using the maven plugin in my repository) when using jj fix, see https://jj-vcs.github.io/jj/latest/cli-reference/#jj-fix.

The problem is that the requirement from jujutsu is that the tool that does formatting should respect the following rule:

The external tools must accept the current file content on standard input, and return the updated file content on standard output. A tool's output will not be used unless it exits with a successful exit code. Output on standard error will be passed through to the terminal.

Would that be something that could be added to spotless?

Thanks!

[nedtwigg]

Spotless Gradle supports this, but the Maven support is immature. @SapiensAnatis has made some progress here.

[SapiensAnatis]

I didn’t write the IDE hook to be clear, but it sounds broadly like what you are looking for.

If you use these arguments: https://github.com/SapiensAnatis/vscode-spotless-maven/blob/5cb5fa5a1cc444e47aa50e33547b2ffefdf56dec/src/maven/basicMavenExecutor.ts#L28

You will get a process that takes a document on stdin and then returns on stderr IS CLEAN / IS DIRTY and on stdout the formatted document.

The only problem is I can’t remember if you get the formatted document on stdout if Spotless returns IS CLEAN. In the vscode extension we just return ‘no changes’ if we get that back.

[victornoel]

Hey @SapiensAnatis @nedtwigg thank you for the feedback, I will do a bit of experimentation with that to see how it behaves and come back here to share my findings!

[victornoel]

I can already share this:

• on "IS CLEAN", the file is not outputted indeed, that could be a problem for jj fix but I'm sure I will find a temporary workaround until we decide here if we should change the behaviour or not
• on "IS DIRTY" it looks like it's doing exactly what I am expecting
• in the long run there will also be the concern of performance
  • mvn is slow to startup, so for tens of file it's going to be long to run… well, it's always better than doing it by hand 😃
  • I'm not sure how this can be solved in practice: I mean, spotless already starts a separate npm process to be able to format multiple files faster, now will we need a separate mvn process to submit the files to format? 😅

[SapiensAnatis]

Performance wise I think the way forward is to use mvnd which runs as a daemon and is much faster for ad-hoc formatting. However it doesn’t seem to be compatible with the IDE hook at the minute, I’ve raised a few issues over there so hopefully those will be fixed soon!

[victornoel]

I did a few tests with mvnd using both -DspotlessFiles and -DspotlessIdeHook (without stdin/out) and performances are still not great

It's taking something like 2.8sec with -DspotlessFiles and 3.8sec with -DspotlessIdeHook. The IDE Hook actually restarts the npm process every time ^^

running spotless:apply on ALL my files takes the same time btw, so the overhead is somewhere else, which means there is maybe some margin for improvement on the spotless side :)

[SapiensAnatis]

Oh the IDE hook is really only for a single file (hence IDE). On a single file it takes about 0.4s for me (assuming the daemon is warm) but if you are formatting your entire solution it will probably be slower.

[victornoel]

@SapiensAnatis I meant a single file yes and it was clean and indexed so actually nothing was really done against the file and I let the daemon warm a bit indeed by calling the same command many times. In both cases (see below), most of the time is spent (from my subjective point of view) just after the line "[INFO] --- spotless:2.43.0:apply (default-cli) @ app ---".

Using the hook

• we can disregard the error at the end of that one, it's just https://github.com/apache/maven-mvnd/issues/1271 ;)
• see how npm is started again here, it's not the case in the next example

JAVA_HOME=/usr/lib/jvm/java-23-openjdk mvnd -N spotless:apply -DspotlessIdeHook=$PWD/src/main/java/Test.java
[INFO] Processing build on daemon a98d8404
[INFO] Scanning for projects...
[INFO] BuildTimeEventSpy is registered.
[INFO] 
[INFO] Using the SmartBuilder implementation with a thread count of 11
[INFO] 
[INFO] ------------------------< my.app:app >------------------------
[INFO] Building Parent 1.0.0-SNAPSHOT
[INFO]   from pom.xml
[INFO] --------------------------------[ pom ]---------------------------------
[INFO] 
[INFO] --- spotless:2.43.0:apply (default-cli) @ app ---
[INFO] creating formatter function (starting server)
[INFO] [BEGIN] Starting npm based server in /home/victor/Codebases/app/target/spotless-prettier-node-modules-b900751a430598b577bf2148e1ea287e with StandardNpmProcessFactory.
[INFO] [END] Starting npm based server in /home/victor/Codebases/app/target/spotless-prettier-node-modules-b900751a430598b577bf2148e1ea287e with StandardNpmProcessFactory. (took 2ms)
[WARNING] [stderr] IS CLEAN
[INFO] Closing formatting function (ending server).
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  3.706 s (Wall Clock)
[INFO] Finished at: 2025-02-21T09:43:15+01:00
[INFO] ------------------------------------------------------------------------
java.lang.NullPointerException: Cannot invoke "org.mvndaemon.mvnd.logging.smart.LoggingOutputStream.forceFlush()" because "this.out" is null

and not the hook:

JAVA_HOME=/usr/lib/jvm/java-23-openjdk mvnd -N spotless:apply -DspotlessFiles=$PWD/src/main/java/Test.java
[INFO] Processing build on daemon a98d8404
[INFO] Scanning for projects...
[INFO] BuildTimeEventSpy is registered.
[INFO] 
[INFO] Using the SmartBuilder implementation with a thread count of 11
[INFO] 
[INFO] ------------------------< my.app:app >------------------------
[INFO] Building Parent 1.0.0-SNAPSHOT
[INFO]   from pom.xml
[INFO] --------------------------------[ pom ]---------------------------------
[INFO] 
[INFO] --- spotless:2.43.0:apply (default-cli) @ app ---
[INFO] Spotless.Java is keeping 1 files clean - 0 were changed to be clean, 0 were already clean, 1 were skipped because caching determined they were already clean
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.638 s (Wall Clock)
[INFO] Finished at: 2025-02-21T09:45:00+01:00
[INFO] ------------------------------------------------------------------------

[SapiensAnatis]

Ah, my numbers were from running on Java directly. I guess there may be some overhead if you need to invoke npm/prettier.

[victornoel]

Ah, my numbers were from running on Java directly. I guess there may be some overhead if you need to invoke npm/prettier.

something to investigate, maybe in another ticket though ^^ I will try to get a feel of why it happens and create one if I feel like it's a good idea :)