Merge pull request #345 from numtide/fix/fail-on-change
Some checks failed
gh-pages / build (push) Has been cancelled
golangci-lint / lint (push) Has been cancelled
gh-pages / deploy (push) Has been cancelled

fix: use second precision when comparing file mod times
This commit is contained in:
Brian McGee 2024-07-07 11:17:11 +01:00 committed by GitHub
commit 65152cb373
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
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
}