fix: use second precision when comparing file mod times

We were using whatever precision the underlying file system was giving us, and in some cases that can be very detailed. Some formatters mess with the mod time, but not to the same precision (e.g. dos2unix).

POSIX also specifies that mod time should be EPOCH (second) precision.

This change brings us back in line with how 1.x worked, and should resolve issues with false fail on change errors.
This commit is contained in:
Brian McGee 2024-07-06 12:20:43 +01:00
parent 23e563b239
commit 85ce0a2f74
No known key found for this signature in database
GPG Key ID: D49016E76AD1E8C0
8 changed files with 110 additions and 52 deletions

View File

@ -11,6 +11,7 @@ import (
"runtime"
"runtime/pprof"
"syscall"
"time"
"git.numtide.com/numtide/treefmt/format"
"git.numtide.com/numtide/treefmt/stats"
@ -389,20 +390,26 @@ func (f *Format) detectFormatted(ctx context.Context) func() error {
return nil
}
// look up current file info
currentInfo, err := os.Stat(file.Path)
// check if the file has changed
changed, newInfo, err := file.HasChanged()
if err != nil {
return fmt.Errorf("failed to stat processed file: %w", err)
return err
}
// check if the file has changed
if !(file.Info.ModTime() == currentInfo.ModTime() && file.Info.Size() == currentInfo.Size()) {
if changed {
// record the change
stats.Add(stats.Formatted, 1)
// log the change for diagnostics
log.Debugf("file has been changed: %s", file.Path)
log.Debug(
"file has changed",
"path", file.Path,
"prev_size", file.Info.Size(),
"current_size", newInfo.Size(),
"prev_mod_time", file.Info.ModTime().Truncate(time.Second),
"current_mod_time", newInfo.ModTime().Truncate(time.Second),
)
// update the file info
file.Info = currentInfo
file.Info = newInfo
}
// mark as processed

View File

@ -9,6 +9,7 @@ import (
"path/filepath"
"regexp"
"testing"
"time"
"git.numtide.com/numtide/treefmt/config"
"git.numtide.com/numtide/treefmt/format"
@ -159,22 +160,22 @@ func TestSpecifyingFormatters(t *testing.T) {
setup()
_, err := cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 3, 3)
assertStats(t, as, 32, 32, 3, 3)
setup()
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--formatters", "elm,nix")
as.NoError(err)
assertStats(t, as, 31, 31, 2, 2)
assertStats(t, as, 32, 32, 2, 2)
setup()
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "-f", "ruby,nix")
as.NoError(err)
assertStats(t, as, 31, 31, 2, 2)
assertStats(t, as, 32, 32, 2, 2)
setup()
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--formatters", "nix")
as.NoError(err)
assertStats(t, as, 31, 31, 1, 1)
assertStats(t, as, 32, 32, 1, 1)
// test bad names
setup()
@ -204,7 +205,7 @@ func TestIncludesAndExcludes(t *testing.T) {
test.WriteConfig(t, configPath, cfg)
_, err := cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 31, 0)
assertStats(t, as, 32, 32, 32, 0)
// globally exclude nix files
cfg.Global.Excludes = []string{"*.nix"}
@ -212,7 +213,7 @@ func TestIncludesAndExcludes(t *testing.T) {
test.WriteConfig(t, configPath, cfg)
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 30, 0)
assertStats(t, as, 32, 32, 31, 0)
// add haskell files to the global exclude
cfg.Global.Excludes = []string{"*.nix", "*.hs"}
@ -220,7 +221,7 @@ func TestIncludesAndExcludes(t *testing.T) {
test.WriteConfig(t, configPath, cfg)
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 24, 0)
assertStats(t, as, 32, 32, 25, 0)
echo := cfg.Formatters["echo"]
@ -230,7 +231,7 @@ func TestIncludesAndExcludes(t *testing.T) {
test.WriteConfig(t, configPath, cfg)
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 22, 0)
assertStats(t, as, 32, 32, 23, 0)
// remove go files from the echo formatter
echo.Excludes = []string{"*.py", "*.go"}
@ -238,7 +239,7 @@ func TestIncludesAndExcludes(t *testing.T) {
test.WriteConfig(t, configPath, cfg)
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 21, 0)
assertStats(t, as, 32, 32, 22, 0)
// adjust the includes for echo to only include elm files
echo.Includes = []string{"*.elm"}
@ -246,7 +247,7 @@ func TestIncludesAndExcludes(t *testing.T) {
test.WriteConfig(t, configPath, cfg)
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 1, 0)
assertStats(t, as, 32, 32, 1, 0)
// add js files to echo formatter
echo.Includes = []string{"*.elm", "*.js"}
@ -254,7 +255,7 @@ func TestIncludesAndExcludes(t *testing.T) {
test.WriteConfig(t, configPath, cfg)
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 2, 0)
assertStats(t, as, 32, 32, 2, 0)
}
func TestCache(t *testing.T) {
@ -281,7 +282,7 @@ func TestCache(t *testing.T) {
test.WriteConfig(t, configPath, cfg)
_, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 31, 0)
assertStats(t, as, 32, 32, 32, 0)
out, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
@ -290,7 +291,7 @@ func TestCache(t *testing.T) {
// clear cache
_, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir, "-c")
as.NoError(err)
assertStats(t, as, 31, 31, 31, 0)
assertStats(t, as, 32, 32, 32, 0)
out, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
@ -299,7 +300,7 @@ func TestCache(t *testing.T) {
// clear cache
_, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir, "-c")
as.NoError(err)
assertStats(t, as, 31, 31, 31, 0)
assertStats(t, as, 32, 32, 32, 0)
out, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir)
as.NoError(err)
@ -308,7 +309,7 @@ func TestCache(t *testing.T) {
// no cache
_, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir, "--no-cache")
as.NoError(err)
assertStats(t, as, 31, 31, 31, 0)
assertStats(t, as, 32, 32, 32, 0)
}
func TestChangeWorkingDirectory(t *testing.T) {
@ -342,7 +343,7 @@ func TestChangeWorkingDirectory(t *testing.T) {
// this should fail if the working directory hasn't been changed first
_, err = cmd(t, "-C", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 31, 0)
assertStats(t, as, 32, 32, 32, 0)
}
func TestFailOnChange(t *testing.T) {
@ -365,6 +366,9 @@ func TestFailOnChange(t *testing.T) {
_, err := cmd(t, "--fail-on-change", "--config-file", configPath, "--tree-root", tempDir)
as.ErrorIs(err, ErrFailOnChange)
// we have second precision mod time tracking
time.Sleep(time.Second)
// test with no cache
test.WriteConfig(t, configPath, cfg)
_, err = cmd(t, "--fail-on-change", "--config-file", configPath, "--tree-root", tempDir, "--no-cache")
@ -411,31 +415,31 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
args := []string{"--config-file", configPath, "--tree-root", tempDir}
_, err := cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 31, 3, 0)
assertStats(t, as, 32, 32, 3, 0)
// tweak mod time of elm formatter
as.NoError(test.RecreateSymlink(t, binPath+"/"+"elm-format"))
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 31, 3, 0)
assertStats(t, as, 32, 32, 3, 0)
// check cache is working
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 0, 0, 0)
assertStats(t, as, 32, 0, 0, 0)
// tweak mod time of python formatter
as.NoError(test.RecreateSymlink(t, binPath+"/"+"black"))
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 31, 3, 0)
assertStats(t, as, 32, 32, 3, 0)
// check cache is working
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 0, 0, 0)
assertStats(t, as, 32, 0, 0, 0)
// add go formatter
cfg.Formatters["go"] = &config.Formatter{
@ -447,12 +451,12 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 31, 4, 0)
assertStats(t, as, 32, 32, 4, 0)
// check cache is working
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 0, 0, 0)
assertStats(t, as, 32, 0, 0, 0)
// remove python formatter
delete(cfg.Formatters, "python")
@ -460,12 +464,12 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 31, 2, 0)
assertStats(t, as, 32, 32, 2, 0)
// check cache is working
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 0, 0, 0)
assertStats(t, as, 32, 0, 0, 0)
// remove elm formatter
delete(cfg.Formatters, "elm")
@ -473,12 +477,12 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 31, 1, 0)
assertStats(t, as, 32, 32, 1, 0)
// check cache is working
_, err = cmd(t, args...)
as.NoError(err)
assertStats(t, as, 31, 0, 0, 0)
assertStats(t, as, 32, 0, 0, 0)
}
func TestGitWorktree(t *testing.T) {
@ -524,7 +528,7 @@ func TestGitWorktree(t *testing.T) {
// add everything to the worktree
as.NoError(wt.AddGlob("."))
as.NoError(err)
run(31, 31, 31, 0)
run(32, 32, 32, 0)
// remove python directory
as.NoError(wt.RemoveGlob("python/*"))
@ -533,7 +537,7 @@ func TestGitWorktree(t *testing.T) {
// walk with filesystem instead of git
_, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--walk", "filesystem")
as.NoError(err)
assertStats(t, as, 59, 59, 59, 0)
assertStats(t, as, 61, 61, 61, 0)
}
func TestPathsArg(t *testing.T) {
@ -568,7 +572,7 @@ func TestPathsArg(t *testing.T) {
// without any path args
_, err = cmd(t, "-C", tempDir)
as.NoError(err)
assertStats(t, as, 31, 31, 31, 0)
assertStats(t, as, 32, 32, 32, 0)
// specify some explicit paths
_, err = cmd(t, "-C", tempDir, "-c", "elm/elm.json", "haskell/Nested/Foo.hs")

View File

@ -29,11 +29,11 @@
]
},
"locked": {
"lastModified": 1713532798,
"narHash": "sha256-wtBhsdMJA3Wa32Wtm1eeo84GejtI43pMrFrmwLXrsEc=",
"lastModified": 1717408969,
"narHash": "sha256-Q0OEFqe35fZbbRPPRdrjTUUChKVhhWXz3T9ZSKmaoVY=",
"owner": "numtide",
"repo": "devshell",
"rev": "12e914740a25ea1891ec619bb53cf5e6ca922e40",
"rev": "1ebbe68d57457c8cae98145410b164b5477761f4",
"type": "github"
},
"original": {
@ -44,11 +44,11 @@
},
"flake-compat": {
"locked": {
"lastModified": 1688025799,
"narHash": "sha256-ktpB4dRtnksm9F5WawoIkEneh1nrEvuxb5lJFt1iOyw=",
"lastModified": 1717312683,
"narHash": "sha256-FrlieJH50AuvagamEvWMIE6D2OAnERuDboFDYAED/dE=",
"owner": "nix-community",
"repo": "flake-compat",
"rev": "8bf105319d44f6b9f0d764efa4fdef9f1cc9ba1c",
"rev": "38fd3954cf65ce6faf3d0d45cd26059e059f07ea",
"type": "github"
},
"original": {
@ -86,11 +86,11 @@
]
},
"locked": {
"lastModified": 1710154385,
"narHash": "sha256-4c3zQ2YY4BZOufaBJB4v9VBBeN2dH7iVdoJw8SDNCfI=",
"lastModified": 1717050755,
"narHash": "sha256-C9IEHABulv2zEDFA+Bf0E1nmfN4y6MIUe5eM2RCrDC0=",
"owner": "nix-community",
"repo": "gomod2nix",
"rev": "872b63ddd28f318489c929d25f1f0a3c6039c971",
"rev": "31b6d2e40b36456e792cd6cf50d5a8ddd2fa59a1",
"type": "github"
},
"original": {
@ -116,11 +116,11 @@
},
"nixpkgs": {
"locked": {
"lastModified": 1714253743,
"narHash": "sha256-mdTQw2XlariysyScCv2tTE45QSU9v/ezLcHJ22f0Nxc=",
"lastModified": 1720031269,
"narHash": "sha256-rwz8NJZV+387rnWpTYcXaRNvzUSnnF9aHONoJIYmiUQ=",
"owner": "nixos",
"repo": "nixpkgs",
"rev": "58a1abdbae3217ca6b702f03d3b35125d88a2994",
"rev": "9f4128e00b0ae8ec65918efeba59db998750ead6",
"type": "github"
},
"original": {
@ -178,11 +178,11 @@
]
},
"locked": {
"lastModified": 1715940852,
"narHash": "sha256-wJqHMg/K6X3JGAE9YLM0LsuKrKb4XiBeVaoeMNlReZg=",
"lastModified": 1719887753,
"narHash": "sha256-p0B2r98UtZzRDM5miGRafL4h7TwGRC4DII+XXHDHqek=",
"owner": "numtide",
"repo": "treefmt-nix",
"rev": "2fba33a182602b9d49f0b2440513e5ee091d838b",
"rev": "bdb6355009562d8f9313d9460c0d3860f525bc6c",
"type": "github"
},
"original": {

View File

@ -16,6 +16,8 @@ with pkgs; [
statix
deadnix
terraform
dos2unix
yamlfmt
# util for unit testing
(pkgs.writeShellApplication {
name = "test-fmt";

View File

@ -91,5 +91,14 @@ command = "terraform"
options = ["fmt"]
includes = ["*.tf"]
[formatter.dos2unix]
command = "dos2unix"
includes = ["*.yaml"]
options = ["--keepdate"]
[formatter.yamlfmt]
command = "yamlfmt"
includes = ["*.yaml"]
[formatter.foo-fmt]
command = "foo-fmt"

View File

@ -0,0 +1,5 @@
hello: world
list:
- one
- two
- three

View File

@ -4,6 +4,7 @@ import (
"io"
"os"
"testing"
"time"
"git.numtide.com/numtide/treefmt/config"
@ -27,6 +28,11 @@ func WriteConfig(t *testing.T, path string, cfg config.Config) {
func TempExamples(t *testing.T) string {
tempDir := t.TempDir()
require.NoError(t, cp.Copy("../test/examples", tempDir), "failed to copy test data to temp dir")
// we have second precision mod time tracking, so we wait a second before returning, so we don't trigger false
//positives for things like fail on change
time.Sleep(time.Second)
return tempDir
}

View File

@ -4,6 +4,8 @@ import (
"context"
"fmt"
"io/fs"
"os"
"time"
)
type Type string
@ -20,6 +22,29 @@ type File struct {
Info fs.FileInfo
}
func (f File) HasChanged() (bool, fs.FileInfo, error) {
// get the file's current state
current, err := os.Stat(f.Path)
if err != nil {
return false, nil, fmt.Errorf("failed to stat %s: %w", f.Path, err)
}
// check the size first
if f.Info.Size() != current.Size() {
return true, current, nil
}
// POSIX specifies EPOCH time for Mod time, but some filesystems give more precision.
// Some formatters mess with the mod time (e.g., dos2unix) but not to the same precision,
// triggering false positives.
// We truncate everything below a second.
if f.Info.ModTime().Truncate(time.Second) != current.ModTime().Truncate(time.Second) {
return true, current, nil
}
return false, nil, nil
}
func (f File) String() string {
return f.Path
}