diff --git a/go/buildutil/allpackages.go b/go/buildutil/allpackages.go index c95db42c35..0f909eeb20 100644 --- a/go/buildutil/allpackages.go +++ b/go/buildutil/allpackages.go @@ -30,11 +30,8 @@ import ( // func AllPackages(ctxt *build.Context) []string { var list []string - var mu sync.Mutex ForEachPackage(ctxt, func(pkg string, _ error) { - mu.Lock() list = append(list, pkg) - mu.Unlock() }) sort.Strings(list) return list @@ -47,27 +44,42 @@ func AllPackages(ctxt *build.Context) []string { // If the package directory exists but could not be read, the second // argument to the found function provides the error. // -// The found function and the build.Context file system interface -// accessors must be concurrency safe. +// All I/O is done via the build.Context file system interface, +// which must be concurrency-safe. // func ForEachPackage(ctxt *build.Context, found func(importPath string, err error)) { // We use a counting semaphore to limit // the number of parallel calls to ReadDir. sema := make(chan bool, 20) + ch := make(chan item) + var wg sync.WaitGroup for _, root := range ctxt.SrcDirs() { root := root wg.Add(1) go func() { - allPackages(ctxt, sema, root, found) + allPackages(ctxt, sema, root, ch) wg.Done() }() } - wg.Wait() + go func() { + wg.Wait() + close(ch) + }() + + // All calls to found occur in the caller's goroutine. + for i := range ch { + found(i.importPath, i.err) + } } -func allPackages(ctxt *build.Context, sema chan bool, root string, found func(string, error)) { +type item struct { + importPath string + err error // (optional) +} + +func allPackages(ctxt *build.Context, sema chan bool, root string, ch chan<- item) { root = filepath.Clean(root) + string(os.PathSeparator) var wg sync.WaitGroup @@ -92,7 +104,7 @@ func allPackages(ctxt *build.Context, sema chan bool, root string, found func(st files, err := ReadDir(ctxt, dir) <-sema if pkg != "" || err != nil { - found(pkg, err) + ch <- item{pkg, err} } for _, fi := range files { fi := fi diff --git a/refactor/rename/mvpkg.go b/refactor/rename/mvpkg.go index 4d922f76fd..bf8edd6f5a 100644 --- a/refactor/rename/mvpkg.go +++ b/refactor/rename/mvpkg.go @@ -26,7 +26,6 @@ import ( "runtime" "strconv" "strings" - "sync" "text/template" "golang.org/x/tools/go/buildutil" @@ -133,13 +132,12 @@ func srcDir(ctxt *build.Context, pkg string) (string, error) { // subpackages returns the set of packages in the given srcDir whose // import paths start with dir. func subpackages(ctxt *build.Context, srcDir string, dir string) map[string]bool { - var mu sync.Mutex subs := map[string]bool{dir: true} // Find all packages under srcDir whose import paths start with dir. buildutil.ForEachPackage(ctxt, func(pkg string, err error) { if err != nil { - log.Fatalf("unexpected error in ForEackPackage: %v", err) + log.Fatalf("unexpected error in ForEachPackage: %v", err) } if !strings.HasPrefix(pkg, path.Join(dir, "")) { @@ -157,9 +155,7 @@ func subpackages(ctxt *build.Context, srcDir string, dir string) map[string]bool return } - mu.Lock() subs[pkg] = true - mu.Unlock() }) return subs diff --git a/refactor/rename/mvpkg_test.go b/refactor/rename/mvpkg_test.go index b9f29d6b6a..3c915b44a2 100644 --- a/refactor/rename/mvpkg_test.go +++ b/refactor/rename/mvpkg_test.go @@ -15,7 +15,6 @@ import ( "path/filepath" "regexp" "strings" - "sync" "testing" "golang.org/x/tools/go/buildutil" @@ -212,7 +211,6 @@ var _ a.T for _, test := range tests { ctxt := test.ctxt - var mu sync.Mutex got := make(map[string]string) // Populate got with starting file set. rewriteFile and moveDirectory // will mutate got to produce resulting file set. @@ -225,19 +223,17 @@ var _ a.T return } f, err := ctxt.OpenFile(path) - defer f.Close() if err != nil { t.Errorf("unexpected error opening file: %s", err) return } bytes, err := ioutil.ReadAll(f) + f.Close() if err != nil { t.Errorf("unexpected error reading file: %s", err) return } - mu.Lock() got[path] = string(bytes) - defer mu.Unlock() }) rewriteFile = func(fset *token.FileSet, f *ast.File, orig string) error { var out bytes.Buffer