numtide / treefmt

one CLI to format your repo [maintainers=@zimbatm,@brianmcgee]
https://treefmt.com
MIT License
575 stars 35 forks source link

obscure --fail-on-change error #396

Open teto opened 2 weeks ago

teto commented 2 weeks ago

Describe the bug

I am trying to enable treefmt on CI but I find the behavior elusive: here are 2 consecutive runs:

[teto]$ (nix)treefmt --fail-on-change
WARN format: no formatter for path: .gitlab-ci.yml
treefmt: error: unexpected changes detected, --fail-on-change is enabled

[teto]$ (nix)treefmt --fail-on-change
traversed 4110 files
emitted 0 files for processing
formatted 0 files (0 changed) in 34ms
  1. Why is it failing on the first run ? is it because of WARN format: no formatter for path: .gitlab-ci.yml or another path not displayed ? I dont think it should fail when a file is not covered by a formatter, it makes the out of the box behavior quite annoying.
  2. why it doesn't fail on the second run ? I haven't done anything between the 2 runs

Expected behavior

System information

treefmt v2.0.3

Additional context

teto commented 2 weeks ago

I tried to convert warnings into debug in case the WARN format ... was making the check fail, which gives:

$ treefmt --fail-on-change -udebug --no-cache
treefmt: error: unexpected changes detected, --fail-on-change is enabled

I guess it fails because of the same reasons than in my initial issue but now I have little feedback to address the issue.

brianmcgee commented 1 week ago

@teto if you add -v, -vv you will get info and debug level logging.

It's recommended to use --fail-on-change with --no-cache. There's a --ci flag which has been available since 2.0.4 that does a few things that ensure a better experience in a CI environment https://treefmt.com/usage#ci.

I recommend upgrading to 2.0.5, which is in nixpkgs unstable now https://nixpkgs-tracker.ocfox.me/?pr=336307.

If you use treefmt-nix, I've just created a PR to update the default version to 2.0.5. It currently uses 2.0.4. https://github.com/numtide/treefmt-nix/pull/229

teto commented 1 week ago

if you add -v, -vv you will get info and debug level logging.

I think people are more used to the inverse logic, ie. the tool gives useful output by default. For the minority who want a quiet output, they can redirect to /dev/null or use a --quiet flag. Adding -vv I could see:

...
INFO format | just: 1 file(s) processed in 25.524693ms
DEBU format: file has changed path=/home/teto/nova/justfile prev_size=2168 current_size=2168 prev_mod_time="2024-08-30 10:50:11 +0200 CEST" current_mod_time="2024-09-02 12:41:41 +0200 CEST"

I ran a simple treefmt, reran treefmt --fail-on-change which now succeeds, yet git diff/git status shows no difference. I tried to check if I had a git config to ignore whitespace change but couldn't find any. Does it mean it just refreshed the cache but the actual file was correctly formatted from the start ?

So the WARN format: no formatter for path: .gitlab-ci.yml were red herrings that did not cause the failure. I am shown information I dont care but the one that could help me fix the issue is hidden :(

If you use treefmt-nix, I've just created a PR to update the default version to 2.0.5. It currently uses 2.0.4

I see it's merged I will try to update ty !

It's recommended to use --fail-on-change with --no-cache. There's a --ci flag which has been available since 2.0.4 that does a few things that ensure a better experience in a CI environment https://treefmt.com/usage#ci.

brianmcgee commented 1 week ago

I think you're seeing the same issue as https://github.com/numtide/treefmt/issues/365. Moving to 2.0.5 is the fix.

So the WARN format: no formatter for path: .gitlab-ci.yml were red herrings that did not cause the failure. I am shown information I dont care but the one that could help me fix the issue is hidden :(

2.0.5 now logs at error level the file has changed entry when --fail-on-change is enabled, making it clearer by default what happened.

I think people are more used to the inverse logic, ie. the tool gives useful output by default. For the minority who want a quiet output, they can redirect to /dev/null or use a --quiet flag.

I disagree that there is a minority wanting quiet output by default. Regardless, this is in keeping with v1 behaviour, if not a little less noisy by default.

In general, I want to stabilise the move to Go with 2.0.x. Once that happens (which I feel is pretty close), I'm happy to start diverging more clearly with 2.1.x.