Inconsistent treefmt behavior with just: Intermittent file change reporting

[willbush]

Describe the bug

treefmt --no-cache using just to format a justfile intermittently shows formatted 1 files (1 changed) or formatted 1 files (0 changed).

Also noticed when making repro steps below that treefmt says its formatting the justfile, but nothing changes it git status.

I first ran into this when using nix flake check trying to setup CI. Found this issue: numtide/treefmt-nix#156 note that using treefmtEval.config.build.check self through treefmt-nix uses --no-cache by default.

Then narrowed it down to treefmt --no-cache and just.

To Reproduce

❯ mkdir test && cd test

~/test
❯ git init
Initialized empty Git repository in /home/will/test/.git/

test on  main
❯ cat << 'EOF' > flake.nix
{
  inputs = {
    nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable";
    utils.url = "github:numtide/flake-utils";
    treefmt-nix = {
      url = "github:numtide/treefmt-nix";
      inputs.nixpkgs.follows = "nixpkgs";
    };
  };
  outputs =
    {
      self,
      nixpkgs,
      utils,
      treefmt-nix,
    }:
    utils.lib.eachDefaultSystem (
      system:

      let
        pkgs = nixpkgs.legacyPackages.${system};
        treefmtEval = treefmt-nix.lib.evalModule pkgs {
          # Used to find the project root
          projectRootFile = ".git/config";
          programs.just.enable = true;
        };
      in
      {
        devShells.default = pkgs.mkShell {
          buildInputs = [
            pkgs.just
            treefmtEval.config.build.wrapper # treefmt configured
          ];
        };
      }
    );
}
EOF


test on  main [?]
❯ cat << 'EOF' > justfile
default:
    just --list
EOF


test on  main [?]
❯ git add -A

test on  main [+]
❯ nix develop
warning: Git tree '/home/will/test' is dirty
warning: creating lock file '/home/will/test/flake.lock':
• Added input 'nixpkgs':
    'github:nixos/nixpkgs/b73c2221a46c13557b1b3be9c2070cc42cf01eb3?narHash=sha256-QOS0ykELUmPbrrUGmegAUlpmUFznDQeR4q7rFhl8eQg%3D' (2024-07-27)
• Added input 'treefmt-nix':
    'github:numtide/treefmt-nix/768acdb06968e53aa1ee8de207fd955335c754b7?narHash=sha256-uru7JzOa33YlSRwf9sfXpJG%2BUAV%2BbnBEYMjrzKrQZFw%3D' (2024-07-30)
• Added input 'treefmt-nix/nixpkgs':
    follows 'nixpkgs'
• Added input 'utils':
    'github:numtide/flake-utils/b1d9ab70662946ef0850d488da1c9019f3a9752a?narHash=sha256-SZ5L6eA7HJ/nmkzGG7/ISclqe6oZdOZTNoesiInkXPQ%3D' (2024-03-11)
• Added input 'utils/systems':
    'github:nix-systems/default/da67096a3b9bf56a91d16901293e51ba5b49a27e?narHash=sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768%3D' (2023-04-09)
warning: Git tree '/home/will/test' is dirty

[will@blazar:~/test]$ git add -A

[will@blazar:~/test]$ git status
On branch main

No commits yet

Changes to be committed:
  (use "git rm --cached <file>..." to unstage)
	new file:   flake.lock
	new file:   flake.nix
	new file:   justfile


[will@blazar:~/test]$ treefmt
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 3ms

[will@blazar:~/test]$ treefmt
traversed 3 files
emitted 0 files for processing
formatted 0 files (0 changed) in 0s

[will@blazar:~/test]$ git status
On branch main

No commits yet

Changes to be committed:
  (use "git rm --cached <file>..." to unstage)
	new file:   flake.lock
	new file:   flake.nix
	new file:   justfile


[will@blazar:~/test]$ treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 3ms

[will@blazar:~/test]$ treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 3ms

[will@blazar:~/test]$ treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (0 changed) in 3ms

[will@blazar:~/test]$ treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 3ms

Expected behavior

formatted 1 files (0 changed). I think. I'm not sure what the difference between changed and formatted files means.

System information

$ treefmt --version
treefmt v2.0.3

$ just --version
just 1.32.0

$ nixos-version
24.11.20240727.b73c222 (Vicuna)

[brianmcgee]

Thanks for the reproducer @willbush, I'll try to have a look in the next couple days.

FYI formatted is the number of files that were run through formatters, changed is the number of files that were actually changed after applying the formatters (we check their size and mod time afterwards)

[zimbatm]

This could be an issue with the just formatter not respecting the treefmt spec and writing to the file on every run.

[brianmcgee]

I followed the steps to reproduce and this is what I found.

❯ treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 4ms

❯ treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 4ms

❯ treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 4ms

❯ treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 4ms

This was me manually running the command, with a natural second or two delay between executions as I retrieved the command from my history and hit enter. At this point, I suspected I knew how to make it happen, and I did with the following:

❯ treefmt --no-cache && \
  treefmt --no-cache && \
  treefmt --no-cache && \
  treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 4ms
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (0 changed) in 3ms
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (0 changed) in 4ms
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (0 changed) in 4ms

To better illustrate what is happening, look at this output:

❯ treefmt --no-cache && \
  echo -e "\nmodtime = $(stat --format=%y justfile)\n" && \
  treefmt --no-cache && \
  echo -e "\nmodtime = $(stat --format=%y justfile)\n" && \
  sleep 1 && \
  treefmt --no-cache
WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 4ms

modtime = 2024-08-01 14:41:28.434694125 +0100

WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (0 changed) in 4ms

modtime = 2024-08-01 14:41:28.445694253 +0100

WARN format: no formatter for path: flake.nix
traversed 3 files
emitted 3 files for processing
formatted 1 files (1 changed) in 4ms

Sometimes, treefmt thinks the file has changed after applying the formatter, and sometimes, it doesn't because of this change: 85ce0a2.

When comparing modtime, we truncate to second-level precision before comparing.

We capture the modtime of every file being processed, then, after applying the formatters, capture the mod time again and compare. In the examples above, when executed quickly in succession, we fail to detect the change even though the modtime has been changed.

I think this also explains what @jaen was reporting in #316. If you have a checkout, and then run treefmt --no-cache --fail-on-change, and everything completes within the same second, it would fail to report some changes if the modtime was bumped by milliseconds within the same second.

The change to how we compare mod times was made to account for dos2unix behaving badly.

It would seem that the heuristic of size and modtime comparison is not enough. @zimbatm and I have been discussing more robust change detection. It seems that now's the time to give it some proper thought.

[brianmcgee]

In the short term, I've done the simple thing and added a wait, which sits behind a new ci flag #368

[willbush]

Sweet thanks for the fast turn around!
I suppose in the future if the comparison heuristic changes and delay is not needed, will --ci remain without delay?

[brianmcgee]

That's the plan. The ci flag is useful in itself. When we figure out a better comparison heuristic the delay fudge can go away like you said.