Skip to content

Commit

Permalink
fix: use second precision when comparing file mod times
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@brianmcgee
brianmcgee committed Jul 6, 2024
1 parent 23e563b commit 85ce0a2
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 52 deletions.
21 changes: 14 additions & 7 deletions cli/format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"runtime"
"runtime/pprof"
"syscall"
"time"

"git.numtide.com/numtide/treefmt/format"
"git.numtide.com/numtide/treefmt/stats"
Expand Down Expand Up @@ -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
Expand Down
64 changes: 34 additions & 30 deletions cli/format_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path/filepath"
"regexp"
"testing"
"time"

"git.numtide.com/numtide/treefmt/config"
"git.numtide.com/numtide/treefmt/format"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -204,23 +205,23 @@ 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"}

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"}

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"]

Expand All @@ -230,31 +231,31 @@ 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"}

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"}

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"}

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) {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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")
Expand Down Expand Up @@ -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{
Expand All @@ -447,38 +451,38 @@ 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")
test.WriteConfig(t, configPath, cfg)

_, 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")
test.WriteConfig(t, configPath, cfg)

_, 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) {
Expand Down Expand Up @@ -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/*"))
Expand All @@ -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) {
Expand Down Expand Up @@ -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")
Expand Down
30 changes: 15 additions & 15 deletions flake.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions nix/packages/treefmt/formatters.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ with pkgs; [
statix
deadnix
terraform
dos2unix
yamlfmt
# util for unit testing
(pkgs.writeShellApplication {
name = "test-fmt";
Expand Down
Loading

0 comments on commit 85ce0a2

Please # to comment.