qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-10-01 03:18:33 -06:00
Code Releases Activity

go/packages: handle case when go list returns absolute ImportPath

Browse Source 
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>
This commit is contained in:
Michael Matloob 2019-08-02 16:21:37 -04:00
parent e1fc249b66
commit c5a2fd39b7
3 changed files with 63 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
go/packages/golist.go
View File

@ -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 {

11
go/packages/golist_overlay.go
View File

@ -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 {

6
go/packages/packages_test.go
View File

@ -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()