220302.2 · 5daeaefe4de0b5d6326db5e0ad31fc441e4470dd
Correctly follow symlinks, fix adapter funcs, fix Windows tests
This also tunes DefaultNumWorkers for macOS/Darwin which results in a 22% speed up: ``` name old time/op new time/op delta FastWalk-10 16.8ms ± 1% 13.0ms ± 2% -22.20% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta FastWalk-10 2.13MB ± 0% 2.14MB ± 0% ~ (p=0.151 n=5+5)
name old allocs/op new allocs/op delta FastWalk-10 37.7k ± 0% 37.7k ± 0% -0.01% (p=0.016 n=5+5) ```
reading declarations
@@ -7,57 +7,94 @@ import ( ) func isDir(path string, d fs.DirEntry) bool {- typ := d.Type()- if typ&os.ModeSymlink != 0 {+ if d.IsDir() {+ return true+ }+ if d.Type()&os.ModeSymlink != 0 { if fi, err := StatDirEntry(path, d); err == nil {- typ = fi.Mode().Type()+ return fi.IsDir() } }- return typ.IsDir()+ return false } -// TODO: the following wrappers may be incorrect since we do not-// call the WalkDirFunc in all cases.--// FollowSymlinks wraps walkFn so that symlinks are followed.-func FollowSymlinks(walkFn fs.WalkDirFunc) fs.WalkDirFunc {+// IgnoreDuplicateDirs wraps fs.WalkDirFunc walkFn to make it follow symbolic+// links and ignore duplicate directories (if a symlink points to a directory+// that has already been traversed it is skipped). The walkFn is called for+// for skipped directories, but the directory is not traversed (this is+// required for error handling).+//+// The Config.Follow setting has no effect on the behavior of Walk when+// this wrapper is used.+//+// In most use cases, the returned fs.WalkDirFunc should not be reused between+// in another call to Walk. If it is reused, any previously visited file will+// be skipped.+//+// NOTE: The order of traversal is undefined. Given an "example" directory+// like the one below where "dir" is a directory and "smydir1" and "smydir2"+// are links to it, only one of "dir", "smydir1", or "smydir2" will be+// traversed, but which one is undefined.+//+// example+// ├── dir+// ├── smydir1 -> dir+// └── smydir2 -> dir+//+func IgnoreDuplicateDirs(walkFn fs.WalkDirFunc) fs.WalkDirFunc { filter := NewEntryFilter() return func(path string, d fs.DirEntry, err error) error {+ // Call walkFn before checking the entry filter so that we+ // don't record directories that are skipped with SkipDir.+ err = walkFn(path, d, err)+ if err != nil {+ if err != filepath.SkipDir && isDir(path, d) {+ filter.Entry(path, d)+ }+ return err+ } if isDir(path, d) {- // TODO: call walkFn on these files or is that already handled? if filter.Entry(path, d) { return filepath.SkipDir }- if d.Type()&os.ModeSymlink != 0 {+ if d.Type() == os.ModeSymlink { return ErrTraverseLink }- return nil }- return walkFn(path, d, err)+ return nil } } // IgnoreDuplicateFiles wraps walkFn so that symlinks are followed and duplicate // files are ignored. If a symlink resolves to a file that has already been // visited it will be skipped.+//+// In most use cases, the returned fs.WalkDirFunc should not be reused between+// in another call to Walk. If it is reused, any previously visited file will+// be skipped.+//+// This can significantly slow Walk as os.Stat() is called for each path+// (on Windows, os.Stat() is only needed for symlinks). func IgnoreDuplicateFiles(walkFn fs.WalkDirFunc) fs.WalkDirFunc { filter := NewEntryFilter()- return func(path string, ent fs.DirEntry, err error) error {- typ := ent.Type()- if filter.Entry(path, ent) {- if typ.IsDir() {+ return func(path string, d fs.DirEntry, err error) error {+ // Skip all duplicate files, directories, and links+ if filter.Entry(path, d) {+ if isDir(path, d) { return filepath.SkipDir } return nil }- if typ&os.ModeSymlink != 0 {- return ErrTraverseLink+ err = walkFn(path, d, err)+ if err == nil && d.Type() == os.ModeSymlink && isDir(path, d) {+ err = ErrTraverseLink }- return walkFn(path, ent, err)+ return err } } // IgnorePermissionErrors wraps walkFn so that permission errors are ignored.+// The returned fs.WalkDirFunc may be reused. func IgnorePermissionErrors(walkFn fs.WalkDirFunc) fs.WalkDirFunc { return func(path string, d fs.DirEntry, err error) error { if err != nil && os.IsPermission(err) {
@@ -0,0 +1,237 @@+package fastwalk_test++import (+ "errors"+ "io/fs"+ "io/ioutil"+ "os"+ "path/filepath"+ "reflect"+ "runtime"+ "sort"+ "strings"+ "sync"+ "sync/atomic"+ "testing"++ "github.com/charlievieth/fastwalk"+)++func TestIgnoreDuplicateDirs(t *testing.T) {+ tempdir, err := ioutil.TempDir("", "test-fast-walk")+ if err != nil {+ t.Fatal(err)+ }+ // on macOS the tempdir is a symlink+ tempdir, err = filepath.EvalSymlinks(tempdir)+ if err != nil {+ t.Fatal(err)+ }+ defer cleanupOrLogTempDir(t, tempdir)++ files := map[string]string{+ "bar/bar.go": "one",+ "foo/foo.go": "two",+ "skip/baz.go": "three", // we skip "skip", but visit "baz.go" via "symdir"+ "symdir": "LINK:skip",+ "bar/symdir": "LINK:../foo/",+ "bar/loop": "LINK:../bar/", // symlink loop+ }+ testCreateFiles(t, tempdir, files)++ want := map[string]os.FileMode{+ "": os.ModeDir,+ "/src": os.ModeDir,+ "/src/bar": os.ModeDir,+ "/src/bar/bar.go": 0,+ "/src/bar/symdir": os.ModeSymlink,+ "/src/bar/loop": os.ModeSymlink,+ "/src/foo": os.ModeDir,+ "/src/foo/foo.go": 0,+ "/src/symdir": os.ModeSymlink,+ "/src/symdir/baz.go": 0,+ "/src/skip": os.ModeDir,+ }++ runTest := func(t *testing.T, conf *fastwalk.Config) {+ var mu sync.Mutex+ got := make(map[string]os.FileMode)+ walkFn := fastwalk.IgnoreDuplicateDirs(func(path string, de fs.DirEntry, err error) error {+ requireNoError(t, err)+ if err != nil {+ return err+ }++ // Resolve links for regular files since we don't know which directory+ // or link we traversed to visit them. Exclude "baz.go" because we want+ // to test that we visited it through it's link.+ if de.Type().IsRegular() && de.Name() != "baz.go" {+ realpath, err := filepath.EvalSymlinks(path)+ if err != nil {+ t.Error(err)+ return err+ }+ path = realpath+ }+ if !strings.HasPrefix(path, tempdir) {+ t.Errorf("Path %q not a child of TMPDIR %q", path, tempdir)+ return errors.New("abort")+ }+ key := filepath.ToSlash(strings.TrimPrefix(path, tempdir))++ mu.Lock()+ defer mu.Unlock()+ got[key] = de.Type().Type()++ if de.Name() == "skip" {+ return filepath.SkipDir+ }+ return nil+ })+ if err := fastwalk.Walk(conf, tempdir, walkFn); err != nil {+ t.Error("fastwalk:", err)+ }+ if !reflect.DeepEqual(want, got) {+ t.Errorf("walk mismatch.\n got:\n%v\nwant:\n%v", formatFileModes(got), formatFileModes(want))+ diffFileModes(t, got, want)+ }+ }++ t.Run("NoFollow", func(t *testing.T) {+ runTest(t, &fastwalk.Config{Follow: false})+ })++ // Test that setting Follow to true has no impact on the behavior+ t.Run("Follow", func(t *testing.T) {+ runTest(t, &fastwalk.Config{Follow: true})+ })++ t.Run("Error", func(t *testing.T) {+ tempdir := t.TempDir()+ if err := os.WriteFile(tempdir+"/error_test", []byte("error"), 0644); err != nil {+ t.Fatal(err)+ }+ want := errors.New("my error")+ var callCount int32+ walkFn := fastwalk.IgnoreDuplicateDirs(func(path string, de fs.DirEntry, err error) error {+ atomic.AddInt32(&callCount, 1)+ return want+ })+ err := fastwalk.Walk(nil, tempdir, walkFn)+ if !errors.Is(err, want) {+ t.Errorf("Error: want: %v got: %v", want, err)+ }++ })+}++func TestIgnoreDuplicateFiles(t *testing.T) {+ tempdir := t.TempDir()+ files := map[string]string{+ "foo/foo.go": "one",+ "bar/bar.go": "LINK:../foo/foo.go",+ "bar/baz.go": "two",+ "broken/broken.go": "LINK:../nonexistent",+ "bar/loop": "LINK:../bar/", // symlink loop+ "file.go": "three",++ // Use multiple symdirs to increase the chance that one+ // of these and not "foo" is followed first.+ "symdir1": "LINK:foo",+ "symdir2": "LINK:foo",+ "symdir3": "LINK:foo",+ "symdir4": "LINK:foo",+ }+ if runtime.GOOS == "windows" {+ delete(files, "broken/broken.go")+ }+ testCreateFiles(t, tempdir, files)++ var expectedContents []string+ for _, contents := range files {+ if !strings.HasPrefix(contents, "LINK:") {+ expectedContents = append(expectedContents, contents)+ }+ }+ sort.Strings(expectedContents)++ var (+ mu sync.Mutex+ seen []os.FileInfo+ contents []string+ )+ walkFn := fastwalk.IgnoreDuplicateFiles(func(path string, de fs.DirEntry, err error) error {+ requireNoError(t, err)+ fi1, err := fastwalk.StatDirEntry(path, de)+ if err != nil {+ t.Error(err)+ return err+ }+ mu.Lock()+ defer mu.Unlock()+ for _, fi2 := range seen {+ if os.SameFile(fi1, fi2) {+ t.Errorf("Visited file twice: %q (%s) and %q (%s)",+ path, fi1.Mode(), fi2.Name(), fi2.Mode())+ }+ }+ seen = append(seen, fi1)+ if fi1.Mode().IsRegular() {+ data, err := os.ReadFile(path)+ if err != nil {+ return err+ }+ contents = append(contents, string(data))+ }+ return nil+ })+ if err := fastwalk.Walk(nil, tempdir, walkFn); err != nil {+ t.Fatal(err)+ }++ sort.Strings(contents)+ if !reflect.DeepEqual(expectedContents, contents) {+ t.Errorf("File contents want: %q got: %q", expectedContents, contents)+ }+}++func TestIgnorePermissionErrors(t *testing.T) {+ var called bool+ fn := fastwalk.IgnorePermissionErrors(func(path string, _ fs.DirEntry, err error) error {+ called = true+ if err != nil {+ t.Fatal(err)+ }+ return nil+ })++ t.Run("PermissionError", func(t *testing.T) {+ err := fn("", nil, &os.PathError{Op: "open", Path: "foo.go", Err: os.ErrPermission})+ if err != nil {+ t.Fatal(err)+ }+ if called {+ t.Fatal("walkFn should not have been called with os.ErrPermission")+ }+ })++ t.Run("NilError", func(t *testing.T) {+ called = false+ if err := fn("", nil, nil); err != nil {+ t.Fatal(err)+ }+ if !called {+ t.Fatal("walkFn should have been called with nil error")+ }+ })++ t.Run("OtherError", func(t *testing.T) {+ fn := fastwalk.IgnorePermissionErrors(func(path string, _ fs.DirEntry, err error) error {+ return err+ })+ want := &os.PathError{Op: "open", Path: "foo.go", Err: os.ErrExist}+ if got := fn("", nil, want); got != want {+ t.Fatalf("want error: %v got: %v", want, got)+ }+ })+}
@@ -4,6 +4,7 @@ import ( "fmt" "io/fs" "os"+ "testing" "time" ) @@ -27,3 +28,27 @@ func FormatFileInfo(fi fs.FileInfo) string { Sys: fmt.Sprintf("%+v", fi.Sys()), }) }++func TestCleanRootPath(t *testing.T) {+ tests := map[string]string{+ "": "",+ "/": "/",+ "//": "/",+ "/foo": "/foo",+ "/foo/": "/foo",+ "a": "a",+ `C:/`: `C:`,+ }+ if os.PathSeparator != '/' {+ const sep = string(os.PathSeparator)+ tests["C:"+sep] = `C:`+ tests["C:"+sep+sep] = `C:`+ tests[sep+sep] = sep+ }+ for in, want := range tests {+ got := cleanRootPath(in)+ if got != want {+ t.Errorf("cleanRootPath(%q) = %q; want: %q", in, got, want)+ }+ }+}
@@ -7,6 +7,7 @@ import ( "os" "path/filepath" "runtime"+ "sync" "testing" "time" @@ -14,80 +15,64 @@ import ( ) func TestEntryFilter(t *testing.T) {- dirEntry := func(t *testing.T, name string) fs.DirEntry {- t.Helper()- fi, err := os.Lstat(name)- if err != nil {- t.Fatal(err)- }- return fs.FileInfoToDirEntry(fi)- }- tempdir := t.TempDir()-- fileName := filepath.Join(tempdir, "file.txt")- if err := writeFile(fileName, "file.txt", 0644); err != nil {- t.Fatal(err)+ files := map[string]string{+ "foo/foo.go": "one",+ "bar/bar.go": "LINK:../foo/foo.go",+ "bar/baz.go": "two",+ "bar/loop": "LINK:../bar/", // symlink loop+ "file.go": "three",++ // Use multiple symdirs to increase the chance that one+ // of these and not "foo" is followed first.+ "symdir1": "LINK:foo",+ "symdir2": "LINK:foo",+ "symdir3": "LINK:foo",+ "symdir4": "LINK:foo", }+ testCreateFiles(t, tempdir, files) - linkDir := filepath.Join(tempdir, "dir")- if err := os.Mkdir(linkDir, 0755); err != nil {- t.Fatal(err)- }- linkName := filepath.Join(linkDir, "link.link")- if err := symlink(t, "../"+filepath.Base(fileName), linkName); err != nil {+ var mu sync.Mutex+ var seen []os.FileInfo+ filter := fastwalk.NewEntryFilter()+ walkFn := fastwalk.IgnoreDuplicateFiles(func(path string, de fs.DirEntry, err error) error {+ requireNoError(t, err)+ fi1, err := fastwalk.StatDirEntry(path, de)+ if err != nil {+ t.Error(err)+ return err+ }+ mu.Lock()+ defer mu.Unlock()+ if !filter.Entry(path, de) {+ for _, fi2 := range seen {+ if os.SameFile(fi1, fi2) {+ t.Errorf("Visited file twice: %q (%s) and %q (%s)",+ path, fi1.Mode(), fi2.Name(), fi2.Mode())+ }+ }+ }+ seen = append(seen, fi1)+ return nil+ })+ if err := fastwalk.Walk(nil, tempdir, walkFn); err != nil { t.Fatal(err) } - // Sanity check- {- fi1, err := os.Stat(fileName)+ // Test that true is returned for a non-existent directory+ // On Windows the Info field of the returned DirEntry+ // is already populated so this will succeed.+ if runtime.GOOS != "windows" {+ path := filepath.Join(tempdir, "src", "foo/foo.go")+ fi, err := os.Lstat(path) if err != nil { t.Fatal(err) }- fi2, err := os.Stat(linkName)- if err != nil {+ if err := os.Remove(path); err != nil { t.Fatal(err) }- if !os.SameFile(fi1, fi2) {- t.Fatalf("os.SameFile(%q, %q) == false", fileName, linkName)- }- }-- filter := fastwalk.NewEntryFilter()- if seen := filter.Entry(fileName, dirEntry(t, fileName)); seen {- t.Errorf("filter.Entry(%q) = %t want: %t",- filepath.Dir(fileName), seen, false)- }- if seen := filter.Entry(linkName, dirEntry(t, linkName)); !seen {- t.Errorf("filter.Entry(%q) = %t want: %t",- filepath.Dir(linkName), seen, true)- }-- // Entry should return true (aka seen) if there is an error- // stat'ing the file.-- infos, err := os.ReadDir(tempdir)- if err != nil {- t.Fatal(err)- }-- // Remove the files- if err := os.RemoveAll(tempdir); err != nil {- t.Fatal(err)- }-- for _, de := range infos {- // On Windows the Info field of the returned DirEntry- // is already populated so this will succeed.- if runtime.GOOS != "windows" {- if _, err := de.Info(); err == nil {- t.Fatal(de.Name())- }- }- path := filepath.Join(tempdir, de.Name())- if seen := filter.Entry(path, de); !seen {- t.Errorf("filter.Entry(%q) = %t want: %t", de.Name(), seen, true)+ if !filter.Entry(path, fs.FileInfoToDirEntry(fi)) {+ t.Error("EntryFilter should return true when the file does not exist") } } }
@@ -48,6 +48,8 @@ func (e *EntryFilter) seen(dev, ino uint64) (seen bool) { return seen } +// TODO: this name is confusing and should be fixed+ // Entry returns if path and fs.DirEntry have been seen before. func (e *EntryFilter) Entry(path string, de fs.DirEntry) (seen bool) { fi, err := StatDirEntry(path, de)
@@ -42,7 +42,7 @@ func generateDevIno(rr *rand.Rand, ndev, size int) []devIno { return pairs } -func TestEntryFilterFiles(t *testing.T) {+func TestEntryFilter_Unix(t *testing.T) { rr := rand.New(rand.NewSource(1)) pairs := generateDevIno(rr, 2, 100) @@ -59,7 +59,7 @@ func TestEntryFilterFiles(t *testing.T) { } } -func TestEntryFilterFiles_Parallel(t *testing.T) {+func TestEntryFilter_Unix_Parallel(t *testing.T) { if testing.Short() { t.Skip("Short test") }@@ -106,32 +106,12 @@ func TestEntryFilterFiles_Parallel(t *testing.T) { wg.Wait() } -// TODO: delete?-func BenchmarkEntryFilter_Seen(b *testing.B) {- rr := rand.New(rand.NewSource(1))- // pairs := GenerateDevIno(rr, 2, 8192)- pairs := generateDevIno(rr, 1, 8192)- x := NewEntryFilter()-- for _, p := range pairs {- x.seen(p.Dev, p.Ino)- }- if len(pairs) != 8192 {- panic("nope!")- }-- b.ResetTimer()- for i := 0; i < b.N; i++ {- p := pairs[i%8192]- x.seen(p.Dev, p.Ino)+func BenchmarkEntryFilter_Unix(b *testing.B) {+ if testing.Short() {+ b.Skip("Skipping: short test") }-}--// TODO: delete?-func BenchmarkEntryFilter_Seen_Parallel(b *testing.B) { rr := rand.New(rand.NewSource(1))- // pairs := GenerateDevIno(rr, 2, 8192)- pairs := generateDevIno(rr, 1, 8192)+ pairs := generateDevIno(rr, 2, 8192) x := NewEntryFilter() for _, p := range pairs {@@ -142,10 +122,19 @@ func BenchmarkEntryFilter_Seen_Parallel(b *testing.B) { } b.ResetTimer()- b.RunParallel(func(pb *testing.PB) {- for i := 0; pb.Next(); i++ {+ b.Run("Sequential", func(b *testing.B) {+ for i := 0; i < b.N; i++ { p := pairs[i%8192] x.seen(p.Dev, p.Ino) } })++ b.Run("Parallel", func(b *testing.B) {+ b.RunParallel(func(pb *testing.PB) {+ for i := 0; pb.Next(); i++ {+ p := pairs[i%8192]+ x.seen(p.Dev, p.Ino)+ }+ })+ }) }
@@ -54,29 +54,57 @@ var ErrTraverseLink = errors.New("fastwalk: traverse symlink, assuming target is // Child directories will still be traversed. var ErrSkipFiles = errors.New("fastwalk: skip remaining files in directory") -// DefaultNumWorkers is the default number of worker goroutines to use in-// fastwalk.Walk and is in the range of 4 to 32.-var DefaultNumWorkers = func() int {- numCPU := runtime.NumCPU()+// SkipDir is used as a return value from WalkDirFuncs to indicate that+// the directory named in the call is to be skipped. It is not returned+// as an error by any function.+var SkipDir = fs.SkipDir++// DefaultNumWorkers returns the default number of worker goroutines to use in+// fastwalk.Walk and is the value of runtime.GOMAXPROCS(-1) clamped to a range+// of 4 to 32 except on Darwin where it is either 4 (8 cores or less) or 6+// (more than 8 cores). This is because Walk / IO performance on Darwin+// degrades with more concurrency.+//+// The optimal number for your workload may be lower or higher. The results+// of BenchmarkFastWalkNumWorkers benchmark may be informative.+func DefaultNumWorkers() int {+ numCPU := runtime.GOMAXPROCS(-1) if numCPU < 4 { return 4 }+ // Darwin IO performance on APFS slows with more workers.+ // Stat performance is best around 2-4 and file IO is best+ // around 4-6. More workers only benefit CPU intensive tasks.+ if runtime.GOOS == "darwin" {+ if numCPU <= 8 {+ return 4+ }+ return 6+ } if numCPU > 32 { return 32 } return numCPU-}()+} // DefaultConfig is the default Config used when none is supplied. var DefaultConfig = Config{ Follow: false,- NumWorkers: DefaultNumWorkers,+ NumWorkers: DefaultNumWorkers(), } type Config struct {- // TODO: better document Follow+ // TODO: do we want to pass a sentinel error to WalkFunc if+ // a symlink loop is detected? - // Follow symbolic links ignoring duplicate directories.+ // Follow symbolic links ignoring directories that would lead+ // to infinite loops; that is, entering a previously visited+ // directory that is an ancestor of the last file encountered.+ //+ // The sentinel error ErrTraverseLink is ignored when Follow+ // is true (this to prevent users from defeating the loop+ // detection logic), but SkipDir and ErrSkipFiles are still+ // respected. Follow bool // Number of parallel workers to use. If NumWorkers if ≤ 0 then@@ -147,16 +175,7 @@ func Walk(conf *Config, root string, walkFn fs.WalkDirFunc) error { numWorkers := conf.NumWorkers if numWorkers <= 0 {- // TODO(bradfitz): We used a minimum of 4 to give the kernel more info- // about multiple things we want, in hopes its I/O scheduling can take- // advantage of that. Hopefully most are in cache. Maybe 4 is even too- // low of a minimum. Profile more.- numWorkers = 4- if n := runtime.NumCPU(); n > numWorkers {- // TODO(CEV): profile to see if we still want to limit- // concurrency on macOS.- numWorkers = n- }+ numWorkers = DefaultNumWorkers() } w := &walker{@@ -167,9 +186,13 @@ func Walk(conf *Config, root string, walkFn fs.WalkDirFunc) error { // buffered for correctness & not leaking goroutines: resc: make(chan error, numWorkers),++ follow: conf.Follow, }- if conf.Follow {- w.filter = NewEntryFilter()+ if w.follow {+ if fi, err := os.Stat(root); err == nil {+ w.ignoredDirs = append(w.ignoredDirs, fi)+ } } defer close(w.donec)@@ -178,6 +201,8 @@ func Walk(conf *Config, root string, walkFn fs.WalkDirFunc) error { wg.Add(1) go w.doWork(&wg) }++ root = cleanRootPath(root) todo := []walkItem{{dir: root, info: fileInfoToDirEntry(filepath.Dir(root), fi)}} out := 0 for {@@ -243,7 +268,8 @@ type walker struct { enqueuec chan walkItem // from workers resc chan error // from workers - filter *EntryFilter // track files when walking links+ ignoredDirs []os.FileInfo+ follow bool } type walkItem struct {@@ -259,40 +285,79 @@ func (w *walker) enqueue(it walkItem) { } } -func (w *walker) onDirEnt(dirName, baseName string, de fs.DirEntry) error {- joined := dirName + string(os.PathSeparator) + baseName- typ := de.Type()- if typ == os.ModeSymlink && w.filter != nil {- // Check if the symlink points to a directory before potentially- // enqueuing it.- if fi, _ := StatDirEntry(joined, de); fi != nil && fi.IsDir() {- if !w.filter.Entry(joined, de) {- w.enqueue(walkItem{dir: joined, info: de})- }+func (w *walker) shouldSkipDir(fi os.FileInfo) bool {+ for _, ignored := range w.ignoredDirs {+ if os.SameFile(ignored, fi) {+ return true }- return nil }- if typ == os.ModeDir {- if w.filter == nil || !w.filter.Entry(joined, de) {- w.enqueue(walkItem{dir: joined, info: de})+ return false+}++func (w *walker) shouldTraverse(path string, de fs.DirEntry) bool {+ // TODO: do we need to use filepath.EvalSymlinks() here?+ ts, err := StatDirEntry(path, de)+ if err != nil {+ return false+ }+ if !ts.IsDir() {+ return false+ }+ if w.shouldSkipDir(ts) {+ return false+ }+ for {+ parent := filepath.Dir(path)+ if parent == path {+ return true }+ parentInfo, err := os.Stat(parent)+ if err != nil {+ return false+ }+ if os.SameFile(ts, parentInfo) {+ return false+ }+ path = parent+ }+}++func joinPaths(dir, base string) string {+ // Handle the case where the root path argument to Walk is "/"+ // without this the returned path is prefixed with "//".+ if os.PathSeparator == '/' && dir == "/" {+ return dir + base+ }+ return dir + string(os.PathSeparator) + base+}++func (w *walker) onDirEnt(dirName, baseName string, de fs.DirEntry) error {+ joined := joinPaths(dirName, baseName)+ typ := de.Type()+ if typ == os.ModeDir {+ w.enqueue(walkItem{dir: joined, info: de}) return nil } err := w.fn(joined, de, nil) if typ == os.ModeSymlink {- // TODO: note that this only occurs when not-following symlinks- // (aka: w.seen == nil) if err == ErrTraverseLink {- // Set callbackDone so we don't call it twice for both the- // symlink-as-symlink and the symlink-as-directory later:- w.enqueue(walkItem{dir: joined, callbackDone: true})- return nil+ if !w.follow {+ // Set callbackDone so we don't call it twice for both the+ // symlink-as-symlink and the symlink-as-directory later:+ w.enqueue(walkItem{dir: joined, info: de, callbackDone: true})+ return nil+ }+ err = nil // Ignore ErrTraverseLink when Follow is true. } if err == filepath.SkipDir { // Permit SkipDir on symlinks too. return nil }+ if err == nil && w.follow && w.shouldTraverse(joined, de) {+ // Traverse symlink+ w.enqueue(walkItem{dir: joined, info: de, callbackDone: true})+ } } return err }@@ -315,3 +380,15 @@ func (w *walker) walk(root string, info fs.DirEntry, runUserCallback bool) error } return nil }++func cleanRootPath(root string) string {+ for i := len(root) - 1; i >= 0; i-- {+ if !os.IsPathSeparator(root[i]) {+ return root[:i+1]+ }+ }+ if root != "" {+ return root[0:1] // root is all path separators ("//")+ }+ return root+}
@@ -2,6 +2,7 @@ package fastwalk_test import ( "bytes"+ "crypto/md5" "errors" "flag" "fmt"@@ -75,13 +76,19 @@ func symlink(t testing.TB, oldname, newname string) error { return err } -func testFastWalkConf(t *testing.T, conf *fastwalk.Config, files map[string]string, callback fs.WalkDirFunc, want map[string]os.FileMode) {- tempdir, err := ioutil.TempDir("", "test-fast-walk")- if err != nil {- t.Fatal(err)+func cleanupOrLogTempDir(t *testing.T, tempdir string) {+ if e := recover(); e != nil {+ t.Log("TMPDIR:", tempdir)+ t.Fatal(e)+ }+ if t.Failed() {+ t.Log("TMPDIR:", tempdir)+ } else {+ os.RemoveAll(tempdir) }- defer os.RemoveAll(tempdir)+} +func testCreateFiles(t *testing.T, tempdir string, files map[string]string) { symlinks := map[string]string{} for path, contents := range files { file := filepath.Join(tempdir, "/src", path)@@ -106,10 +113,24 @@ func testFastWalkConf(t *testing.T, conf *fastwalk.Config, files map[string]stri t.Fatal(err) } }+}++func testFastWalkConf(t *testing.T, conf *fastwalk.Config, files map[string]string, callback fs.WalkDirFunc, want map[string]os.FileMode) {+ tempdir, err := ioutil.TempDir("", "test-fast-walk")+ if err != nil {+ t.Fatal(err)+ }+ defer cleanupOrLogTempDir(t, tempdir)++ testCreateFiles(t, tempdir, files) got := map[string]os.FileMode{} var mu sync.Mutex err = fastwalk.Walk(conf, tempdir, func(path string, de fs.DirEntry, err error) error {+ if de == nil {+ t.Errorf("nil fs.DirEntry on %q", path)+ return nil+ } mu.Lock() defer mu.Unlock() if !strings.HasPrefix(path, tempdir) {@@ -128,6 +149,7 @@ func testFastWalkConf(t *testing.T, conf *fastwalk.Config, files map[string]stri } if !reflect.DeepEqual(got, want) { t.Errorf("walk mismatch.\n got:\n%v\nwant:\n%v", formatFileModes(got), formatFileModes(want))+ diffFileModes(t, got, want) } } @@ -215,10 +237,14 @@ func TestFastWalk_DirEntryType(t *testing.T) { "symdir": "LINK:foo", "broken/broken.go": "LINK:../nonexistent", },- func(path string, typ fs.DirEntry, err error) error {+ func(path string, de fs.DirEntry, err error) error { requireNoError(t, err)- if _, ok := typ.(fastwalk.DirEntry); !ok {- t.Errorf("%q: not a fastwalk.DirEntry: %T", path, typ)+ if _, ok := de.(fastwalk.DirEntry); !ok {+ t.Errorf("%q: not a fastwalk.DirEntry: %T", path, de)+ }+ if de.Type() != de.Type().Type() {+ t.Errorf("%s: type mismatch got: %q want: %q",+ path, de.Type(), de.Type().Type()) } return nil },@@ -295,10 +321,9 @@ func TestFastWalk_SkipFiles(t *testing.T) { func TestFastWalk_TraverseSymlink(t *testing.T) { testFastWalk(t, map[string]string{- "foo/foo.go": "one",- "bar/bar.go": "two",- "skip/skip.go": "skip",- "symdir": "LINK:foo",+ "foo/foo.go": "one",+ "bar/bar.go": "two",+ "symdir": "LINK:foo", }, func(path string, de fs.DirEntry, err error) error { requireNoError(t, err)@@ -315,74 +340,123 @@ func TestFastWalk_TraverseSymlink(t *testing.T) { "/src/bar/bar.go": 0, "/src/foo": os.ModeDir, "/src/foo/foo.go": 0,- "/src/skip": os.ModeDir,- "/src/skip/skip.go": 0, "/src/symdir": os.ModeSymlink, "/src/symdir/foo.go": 0, }) } -func TestFastWalk_TraverseSymlink_Follow(t *testing.T) {+func TestFastWalk_Follow(t *testing.T) {+ subTests := []struct {+ Name string+ OnLink func(path string, d fs.DirEntry) error+ }{+ // Test that the walk func does *not* need to return+ // ErrTraverseLink for links to be followed.+ {+ Name: "Default",+ OnLink: func(path string, d fs.DirEntry) error { return nil },+ },++ // Test that returning ErrTraverseLink does not interfere+ // with the Follow logic.+ {+ Name: "ErrTraverseLink",+ OnLink: func(path string, d fs.DirEntry) error {+ if d.Type()&os.ModeSymlink != 0 {+ if fi, err := fastwalk.StatDirEntry(path, d); err == nil && fi.IsDir() {+ return fastwalk.ErrTraverseLink+ }+ }+ return nil+ },+ },+ }+ for _, x := range subTests {+ t.Run(x.Name, func(t *testing.T) {+ conf := fastwalk.Config{+ Follow: true,+ }+ testFastWalkConf(t, &conf, map[string]string{+ "foo/foo.go": "one",+ "bar/bar.go": "two",+ "foo/symlink": "LINK:foo.go",+ "bar/symdir": "LINK:../foo/",+ "bar/link1": "LINK:../foo/",+ },+ func(path string, de fs.DirEntry, err error) error {+ requireNoError(t, err)+ if err != nil {+ return err+ }+ if de.Type()&os.ModeSymlink != 0 {+ return x.OnLink(path, de)+ }+ return nil+ },+ map[string]os.FileMode{+ "": os.ModeDir,+ "/src": os.ModeDir,+ "/src/bar": os.ModeDir,+ "/src/bar/bar.go": 0,+ "/src/bar/link1": os.ModeSymlink,+ "/src/bar/link1/foo.go": 0,+ "/src/bar/link1/symlink": os.ModeSymlink,+ "/src/bar/symdir": os.ModeSymlink,+ "/src/bar/symdir/foo.go": 0,+ "/src/bar/symdir/symlink": os.ModeSymlink,+ "/src/foo": os.ModeDir,+ "/src/foo/foo.go": 0,+ "/src/foo/symlink": os.ModeSymlink,+ })+ })+ }+}++func TestFastWalk_Follow_SkipDir(t *testing.T) { conf := fastwalk.Config{ Follow: true, } testFastWalkConf(t, &conf, map[string]string{- "foo/foo.go": "one",- "bar/bar.go": "two",- "skip/skip.go": "skip",- "foo/symdir": "LINK:foo",- "bar/symdir": "LINK:../foo",+ ".dot/baz.go": "one",+ "bar/bar.go": "three",+ "bar/dot": "LINK:../.dot/",+ "bar/symdir": "LINK:../foo/",+ "foo/foo.go": "two",+ "foo/symlink": "LINK:foo.go", }, func(path string, de fs.DirEntry, err error) error { requireNoError(t, err)- typ := de.Type().Type()- if typ == os.ModeSymlink {- t.Errorf("unexpected symlink: %q", path)+ if err != nil {+ return err+ }+ if strings.HasPrefix(de.Name(), ".") {+ return filepath.SkipDir } return nil }, map[string]os.FileMode{- "": os.ModeDir,- "/src": os.ModeDir,- "/src/bar": os.ModeDir,- "/src/bar/bar.go": 0,- "/src/foo": os.ModeDir,- "/src/foo/foo.go": 0,- "/src/skip": os.ModeDir,- "/src/skip/skip.go": 0,+ "": os.ModeDir,+ "/src": os.ModeDir,+ "/src/.dot": os.ModeDir,+ "/src/bar": os.ModeDir,+ "/src/bar/bar.go": 0,+ "/src/bar/dot": os.ModeSymlink,+ "/src/bar/dot/baz.go": 0,+ "/src/bar/symdir": os.ModeSymlink,+ "/src/bar/symdir/foo.go": 0,+ "/src/bar/symdir/symlink": os.ModeSymlink,+ "/src/foo": os.ModeDir,+ "/src/foo/foo.go": 0,+ "/src/foo/symlink": os.ModeSymlink, }) } -func TestFastWalk_FollowSymlinks(t *testing.T) {- testFastWalk(t, map[string]string{- "foo/foo.go": "one",- "bar/bar.go": "two",- "skip/skip.go": "skip",- "bar/loop": "LINK:../bar/",- },- fastwalk.FollowSymlinks(func(path string, de fs.DirEntry, err error) error {- requireNoError(t, err)- return nil- }),- map[string]os.FileMode{- "": os.ModeDir,- "/src": os.ModeDir,- "/src/bar": os.ModeDir,- "/src/bar/loop": os.ModeSymlink,- "/src/bar/bar.go": 0,- "/src/foo": os.ModeDir,- "/src/foo/foo.go": 0,- "/src/skip": os.ModeDir,- "/src/skip/skip.go": 0,- })-}--func TestFastWalk_SymlinkLoop(t *testing.T) {+func TestFastWalk_Follow_SymlinkLoop(t *testing.T) { tempdir, err := ioutil.TempDir("", "test-fast-walk") if err != nil { t.Fatal(err) }- defer os.RemoveAll(tempdir)+ defer cleanupOrLogTempDir(t, tempdir) if err := writeFile(tempdir+"/src/foo.go", "hello", 0644); err != nil { t.Fatal(err)@@ -396,13 +470,12 @@ func TestFastWalk_SymlinkLoop(t *testing.T) { } var walked int32 err = fastwalk.Walk(&conf, tempdir, func(path string, de fs.DirEntry, err error) error {- requireNoError(t, err)+ if err != nil {+ return err+ } if n := atomic.AddInt32(&walked, 1); n > 20 { return fmt.Errorf("symlink loop: %d", n) }- if de.Type()&os.ModeSymlink != 0 {- return fastwalk.ErrTraverseLink- } return nil }) if err != nil {@@ -410,6 +483,43 @@ func TestFastWalk_SymlinkLoop(t *testing.T) { } } +// Test that ErrTraverseLink is ignored when following symlinks+// if it would cause a symlink loop.+func TestFastWalk_Follow_ErrTraverseLink(t *testing.T) {+ conf := fastwalk.Config{+ Follow: true,+ }+ testFastWalkConf(t, &conf, map[string]string{+ "foo/foo.go": "one",+ "bar/bar.go": "two",+ "bar/symdir": "LINK:../foo/",+ "bar/loop": "LINK:../bar/", // symlink loop+ },+ func(path string, de fs.DirEntry, err error) error {+ requireNoError(t, err)+ if err != nil {+ return err+ }+ if de.Type()&os.ModeSymlink != 0 {+ if fi, err := fastwalk.StatDirEntry(path, de); err == nil && fi.IsDir() {+ return fastwalk.ErrTraverseLink+ }+ }+ return nil+ },+ map[string]os.FileMode{+ "": os.ModeDir,+ "/src": os.ModeDir,+ "/src/bar": os.ModeDir,+ "/src/bar/bar.go": 0,+ "/src/bar/loop": os.ModeSymlink,+ "/src/bar/symdir": os.ModeSymlink,+ "/src/bar/symdir/foo.go": 0,+ "/src/foo": os.ModeDir,+ "/src/foo/foo.go": 0,+ })+}+ func TestFastWalk_Error(t *testing.T) { tmp := t.TempDir() for _, child := range []string{@@ -506,6 +616,54 @@ func TestFastWalk_ErrPermission(t *testing.T) { } if !reflect.DeepEqual(got, want) { t.Errorf("walk mismatch.\n got:\n%v\nwant:\n%v", formatFileModes(got), formatFileModes(want))+ diffFileModes(t, got, want)+ }+}++func diffFileModes(t *testing.T, got, want map[string]os.FileMode) {+ type Mode struct {+ Name string+ Mode os.FileMode+ }+ var extra []Mode+ for k, v := range got {+ if _, ok := want[k]; !ok {+ extra = append(extra, Mode{k, v})+ }+ }+ var missing []Mode+ for k, v := range want {+ if _, ok := got[k]; !ok {+ missing = append(missing, Mode{k, v})+ }+ }+ var delta []Mode+ for k, v := range got {+ if vv, ok := want[k]; ok && vv != v {+ delta = append(delta, Mode{k, v}, Mode{k, vv})+ }+ }+ w := new(strings.Builder)+ printMode := func(name string, modes []Mode) {+ if len(modes) == 0 {+ return+ }+ sort.Slice(modes, func(i, j int) bool {+ return modes[i].Name < modes[j].Name+ })+ if w.Len() == 0 {+ w.WriteString("\n")+ }+ fmt.Fprintf(w, "%s:\n", name)+ for _, m := range modes {+ fmt.Fprintf(w, " %-20s: %s\n", m.Name, m.Mode.String())+ }+ }+ printMode("Extra", extra)+ printMode("Missing", missing)+ printMode("Delta", delta)+ if w.Len() != 0 {+ t.Error(w.String()) } } @@ -526,35 +684,156 @@ func init() { } } -func benchmarkFastWalk(b *testing.B, conf *fastwalk.Config) {+func noopWalkFunc(_ string, _ fs.DirEntry, _ error) error { return nil }++func benchmarkFastWalk(b *testing.B, conf *fastwalk.Config,+ adapter func(fs.WalkDirFunc) fs.WalkDirFunc) {+ b.ReportAllocs()- for i := 0; i < b.N; i++ {- err := fastwalk.Walk(conf, *benchDir, func(_ string, _ fs.DirEntry, _ error) error {- return nil- })- if err != nil {- b.Fatal(err)+ if adapter != nil {+ walkFn := noopWalkFunc+ for i := 0; i < b.N; i++ {+ err := fastwalk.Walk(conf, *benchDir, adapter(walkFn))+ if err != nil {+ b.Fatal(err)+ }+ }+ } else {+ for i := 0; i < b.N; i++ {+ err := fastwalk.Walk(conf, *benchDir, noopWalkFunc)+ if err != nil {+ b.Fatal(err)+ } } } } func BenchmarkFastWalk(b *testing.B) {- benchmarkFastWalk(b, nil)+ benchmarkFastWalk(b, nil, nil) } func BenchmarkFastWalkFollow(b *testing.B) {- benchmarkFastWalk(b, &fastwalk.Config{- Follow: true,+ benchmarkFastWalk(b, &fastwalk.Config{Follow: true}, nil)+}++func BenchmarkFastWalkAdapters(b *testing.B) {+ if testing.Short() {+ b.Skip("Skipping: short test")+ }+ b.Run("IgnoreDuplicateDirs", func(b *testing.B) {+ benchmarkFastWalk(b, nil, fastwalk.IgnoreDuplicateDirs)+ })++ b.Run("IgnoreDuplicateFiles", func(b *testing.B) {+ benchmarkFastWalk(b, nil, fastwalk.IgnoreDuplicateFiles) }) } +// Benchmark various tasks with different worker counts.+//+// Observations:+// * Linux (Intel i9-9900K / Samsung Pro NVMe): consistently benefits from+// more workers+// * Darwin (m1): IO heavy tasks (Readfile and Stat) and Traversal perform+// best with 4 workers, and only CPU bound tasks benefit from more workers+// func BenchmarkFastWalkNumWorkers(b *testing.B) {- maxWorkers := runtime.NumCPU() * 2- for i := 2; i <= maxWorkers; i += 2 {- b.Run(fmt.Sprint(i), func(b *testing.B) {- })+ if testing.Short() {+ b.Skip("Skipping: short test")+ }++ runBench := func(b *testing.B, walkFn fs.WalkDirFunc) {+ maxWorkers := runtime.NumCPU()+ for i := 2; i <= maxWorkers; i += 2 {+ b.Run(fmt.Sprint(i), func(b *testing.B) {+ conf := fastwalk.Config{+ NumWorkers: i,+ }+ for i := 0; i < b.N; i++ {+ if err := fastwalk.Walk(&conf, *benchDir, walkFn); err != nil {+ b.Fatal(err)+ }+ }+ })+ } }- benchmarkFastWalk(b, nil)++ // Bench pure traversal speed+ b.Run("NoOp", func(b *testing.B) {+ runBench(b, func(path string, d fs.DirEntry, err error) error {+ return err+ })+ })++ // No IO and light CPU+ b.Run("NoIO", func(b *testing.B) {+ runBench(b, func(path string, d fs.DirEntry, err error) error {+ if err == nil {+ fmt.Fprintf(io.Discard, "%s: %q\n", d.Type(), path)+ }+ return err+ })+ })++ // Stat each regular file+ b.Run("Stat", func(b *testing.B) {+ runBench(b, func(path string, d fs.DirEntry, err error) error {+ if err == nil && d.Type().IsRegular() {+ d.Info()+ }+ return err+ })+ })++ // IO heavy task+ b.Run("ReadFile", func(b *testing.B) {+ runBench(b, func(path string, d fs.DirEntry, err error) error {+ if err != nil || !d.Type().IsRegular() {+ return err+ }+ f, err := os.Open(path)+ if err != nil {+ if os.IsNotExist(err) || os.IsPermission(err) {+ return nil+ }+ return err+ }+ defer f.Close()++ _, err = io.Copy(io.Discard, f)+ return err+ })+ })++ // CPU and IO heavy task+ b.Run("Hash", func(b *testing.B) {+ bufPool := &sync.Pool{+ New: func() interface{} {+ b := make([]byte, 96*1024)+ return &b+ },+ }+ runBench(b, func(path string, d fs.DirEntry, err error) error {+ if err != nil || !d.Type().IsRegular() {+ return err+ }+ f, err := os.Open(path)+ if err != nil {+ if os.IsNotExist(err) || os.IsPermission(err) {+ return nil+ }+ return err+ }+ defer f.Close()++ p := bufPool.Get().(*[]byte)+ h := md5.New()+ _, err = io.CopyBuffer(h, f, *p)+ bufPool.Put(p)+ _ = h.Sum(nil)+ return err+ })+ }) } var benchWalkFunc = flag.String("walkfunc", "fastwalk", "The function to use for BenchmarkWalkComparison")@@ -562,9 +841,12 @@ var benchWalkFunc = flag.String("walkfunc", "fastwalk", "The function to use for // BenchmarkWalkComparison generates benchmarks using different walk functions // so that the results can be easily compared with `benchcmp` and `benchstat`. func BenchmarkWalkComparison(b *testing.B) {+ if testing.Short() {+ b.Skip("Skipping: short test")+ } switch *benchWalkFunc { case "fastwalk":- benchmarkFastWalk(b, nil)+ benchmarkFastWalk(b, nil, nil) case "godirwalk": opts := godirwalk.Options{ Unsorted: true,
@@ -78,9 +78,7 @@ func readDir(dirName string, fn func(dirName, entName string, de fs.DirEntry) er } } -// TODO: we can probably use syscall.Dirent.Ino + the parents Dev to skip a call to stat func parseDirEnt(buf []byte) (consumed int, name string, typ os.FileMode) {- // golang.org/issue/37269 dirent := &syscall.Dirent{} copy((*[unsafe.Sizeof(syscall.Dirent{})]byte)(unsafe.Pointer(dirent))[:], buf)
@@ -6,7 +6,6 @@ package fastwalk import ( "syscall" "unsafe"- _ "unsafe" ) // TODO(charlie): implement the closedir and readdir_r functions here instead