go/packages: handle case when go list returns absolute ImportPath
go list -e, when given an absolute path, will find the package contained at
that directory. But when no package exists there, it will return a fake package
with an error and the ImportPath set to the absolute path provided to go list.
This change modifies the go list driver to convert that absolute path to what
its package path would be if it's contained in a known module or GOPATH entry.
This will allow the package to be properly "reclaimed" when overlays are
processed.
Fixes golang/go#33157
Change-Id: I5ac032879884f52edbe45e00ed3949d84a71715e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/188764
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/go/packages/golist.go b/go/packages/golist.go
index 44df821..ca1bb6f 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -13,6 +13,7 @@
"log"
"os"
"os/exec"
+ "path"
"path/filepath"
"reflect"
"regexp"
@@ -86,6 +87,23 @@
}()
}
+ // start fetching rootDirs
+ var rootDirs map[string]string
+ var rootDirsReady = make(chan struct{})
+ go func() {
+ rootDirs = determineRootDirs(cfg)
+ close(rootDirsReady)
+ }()
+ getRootDirs := func() map[string]string {
+ <-rootDirsReady
+ return rootDirs
+ }
+
+ // always pass getRootDirs to golistDriver
+ golistDriver := func(cfg *Config, patterns ...string) (*driverResponse, error) {
+ return golistDriver(cfg, getRootDirs, patterns...)
+ }
+
// Determine files requested in contains patterns
var containFiles []string
var packagesNamed []string
@@ -147,7 +165,7 @@
var containsCandidates []string
if len(containFiles) != 0 {
- if err := runContainsQueries(cfg, golistDriver, response, containFiles); err != nil {
+ if err := runContainsQueries(cfg, golistDriver, response, containFiles, getRootDirs); err != nil {
return nil, err
}
}
@@ -158,7 +176,7 @@
}
}
- modifiedPkgs, needPkgs, err := processGolistOverlay(cfg, response)
+ modifiedPkgs, needPkgs, err := processGolistOverlay(cfg, response, getRootDirs)
if err != nil {
return nil, err
}
@@ -166,7 +184,7 @@
containsCandidates = append(containsCandidates, modifiedPkgs...)
containsCandidates = append(containsCandidates, needPkgs...)
}
- if err := addNeededOverlayPackages(cfg, golistDriver, response, needPkgs); err != nil {
+ if err := addNeededOverlayPackages(cfg, golistDriver, response, needPkgs, getRootDirs); err != nil {
return nil, err
}
// Check candidate packages for containFiles.
@@ -198,7 +216,7 @@
return response.dr, nil
}
-func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDeduper, pkgs []string) error {
+func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDeduper, pkgs []string, getRootDirs func() map[string]string) error {
if len(pkgs) == 0 {
return nil
}
@@ -209,17 +227,17 @@
for _, pkg := range dr.Packages {
response.addPackage(pkg)
}
- _, needPkgs, err := processGolistOverlay(cfg, response)
+ _, needPkgs, err := processGolistOverlay(cfg, response, getRootDirs)
if err != nil {
return err
}
- if err := addNeededOverlayPackages(cfg, driver, response, needPkgs); err != nil {
+ if err := addNeededOverlayPackages(cfg, driver, response, needPkgs, getRootDirs); err != nil {
return err
}
return nil
}
-func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, queries []string) error {
+func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, queries []string, rootDirs func() map[string]string) error {
for _, query := range queries {
// TODO(matloob): Do only one query per directory.
fdir := filepath.Dir(query)
@@ -567,7 +585,7 @@
// golistDriver uses the "go list" command to expand the pattern
// words and return metadata for the specified packages. dir may be
// "" and env may be nil, as per os/exec.Command.
-func golistDriver(cfg *Config, words ...string) (*driverResponse, error) {
+func golistDriver(cfg *Config, rootsDirs func() map[string]string, words ...string) (*driverResponse, error) {
// go list uses the following identifiers in ImportPath and Imports:
//
// "p" -- importable package or main (command)
@@ -608,6 +626,20 @@
return nil, fmt.Errorf("package missing import path: %+v", p)
}
+ // Work around https://golang.org/issue/33157:
+ // go list -e, when given an absolute path, will find the package contained at
+ // that directory. But when no package exists there, it will return a fake package
+ // with an error and the ImportPath set to the absolute path provided to go list.
+ // Try toto convert that absolute path to what its package path would be if it's
+ // contained in a known module or GOPATH entry. This will allow the package to be
+ // properly "reclaimed" when overlays are processed.
+ if filepath.IsAbs(p.ImportPath) && p.Error != nil {
+ pkgPath, ok := getPkgPath(p.ImportPath, rootsDirs)
+ if ok {
+ p.ImportPath = pkgPath
+ }
+ }
+
if old, found := seen[p.ImportPath]; found {
if !reflect.DeepEqual(p, old) {
return nil, fmt.Errorf("internal error: go list gives conflicting information for package %v", p.ImportPath)
@@ -711,6 +743,27 @@
return &response, nil
}
+// getPkgPath finds the package path of a directory if it's relative to a root directory.
+func getPkgPath(dir string, rootDirs func() map[string]string) (string, bool) {
+ for rdir, rpath := range rootDirs() {
+ // TODO(matloob): This doesn't properly handle symlinks.
+ r, err := filepath.Rel(rdir, dir)
+ if err != nil {
+ continue
+ }
+ if rpath != "" {
+ // We choose only ore root even though the directory even it can belong in multiple modules
+ // or GOPATH entries. This is okay because we only need to work with absolute dirs when a
+ // file is missing from disk, for instance when gopls calls go/packages in an overlay.
+ // Once the file is saved, gopls, or the next invocation of the tool will get the correct
+ // result straight from golist.
+ // TODO(matloob): Implement module tiebreaking?
+ return path.Join(rpath, filepath.ToSlash(r)), true
+ }
+ }
+ return "", false
+}
+
// absJoin absolutizes and flattens the lists of files.
func absJoin(dir string, fileses ...[]string) (res []string) {
for _, files := range fileses {
diff --git a/go/packages/golist_overlay.go b/go/packages/golist_overlay.go
index ffc7a36..298308c 100644
--- a/go/packages/golist_overlay.go
+++ b/go/packages/golist_overlay.go
@@ -10,7 +10,6 @@
"path/filepath"
"strconv"
"strings"
- "sync"
)
// processGolistOverlay provides rudimentary support for adding
@@ -18,7 +17,7 @@
// sometimes incorrect.
// TODO(matloob): Handle unsupported cases, including the following:
// - determining the correct package to add given a new import path
-func processGolistOverlay(cfg *Config, response *responseDeduper) (modifiedPkgs, needPkgs []string, err error) {
+func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func() map[string]string) (modifiedPkgs, needPkgs []string, err error) {
havePkgs := make(map[string]string) // importPath -> non-test package ID
needPkgsSet := make(map[string]bool)
modifiedPkgsSet := make(map[string]bool)
@@ -29,9 +28,6 @@
havePkgs[pkg.PkgPath] = pkg.ID
}
- var rootDirs map[string]string
- var onceGetRootDirs sync.Once
-
// If no new imports are added, it is safe to avoid loading any needPkgs.
// Otherwise, it's hard to tell which package is actually being loaded
// (due to vendoring) and whether any modified package will show up
@@ -76,13 +72,10 @@
}
// The overlay could have included an entirely new package.
if pkg == nil {
- onceGetRootDirs.Do(func() {
- rootDirs = determineRootDirs(cfg)
- })
// Try to find the module or gopath dir the file is contained in.
// Then for modules, add the module opath to the beginning.
var pkgPath string
- for rdir, rpath := range rootDirs {
+ for rdir, rpath := range rootDirs() {
// TODO(matloob): This doesn't properly handle symlinks.
r, err := filepath.Rel(rdir, dir)
if err != nil {
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index c9d0792..a736d1e 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -930,12 +930,6 @@
"b/b.go": `package b; import "golang.org/fake/c"; const B = "b" + c.C`,
"c/c.go": `package c; const C = "c"`,
"d/d.go": `package d; const D = "d"`,
-
- // TODO: Remove these temporary files when golang.org/issue/33157 is resolved.
- filepath.Join("e/e_temp.go"): ``,
- filepath.Join("f/f_temp.go"): ``,
- filepath.Join("g/g_temp.go"): ``,
- filepath.Join("h/h_temp.go"): ``,
}}})
defer exported.Cleanup()
