Skip to content

Commit

Permalink
cmd/go/internal/search: record errors in the Match struct
Browse files Browse the repository at this point in the history
Previously, we would either invoke base.Fatalf (which is too aggressive),
or log.Print (which is too passive).

Updates #32917

Change-Id: I5475e873e76948de7df65dca08bc0ce67a7fc827
Reviewed-on: https://go-review.googlesource.com/c/go/+/185344
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
  • Loading branch information
Bryan C. Mills committed Feb 28, 2020
1 parent d464c7c commit 156c607
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 15 deletions.
9 changes: 7 additions & 2 deletions src/cmd/go/internal/get/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,15 @@ 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) {
args = search.MatchPackagesInFS(arg).Pkgs
match = search.MatchPackagesInFS(arg)
} else {
args = search.MatchPackages(arg).Pkgs
match = search.MatchPackages(arg)
}
args = match.Pkgs
for _, err := range match.Errs {
base.Errorf("%s", err)
}
isWildcard = true
}
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/go/internal/load/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -2091,6 +2091,9 @@ func PackagesAndErrors(patterns []string) []*Package {
seenPkg[p] = true
pkgs = append(pkgs, p)
}

// TODO: if len(m.Pkgs) == 0 && len(m.Errs) > 0, should we add a *Package
// with a non-nil Error field?
}

// Now that CmdlinePkg is set correctly,
Expand Down
4 changes: 4 additions & 0 deletions src/cmd/go/internal/modget/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,10 @@ func runGet(cmd *base.Command, args []string) {
// we'll print a warning after the outer loop.
if !search.IsRelativePath(arg.path) && !match.Literal && arg.path != "all" {
addQuery(&query{querySpec: querySpec{path: arg.path, vers: arg.vers}, arg: arg.raw})
} else {
for _, err := range match.Errs {
base.Errorf("go get: %v", err)
}
}
continue
}
Expand Down
8 changes: 6 additions & 2 deletions src/cmd/go/internal/modload/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
if m.Literal {
dirs = []string{m.Pattern}
} else {
dirs = search.MatchPackagesInFS(m.Pattern).Pkgs
match := search.MatchPackagesInFS(m.Pattern)
dirs = match.Pkgs
m.Errs = match.Errs
}
fsDirs[i] = dirs
}
Expand Down Expand Up @@ -187,7 +189,9 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {

case search.IsMetaPackage(m.Pattern): // std, cmd
if len(m.Pkgs) == 0 {
m.Pkgs = search.MatchPackages(m.Pattern).Pkgs
match := search.MatchPackages(m.Pattern)
m.Pkgs = match.Pkgs
m.Errs = match.Errs
}

default:
Expand Down
66 changes: 55 additions & 11 deletions src/cmd/go/internal/search/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"cmd/go/internal/cfg"
"fmt"
"go/build"
"log"
"os"
"path"
"path/filepath"
Expand All @@ -22,6 +21,35 @@ type Match struct {
Pattern string // the pattern itself
Literal bool // whether it is a literal (no wildcards)
Pkgs []string // matching packages (dirs or import paths)
Errs []error // errors matching the patterns to packages, NOT errors loading those packages

// Errs may be non-empty even if len(Pkgs) > 0, indicating that some matching
// packages could be located but results may be incomplete.
// If len(Pkgs) == 0 && len(Errs) == 0, the pattern is well-formed but did not
// match any packages.
}

// AddError appends a MatchError wrapping err to m.Errs.
func (m *Match) AddError(err error) {
m.Errs = append(m.Errs, &MatchError{Match: m, Err: err})
}

// A MatchError indicates an error that occurred while attempting to match a
// pattern.
type MatchError struct {
*Match
Err error
}

func (e *MatchError) Error() string {
if e.Literal {
return fmt.Sprintf("matching %s: %v", e.Pattern, e.Err)
}
return fmt.Sprintf("pattern %s: %v", e.Pattern, e.Err)
}

func (e *MatchError) Unwrap() error {
return e.Err
}

// MatchPackages returns all the packages that can be found
Expand Down Expand Up @@ -56,7 +84,7 @@ func MatchPackages(pattern string) *Match {
if pattern == "cmd" {
root += "cmd" + string(filepath.Separator)
}
filepath.Walk(root, func(path string, fi os.FileInfo, err error) error {
err := filepath.Walk(root, func(path string, fi os.FileInfo, err error) error {
if err != nil || path == src {
return nil
}
Expand Down Expand Up @@ -100,21 +128,29 @@ func MatchPackages(pattern string) *Match {
pkg, err := cfg.BuildContext.ImportDir(path, 0)
if err != nil {
if _, noGo := err.(*build.NoGoError); noGo {
// The package does not actually exist, so record neither the package
// nor the error.
return nil
}
// There was an error importing path, but not matching it,
// which is all that Match promises to do.
// Ignore the import error.
}

// If we are expanding "cmd", skip main
// packages under cmd/vendor. At least as of
// March, 2017, there is one there for the
// vendored pprof tool.
if pattern == "cmd" && strings.HasPrefix(pkg.ImportPath, "cmd/vendor") && pkg.Name == "main" {
if pattern == "cmd" && pkg != nil && strings.HasPrefix(pkg.ImportPath, "cmd/vendor") && pkg.Name == "main" {
return nil
}

m.Pkgs = append(m.Pkgs, name)
return nil
})
if err != nil {
m.AddError(err)
}
}
return m
}
Expand Down Expand Up @@ -166,15 +202,16 @@ func MatchPackagesInFS(pattern string) *Match {
if modRoot != "" {
abs, err := filepath.Abs(dir)
if err != nil {
base.Fatalf("go: %v", err)
m.AddError(err)
return m
}
if !hasFilepathPrefix(abs, modRoot) {
base.Fatalf("go: pattern %s refers to dir %s, outside module root %s", pattern, abs, modRoot)
return nil
m.AddError(fmt.Errorf("directory %s is outside module root (%s)", abs, modRoot))
return m
}
}

filepath.Walk(dir, func(path string, fi os.FileInfo, err error) error {
err := filepath.Walk(dir, func(path string, fi os.FileInfo, err error) error {
if err != nil || !fi.IsDir() {
return nil
}
Expand Down Expand Up @@ -218,14 +255,21 @@ func MatchPackagesInFS(pattern string) *Match {
// behavior means people miss serious mistakes.
// See golang.org/issue/11407.
if p, err := cfg.BuildContext.ImportDir(path, 0); err != nil && (p == nil || len(p.InvalidGoFiles) == 0) {
if _, noGo := err.(*build.NoGoError); !noGo {
log.Print(err)
if _, noGo := err.(*build.NoGoError); noGo {
// The package does not actually exist, so record neither the package
// nor the error.
return nil
}
return nil
// There was an error importing path, but not matching it,
// which is all that Match promises to do.
// Ignore the import error.
}
m.Pkgs = append(m.Pkgs, name)
return nil
})
if err != nil {
m.AddError(err)
}
return m
}

Expand Down Expand Up @@ -316,7 +360,7 @@ func replaceVendor(x, repl string) string {
// WarnUnmatched warns about patterns that didn't match any packages.
func WarnUnmatched(matches []*Match) {
for _, m := range matches {
if len(m.Pkgs) == 0 {
if len(m.Pkgs) == 0 && len(m.Errs) == 0 {
fmt.Fprintf(os.Stderr, "go: warning: %q matched no packages\n", m.Pattern)
}
}
Expand Down

0 comments on commit 156c607

Please sign in to comment.