From 0c428a56176353d52170f318e998f342b08dacd2 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 10 Dec 2015 22:34:17 -0500 Subject: [PATCH] go/build: support vendor directories in Import This fix, plus a one-line change to golang.org/x/tools/go/loader, is sufficient to let that loader package process source code using vendored packages. For example, GOPATH="" ssadump net/http # uses vendored http2 used to fail, not able to find net/http's import of the vendored copy of golang.org/x/net/http2/hpack. This CL plus the fix to loader (CL 17727) suffices to get ssadump working, as well as - I expect - most other source code processing built on golang.org/x/tools/go/loader. Fixes #12278. Change-Id: I83715e757419171159f67d49bb453636afdd91f0 Reviewed-on: https://go-review.googlesource.com/17726 Reviewed-by: Ian Lance Taylor --- src/cmd/go/pkg.go | 54 ++++++++--------------- src/cmd/go/vendor_test.go | 2 +- src/go/build/build.go | 89 ++++++++++++++++++++++++++++++++------ src/go/build/build_test.go | 27 ++++++++++++ 4 files changed, 123 insertions(+), 49 deletions(-) diff --git a/src/cmd/go/pkg.go b/src/cmd/go/pkg.go index 6c3a09a2d2..2f8799a608 100644 --- a/src/cmd/go/pkg.go +++ b/src/cmd/go/pkg.go @@ -118,7 +118,7 @@ func (p *Package) vendored(imports []string) []string { seen := make(map[string]bool) var all []string for _, path := range imports { - path, _ = vendoredImportPath(p, path) + path = vendoredImportPath(p, path) if !seen[path] { seen[path] = true all = append(all, path) @@ -256,6 +256,7 @@ func reloadPackage(arg string, stk *importStack) *Package { // The variable is obnoxiously long so that years from now when people find it in // their profiles and wonder what it does, there is some chance that a web search // might answer the question. +// There is a copy of this variable in src/go/build/build.go. Delete that one when this one goes away. var go15VendorExperiment = os.Getenv("GO15VENDOREXPERIMENT") != "0" // dirToImportPath returns the pseudo-import path we use for a package @@ -312,11 +313,14 @@ func loadImport(path, srcDir string, parent *Package, stk *importStack, importPo importPath := path origPath := path isLocal := build.IsLocalImport(path) - var vendorSearch []string if isLocal { importPath = dirToImportPath(filepath.Join(srcDir, path)) } else if mode&useVendor != 0 { - path, vendorSearch = vendoredImportPath(parent, path) + // We do our own vendor resolution, because we want to + // find out the key to use in packageCache without the + // overhead of repeated calls to buildContext.Import. + // The code is also needed in a few other places anyway. + path = vendoredImportPath(parent, path) importPath = path } @@ -343,29 +347,14 @@ func loadImport(path, srcDir string, parent *Package, stk *importStack, importPo // // TODO: After Go 1, decide when to pass build.AllowBinary here. // See issue 3268 for mistakes to avoid. - bp, err := buildContext.Import(path, srcDir, build.ImportComment) - - // If we got an error from go/build about package not found, - // it contains the directories from $GOROOT and $GOPATH that - // were searched. Add to that message the vendor directories - // that were searched. - if err != nil && len(vendorSearch) > 0 { - // NOTE(rsc): The direct text manipulation here is fairly awful, - // but it avoids defining new go/build API (an exported error type) - // late in the Go 1.5 release cycle. If this turns out to be a more general - // problem we could define a real error type when the decision can be - // considered more carefully. - text := err.Error() - if strings.Contains(text, "cannot find package \"") && strings.Contains(text, "\" in any of:\n\t") { - old := strings.SplitAfter(text, "\n") - lines := []string{old[0]} - for _, dir := range vendorSearch { - lines = append(lines, "\t"+dir+" (vendor tree)\n") - } - lines = append(lines, old[1:]...) - err = errors.New(strings.Join(lines, "")) - } + buildMode := build.ImportComment + if go15VendorExperiment && mode&useVendor != 0 && path == origPath { + // We've already searched the vendor directories and didn't find anything. + // Let Import search them again so that, if the package is not found anywhere, + // the error includes the vendor directories in the list of places considered. + buildMode |= build.AllowVendor } + bp, err := buildContext.Import(path, srcDir, buildMode) bp.ImportPath = importPath if gobin != "" { bp.BinDir = gobin @@ -411,12 +400,9 @@ func isDir(path string) bool { // If parent is x/y/z, then path might expand to x/y/z/vendor/path, x/y/vendor/path, // x/vendor/path, vendor/path, or else stay path if none of those exist. // vendoredImportPath returns the expanded path or, if no expansion is found, the original. -// If no expansion is found, vendoredImportPath also returns a list of vendor directories -// it searched along the way, to help prepare a useful error message should path turn -// out not to exist. -func vendoredImportPath(parent *Package, path string) (found string, searched []string) { +func vendoredImportPath(parent *Package, path string) (found string) { if parent == nil || parent.Root == "" || !go15VendorExperiment { - return path, nil + return path } dir := filepath.Clean(parent.Dir) root := filepath.Join(parent.Root, "src") @@ -451,14 +437,12 @@ func vendoredImportPath(parent *Package, path string) (found string, searched [] // and found c:\gopath\src\vendor\path. // We chopped \foo\bar (length 8) but the import path is "foo/bar" (length 7). // Use "vendor/path" without any prefix. - return vpath, nil + return vpath } - return parent.ImportPath[:len(parent.ImportPath)-chopped] + "/" + vpath, nil + return parent.ImportPath[:len(parent.ImportPath)-chopped] + "/" + vpath } - // Note the existence of a vendor directory in case path is not found anywhere. - searched = append(searched, targ) } - return path, searched + return path } // reusePackage reuses package p to satisfy the import at the top diff --git a/src/cmd/go/vendor_test.go b/src/cmd/go/vendor_test.go index 1e8cf9c8d2..766392fe3a 100644 --- a/src/cmd/go/vendor_test.go +++ b/src/cmd/go/vendor_test.go @@ -102,7 +102,7 @@ func TestVendorImportError(t *testing.T) { re := regexp.MustCompile(`cannot find package "notfound" in any of: .*[\\/]testdata[\\/]src[\\/]vend[\\/]x[\\/]vendor[\\/]notfound \(vendor tree\) - .*[\\/]testdata[\\/]src[\\/]vend[\\/]vendor[\\/]notfound \(vendor tree\) + .*[\\/]testdata[\\/]src[\\/]vend[\\/]vendor[\\/]notfound .*[\\/]src[\\/]notfound \(from \$GOROOT\) .*[\\/]testdata[\\/]src[\\/]notfound \(from \$GOPATH\)`) diff --git a/src/go/build/build.go b/src/go/build/build.go index 5016405ab5..580326fecf 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -110,7 +110,7 @@ func (ctxt *Context) splitPathList(s string) []string { return filepath.SplitList(s) } -// isAbsPath calls ctxt.IsAbsSPath (if not nil) or else filepath.IsAbs. +// isAbsPath calls ctxt.IsAbsPath (if not nil) or else filepath.IsAbs. func (ctxt *Context) isAbsPath(path string) bool { if f := ctxt.IsAbsPath; f != nil { return f(path) @@ -343,6 +343,19 @@ const ( // or finds conflicting comments in multiple source files. // See golang.org/s/go14customimport for more information. ImportComment + + // If AllowVendor is set, Import searches vendor directories + // that apply in the given source directory before searching + // the GOROOT and GOPATH roots. + // If an Import finds and returns a package using a vendor + // directory, the resulting ImportPath is the complete path + // to the package, including the path elements leading up + // to and including "vendor". + // For example, if Import("y", "x/subdir", AllowVendor) finds + // "x/vendor/y", the returned package's ImportPath is "x/vendor/y", + // not plain "y". + // See golang.org/s/go15vendor for more information. + AllowVendor ) // A Package describes the Go package found in a directory. @@ -474,15 +487,22 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa switch ctxt.Compiler { case "gccgo": pkgtargetroot = "pkg/gccgo_" + ctxt.GOOS + "_" + ctxt.GOARCH + suffix - dir, elem := pathpkg.Split(p.ImportPath) - pkga = pkgtargetroot + "/" + dir + "lib" + elem + ".a" case "gc": pkgtargetroot = "pkg/" + ctxt.GOOS + "_" + ctxt.GOARCH + suffix - pkga = pkgtargetroot + "/" + p.ImportPath + ".a" default: // Save error for end of function. pkgerr = fmt.Errorf("import %q: unknown compiler %q", path, ctxt.Compiler) } + setPkga := func() { + switch ctxt.Compiler { + case "gccgo": + dir, elem := pathpkg.Split(p.ImportPath) + pkga = pkgtargetroot + "/" + dir + "lib" + elem + ".a" + case "gc": + pkga = pkgtargetroot + "/" + p.ImportPath + ".a" + } + } + setPkga() binaryOnly := false if IsLocalImport(path) { @@ -543,9 +563,50 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa // tried records the location of unsuccessful package lookups var tried struct { + vendor []string goroot string gopath []string } + gopath := ctxt.gopath() + + // Vendor directories get first chance to satisfy import. + if mode&AllowVendor != 0 && srcDir != "" { + searchVendor := func(root string, isGoroot bool) bool { + sub, ok := ctxt.hasSubdir(root, srcDir) + if !ok || !strings.HasPrefix(sub, "src/") || strings.Contains(sub, "/testdata/") { + return false + } + for { + vendor := ctxt.joinPath(root, sub, "vendor") + if ctxt.isDir(vendor) { + dir := ctxt.joinPath(vendor, path) + if ctxt.isDir(dir) { + p.Dir = dir + p.ImportPath = strings.TrimPrefix(pathpkg.Join(sub, "vendor", path), "src/") + p.Goroot = isGoroot + p.Root = root + setPkga() // p.ImportPath changed + return true + } + tried.vendor = append(tried.vendor, dir) + } + i := strings.LastIndex(sub, "/") + if i < 0 { + break + } + sub = sub[:i] + } + return false + } + if searchVendor(ctxt.GOROOT, true) { + goto Found + } + for _, root := range gopath { + if searchVendor(root, false) { + goto Found + } + } + } // Determine directory from import path. if ctxt.GOROOT != "" { @@ -560,7 +621,7 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa } tried.goroot = dir } - for _, root := range ctxt.gopath() { + for _, root := range gopath { dir := ctxt.joinPath(root, "src", path) isDir := ctxt.isDir(dir) binaryOnly = !isDir && mode&AllowBinary != 0 && pkga != "" && ctxt.isFile(ctxt.joinPath(root, pkga)) @@ -574,20 +635,22 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa // package was not found var paths []string + format := "\t%s (vendor tree)" + for _, dir := range tried.vendor { + paths = append(paths, fmt.Sprintf(format, dir)) + format = "\t%s" + } if tried.goroot != "" { paths = append(paths, fmt.Sprintf("\t%s (from $GOROOT)", tried.goroot)) } else { paths = append(paths, "\t($GOROOT not set)") } - var i int - var format = "\t%s (from $GOPATH)" - for ; i < len(tried.gopath); i++ { - if i > 0 { - format = "\t%s" - } - paths = append(paths, fmt.Sprintf(format, tried.gopath[i])) + format = "\t%s (from $GOPATH)" + for _, dir := range tried.gopath { + paths = append(paths, fmt.Sprintf(format, dir)) + format = "\t%s" } - if i == 0 { + if len(tried.gopath) == 0 { paths = append(paths, "\t($GOPATH not set)") } return p, fmt.Errorf("cannot find package %q in any of:\n%s", path, strings.Join(paths, "\n")) diff --git a/src/go/build/build_test.go b/src/go/build/build_test.go index 3b7e312a07..d0a2219ebc 100644 --- a/src/go/build/build_test.go +++ b/src/go/build/build_test.go @@ -297,3 +297,30 @@ func TestShellSafety(t *testing.T) { } } } + +func TestImportVendor(t *testing.T) { + ctxt := Default + ctxt.GOPATH = "" + p, err := ctxt.Import("golang.org/x/net/http2/hpack", filepath.Join(ctxt.GOROOT, "src/net/http"), AllowVendor) + if err != nil { + t.Fatalf("cannot find vendored golang.org/x/net/http2/hpack from net/http directory: %v", err) + } + want := "vendor/golang.org/x/net/http2/hpack" + if p.ImportPath != want { + t.Fatalf("Import succeeded but found %q, want %q", p.ImportPath, want) + } +} + +func TestImportVendorFailure(t *testing.T) { + ctxt := Default + ctxt.GOPATH = "" + p, err := ctxt.Import("x.com/y/z", filepath.Join(ctxt.GOROOT, "src/net/http"), AllowVendor) + if err == nil { + t.Fatalf("found made-up package x.com/y/z in %s", p.Dir) + } + + e := err.Error() + if !strings.Contains(e, " (vendor tree)") { + t.Fatalf("error on failed import does not mention GOROOT/src/vendor directory:\n%s", e) + } +}