1
0
mirror of https://github.com/golang/go synced 2024-11-18 11:14:39 -07:00

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>
This commit is contained in:
Alan Donovan 2015-12-18 14:11:09 -05:00 committed by Alan Donovan
parent 27534aa951
commit fa833fdef5
2 changed files with 51 additions and 11 deletions

View File

@ -53,9 +53,10 @@ func (g Graph) Search(roots ...string) map[string]bool {
// Build scans the specified Go workspace and builds the forward and // Build scans the specified Go workspace and builds the forward and
// reverse import dependency graphs for all its packages. // 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. // whose loading was not entirely successful.
// A package may appear in the graph and in the errors mapping. // 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) { func Build(ctxt *build.Context) (forward, reverse Graph, errors map[string]error) {
type importEdge struct { type importEdge struct {
from, to string from, to string
@ -67,6 +68,7 @@ func Build(ctxt *build.Context) (forward, reverse Graph, errors map[string]error
ch := make(chan interface{}) ch := make(chan interface{})
sema := make(chan int, 20) // I/O concurrency limiting semaphore
var wg sync.WaitGroup var wg sync.WaitGroup
buildutil.ForEachPackage(ctxt, func(path string, err error) { buildutil.ForEachPackage(ctxt, func(path string, err error) {
wg.Add(1) wg.Add(1)
@ -77,7 +79,10 @@ func Build(ctxt *build.Context) (forward, reverse Graph, errors map[string]error
return return
} }
bp, err := ctxt.Import(path, "", 0) sema <- 1
bp, err := ctxt.Import(path, "", buildutil.AllowVendor)
<-sema
if err != nil { if err != nil {
if _, ok := err.(*build.NoGoError); ok { if _, ok := err.(*build.NoGoError); ok {
// empty directory is not an error // empty directory is not an error
@ -86,15 +91,43 @@ func Build(ctxt *build.Context) (forward, reverse Graph, errors map[string]error
} }
// Even in error cases, Import usually returns a package. // 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 { if bp != nil {
for _, imp := range bp.Imports { for _, imp := range bp.Imports {
ch <- importEdge{path, imp} ch <- importEdge{path, absolutize(imp)}
} }
for _, imp := range bp.TestImports { for _, imp := range bp.TestImports {
ch <- importEdge{path, imp} ch <- importEdge{path, absolutize(imp)}
} }
for _, imp := range bp.XTestImports { for _, imp := range bp.XTestImports {
ch <- importEdge{path, imp} ch <- importEdge{path, absolutize(imp)}
} }
} }
}() }()

View File

@ -9,11 +9,13 @@
package importgraph_test package importgraph_test
import ( import (
"fmt"
"go/build" "go/build"
"runtime"
"sort" "sort"
"strings"
"testing" "testing"
"golang.org/x/tools/go/buildutil"
"golang.org/x/tools/refactor/importgraph" "golang.org/x/tools/refactor/importgraph"
_ "crypto/hmac" // just for test, below _ "crypto/hmac" // just for test, below
@ -22,15 +24,12 @@ import (
const this = "golang.org/x/tools/refactor/importgraph" const this = "golang.org/x/tools/refactor/importgraph"
func TestBuild(t *testing.T) { func TestBuild(t *testing.T) {
saved := runtime.GOMAXPROCS(8) // Build is highly parallel
defer runtime.GOMAXPROCS(saved)
forward, reverse, errors := importgraph.Build(&build.Default) forward, reverse, errors := importgraph.Build(&build.Default)
// Test direct edges. // Test direct edges.
// We throw in crypto/hmac to prove that external test files // We throw in crypto/hmac to prove that external test files
// (such as this one) are inspected. // (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] { if !forward[this][p] {
t.Errorf("forward[importgraph][%s] not found", p) t.Errorf("forward[importgraph][%s] not found", p)
} }
@ -40,7 +39,7 @@ func TestBuild(t *testing.T) {
} }
// Test non-existent direct edges // Test non-existent direct edges
for _, p := range []string{"fmt", "errors", "reflect"} { for _, p := range []string{"errors", "reflect"} {
if forward[this][p] { if forward[this][p] {
t.Errorf("unexpected: forward[importgraph][%s] found", p) t.Errorf("unexpected: forward[importgraph][%s] found", p)
} }
@ -49,6 +48,14 @@ func TestBuild(t *testing.T) {
} }
} }
// 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. // Test Search is reflexive.
if !forward.Search(this)[this] { if !forward.Search(this)[this] {
t.Errorf("irreflexive: forward.Search(importgraph)[importgraph] not found") t.Errorf("irreflexive: forward.Search(importgraph)[importgraph] not found")