diff --git a/go/packages/golist.go b/go/packages/golist.go index 44df8210d7..ca1bb6fc66 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -13,6 +13,7 @@ import ( "log" "os" "os/exec" + "path" "path/filepath" "reflect" "regexp" @@ -86,6 +87,23 @@ func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) { }() } + // 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 @@ extractQueries: 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 @@ extractQueries: } } - modifiedPkgs, needPkgs, err := processGolistOverlay(cfg, response) + modifiedPkgs, needPkgs, err := processGolistOverlay(cfg, response, getRootDirs) if err != nil { return nil, err } @@ -166,7 +184,7 @@ extractQueries: 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 @@ extractQueries: 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 @@ func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDedu 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 @@ func otherFiles(p *jsonPackage) [][]string { // 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 @@ func golistDriver(cfg *Config, words ...string) (*driverResponse, error) { 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 @@ func golistDriver(cfg *Config, words ...string) (*driverResponse, error) { 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 ffc7a367f6..298308ca7a 100644 --- a/go/packages/golist_overlay.go +++ b/go/packages/golist_overlay.go @@ -10,7 +10,6 @@ import ( "path/filepath" "strconv" "strings" - "sync" ) // processGolistOverlay provides rudimentary support for adding @@ -18,7 +17,7 @@ import ( // 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 @@ func processGolistOverlay(cfg *Config, response *responseDeduper) (modifiedPkgs, 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 @@ func processGolistOverlay(cfg *Config, response *responseDeduper) (modifiedPkgs, } // 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 c9d07926bd..a736d1e2a5 100644 --- a/go/packages/packages_test.go +++ b/go/packages/packages_test.go @@ -930,12 +930,6 @@ func testNewPackagesInOverlay(t *testing.T, exporter packagestest.Exporter) { "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()