diff --git a/cache/cache.go b/cache/cache.go index a6c743f..fe1c1ac 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -218,11 +218,6 @@ func ChangeSet(ctx context.Context, walker walk.Walker, filesCh chan<- *walk.Fil } } - // ignore symlinks - if file.Info.Mode()&os.ModeSymlink == os.ModeSymlink { - return nil - } - // open a new read tx if there isn't one in progress // we have to periodically open a new read tx to prevent writes from being blocked if tx == nil { diff --git a/cli/format.go b/cli/format.go index 7218a95..95ff37b 100644 --- a/cli/format.go +++ b/cli/format.go @@ -237,12 +237,9 @@ func (f *Format) walkFilesystem(ctx context.Context) func() error { case <-ctx.Done(): return ctx.Err() default: - // ignore symlinks and directories - if !(file.Info.IsDir() || file.Info.Mode()&os.ModeSymlink == os.ModeSymlink) { - stats.Add(stats.Traversed, 1) - stats.Add(stats.Emitted, 1) - f.filesCh <- file - } + stats.Add(stats.Traversed, 1) + stats.Add(stats.Emitted, 1) + f.filesCh <- file return nil } }) diff --git a/walk/filesystem.go b/walk/filesystem.go index 18cc3df..729a497 100644 --- a/walk/filesystem.go +++ b/walk/filesystem.go @@ -4,39 +4,45 @@ import ( "context" "fmt" "io/fs" + "os" "path/filepath" ) type filesystemWalker struct { - root string - pathsCh chan string + root string + pathsCh chan string + relPathOffset int } func (f filesystemWalker) Root() string { return f.root } -func (f filesystemWalker) Walk(_ context.Context, fn WalkFunc) error { - relPathOffset := len(f.root) + 1 - - relPathFn := func(path string) (string, error) { - // quick optimisation for the majority of use cases - // todo check that root is a prefix in path? - if len(path) >= relPathOffset { - return path[relPathOffset:], nil - } - return filepath.Rel(f.root, path) +func (f filesystemWalker) relPath(path string) (string, error) { + // quick optimization for the majority of use cases + if len(path) >= f.relPathOffset && path[:len(f.root)] == f.root { + return path[f.relPathOffset:], nil } + // fallback to proper relative path resolution + return filepath.Rel(f.root, path) +} +func (f filesystemWalker) Walk(_ context.Context, fn WalkFunc) error { walkFn := func(path string, info fs.FileInfo, _ error) error { if info == nil { return fmt.Errorf("no such file or directory '%s'", path) } - relPath, err := relPathFn(path) + // ignore directories and symlinks + if info.IsDir() || info.Mode()&os.ModeSymlink == os.ModeSymlink { + return nil + } + + relPath, err := f.relPath(path) if err != nil { return fmt.Errorf("failed to determine a relative path for %s: %w", path, err) } + file := File{ Path: path, RelPath: relPath, @@ -55,5 +61,9 @@ func (f filesystemWalker) Walk(_ context.Context, fn WalkFunc) error { } func NewFilesystem(root string, paths chan string) (Walker, error) { - return filesystemWalker{root, paths}, nil + return filesystemWalker{ + root: root, + pathsCh: paths, + relPathOffset: len(root) + 1, + }, nil } diff --git a/walk/filesystem_test.go b/walk/filesystem_test.go new file mode 100644 index 0000000..503ec0d --- /dev/null +++ b/walk/filesystem_test.go @@ -0,0 +1,76 @@ +package walk + +import ( + "context" + "os" + "testing" + + "git.numtide.com/numtide/treefmt/test" + "github.com/stretchr/testify/require" +) + +var examplesPaths = []string{ + "elm/elm.json", + "elm/src/Main.elm", + "go/go.mod", + "go/main.go", + "haskell/CHANGELOG.md", + "haskell/Foo.hs", + "haskell/Main.hs", + "haskell/Nested/Foo.hs", + "haskell/Setup.hs", + "haskell/haskell.cabal", + "haskell/treefmt.toml", + "haskell-frontend/CHANGELOG.md", + "haskell-frontend/Main.hs", + "haskell-frontend/Setup.hs", + "haskell-frontend/haskell-frontend.cabal", + "html/index.html", + "html/scripts/.gitkeep", + "javascript/source/hello.js", + "nix/sources.nix", + "nixpkgs.toml", + "python/main.py", + "python/requirements.txt", + "python/virtualenv_proxy.py", + "ruby/bundler.rb", + "rust/Cargo.toml", + "rust/src/main.rs", + "shell/foo.sh", + "terraform/main.tf", + "terraform/two.tf", + "touch.toml", + "treefmt.toml", + "yaml/test.yaml", +} + +func TestFilesystemWalker_Walk(t *testing.T) { + tempDir := test.TempExamples(t) + + paths := make(chan string, 1) + go func() { + paths <- tempDir + close(paths) + }() + + as := require.New(t) + + walker, err := NewFilesystem(tempDir, paths) + as.NoError(err) + + idx := 0 + err = walker.Walk(context.Background(), func(file *File, err error) error { + as.Equal(examplesPaths[idx], file.RelPath) + idx += 1 + return nil + }) + as.NoError(err) + + // capture current cwd, so we can replace it after the test is finished + cwd, err := os.Getwd() + as.NoError(err) + t.Cleanup(func() { + // return to the previous working directory + as.NoError(os.Chdir(cwd)) + }) +} diff --git a/walk/git.go b/walk/git.go index 58afa4e..4dc9196 100644 --- a/walk/git.go +++ b/walk/git.go @@ -13,26 +13,26 @@ import ( ) type gitWalker struct { - root string - paths chan string - repo *git.Repository + root string + paths chan string + repo *git.Repository + relPathOffset int } -func (g *gitWalker) Root() string { +func (g gitWalker) Root() string { return g.root } -func (g *gitWalker) Walk(ctx context.Context, fn WalkFunc) error { - // for quick relative paths - relPathOffset := len(g.root) + 1 - - relPathFn := func(path string) (relPath string) { - if len(path) >= relPathOffset { - relPath = path[relPathOffset:] - } - return +func (g gitWalker) relPath(path string) (string, error) { + // quick optimization for the majority of use cases + if len(path) >= g.relPathOffset && path[:len(g.root)] == g.root { + return path[g.relPathOffset:], nil } + // fallback to proper relative path resolution + return filepath.Rel(g.root, path) +} +func (g gitWalker) Walk(ctx context.Context, fn WalkFunc) error { idx, err := g.repo.Storer.Index() if err != nil { return fmt.Errorf("failed to open git index: %w", err) @@ -50,14 +50,28 @@ func (g *gitWalker) Walk(ctx context.Context, fn WalkFunc) error { case <-ctx.Done(): return ctx.Err() default: - path := filepath.Join(g.root, entry.Name) + // we only want regular files, not directories or symlinks + if !entry.Mode.IsRegular() { + continue + } // stat the file + path := filepath.Join(g.root, entry.Name) + info, err := os.Lstat(path) + if err != nil { + return fmt.Errorf("failed to stat %s: %w", path, err) + } + + // determine a relative path + relPath, err := g.relPath(path) + if err != nil { + return fmt.Errorf("failed to determine a relative path for %s: %w", path, err) + } file := File{ Path: path, - RelPath: relPathFn(path), + RelPath: relPath, Info: info, } @@ -93,9 +107,9 @@ func (g *gitWalker) Walk(ctx context.Context, fn WalkFunc) error { return nil } - relPath, err := filepath.Rel(g.root, path) + relPath, err := g.relPath(path) if err != nil { - return err + return fmt.Errorf("failed to determine a relative path for %s: %w", path, err) } if _, ok := cache[relPath]; !ok { @@ -105,7 +119,7 @@ func (g *gitWalker) Walk(ctx context.Context, fn WalkFunc) error { file := File{ Path: path, - RelPath: relPathFn(path), + RelPath: relPath, Info: info, } @@ -121,5 +135,10 @@ func NewGit(root string, paths chan string) (Walker, error) { if err != nil { return nil, fmt.Errorf("failed to open git repo: %w", err) } - return &gitWalker{root, paths, repo}, nil + return &gitWalker{ + root: root, + paths: paths, + repo: repo, + relPathOffset: len(root) + 1, + }, nil }