From 00c5fa58688ea7ffa51656ec1d88eb2c67844802 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Thu, 25 Oct 2018 18:51:23 -0400 Subject: [PATCH] go/packages: find mismatched top level packages in name= Treat the top-level package of a module specially so that the VCS working dir name doesn't have to match the package name. Generally we take the position that directory names should match packages, but for the top-level package that's a pretty strict requirement. Now we always check the top-level directory, just in case. Change-Id: I2e96751bf46736fbaef434fa9788fdc1339c37b9 Reviewed-on: https://go-review.googlesource.com/c/144817 Reviewed-by: Ian Cottrell Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot --- go/packages/golist.go | 42 ++++++++++++++++++++++------- go/packages/packages_test.go | 9 ++++--- go/packages/packagestest/modules.go | 3 ++- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/go/packages/golist.go b/go/packages/golist.go index ce4e1ed9..6f336c86 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -174,7 +174,7 @@ var modCacheRegexp = regexp.MustCompile(`(.*)@([^/\\]*)(.*)`) func runNamedQueries(cfg *Config, driver driver, addPkg func(*Package), queries []string) ([]string, error) { // Determine which directories are relevant to scan. - roots, modulesEnabled, err := roots(cfg) + roots, modRoot, err := roots(cfg) if err != nil { return nil, err } @@ -209,7 +209,26 @@ func runNamedQueries(cfg *Config, driver driver, addPkg func(*Package), queries } } } - gopathwalk.Walk(roots, add, gopathwalk.Options{ModulesEnabled: modulesEnabled}) + gopathwalk.Walk(roots, add, gopathwalk.Options{ModulesEnabled: modRoot != ""}) + + // Weird special case: the top-level package in a module will be in + // whatever directory the user checked the repository out into. It's + // more reasonable for that to not match the package name. So, if there + // are any Go files in the mod root, query it just to be safe. + if modRoot != "" { + rel, err := filepath.Rel(cfg.Dir, modRoot) + if err != nil { + panic(err) // See above. + } + + files, err := ioutil.ReadDir(modRoot) + for _, f := range files { + if strings.HasSuffix(f.Name(), ".go") { + simpleMatches = append(simpleMatches, rel) + break + } + } + } var results []string addResponse := func(r *driverResponse) { @@ -291,36 +310,39 @@ func runNamedQueries(cfg *Config, driver driver, addPkg func(*Package), queries // roots selects the appropriate paths to walk based on the passed-in configuration, // particularly the environment and the presence of a go.mod in cfg.Dir's parents. -func roots(cfg *Config) ([]gopathwalk.Root, bool, error) { +func roots(cfg *Config) ([]gopathwalk.Root, string, error) { stdout, err := invokeGo(cfg, "env", "GOROOT", "GOPATH", "GOMOD") if err != nil { - return nil, false, err + return nil, "", err } fields := strings.Split(stdout.String(), "\n") if len(fields) != 4 || len(fields[3]) != 0 { - return nil, false, fmt.Errorf("go env returned unexpected output: %q", stdout.String()) + return nil, "", fmt.Errorf("go env returned unexpected output: %q", stdout.String()) } goroot, gopath, gomod := fields[0], filepath.SplitList(fields[1]), fields[2] - modsEnabled := gomod != "" + var modDir string + if gomod != "" { + modDir = filepath.Dir(gomod) + } var roots []gopathwalk.Root // Always add GOROOT. roots = append(roots, gopathwalk.Root{filepath.Join(goroot, "/src"), gopathwalk.RootGOROOT}) // If modules are enabled, scan the module dir. - if modsEnabled { - roots = append(roots, gopathwalk.Root{filepath.Dir(gomod), gopathwalk.RootCurrentModule}) + if modDir != "" { + roots = append(roots, gopathwalk.Root{modDir, gopathwalk.RootCurrentModule}) } // Add either GOPATH/src or GOPATH/pkg/mod, depending on module mode. for _, p := range gopath { - if modsEnabled { + if modDir != "" { roots = append(roots, gopathwalk.Root{filepath.Join(p, "/pkg/mod"), gopathwalk.RootModuleCache}) } else { roots = append(roots, gopathwalk.Root{filepath.Join(p, "/src"), gopathwalk.RootGOPATH}) } } - return roots, modsEnabled, nil + return roots, modDir, nil } // These functions were copied from goimports. See further documentation there. diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go index 8fc514c7..6a3eaecb 100644 --- a/go/packages/packages_test.go +++ b/go/packages/packages_test.go @@ -1146,10 +1146,13 @@ func TestName_Modules(t *testing.T) { t.Skip("pre-modules version of Go") } + // Test the top-level package case described in runNamedQueries. + // Note that overriding GOPATH below prevents Export from + // creating more than one module. exported := packagestest.Export(t, packagestest.Modules, []packagestest.Module{{ - Name: "golang.org/fake", + Name: "golang.org/pkg", Files: map[string]interface{}{ - "pkg/pkg.go": `package pkg`, + "pkg.go": `package pkg`, }}}) defer exported.Cleanup() @@ -1171,7 +1174,7 @@ func TestName_Modules(t *testing.T) { wantGraph := ` * github.com/heschik/tools-testrepo/pkg * github.com/heschik/tools-testrepo/v2/pkg -* golang.org/fake/pkg +* golang.org/pkg `[1:] if graph != wantGraph { t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph) diff --git a/go/packages/packagestest/modules.go b/go/packages/packagestest/modules.go index 8897a531..f928831b 100644 --- a/go/packages/packagestest/modules.go +++ b/go/packages/packagestest/modules.go @@ -8,12 +8,13 @@ import ( "archive/zip" "bytes" "fmt" - "golang.org/x/tools/go/packages" "io/ioutil" "os" "os/exec" "path" "path/filepath" + + "golang.org/x/tools/go/packages" ) // Modules is the exporter that produces module layouts.