From 94b14834a20132093826ea5e2da5502a13908ad3 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Thu, 8 Mar 2018 16:10:10 -0500 Subject: [PATCH] imports: extract fastWalk into new package internal/fastwalk It is going to be used by a new tool. Moved to an internal package so it does not become a publicly supported api. Modified the tests so they don't depend on the fix_test infrastructure. Change-Id: Ib8ebef24dc23e180960af04aa3d06b5f41a7c02b Reviewed-on: https://go-review.googlesource.com/99678 Reviewed-by: Brad Fitzpatrick --- imports/fix.go | 10 ++- {imports => internal/fastwalk}/fastwalk.go | 38 +++++----- .../fastwalk}/fastwalk_dirent_fileno.go | 2 +- .../fastwalk}/fastwalk_dirent_ino.go | 2 +- .../fastwalk}/fastwalk_portable.go | 2 +- .../fastwalk}/fastwalk_test.go | 69 ++++++++++++------- .../fastwalk}/fastwalk_unix.go | 2 +- 7 files changed, 73 insertions(+), 52 deletions(-) rename {imports => internal/fastwalk}/fastwalk.go (83%) rename {imports => internal/fastwalk}/fastwalk_dirent_fileno.go (94%) rename {imports => internal/fastwalk}/fastwalk_dirent_ino.go (94%) rename {imports => internal/fastwalk}/fastwalk_portable.go (97%) rename {imports => internal/fastwalk}/fastwalk_test.go (70%) rename {imports => internal/fastwalk}/fastwalk_unix.go (99%) diff --git a/imports/fix.go b/imports/fix.go index 68961ba656..73f939a13f 100644 --- a/imports/fix.go +++ b/imports/fix.go @@ -22,6 +22,7 @@ import ( "sync" "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/internal/fastwalk" ) // Debug controls verbose logging. @@ -545,16 +546,13 @@ func skipDir(fi os.FileInfo) bool { return false } -// shouldTraverse reports whether the symlink fi should, found in dir, +// shouldTraverse reports whether the symlink fi, found in dir, // should be followed. It makes sure symlinks were never visited // before to avoid symlink loops. func shouldTraverse(dir string, fi os.FileInfo) bool { path := filepath.Join(dir, fi.Name()) target, err := filepath.EvalSymlinks(path) if err != nil { - if !os.IsNotExist(err) { - fmt.Fprintln(os.Stderr, err) - } return false } ts, err := os.Stat(target) @@ -674,12 +672,12 @@ func scanGoDirs(goRoot bool) { return nil } if shouldTraverse(dir, fi) { - return traverseLink + return fastwalk.TraverseLink } } return nil } - if err := fastWalk(srcDir, walkFn); err != nil { + if err := fastwalk.Walk(srcDir, walkFn); err != nil { log.Printf("goimports: scanning directory %v: %v", srcDir, err) } } diff --git a/imports/fastwalk.go b/internal/fastwalk/fastwalk.go similarity index 83% rename from imports/fastwalk.go rename to internal/fastwalk/fastwalk.go index 31e6e27b0d..5cc7df53ff 100644 --- a/imports/fastwalk.go +++ b/internal/fastwalk/fastwalk.go @@ -2,17 +2,9 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// A faster implementation of filepath.Walk. -// -// filepath.Walk's design necessarily calls os.Lstat on each file, -// even if the caller needs less info. And goimports only need to know -// the type of each file. The kernel interface provides the type in -// the Readdir call but the standard library ignored it. -// fastwalk_unix.go contains a fork of the syscall routines. -// -// See golang.org/issue/16399 - -package imports +// Package fastwalk provides a faster version of filepath.Walk for file system +// scanning tools. +package fastwalk import ( "errors" @@ -22,10 +14,22 @@ import ( "sync" ) -// traverseLink is a sentinel error for fastWalk, similar to filepath.SkipDir. -var traverseLink = errors.New("traverse symlink, assuming target is a directory") +// TraverseLink is used as a return value from WalkFuncs to indicate that the +// symlink named in the call may be traversed. +var TraverseLink = errors.New("fastwalk: traverse symlink, assuming target is a directory") -// fastWalk walks the file tree rooted at root, calling walkFn for +// Walk is a faster implementation of filepath.Walk. +// +// filepath.Walk's design necessarily calls os.Lstat on each file, +// even if the caller needs less info. +// Many tools need only the type of each file. +// On some platforms, this information is provided directly by the readdir +// system call, avoiding the need to stat each file individually. +// fastwalk_unix.go contains a fork of the syscall routines. +// +// See golang.org/issue/16399 +// +// Walk walks the file tree rooted at root, calling walkFn for // each file or directory in the tree, including root. // // If fastWalk returns filepath.SkipDir, the directory is skipped. @@ -36,10 +40,10 @@ var traverseLink = errors.New("traverse symlink, assuming target is a directory" // any permission bits. // * multiple goroutines stat the filesystem concurrently. The provided // walkFn must be safe for concurrent use. -// * fastWalk can follow symlinks if walkFn returns the traverseLink +// * fastWalk can follow symlinks if walkFn returns the TraverseLink // sentinel error. It is the walkFn's responsibility to prevent // fastWalk from going into symlink cycles. -func fastWalk(root string, walkFn func(path string, typ os.FileMode) error) error { +func Walk(root string, walkFn func(path string, typ os.FileMode) error) error { // TODO(bradfitz): make numWorkers configurable? 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 @@ -158,7 +162,7 @@ func (w *walker) onDirEnt(dirName, baseName string, typ os.FileMode) error { err := w.fn(joined, typ) if typ == os.ModeSymlink { - if err == traverseLink { + if err == TraverseLink { // 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}) diff --git a/imports/fastwalk_dirent_fileno.go b/internal/fastwalk/fastwalk_dirent_fileno.go similarity index 94% rename from imports/fastwalk_dirent_fileno.go rename to internal/fastwalk/fastwalk_dirent_fileno.go index f1fd64949d..ccffec5adc 100644 --- a/imports/fastwalk_dirent_fileno.go +++ b/internal/fastwalk/fastwalk_dirent_fileno.go @@ -4,7 +4,7 @@ // +build freebsd openbsd netbsd -package imports +package fastwalk import "syscall" diff --git a/imports/fastwalk_dirent_ino.go b/internal/fastwalk/fastwalk_dirent_ino.go similarity index 94% rename from imports/fastwalk_dirent_ino.go rename to internal/fastwalk/fastwalk_dirent_ino.go index 9fc6de0387..ab7fbc0a9a 100644 --- a/imports/fastwalk_dirent_ino.go +++ b/internal/fastwalk/fastwalk_dirent_ino.go @@ -5,7 +5,7 @@ // +build linux darwin // +build !appengine -package imports +package fastwalk import "syscall" diff --git a/imports/fastwalk_portable.go b/internal/fastwalk/fastwalk_portable.go similarity index 97% rename from imports/fastwalk_portable.go rename to internal/fastwalk/fastwalk_portable.go index 6c2658347d..e8ea50d65a 100644 --- a/imports/fastwalk_portable.go +++ b/internal/fastwalk/fastwalk_portable.go @@ -4,7 +4,7 @@ // +build appengine !linux,!darwin,!freebsd,!openbsd,!netbsd -package imports +package fastwalk import ( "io/ioutil" diff --git a/imports/fastwalk_test.go b/internal/fastwalk/fastwalk_test.go similarity index 70% rename from imports/fastwalk_test.go rename to internal/fastwalk/fastwalk_test.go index 5b307d11ba..e9ab0d59f7 100644 --- a/imports/fastwalk_test.go +++ b/internal/fastwalk/fastwalk_test.go @@ -2,12 +2,13 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package imports +package fastwalk_test import ( "bytes" "flag" "fmt" + "io/ioutil" "os" "path/filepath" "reflect" @@ -16,6 +17,8 @@ import ( "strings" "sync" "testing" + + "golang.org/x/tools/internal/fastwalk" ) func formatFileModes(m map[string]os.FileMode) string { @@ -32,30 +35,46 @@ func formatFileModes(m map[string]os.FileMode) string { } func testFastWalk(t *testing.T, files map[string]string, callback func(path string, typ os.FileMode) error, want map[string]os.FileMode) { - testConfig{ - gopathFiles: files, - }.test(t, func(t *goimportTest) { - got := map[string]os.FileMode{} - var mu sync.Mutex - if err := fastWalk(t.gopath, func(path string, typ os.FileMode) error { - mu.Lock() - defer mu.Unlock() - if !strings.HasPrefix(path, t.gopath) { - t.Fatalf("bogus prefix on %q, expect %q", path, t.gopath) - } - key := filepath.ToSlash(strings.TrimPrefix(path, t.gopath)) - if old, dup := got[key]; dup { - t.Fatalf("callback called twice for key %q: %v -> %v", key, old, typ) - } - got[key] = typ - return callback(path, typ) - }); err != nil { - t.Fatalf("callback returned: %v", err) + tempdir, err := ioutil.TempDir("", "test-fast-walk") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempdir) + for path, contents := range files { + file := filepath.Join(tempdir, "/src", path) + if err := os.MkdirAll(filepath.Dir(file), 0755); err != nil { + t.Fatal(err) } - if !reflect.DeepEqual(got, want) { - t.Errorf("walk mismatch.\n got:\n%v\nwant:\n%v", formatFileModes(got), formatFileModes(want)) + var err error + if strings.HasPrefix(contents, "LINK:") { + err = os.Symlink(strings.TrimPrefix(contents, "LINK:"), file) + } else { + err = ioutil.WriteFile(file, []byte(contents), 0644) } - }) + if err != nil { + t.Fatal(err) + } + } + got := map[string]os.FileMode{} + var mu sync.Mutex + if err := fastwalk.Walk(tempdir, func(path string, typ os.FileMode) error { + mu.Lock() + defer mu.Unlock() + if !strings.HasPrefix(path, tempdir) { + t.Fatalf("bogus prefix on %q, expect %q", path, tempdir) + } + key := filepath.ToSlash(strings.TrimPrefix(path, tempdir)) + if old, dup := got[key]; dup { + t.Fatalf("callback called twice for key %q: %v -> %v", key, old, typ) + } + got[key] = typ + return callback(path, typ) + }); err != nil { + t.Fatalf("callback returned: %v", err) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("walk mismatch.\n got:\n%v\nwant:\n%v", formatFileModes(got), formatFileModes(want)) + } } func TestFastWalk_Basic(t *testing.T) { @@ -140,7 +159,7 @@ func TestFastWalk_TraverseSymlink(t *testing.T) { }, func(path string, typ os.FileMode) error { if typ == os.ModeSymlink { - return traverseLink + return fastwalk.TraverseLink } return nil }, @@ -163,7 +182,7 @@ var benchDir = flag.String("benchdir", runtime.GOROOT(), "The directory to scan func BenchmarkFastWalk(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { - err := fastWalk(*benchDir, func(path string, typ os.FileMode) error { return nil }) + err := fastwalk.Walk(*benchDir, func(path string, typ os.FileMode) error { return nil }) if err != nil { b.Fatal(err) } diff --git a/imports/fastwalk_unix.go b/internal/fastwalk/fastwalk_unix.go similarity index 99% rename from imports/fastwalk_unix.go rename to internal/fastwalk/fastwalk_unix.go index e0fc8b7ce6..67db6caf96 100644 --- a/imports/fastwalk_unix.go +++ b/internal/fastwalk/fastwalk_unix.go @@ -5,7 +5,7 @@ // +build linux darwin freebsd openbsd netbsd // +build !appengine -package imports +package fastwalk import ( "bytes"