Skip to content

Commit 65152cb

Browse files
brianmcgeebrianmcgee
authoredJul 7, 2024··
Merge pull request #345 from numtide/fix/fail-on-change
fix: use second precision when comparing file mod times
·
v2.2.1v2.0.3
2 parents 23e563b + 85ce0a2 commit 65152cb

File tree

8 files changed

+110
-52
lines changed

8 files changed

+110
-52
lines changed
 

‎cli/format.go

+14-7
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"runtime"
1212
"runtime/pprof"
1313
"syscall"
14+
"time"
1415

1516
"git.numtide.com/numtide/treefmt/format"
1617
"git.numtide.com/numtide/treefmt/stats"
@@ -389,20 +390,26 @@ func (f *Format) detectFormatted(ctx context.Context) func() error {
389390
return nil
390391
}
391392

392-
// look up current file info
393-
currentInfo, err := os.Stat(file.Path)
393+
// check if the file has changed
394+
changed, newInfo, err := file.HasChanged()
394395
if err != nil {
395-
return fmt.Errorf("failed to stat processed file: %w", err)
396+
return err
396397
}
397398

398-
// check if the file has changed
399-
if !(file.Info.ModTime() == currentInfo.ModTime() && file.Info.Size() == currentInfo.Size()) {
399+
if changed {
400400
// record the change
401401
stats.Add(stats.Formatted, 1)
402402
// log the change for diagnostics
403-
log.Debugf("file has been changed: %s", file.Path)
403+
log.Debug(
404+
"file has changed",
405+
"path", file.Path,
406+
"prev_size", file.Info.Size(),
407+
"current_size", newInfo.Size(),
408+
"prev_mod_time", file.Info.ModTime().Truncate(time.Second),
409+
"current_mod_time", newInfo.ModTime().Truncate(time.Second),
410+
)
404411
// update the file info
405-
file.Info = currentInfo
412+
file.Info = newInfo
406413
}
407414

408415
// mark as processed

‎cli/format_test.go

+34-30
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"path/filepath"
1010
"regexp"
1111
"testing"
12+
"time"
1213

1314
"git.numtide.com/numtide/treefmt/config"
1415
"git.numtide.com/numtide/treefmt/format"
@@ -159,22 +160,22 @@ func TestSpecifyingFormatters(t *testing.T) {
159160
setup()
160161
_, err := cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
161162
as.NoError(err)
162-
assertStats(t, as, 31, 31, 3, 3)
163+
assertStats(t, as, 32, 32, 3, 3)
163164

164165
setup()
165166
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--formatters", "elm,nix")
166167
as.NoError(err)
167-
assertStats(t, as, 31, 31, 2, 2)
168+
assertStats(t, as, 32, 32, 2, 2)
168169

169170
setup()
170171
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "-f", "ruby,nix")
171172
as.NoError(err)
172-
assertStats(t, as, 31, 31, 2, 2)
173+
assertStats(t, as, 32, 32, 2, 2)
173174

174175
setup()
175176
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--formatters", "nix")
176177
as.NoError(err)
177-
assertStats(t, as, 31, 31, 1, 1)
178+
assertStats(t, as, 32, 32, 1, 1)
178179

179180
// test bad names
180181
setup()
@@ -204,23 +205,23 @@ func TestIncludesAndExcludes(t *testing.T) {
204205
test.WriteConfig(t, configPath, cfg)
205206
_, err := cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
206207
as.NoError(err)
207-
assertStats(t, as, 31, 31, 31, 0)
208+
assertStats(t, as, 32, 32, 32, 0)
208209

209210
// globally exclude nix files
210211
cfg.Global.Excludes = []string{"*.nix"}
211212

212213
test.WriteConfig(t, configPath, cfg)
213214
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
214215
as.NoError(err)
215-
assertStats(t, as, 31, 31, 30, 0)
216+
assertStats(t, as, 32, 32, 31, 0)
216217

217218
// add haskell files to the global exclude
218219
cfg.Global.Excludes = []string{"*.nix", "*.hs"}
219220

220221
test.WriteConfig(t, configPath, cfg)
221222
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
222223
as.NoError(err)
223-
assertStats(t, as, 31, 31, 24, 0)
224+
assertStats(t, as, 32, 32, 25, 0)
224225

225226
echo := cfg.Formatters["echo"]
226227

@@ -230,31 +231,31 @@ func TestIncludesAndExcludes(t *testing.T) {
230231
test.WriteConfig(t, configPath, cfg)
231232
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
232233
as.NoError(err)
233-
assertStats(t, as, 31, 31, 22, 0)
234+
assertStats(t, as, 32, 32, 23, 0)
234235

235236
// remove go files from the echo formatter
236237
echo.Excludes = []string{"*.py", "*.go"}
237238

238239
test.WriteConfig(t, configPath, cfg)
239240
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
240241
as.NoError(err)
241-
assertStats(t, as, 31, 31, 21, 0)
242+
assertStats(t, as, 32, 32, 22, 0)
242243

243244
// adjust the includes for echo to only include elm files
244245
echo.Includes = []string{"*.elm"}
245246

246247
test.WriteConfig(t, configPath, cfg)
247248
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
248249
as.NoError(err)
249-
assertStats(t, as, 31, 31, 1, 0)
250+
assertStats(t, as, 32, 32, 1, 0)
250251

251252
// add js files to echo formatter
252253
echo.Includes = []string{"*.elm", "*.js"}
253254

254255
test.WriteConfig(t, configPath, cfg)
255256
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
256257
as.NoError(err)
257-
assertStats(t, as, 31, 31, 2, 0)
258+
assertStats(t, as, 32, 32, 2, 0)
258259
}
259260

260261
func TestCache(t *testing.T) {
@@ -281,7 +282,7 @@ func TestCache(t *testing.T) {
281282
test.WriteConfig(t, configPath, cfg)
282283
_, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir)
283284
as.NoError(err)
284-
assertStats(t, as, 31, 31, 31, 0)
285+
assertStats(t, as, 32, 32, 32, 0)
285286

286287
out, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir)
287288
as.NoError(err)
@@ -290,7 +291,7 @@ func TestCache(t *testing.T) {
290291
// clear cache
291292
_, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir, "-c")
292293
as.NoError(err)
293-
assertStats(t, as, 31, 31, 31, 0)
294+
assertStats(t, as, 32, 32, 32, 0)
294295

295296
out, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir)
296297
as.NoError(err)
@@ -299,7 +300,7 @@ func TestCache(t *testing.T) {
299300
// clear cache
300301
_, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir, "-c")
301302
as.NoError(err)
302-
assertStats(t, as, 31, 31, 31, 0)
303+
assertStats(t, as, 32, 32, 32, 0)
303304

304305
out, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir)
305306
as.NoError(err)
@@ -308,7 +309,7 @@ func TestCache(t *testing.T) {
308309
// no cache
309310
_, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir, "--no-cache")
310311
as.NoError(err)
311-
assertStats(t, as, 31, 31, 31, 0)
312+
assertStats(t, as, 32, 32, 32, 0)
312313
}
313314

