From ce9bf563f5cba56e2aa5ca26718bdb8462de0f4e Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Wed, 18 Dec 2019 13:32:01 -0500 Subject: [PATCH] go/packages: fix GOPATH vendoring with overlays When GOPATH vendoring is in effect, go list on an import path doesn't work -- we have to find the right vendor directory and list that instead. Updates #36155. Change-Id: If04f5bc15498a8f195fb3eb76c5a9f2f53c91804 Reviewed-on: https://go-review.googlesource.com/c/tools/+/215903 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Michael Matloob Reviewed-by: Rebecca Stambler --- go/packages/golist.go | 12 ++++-- go/packages/golist_overlay.go | 78 ++++++++++++++++++++++++++++------- go/packages/packages_test.go | 28 +++++++++++++ 3 files changed, 101 insertions(+), 17 deletions(-) diff --git a/go/packages/golist.go b/go/packages/golist.go index f098905dfb4..3089711321f 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -86,6 +86,9 @@ type golistState struct { rootsOnce sync.Once rootDirsError error rootDirs map[string]string + + // vendorDirs caches the (non)existence of vendor directories. + vendorDirs map[string]bool } // getEnv returns Go environment variables. Only specific variables are @@ -145,8 +148,9 @@ func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) { } state := &golistState{ - cfg: cfg, - ctx: ctx, + cfg: cfg, + ctx: ctx, + vendorDirs: map[string]bool{}, } // Determine files requested in contains patterns @@ -639,7 +643,9 @@ func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer, stdout := new(bytes.Buffer) stderr := new(bytes.Buffer) goArgs := []string{verb} - goArgs = append(goArgs, cfg.BuildFlags...) + if verb != "env" { + goArgs = append(goArgs, cfg.BuildFlags...) + } goArgs = append(goArgs, args...) cmd := exec.CommandContext(state.ctx, "go", goArgs...) // On darwin the cwd gets resolved to the real path, which breaks anything that diff --git a/go/packages/golist_overlay.go b/go/packages/golist_overlay.go index fe77a11b403..c6925c87bee 100644 --- a/go/packages/golist_overlay.go +++ b/go/packages/golist_overlay.go @@ -5,6 +5,7 @@ import ( "fmt" "go/parser" "go/token" + "os" "path/filepath" "strconv" "strings" @@ -140,13 +141,14 @@ func (state *golistState) processGolistOverlay(response *responseDeduper) (modif if _, found := pkg.Imports[imp]; found { continue } - - // TODO(matloob): Handle cases when the following block isn't correct. - // These include imports of vendored packages, etc. overlayAddsImports = true id, ok := havePkgs[imp] if !ok { - id = imp + var err error + id, err = state.resolveImport(dir, imp) + if err != nil { + return nil, nil, err + } } pkg.Imports[imp] = &Package{ID: id} // Add dependencies to the non-test variant version of this package as well. @@ -156,23 +158,25 @@ func (state *golistState) processGolistOverlay(response *responseDeduper) (modif } } - // toPkgPath tries to guess the package path given the id. - // This isn't always correct -- it's certainly wrong for - // vendored packages' paths. - toPkgPath := func(id string) string { - // TODO(matloob): Handle vendor paths. - i := strings.IndexByte(id, ' ') - if i >= 0 { - return id[:i] + // toPkgPath guesses the package path given the id. + toPkgPath := func(sourceDir, id string) (string, error) { + if i := strings.IndexByte(id, ' '); i >= 0 { + return state.resolveImport(sourceDir, id[:i]) } - return id + return state.resolveImport(sourceDir, id) } // Now that new packages have been created, do another pass to determine // the new set of missing packages. for _, pkg := range response.dr.Packages { for _, imp := range pkg.Imports { - pkgPath := toPkgPath(imp.ID) + if len(pkg.GoFiles) == 0 { + return nil, nil, fmt.Errorf("cannot resolve imports for package %q with no Go files", pkg.PkgPath) + } + pkgPath, err := toPkgPath(filepath.Dir(pkg.GoFiles[0]), imp.ID) + if err != nil { + return nil, nil, err + } if _, ok := havePkgs[pkgPath]; !ok { needPkgsSet[pkgPath] = true } @@ -192,6 +196,52 @@ func (state *golistState) processGolistOverlay(response *responseDeduper) (modif return modifiedPkgs, needPkgs, err } +// resolveImport finds the the ID of a package given its import path. +// In particular, it will find the right vendored copy when in GOPATH mode. +func (state *golistState) resolveImport(sourceDir, importPath string) (string, error) { + env, err := state.getEnv() + if err != nil { + return "", err + } + if env["GOMOD"] != "" { + return importPath, nil + } + + searchDir := sourceDir + for { + vendorDir := filepath.Join(searchDir, "vendor") + exists, ok := state.vendorDirs[vendorDir] + if !ok { + info, err := os.Stat(vendorDir) + exists = err == nil && info.IsDir() + state.vendorDirs[vendorDir] = exists + } + + if exists { + vendoredPath := filepath.Join(vendorDir, importPath) + if info, err := os.Stat(vendoredPath); err == nil && info.IsDir() { + // We should probably check for .go files here, but shame on anyone who fools us. + path, ok, err := state.getPkgPath(vendoredPath) + if err != nil { + return "", err + } + if ok { + return path, nil + } + } + } + + // We know we've hit the top of the filesystem when we Dir / and get /, + // or C:\ and get C:\, etc. + next := filepath.Dir(searchDir) + if next == searchDir { + break + } + searchDir = next + } + return importPath, nil +} + func hasTestFiles(p *Package) bool { for _, f := range p.GoFiles { if strings.HasSuffix(f, "_test.go") { diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go index 4e1efb69b2b..77ca60f8c59 100644 --- a/go/packages/packages_test.go +++ b/go/packages/packages_test.go @@ -1276,6 +1276,34 @@ func main() {} } } +func TestOverlayGOPATHVendoring(t *testing.T) { + exported := packagestest.Export(t, packagestest.GOPATH, []packagestest.Module{{ + Name: "golang.org/fake", + Files: map[string]interface{}{ + "vendor/vendor.com/foo/foo.go": `package foo; const X = "hi"`, + "user/user.go": `package user`, + }, + }}) + defer exported.Cleanup() + + exported.Config.Mode = packages.LoadAllSyntax + exported.Config.Logf = t.Logf + exported.Config.Overlay = map[string][]byte{ + exported.File("golang.org/fake", "user/user.go"): []byte(`package user; import "vendor.com/foo"; var x = foo.X`), + } + initial, err := packages.Load(exported.Config, "golang.org/fake/user") + if err != nil { + t.Fatal(err) + } + user := initial[0] + if len(user.Imports) != 1 { + t.Fatal("no imports for user") + } + if user.Imports["vendor.com/foo"].Name != "foo" { + t.Errorf("failed to load vendored package foo, imports: %#v", user.Imports["vendor.com/foo"]) + } +} + func TestLoadAllSyntaxImportErrors(t *testing.T) { packagestest.TestAll(t, testLoadAllSyntaxImportErrors) }