From 2aa5874490495108f9467a8613868b21872066bf Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 9 Dec 2015 14:39:26 -0800 Subject: [PATCH] cmd/doc: search the tree in breadth-first order This is a simple change to the command that should resolve problems like finding vendored packages before their non-vendored siblings. By searching in breadth-first order, we find the matching package lowest in the hierarchy, which is more likely to be correct than the deeper one, such as a vendored package, that will be found in a depth-first scan. This may be sufficient to resolve the issue, and has the merit that it is very easy to explain. I will leave the issue open for now in case my intuition is wrong. Update #12423 Change-Id: Icf69e8beb1845277203fcb7d19ffb7cca9fa41f5 Reviewed-on: https://go-review.googlesource.com/17691 Reviewed-by: Russ Cox --- src/cmd/doc/dirs.go | 99 +++++++++++++++++++++---------------------- src/cmd/go/alldocs.go | 6 ++- src/cmd/go/doc.go | 6 ++- 3 files changed, 56 insertions(+), 55 deletions(-) diff --git a/src/cmd/doc/dirs.go b/src/cmd/doc/dirs.go index e33cbc8ec7..2982eeeb10 100644 --- a/src/cmd/doc/dirs.go +++ b/src/cmd/doc/dirs.go @@ -6,6 +6,7 @@ package main import ( "go/build" + "log" "os" "path" "path/filepath" @@ -54,65 +55,61 @@ func (d *Dirs) Next() (string, bool) { // walk walks the trees in GOROOT and GOPATH. func (d *Dirs) walk() { - d.walkRoot(build.Default.GOROOT) + d.bfsWalkRoot(build.Default.GOROOT) for _, root := range splitGopath() { - d.walkRoot(root) + d.bfsWalkRoot(root) } close(d.scan) } -// walkRoot walks a single directory. Each Go source directory it finds is -// delivered on d.scan. -func (d *Dirs) walkRoot(root string) { +// bfsWalkRoot walks a single directory hierarchy in breadth-first lexical order. +// Each Go source directory it finds is delivered on d.scan. +func (d *Dirs) bfsWalkRoot(root string) { root = path.Join(root, "src") - slashDot := string(filepath.Separator) + "." - // We put a slash on the pkg so can use simple string comparison below - // yet avoid inadvertent matches, like /foobar matching bar. - visit := func(pathName string, f os.FileInfo, err error) error { - if err != nil { - return nil - } - // One package per directory. Ignore the files themselves. - if !f.IsDir() { - return nil - } - // No .git or other dot nonsense please. - if strings.Contains(pathName, slashDot) { - return filepath.SkipDir - } - // Does the directory contain any Go files? If so, it's a candidate. - if hasGoFiles(pathName) { - d.scan <- pathName - return nil - } - return nil - } - - filepath.Walk(root, visit) -} + // this is the queue of directories to examine in this pass. + this := []string{} + // next is the queue of directories to examine in the next pass. + next := []string{root} -// hasGoFiles tests whether the directory contains at least one file with ".go" -// extension -func hasGoFiles(path string) bool { - dir, err := os.Open(path) - if err != nil { - // ignore unreadable directories - return false - } - defer dir.Close() - - names, err := dir.Readdirnames(0) - if err != nil { - // ignore unreadable directories - return false - } - - for _, name := range names { - if strings.HasSuffix(name, ".go") { - return true + for len(next) > 0 { + this, next = next, this[0:0] + for _, dir := range this { + fd, err := os.Open(dir) + if err != nil { + log.Printf("error opening %s: %v", dir, err) + return // TODO? There may be entry before the error. + } + entries, err := fd.Readdir(0) + fd.Close() + if err != nil { + log.Printf("error reading %s: %v", dir, err) + return // TODO? There may be entry before the error. + } + hasGoFiles := false + for _, entry := range entries { + name := entry.Name() + // For plain files, remember if this directory contains any .go + // source files, but ignore them otherwise. + if !entry.IsDir() { + if !hasGoFiles && strings.HasSuffix(name, ".go") { + hasGoFiles = true + } + continue + } + // Entry is a directory. + // No .git or other dot nonsense please. + if strings.HasPrefix(name, ".") { + continue + } + // Remember this (fully qualified) directory for the next pass. + next = append(next, filepath.Join(dir, name)) + } + if hasGoFiles { + // It's a candidate. + d.scan <- dir + } } - } - return false + } } diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 82f848d2e2..ecacf6d3dc 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -236,8 +236,10 @@ The first item in this list matched by the argument is the one whose documentati is printed. (See the examples below.) However, if the argument starts with a capital letter it is assumed to identify a symbol or method in the current directory. -For packages, the order of scanning is determined lexically, but the GOROOT tree -is always scanned before GOPATH. +For packages, the order of scanning is determined lexically in breadth-first order. +That is, the package presented is the one that matches the search and is nearest +the root and lexically first at its level of the hierarchy. The GOROOT tree is +always scanned in its entirety before GOPATH. If there is no package specified or matched, the package in the current directory is selected, so "go doc Foo" shows the documentation for symbol Foo in diff --git a/src/cmd/go/doc.go b/src/cmd/go/doc.go index bed763679d..9b8b8dfc24 100644 --- a/src/cmd/go/doc.go +++ b/src/cmd/go/doc.go @@ -39,8 +39,10 @@ The first item in this list matched by the argument is the one whose documentati is printed. (See the examples below.) However, if the argument starts with a capital letter it is assumed to identify a symbol or method in the current directory. -For packages, the order of scanning is determined lexically, but the GOROOT tree -is always scanned before GOPATH. +For packages, the order of scanning is determined lexically in breadth-first order. +That is, the package presented is the one that matches the search and is nearest +the root and lexically first at its level of the hierarchy. The GOROOT tree is +always scanned in its entirety before GOPATH. If there is no package specified or matched, the package in the current directory is selected, so "go doc Foo" shows the documentation for symbol Foo in -- 2.48.1