-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inconsistent: error: unexpected changes detected, --fail-on-change is enabled #342
Comments
Update, I just managed to reproduce it once locally in the docker container, but don't seem to be able to reproduce it again. |
@terlar trying upping the logging with |
Yeah, only when I enabled the logging it didn't fail, but I guess I will just have to leave it with logging on and keep running until I run into the issue again. |
Catching the failure and running This is how we generally implement treefmt checks, as you can see here. |
There are no changes, but I have a hunch. When I ran This is just a hunch and no proof. |
or actually seems neither |
That's correct, v2 will not operate on untracked files when using the default |
Turns out one thing that prevented me from reproducing the error was that I also added |
Just managed to reproduce this with
As seen by the output, there was no changes, another commit worked without any changes... or I guess I will have to double check the timestamp in case that is what happens, but both those formatters "should" have fixed that. |
I'll have a look at improving the logging to make it clearer what's going on. |
Okay, I can now reproduce this. It happens every time on the first formatting of the change.
Doesn't change the timestamp nor the file. I will verify if this is related to multiple formatters or only one. |
I can verify that this only happens when using two formatters, in my case |
I tested this with I also reproduce this with
I will create an isolated flake with a reproducer. |
Steps to reproduce:
|
There are no changes to the file and checking the timestamp show there are no changes to modify time.
Also consecutive runs are passing after the initial failure. |
Thanks for the reproducer, I'll dig into this later today. |
@terlar if you look at the stat output, you can see that the mod time is actually changing. It's being replaced with a millisecond precision value afterwards. From some experimentation, this seems to be due to I'm going to do a couple of things:
|
@terlar doing a little manual experimentation, I created a test file like above with: hello: world
list:
- one
- two
- three I then ran the following with ❯ stat -c %Y test.yaml && \
dos2unix -k test.yaml && \
stat -c %Y test.yaml && \
dos2unix -k test.yaml && \
stat -c %Y test.yaml && \
dos2unix -k test.yaml && \
stat -c %Y test.yaml
1720265173
dos2unix: converting file test.yaml to Unix format...
1720265173
dos2unix: converting file test.yaml to Unix format...
1720265173
dos2unix: converting file test.yaml to Unix format...
1720265173 As you can see, the second precision mod time isn't changing. When I run the same test with treefmt/temp-test on fix/fail-on-change [$!+] via ❄️ impure (devshell-env) on ☁️
❯ stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml
1720266379
1720266383
1720266383
1720266383
treefmt/temp-test on fix/fail-on-change [$!+] via ❄️ impure (devshell-env) on ☁️
❯ stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml
1720266383
1720266388
1720266388
1720266388
treefmt/temp-test on fix/fail-on-change [$!+] via ❄️ impure (devshell-env) on ☁️
❯ stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml && \
yamlfmt test.yaml && \
stat --format=%Y test.yaml
1720266388
1720266395
1720266395
1720266395 I ran this test several times without ever touching the test file after first creating it. It appears it doesn't mess with the mod time on first execution on a file which hasn't changed, and then on second execution it changes the mod time, then leaves it along the rest of the time. |
Playing around with the format to make it a bit easier to read: ❯ stat --format=%y test.yaml && \
yamlfmt test.yaml && \
stat --format=%y test.yaml && \
yamlfmt test.yaml && \
stat --format=%y test.yaml && \
yamlfmt test.yaml && \
stat --format=%y test.yaml
2024-07-06 12:36:41.733215531 +0100
2024-07-06 12:36:58.758401183 +0100
2024-07-06 12:36:58.763401237 +0100
2024-07-06 12:36:58.767401281 +0100 I'm not sure what it's doing. |
I was being a bit dense with my test. ❯ stat --format=%Y test.yaml && \
∙ yamlfmt test.yaml && sleep 1 && \
∙ stat --format=%Y test.yaml && \
∙ yamlfmt test.yaml && sleep 1 && \
∙ stat --format=%Y test.yaml && \
∙ yamlfmt test.yaml && sleep 1 && \
∙ stat --format=%Y test.yaml
1720266636
1720267333
1720267334
1720267335 Looks like |
|
@terlar I checked what version of After updating the flake lock, ❯ stat --format=%Y test.yaml && \
yamlfmt test.yaml && sleep 1 && \
stat --format=%Y test.yaml && \
yamlfmt test.yaml && sleep 1 && \
stat --format=%Y test.yaml && \
yamlfmt test.yaml && sleep 1 && \
stat --format=%Y test.yaml
1720267906
1720267906
1720267906
1720267906 I tested with your original test case above on the branch for #345, and it's now behaving as expected: treefmt on fix/fail-on-change [$!+] via 🐹 v1.22.3 via ❄️ impure (devshell-env) on ☁️ took 2s
❯ nix run . -- -C temp-test --no-cache --fail-on-change
WARN format: no formatter for path: treefmt.toml
traversed 2 files
emitted 2 files for processing
formatted 1 files (0 changed) in 4ms
treefmt on fix/fail-on-change [$!+] via 🐹 v1.22.3 via ❄️ impure (devshell-env) on ☁️
❯ nix run . -- -C temp-test --no-cache --fail-on-change
WARN format: no formatter for path: treefmt.toml
traversed 2 files
emitted 2 files for processing
formatted 1 files (0 changed) in 4ms
treefmt on fix/fail-on-change [$!+] via 🐹 v1.22.3 via ❄️ impure (devshell-env) on ☁️
❯ nix run . -- -C temp-test --no-cache --fail-on-change
WARN format: no formatter for path: treefmt.toml
traversed 2 files
emitted 2 files for processing
formatted 1 files (0 changed) in 4ms
treefmt on fix/fail-on-change [$!+] via 🐹 v1.22.3 via ❄️ impure (devshell-env) on ☁️
❯ nix run . -- -C temp-test --no-cache --fail-on-change
WARN format: no formatter for path: treefmt.toml
traversed 2 files
emitted 2 files for processing
formatted 1 files (0 changed) in 4ms TLDR The problem is fixed by moving to second level mod time precision (#345) and upgrading your |
I was running the yamlfmt with the fix all the time. I still see this issue. Try touching the file inbetween. Then it fails again. But yes, that might be treefmt. I will try your branch again. Next time I create a reproducer, I should add the explicit nixpkgs version. Sorry to waste your time with that detour.
|
I tried with that branch and it still fails:
|
Ok, when I touch the file in between, I see the same failures as you are. I'll have another look this evening. |
Thank you, it is a tricky one, and not easy to pinpoint. I am not sure it is specific to my formatters, as another person also reported it with another set of formatters here: Seems to be something with multiple formatters touching the same files and weirdly it only fails first time despite not running with cache. So somehow feels like there is still some kind of cache. Also once again sorry to waste your debug time with not providing a reproducer with explicit yamlfmt version 😿 |
Ah, this just clicked for me. In the file cache we record the mod time and the file size. We consider a file to have changed if they don't match. By touching the file, when treefmt runs, the cache layer thinks the file has been changed and it is emitted for processing. The formatter formats it but doesn't change anything. The new size and mod time gets written to the cache. Next time you run without touching the file everything is good. I need to have a think about this one 🤔 We use this strategy as a light weight change detection mechanism. @zimbatm has talked about configurable change detection strategies which can be turned on at an increased cost. This could be one of those scenarios. |
Ok, I don't think it's quite what I said above, but I think it's something in the caching/change detection. I'm now running late cos I needed to find this 😂 I'll have a look later. |
I am also encountering this (see numtide/treefmt-nix#156) but I am not using |
Looking at this again with fresh eyes, I realized I should have truncated the mod time rather than rounding it. @terlar I no longer see the issue with touching the files in between @montchr I'm having trouble checking out the commit you mentioned to test. |
@brianmcgee Thank you, seems to work now! :) |
@brianmcgee sorry for the confusion, I only posted a comment in the other ticket, created by @zmrocze. I just meant to confirm that the issue wasn't related to yamlfmt. Thank you all for working on a fix for this. It took me a while to realize it wasn't just a misconfiguration on my end. I'll test #345 in my project. |
Describe the bug
I just switched to the new release treefmt v2.0.2 and I am experiencing some weird issues with the
--fail-on-change
flag.I am running
treefmt
via pre-commit (git-hooks.nix).I first noticed the issue in CI which seems to consistently fail for a specific commit. Same commit passed locally. Then next commit that I did to debug the issue failed locally. Then I committed again and it passed. It also passed on CI. The CI is running within a dedicated docker container including pre-commit built with Nix. Same docker image with same hash running locally works every time for any commit I tried so far. However then I am mounting the code as a volume, so it might have something that affects the result.
Given the nature of the reproducibility I cannot provide exact steps. I will look further. Any ideas how to debug this?
The text was updated successfully, but these errors were encountered: