refactor/importgraph: changes for vendor support
Each string in build.Package.{,Test,XTest}Imports must now be
interpreted relative to the Package.Dir. This adds a ton of
redundant I/O.
Also:
- use a counting semaphore to limit concurrent I/O calls
(Fixes golang/go#10306)
- remove obsolete call to runtime.GOMAXPROCS from the test.
Change-Id: Ic556c4cf41cce7a88c0158800c992e66f354c484
Reviewed-on: https://go-review.googlesource.com/18050
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/refactor/importgraph/graph.go b/refactor/importgraph/graph.go
index 8ad8014..fc8691d 100644
--- a/refactor/importgraph/graph.go
+++ b/refactor/importgraph/graph.go
@@ -53,9 +53,10 @@
// Build scans the specified Go workspace and builds the forward and
// reverse import dependency graphs for all its packages.
-// It also returns a mapping from import paths to errors for packages
+// It also returns a mapping from canonical import paths to errors for packages
// whose loading was not entirely successful.
// A package may appear in the graph and in the errors mapping.
+// All package paths are canonical and may contain "/vendor/".
func Build(ctxt *build.Context) (forward, reverse Graph, errors map[string]error) {
type importEdge struct {
from, to string
@@ -67,6 +68,7 @@
ch := make(chan interface{})
+ sema := make(chan int, 20) // I/O concurrency limiting semaphore
var wg sync.WaitGroup
buildutil.ForEachPackage(ctxt, func(path string, err error) {
wg.Add(1)
@@ -77,7 +79,10 @@
return
}
- bp, err := ctxt.Import(path, "", 0)
+ sema <- 1
+ bp, err := ctxt.Import(path, "", buildutil.AllowVendor)
+ <-sema
+
if err != nil {
if _, ok := err.(*build.NoGoError); ok {
// empty directory is not an error
@@ -86,15 +91,43 @@
}
// Even in error cases, Import usually returns a package.
}
+
+ // absolutize resolves an import path relative
+ // to the current package bp.
+ // The absolute form may contain "vendor".
+ // TODO(adonovan): opt: experiment with
+ // overriding the IsDir method with a caching version
+ // to avoid a huge number of redundant I/O calls.
+ absolutize := func(path string) string { return path }
+ if buildutil.AllowVendor != 0 {
+ memo := make(map[string]string)
+ absolutize = func(path string) string {
+ canon, ok := memo[path]
+ if !ok {
+ sema <- 1
+ bp2, _ := ctxt.Import(path, bp.Dir, build.FindOnly|buildutil.AllowVendor)
+ <-sema
+
+ if bp2 != nil {
+ canon = bp2.ImportPath
+ } else {
+ canon = path
+ }
+ memo[path] = canon
+ }
+ return canon
+ }
+ }
+
if bp != nil {
for _, imp := range bp.Imports {
- ch <- importEdge{path, imp}
+ ch <- importEdge{path, absolutize(imp)}
}
for _, imp := range bp.TestImports {
- ch <- importEdge{path, imp}
+ ch <- importEdge{path, absolutize(imp)}
}
for _, imp := range bp.XTestImports {
- ch <- importEdge{path, imp}
+ ch <- importEdge{path, absolutize(imp)}
}
}
}()
diff --git a/refactor/importgraph/graph_test.go b/refactor/importgraph/graph_test.go
index a486c26..595ecdb 100644
--- a/refactor/importgraph/graph_test.go
+++ b/refactor/importgraph/graph_test.go
@@ -9,11 +9,13 @@
package importgraph_test
import (
+ "fmt"
"go/build"
- "runtime"
"sort"
+ "strings"
"testing"
+ "golang.org/x/tools/go/buildutil"
"golang.org/x/tools/refactor/importgraph"
_ "crypto/hmac" // just for test, below
@@ -22,15 +24,12 @@
const this = "golang.org/x/tools/refactor/importgraph"
func TestBuild(t *testing.T) {
- saved := runtime.GOMAXPROCS(8) // Build is highly parallel
- defer runtime.GOMAXPROCS(saved)
-
forward, reverse, errors := importgraph.Build(&build.Default)
// Test direct edges.
// We throw in crypto/hmac to prove that external test files
// (such as this one) are inspected.
- for _, p := range []string{"go/build", "runtime", "testing", "crypto/hmac"} {
+ for _, p := range []string{"go/build", "testing", "crypto/hmac"} {
if !forward[this][p] {
t.Errorf("forward[importgraph][%s] not found", p)
}
@@ -40,7 +39,7 @@
}
// Test non-existent direct edges
- for _, p := range []string{"fmt", "errors", "reflect"} {
+ for _, p := range []string{"errors", "reflect"} {
if forward[this][p] {
t.Errorf("unexpected: forward[importgraph][%s] found", p)
}
@@ -49,6 +48,14 @@
}
}
+ // Test vendor packages appear under their absolute names.
+ if buildutil.AllowVendor != 0 { // hack: Go 1.6+ only
+ if !forward["net/http"]["vendor/golang.org/x/net/http2/hpack"] {
+ t.Errorf("forward[net/http] does not include vendor/golang.org/x/net/http2/hpack: %v",
+ strings.Replace(fmt.Sprint(forward["net/http"]), ":true", "", -1))
+ }
+ }
+
// Test Search is reflexive.
if !forward.Search(this)[this] {
t.Errorf("irreflexive: forward.Search(importgraph)[importgraph] not found")
