Skip to content

Commit

Permalink
cmd/go: distinguish patterns from the results of matching them
Browse files Browse the repository at this point in the history 
To date the go command has always just treated the command line
package patterns as a []string, expanded by pattern matching into
another []string. As a result, the code is not always clear about
whether a particular []string contains patterns or results.
A few different important bugs are caused by not keeping
this distinction clear enough. This CL sets us up well for fixing those,
by introducing an explicit search.Match struct holding the
results of matching a single pattern.

The added clarity here also makes it clear how to avoid duplicate
warnings about unmatched packages.

Fixes #26925. (Test in followup CL.)

Change-Id: Ic2f0606f7ab8b3734a40e22d3cb1e6f58d031061
Reviewed-on: https://go-review.googlesource.com/129058
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
  • Loading branch information
@rsc
rsc committed Aug 17, 2018
1 parent 08d10f9 commit d46587c
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 204 deletions.
45 changes: 15 additions & 30 deletions src/cmd/go/internal/get/get.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,8 @@ func runGet(cmd *base.Command, args []string) {
if *getT {
mode |= load.GetTestDeps
}
args = downloadPaths(args)
for _, arg := range args {
download(arg, nil, &stk, mode)
for _, pkg := range downloadPaths(args) {
download(pkg, nil, &stk, mode)
}
base.ExitIfErrors()

Expand All @@ -184,8 +183,7 @@ func runGet(cmd *base.Command, args []string) {
// This leads to duplicated loads of the standard packages.
load.ClearCmdCache()

args = load.ImportPaths(args)
load.PackagesForBuild(args)
pkgs := load.PackagesForBuild(args)

// Phase 3. Install.
if *getD {
Expand All @@ -195,42 +193,29 @@ func runGet(cmd *base.Command, args []string) {
return
}

work.InstallPackages(args)
work.InstallPackages(args, pkgs)
}

// downloadPaths prepares the list of paths to pass to download.
// It expands ... patterns that can be expanded. If there is no match
// for a particular pattern, downloadPaths leaves it in the result list,
// in the hope that we can figure out the repository from the
// initial ...-free prefix.
func downloadPaths(args []string) []string {
for _, arg := range args {
func downloadPaths(patterns []string) []string {
for _, arg := range patterns {
if strings.Contains(arg, "@") {
base.Fatalf("go: cannot use path@version syntax in GOPATH mode")
}
}

args = load.ImportPathsForGoGet(args)
var out []string
for _, a := range args {
if strings.Contains(a, "...") {
var expand []string
// Use matchPackagesInFS to avoid printing
// warnings. They will be printed by the
// eventual call to importPaths instead.
if build.IsLocalImport(a) {
expand = search.MatchPackagesInFS(a)
} else {
expand = search.MatchPackages(a)
}
if len(expand) > 0 {
out = append(out, expand...)
continue
}
var pkgs []string
for _, m := range search.ImportPathsQuiet(patterns) {
if len(m.Pkgs) == 0 && strings.Contains(m.Pattern, "...") {
pkgs = append(pkgs, m.Pattern)
} else {
pkgs = append(pkgs, m.Pkgs...)
}
out = append(out, a)
}
return out
return pkgs
}

// downloadCache records the import paths we have already
Expand Down Expand Up @@ -311,9 +296,9 @@ func download(arg string, parent *load.Package, stk *load.ImportStack, mode int)
// for p has been replaced in the package cache.
if wildcardOkay && strings.Contains(arg, "...") {
if build.IsLocalImport(arg) {
args = search.MatchPackagesInFS(arg)
args = search.MatchPackagesInFS(arg).Pkgs
} else {
args = search.MatchPackages(arg)
args = search.MatchPackages(arg).Pkgs
}
isWildcard = true
}
Expand Down
45 changes: 16 additions & 29 deletions src/cmd/go/internal/load/pkg.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var (
ModBinDir func() string // return effective bin directory
ModLookup func(path string) (dir, realPath string, err error) // lookup effective meaning of import
ModPackageModuleInfo func(path string) *modinfo.ModulePublic // return module info for Package struct
ModImportPaths func(args []string) []string // expand import paths
ModImportPaths func(args []string) []*search.Match // expand import paths
ModPackageBuildInfo func(main string, deps []string) string // return module info to embed in binary
ModInfoProg func(info string) []byte // wrap module info in .go code for binary
ModImportFromFiles func([]string) // update go.mod to add modules for imports in these files
Expand Down Expand Up @@ -1829,54 +1829,41 @@ func Packages(args []string) []*Package {
// *Package for every argument, even the ones that
// cannot be loaded at all.
// The packages that fail to load will have p.Error != nil.
func PackagesAndErrors(args []string) []*Package {
if len(args) > 0 && strings.HasSuffix(args[0], ".go") {
return []*Package{GoFilesPackage(args)}
func PackagesAndErrors(patterns []string) []*Package {
if len(patterns) > 0 && strings.HasSuffix(patterns[0], ".go") {
return []*Package{GoFilesPackage(patterns)}
}

args = ImportPaths(args)
matches := ImportPaths(patterns)
var (
pkgs []*Package
stk ImportStack
seenArg = make(map[string]bool)
seenPkg = make(map[*Package]bool)
)

for _, arg := range args {
if seenArg[arg] {
continue
}
seenArg[arg] = true
pkg := LoadPackage(arg, &stk)
if seenPkg[pkg] {
continue
for _, m := range matches {
for _, pkg := range m.Pkgs {
p := LoadPackage(pkg, &stk)
if seenPkg[p] {
continue
}
seenPkg[p] = true
pkgs = append(pkgs, p)
}
seenPkg[pkg] = true
pkgs = append(pkgs, pkg)
}

return pkgs
}

func ImportPaths(args []string) []string {
if cmdlineMatchers == nil {
SetCmdlinePatterns(search.CleanImportPaths(args))
}
func ImportPaths(args []string) []*search.Match {
if ModInit(); cfg.ModulesEnabled {
return ModImportPaths(args)
}
return search.ImportPaths(args)
}

func ImportPathsForGoGet(args []string) []string {
if cmdlineMatchers == nil {
SetCmdlinePatterns(search.CleanImportPaths(args))
}
return search.ImportPathsNoDotExpansion(args)
}

// packagesForBuild is like 'packages' but fails if any of
// the packages or their dependencies have errors
// PackagesForBuild is like Packages but exits
// if any of the packages or their dependencies have errors
// (cannot be built).
func PackagesForBuild(args []string) []*Package {
pkgs := PackagesAndErrors(args)
Expand Down
24 changes: 13 additions & 11 deletions src/cmd/go/internal/modcmd/why.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,20 +100,22 @@ func runWhy(cmd *base.Command, args []string) {
sep = "\n"
}
} else {
pkgs := modload.ImportPaths(args) // resolve to packages
loadALL() // rebuild graph, from main module (not from named packages)
matches := modload.ImportPaths(args) // resolve to packages
loadALL() // rebuild graph, from main module (not from named packages)
sep := ""
for _, path := range pkgs {
why := modload.Why(path)
if why == "" {
vendoring := ""
if *whyVendor {
vendoring = " to vendor"
for _, m := range matches {
for _, path := range m.Pkgs {
why := modload.Why(path)
if why == "" {
vendoring := ""
if *whyVendor {
vendoring = " to vendor"
}
why = "(main module does not need" + vendoring + " package " + path + ")\n"
}
why = "(main module does not need" + vendoring + " package " + path + ")\n"
fmt.Printf("%s# %s\n%s", sep, path, why)
sep = "\n"
}
fmt.Printf("%s# %s\n%s", sep, path, why)
sep = "\n"
}
}
}
13 changes: 7 additions & 6 deletions src/cmd/go/internal/modget/get.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func runGet(cmd *base.Command, args []string) {
// and a list of install targets (for the "go install" at the end).
var tasks []*task
var install []string
for _, arg := range search.CleanImportPaths(args) {
for _, arg := range search.CleanPatterns(args) {
// Argument is module query path@vers, or else path with implicit @latest.
path := arg
vers := ""
Expand Down Expand Up @@ -519,8 +519,9 @@ func runGet(cmd *base.Command, args []string) {
// Note that 'go get -u' without any arguments results in len(install) == 1:
// search.CleanImportPaths returns "." for empty args.
work.BuildInit()
var pkgs []string
for _, p := range load.PackagesAndErrors(install) {
pkgs := load.PackagesAndErrors(install)
var todo []*load.Package
for _, p := range pkgs {
// Ignore "no Go source files" errors for 'go get' operations on modules.
if p.Error != nil {
if len(args) == 0 && getU != "" && strings.HasPrefix(p.Error.Err, "no Go files") {
Expand All @@ -534,14 +535,14 @@ func runGet(cmd *base.Command, args []string) {
continue
}
}
pkgs = append(pkgs, p.ImportPath)
todo = append(todo, p)
}

// If -d was specified, we're done after the download: no build.
// (The load.PackagesAndErrors is what did the download
// of the named packages and their dependencies.)
if len(pkgs) > 0 && !*getD {
work.InstallPackages(pkgs)
if len(todo) > 0 && !*getD {
work.InstallPackages(install, todo)
}
}
}
Expand Down
Loading

0 comments on commit d46587c

Please sign in to comment.