314315
func TestChangeWorkingDirectory(t *testing.T) {
@@ -342,7 +343,7 @@ func TestChangeWorkingDirectory(t *testing.T) {
342343
// this should fail if the working directory hasn't been changed first
343344
_, err = cmd(t, "-C", tempDir)
344345
as.NoError(err)
345-
assertStats(t, as, 31, 31, 31, 0)
346+
assertStats(t, as, 32, 32, 32, 0)
346347
}
347348

348349
func TestFailOnChange(t *testing.T) {
@@ -365,6 +366,9 @@ func TestFailOnChange(t *testing.T) {
365366
_, err := cmd(t, "--fail-on-change", "--config-file", configPath, "--tree-root", tempDir)
366367
as.ErrorIs(err, ErrFailOnChange)
367368

369+
// we have second precision mod time tracking
370+
time.Sleep(time.Second)
371+
368372
// test with no cache
369373
test.WriteConfig(t, configPath, cfg)
370374
_, err = cmd(t, "--fail-on-change", "--config-file", configPath, "--tree-root", tempDir, "--no-cache")
@@ -411,31 +415,31 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
411415
args := []string{"--config-file", configPath, "--tree-root", tempDir}
412416
_, err := cmd(t, args...)
413417
as.NoError(err)
414-
assertStats(t, as, 31, 31, 3, 0)
418+
assertStats(t, as, 32, 32, 3, 0)
415419

416420
// tweak mod time of elm formatter
417421
as.NoError(test.RecreateSymlink(t, binPath+"/"+"elm-format"))
418422

419423
_, err = cmd(t, args...)
420424
as.NoError(err)
421-
assertStats(t, as, 31, 31, 3, 0)
425+
assertStats(t, as, 32, 32, 3, 0)
422426

423427
// check cache is working
424428
_, err = cmd(t, args...)
425429
as.NoError(err)
426-
assertStats(t, as, 31, 0, 0, 0)
430+
assertStats(t, as, 32, 0, 0, 0)
427431

428432
// tweak mod time of python formatter
429433
as.NoError(test.RecreateSymlink(t, binPath+"/"+"black"))
430434

431435
_, err = cmd(t, args...)
432436
as.NoError(err)
433-
assertStats(t, as, 31, 31, 3, 0)
437+
assertStats(t, as, 32, 32, 3, 0)
434438

435439
// check cache is working
436440
_, err = cmd(t, args...)
437441
as.NoError(err)
438-
assertStats(t, as, 31, 0, 0, 0)
442+
assertStats(t, as, 32, 0, 0, 0)
439443

440444
// add go formatter
441445
cfg.Formatters["go"] = &config.Formatter{
@@ -447,38 +451,38 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
447451

448452
_, err = cmd(t, args...)
449453
as.NoError(err)
450-
assertStats(t, as, 31, 31, 4, 0)
454+
assertStats(t, as, 32, 32, 4, 0)
451455

452456
// check cache is working
453457
_, err = cmd(t, args...)
454458
as.NoError(err)
455-
assertStats(t, as, 31, 0, 0, 0)
459+
assertStats(t, as, 32, 0, 0, 0)
456460

457461
// remove python formatter
458462
delete(cfg.Formatters, "python")
459463
test.WriteConfig(t, configPath, cfg)
460464

461465
_, err = cmd(t, args...)
462466
as.NoError(err)
463-
assertStats(t, as, 31, 31, 2, 0)
467+
assertStats(t, as, 32, 32, 2, 0)
464468

465469
// check cache is working
466470
_, err = cmd(t, args...)
467471
as.NoError(err)
468-
assertStats(t, as, 31, 0, 0, 0)
472+
assertStats(t, as, 32, 0, 0, 0)
469473

470474
// remove elm formatter
471475
delete(cfg.Formatters, "elm")
472476
test.WriteConfig(t, configPath, cfg)
473477

474478
_, err = cmd(t, args...)
475479
as.NoError(err)
476-
assertStats(t, as, 31, 31, 1, 0)
480+
assertStats(t, as, 32, 32, 1, 0)
477481

478482
// check cache is working
479483
_, err = cmd(t, args...)
480484
as.NoError(err)
481-
assertStats(t, as, 31, 0, 0, 0)
485+
assertStats(t, as, 32, 0, 0, 0)
482486
}
483487

484488
func TestGitWorktree(t *testing.T) {
@@ -524,7 +528,7 @@ func TestGitWorktree(t *testing.T) {
524528
// add everything to the worktree
525529
as.NoError(wt.AddGlob("."))
526530
as.NoError(err)
527-
run(31, 31, 31, 0)
531+
run(32, 32, 32, 0)
528532

529533
// remove python directory
530534
as.NoError(wt.RemoveGlob("python/*"))
@@ -533,7 +537,7 @@ func TestGitWorktree(t *testing.T) {
533537
// walk with filesystem instead of git
534538
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--walk", "filesystem")
535539
as.NoError(err)
536-
assertStats(t, as, 59, 59, 59, 0)
540+
assertStats(t, as, 61, 61, 61, 0)
537541
}
538542

539543
func TestPathsArg(t *testing.T) {
@@ -568,7 +572,7 @@ func TestPathsArg(t *testing.T) {
568572
// without any path args
569573
_, err = cmd(t, "-C", tempDir)
570574
as.NoError(err)
571-
assertStats(t, as, 31, 31, 31, 0)
575+
assertStats(t, as, 32, 32, 32, 0)
572576

573577
// specify some explicit paths
574578
_, err = cmd(t, "-C", tempDir, "-c", "elm/elm.json", "haskell/Nested/Foo.hs")

‎flake.lock

+15-15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎nix/packages/treefmt/formatters.nix

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ with pkgs; [
1616
statix
1717
deadnix
1818
terraform
19+
dos2unix
20+
yamlfmt
1921
# util for unit testing
2022
(pkgs.writeShellApplication {
2123
name = "test-fmt";

‎test/examples/treefmt.toml

+9
Original file line numberDiff line numberDiff line change
@@ -91,5 +91,14 @@ command = "terraform"
9191
options = ["fmt"]
9292
includes = ["*.tf"]
9393

94+
[formatter.dos2unix]
95+
command = "dos2unix"
96+
includes = ["*.yaml"]
97+
options = ["--keepdate"]
98+
99+
[formatter.yamlfmt]
100+
command = "yamlfmt"
101+
includes = ["*.yaml"]
102+
94103
[formatter.foo-fmt]
95104
command = "foo-fmt"

‎test/examples/yaml/test.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
hello: world
2+
list:
3+
- one
4+
- two
5+
- three

‎test/temp.go

+6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"io"
55
"os"
66
"testing"
7+
"time"
78

89
"git.numtide.com/numtide/treefmt/config"
910

@@ -27,6 +28,11 @@ func WriteConfig(t *testing.T, path string, cfg config.Config) {
2728
func TempExamples(t *testing.T) string {
2829
tempDir := t.TempDir()
2930
require.NoError(t, cp.Copy("../test/examples", tempDir), "failed to copy test data to temp dir")
31+
32+
// we have second precision mod time tracking, so we wait a second before returning, so we don't trigger false
33+
//positives for things like fail on change
34+
time.Sleep(time.Second)
35+
3036
return tempDir
3137
}
3238

‎walk/walker.go

+25
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"fmt"
66
"io/fs"
7+
"os"
8+
"time"
79
)
810

911
type Type string
@@ -20,6 +22,29 @@ type File struct {
2022
Info fs.FileInfo
2123
}
2224

25+
func (f File) HasChanged() (bool, fs.FileInfo, error) {
26+
// get the file's current state
27+
current, err := os.Stat(f.Path)
28+
if err != nil {
29+
return false, nil, fmt.Errorf("failed to stat %s: %w", f.Path, err)
30+
}
31+
32+
// check the size first
33+
if f.Info.Size() != current.Size() {
34+
return true, current, nil
35+
}
36+
37+
// POSIX specifies EPOCH time for Mod time, but some filesystems give more precision.
38+
// Some formatters mess with the mod time (e.g., dos2unix) but not to the same precision,
39+
// triggering false positives.
40+
// We truncate everything below a second.
41+
if f.Info.ModTime().Truncate(time.Second) != current.ModTime().Truncate(time.Second) {
42+
return true, current, nil
43+
}
44+
45+
return false, nil, nil
46+
}
47+
2348
func (f File) String() string {
2449
return f.Path
2550
}

0 commit comments

Comments
 (0)
Please sign in to comment.