Skip to content

Commit

Permalink
cmd/go/internal/search: consolidate package-pattern predicates into M…
Browse files Browse the repository at this point in the history
…atch methods

This change consolidates predicates currently scattered throughout
various parts of the package and module loader into methods on the
search.Match type.

That not only makes them more concise, but also encourages
consistency, both in the code and in reasoning about the kinds of
patterns that need to be handled. (For example, the IsLocal predicate
was previously two different calls, either of which could be easily
forgotten at a given call site.)

Factored out from CL 185344 and CL 185345.

Updates #32917

Change-Id: Ifa450ffaf6101f673e0ed69ced001a487d6f9335
Reviewed-on: https://go-review.googlesource.com/c/go/+/221458
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
  • Loading branch information
Bryan C. Mills committed Feb 28, 2020
1 parent 156c607 commit d11e1f9
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 90 deletions.
13 changes: 6 additions & 7 deletions src/cmd/go/internal/get/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package get

import (
"fmt"
"go/build"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -198,8 +197,8 @@ func downloadPaths(patterns []string) []string {
}
var pkgs []string
for _, m := range search.ImportPathsQuiet(patterns) {
if len(m.Pkgs) == 0 && strings.Contains(m.Pattern, "...") {
pkgs = append(pkgs, m.Pattern)
if len(m.Pkgs) == 0 && strings.Contains(m.Pattern(), "...") {
pkgs = append(pkgs, m.Pattern())
} else {
pkgs = append(pkgs, m.Pkgs...)
}
Expand Down Expand Up @@ -285,11 +284,11 @@ func download(arg string, parent *load.Package, stk *load.ImportStack, mode int)
// We delay this until after reloadPackage so that the old entry
// for p has been replaced in the package cache.
if wildcardOkay && strings.Contains(arg, "...") {
var match *search.Match
if build.IsLocalImport(arg) {
match = search.MatchPackagesInFS(arg)
match := search.NewMatch(arg)
if match.IsLocal() {
match.MatchPackagesInFS()
} else {
match = search.MatchPackages(arg)
match.MatchPackages()
}
args = match.Pkgs
for _, err := range match.Errs {
Expand Down
8 changes: 4 additions & 4 deletions src/cmd/go/internal/load/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -2074,13 +2074,13 @@ func PackagesAndErrors(patterns []string) []*Package {
for _, m := range matches {
for _, pkg := range m.Pkgs {
if pkg == "" {
panic(fmt.Sprintf("ImportPaths returned empty package for pattern %s", m.Pattern))
panic(fmt.Sprintf("ImportPaths returned empty package for pattern %s", m.Pattern()))
}
p := loadImport(pre, pkg, base.Cwd, nil, &stk, nil, 0)
p.Match = append(p.Match, m.Pattern)
p.Match = append(p.Match, m.Pattern())
p.Internal.CmdlinePkg = true
if m.Literal {
// Note: do not set = m.Literal unconditionally
if m.IsLiteral() {
// Note: do not set = m.IsLiteral unconditionally
// because maybe we'll see p matching both
// a literal and also a non-literal pattern.
p.Internal.CmdlinePkgLiteral = true
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modget/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func runGet(cmd *base.Command, args []string) {
// If the pattern did not match any packages, look up a new module.
// If the pattern doesn't match anything on the last iteration,
// we'll print a warning after the outer loop.
if !search.IsRelativePath(arg.path) && !match.Literal && arg.path != "all" {
if !search.IsRelativePath(arg.path) && !match.IsLiteral() && arg.path != "all" {
addQuery(&query{querySpec: querySpec{path: arg.path, vers: arg.vers}, arg: arg.raw})
} else {
for _, err := range match.Errs {
Expand Down
42 changes: 19 additions & 23 deletions src/cmd/go/internal/modload/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,20 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
updateMatches := func(matches []*search.Match, iterating bool) {
for i, m := range matches {
switch {
case build.IsLocalImport(m.Pattern) || filepath.IsAbs(m.Pattern):
case m.IsLocal():
// Evaluate list of file system directories on first iteration.
if fsDirs == nil {
fsDirs = make([][]string, len(matches))
}
if fsDirs[i] == nil {
var dirs []string
if m.Literal {
dirs = []string{m.Pattern}
if m.IsLiteral() {
fsDirs[i] = []string{m.Pattern()}
} else {
match := search.MatchPackagesInFS(m.Pattern)
dirs = match.Pkgs
m.Errs = match.Errs
m.MatchPackagesInFS()
// Pull out the matching directories: we are going to resolve them
// to package paths below.
fsDirs[i], m.Pkgs = m.Pkgs, nil
}
fsDirs[i] = dirs
}

// Make a copy of the directory list and translate to import paths.
Expand All @@ -92,9 +91,8 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
// from not being in the build list to being in it and back as
// the exact version of a particular module increases during
// the loader iterations.
m.Pkgs = str.StringList(fsDirs[i])
pkgs := m.Pkgs
m.Pkgs = m.Pkgs[:0]
pkgs := str.StringList(fsDirs[i])
m.Pkgs = pkgs[:0]
for _, pkg := range pkgs {
var dir string
if !filepath.IsAbs(pkg) {
Expand Down Expand Up @@ -172,10 +170,13 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
m.Pkgs = append(m.Pkgs, pkg)
}

case strings.Contains(m.Pattern, "..."):
m.Pkgs = matchPackages(m.Pattern, loaded.tags, true, buildList)
case m.IsLiteral():
m.Pkgs = []string{m.Pattern()}

case m.Pattern == "all":
case strings.Contains(m.Pattern(), "..."):
m.Pkgs = matchPackages(m.Pattern(), loaded.tags, true, buildList)

case m.Pattern() == "all":
loaded.testAll = true
if iterating {
// Enumerate the packages in the main module.
Expand All @@ -187,15 +188,13 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
m.Pkgs = loaded.computePatternAll(m.Pkgs)
}

case search.IsMetaPackage(m.Pattern): // std, cmd
case m.Pattern() == "std" || m.Pattern() == "cmd":
if len(m.Pkgs) == 0 {
match := search.MatchPackages(m.Pattern)
m.Pkgs = match.Pkgs
m.Errs = match.Errs
m.MatchPackages() // Locate the packages within GOROOT/src.
}

default:
m.Pkgs = []string{m.Pattern}
panic(fmt.Sprintf("internal error: modload missing case for pattern %s", m.Pattern()))
}
}
}
Expand All @@ -204,10 +203,7 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {

var matches []*search.Match
for _, pattern := range search.CleanPatterns(patterns) {
matches = append(matches, &search.Match{
Pattern: pattern,
Literal: !strings.Contains(pattern, "...") && !search.IsMetaPackage(pattern),
})
matches = append(matches, search.NewMatch(pattern))
}

loaded = newLoader(tags)
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/go/internal/modload/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ type QueryResult struct {
// module and only the version "latest", without checking for other possible
// modules.
func QueryPackage(path, query string, allowed func(module.Version) bool) ([]QueryResult, error) {
if search.IsMetaPackage(path) || strings.Contains(path, "...") {
m := search.NewMatch(path)
if m.IsLocal() || !m.IsLiteral() {
return nil, fmt.Errorf("pattern %s is not an importable package", path)
}
return QueryPattern(path, query, allowed)
Expand Down
Loading

0 comments on commit d11e1f9

Please sign in to comment.