cmd/go: query modules in parallel

Refactor modload.QueryPackage and modload.QueryPattern to share code.

Fine-tune error reporting and make it consistent between QueryPackage and QueryPattern.

Expand tests for pattern errors.

Update a TODO in modget/get.go and add a test case that demonstrates it.

Updates #26232

Change-Id: I900ca8de338ef9a51b7f85ed93d8bcf837621646
Reviewed-on: https://go-review.googlesource.com/c/go/+/173017
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

1 files changed, 215 insertions, 92 deletions

diff --git a/src/cmd/go/internal/modload/query.go b/src/cmd/go/internal/modload/query.go
index 84950732be..74847a2912 100644
--- a/src/cmd/go/internal/modload/query.go
+++ b/src/cmd/go/internal/modload/query.go
@@ -5,15 +5,17 @@
 package modload
 
 import (
+	"fmt"
+	pathpkg "path"
+	"strings"
+	"sync"
+
 	"cmd/go/internal/modfetch"
 	"cmd/go/internal/modfetch/codehost"
 	"cmd/go/internal/module"
+	"cmd/go/internal/search"
 	"cmd/go/internal/semver"
 	"cmd/go/internal/str"
-	"errors"
-	"fmt"
-	pathpkg "path"
-	"strings"
 )
 
 // Query looks up a revision of a given module given a version query string.
@@ -181,10 +183,10 @@ func Query(path, query string, allowed func(module.Version) bool) (*modfetch.Rev
 		}
 	}
 
-	return nil, fmt.Errorf("no matching versions for query %q", query)
+	return nil, &NoMatchingVersionError{query: query}
 }
 
-// isSemverPrefix reports whether v is a semantic version prefix: v1 or  v1.2 (not v1.2.3).
+// isSemverPrefix reports whether v is a semantic version prefix: v1 or  v1.2 (not wv1.2.3).
 // The caller is assumed to have checked that semver.IsValid(v) is true.
 func isSemverPrefix(v string) bool {
 	dots := 0
@@ -208,114 +210,235 @@ func matchSemverPrefix(p, v string) bool {
 	return len(v) > len(p) && v[len(p)] == '.' && v[:len(p)] == p
 }
 
-// QueryPackage looks up a revision of a module containing path.
+type QueryResult struct {
+	Mod      module.Version
+	Rev      *modfetch.RevInfo
+	Packages []string
+}
+
+// QueryPackage looks up the module(s) containing path at a revision matching
+// query. The results are sorted by module path length in descending order.
 //
-// If multiple modules with revisions matching the query provide the requested
-// package, QueryPackage picks the one with the longest module path.
+// If the package is in the main module, QueryPackage considers only the main
+// 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, "...") {
+		return nil, fmt.Errorf("pattern %s is not an importable package", path)
+	}
+	return QueryPattern(path, query, allowed)
+}
+
+// QueryPattern looks up the module(s) containing at least one package matching
+// the given pattern at the given version. The results are sorted by module path
+// length in descending order.
 //
-// If the path is in the main module and the query is "latest",
-// QueryPackage returns Target as the version.
-func QueryPackage(path, query string, allowed func(module.Version) bool) (module.Version, *modfetch.RevInfo, error) {
+// QueryPattern queries modules with package paths up to the first "..."
+// in the pattern. For the pattern "example.com/a/b.../c", QueryPattern would
+// consider prefixes of "example.com/a". If multiple modules have versions
+// that match the query and packages that match the pattern, QueryPattern
+// picks the one with the longest module path.
+//
+// If any matching package is in the main module, QueryPattern considers only
+// the main module and only the version "latest", without checking for other
+// possible modules.
+func QueryPattern(pattern string, query string, allowed func(module.Version) bool) ([]QueryResult, error) {
+	base := pattern
+	var match func(m module.Version, root string, isLocal bool) (pkgs []string)
+
+	if i := strings.Index(pattern, "..."); i >= 0 {
+		base = pathpkg.Dir(pattern[:i+3])
+		match = func(m module.Version, root string, isLocal bool) []string {
+			return matchPackages(pattern, anyTags, false, []module.Version{m})
+		}
+	} else {
+		match = func(m module.Version, root string, isLocal bool) []string {
+			prefix := m.Path
+			if m == Target {
+				prefix = targetPrefix
+			}
+			if _, ok := dirInModule(pattern, prefix, root, isLocal); ok {
+				return []string{pattern}
+			} else {
+				return nil
+			}
+		}
+	}
+
 	if HasModRoot() {
-		if _, ok := dirInModule(path, targetPrefix, modRoot, true); ok {
+		pkgs := match(Target, modRoot, true)
+		if len(pkgs) > 0 {
 			if query != "latest" {
-				return module.Version{}, nil, fmt.Errorf("can't query specific version (%q) for package %s in the main module (%s)", query, path, Target.Path)
+				return nil, fmt.Errorf("can't query specific version for package %s in the main module (%s)", pattern, Target.Path)
 			}
 			if !allowed(Target) {
-				return module.Version{}, nil, fmt.Errorf("internal error: package %s is in the main module (%s), but version is not allowed", path, Target.Path)
+				return nil, fmt.Errorf("internal error: package %s is in the main module (%s), but version is not allowed", pattern, Target.Path)
 			}
-			return Target, &modfetch.RevInfo{Version: Target.Version}, nil
+			return []QueryResult{{
+				Mod:      Target,
+				Rev:      &modfetch.RevInfo{Version: Target.Version},
+				Packages: pkgs,
+			}}, nil
 		}
 	}
 
-	finalErr := errMissing
-	for p := path; p != "." && p != "/"; p = pathpkg.Dir(p) {
-		info, err := Query(p, query, allowed)
-		if err != nil {
-			if _, ok := err.(*codehost.VCSError); ok {
-				// A VCSError means we know where to find the code,
-				// we just can't. Abort search.
-				return module.Version{}, nil, err
+	// If the path we're attempting is not in the module cache and we don't have a
+	// fetch result cached either, we'll end up making a (potentially slow)
+	// request to the proxy or (often even slower) the origin server.
+	// To minimize latency, execute all of those requests in parallel.
+	type result struct {
+		QueryResult
+		err error
+	}
+	results := make([]result, strings.Count(base, "/")+1) // by descending path length
+	i, p := 0, base
+	var wg sync.WaitGroup
+	wg.Add(len(results))
+	for {
+		go func(p string, r *result) (err error) {
+			defer func() {
+				r.err = err
+				wg.Done()
+			}()
+
+			r.Mod.Path = p
+			if HasModRoot() && p == Target.Path {
+				r.Mod.Version = Target.Version
+				r.Rev = &modfetch.RevInfo{Version: Target.Version}
+				// We already know (from above) that Target does not contain any
+				// packages matching pattern, so leave r.Packages empty.
+			} else {
+				r.Rev, err = Query(p, query, allowed)
+				if err != nil {
+					return err
+				}
+				r.Mod.Version = r.Rev.Version
+				root, isLocal, err := fetch(r.Mod)
+				if err != nil {
+					return err
+				}
+				r.Packages = match(r.Mod, root, isLocal)
 			}
-			if finalErr == errMissing {
-				finalErr = err
+			if len(r.Packages) == 0 {
+				return &packageNotInModuleError{
+					mod:     r.Mod,
+					query:   query,
+					pattern: pattern,
+				}
 			}
-			continue
-		}
-		m := module.Version{Path: p, Version: info.Version}
-		root, isLocal, err := fetch(m)
-		if err != nil {
-			return module.Version{}, nil, err
+			return nil
+		}(p, &results[i])
+
+		j := strings.LastIndexByte(p, '/')
+		if i++; i == len(results) {
+			if j >= 0 {
+				panic("undercounted slashes")
+			}
+			break
 		}
-		_, ok := dirInModule(path, m.Path, root, isLocal)
-		if ok {
-			return m, info, nil
+		if j < 0 {
+			panic("overcounted slashes")
 		}
+		p = p[:j]
 	}
+	wg.Wait()
 
-	return module.Version{}, nil, finalErr
-}
-
-// QueryPattern looks up a module with at least one package matching the
-// given pattern at the given version. It returns a list of matched packages
-// and information about the module.
-//
-// QueryPattern queries modules with package paths up to the first "..."
-// in the pattern. For the pattern "example.com/a/b.../c", QueryPattern would
-// consider prefixes of "example.com/a". If multiple modules have versions
-// that match the query and packages that match the pattern, QueryPattern
-// picks the one with the longest module path.
-func QueryPattern(pattern string, query string, allowed func(module.Version) bool) ([]string, module.Version, *modfetch.RevInfo, error) {
-	i := strings.Index(pattern, "...")
-	if i < 0 {
-		m, info, err := QueryPackage(pattern, query, allowed)
-		if err != nil {
-			return nil, module.Version{}, nil, err
-		} else {
-			return []string{pattern}, m, info, nil
+	// Classify the results. In case of failure, identify the error that the user
+	// is most likely to find helpful.
+	var (
+		successes  []QueryResult
+		mostUseful result
+	)
+	for _, r := range results {
+		if r.err == nil {
+			successes = append(successes, r.QueryResult)
+			continue
 		}
-	}
-	base := pathpkg.Dir(pattern[:i+3])
 
-	// Return the most specific error for the longest module path.
-	const (
-		errNoModule  = 0
-		errNoVersion = 1
-		errNoMatch   = 2
-	)
-	errLevel := errNoModule
-	finalErr := errors.New("cannot find module matching pattern")
+		switch mostUseful.err.(type) {
+		case nil:
+			mostUseful = r
+			continue
+		case *packageNotInModuleError:
+			// Any other error is more useful than one that reports that the main
+			// module does not contain the requested packages.
+			if mostUseful.Mod.Path == Target.Path {
+				mostUseful = r
+				continue
+			}
+		}
 
-	for p := base; p != "." && p != "/"; p = pathpkg.Dir(p) {
-		info, err := Query(p, query, allowed)
-		if err != nil {
-			if _, ok := err.(*codehost.VCSError); ok {
-				// A VCSError means we know where to find the code,
-				// we just can't. Abort search.
-				return nil, module.Version{}, nil, err
+		switch r.err.(type) {
+		case *codehost.VCSError:
+			// A VCSError means that we've located a repository, but couldn't look
+			// inside it for packages. That's a very strong signal, and should
+			// override any others.
+			return nil, r.err
+		case *packageNotInModuleError:
+			if r.Mod.Path == Target.Path {
+				// Don't override a potentially-useful error for some other module with
+				// a trivial error for the main module.
+				continue
 			}
-			if errLevel < errNoVersion {
-				errLevel = errNoVersion
-				finalErr = err
+			// A module with an appropriate prefix exists at the requested version,
+			// but it does not contain the requested package(s).
+			if _, worsePath := mostUseful.err.(*packageNotInModuleError); !worsePath {
+				mostUseful = r
+			}
+		case *NoMatchingVersionError:
+			// A module with an appropriate prefix exists, but not at the requested
+			// version.
+			_, worseError := mostUseful.err.(*packageNotInModuleError)
+			_, worsePath := mostUseful.err.(*NoMatchingVersionError)
+			if !(worseError || worsePath) {
+				mostUseful = r
 			}
-			continue
-		}
-		m := module.Version{Path: p, Version: info.Version}
-		// matchPackages also calls fetch but treats errors as fatal, so we
-		// fetch here first.
-		_, _, err = fetch(m)
-		if err != nil {
-			return nil, module.Version{}, nil, err
-		}
-		pkgs := matchPackages(pattern, anyTags, false, []module.Version{m})
-		if len(pkgs) > 0 {
-			return pkgs, m, info, nil
-		}
-		if errLevel < errNoMatch {
-			errLevel = errNoMatch
-			finalErr = fmt.Errorf("no matching packages in module %s@%s", m.Path, m.Version)
 		}
 	}
 
-	return nil, module.Version{}, nil, finalErr
+	// TODO(#26232): If len(successes) == 0 and some of the errors are 4xx HTTP
+	// codes, have the auth package recheck the failed paths.
+	// If we obtain new credentials for any of them, re-run the above loop.
+
+	if len(successes) == 0 {
+		// All of the possible module paths either did not exist at the requested
+		// version, or did not contain the requested package(s).
+		return nil, mostUseful.err
+	}
+
+	// At least one module at the requested version contained the requested
+	// package(s). Any remaining errors only describe the non-existence of
+	// alternatives, so ignore them.
+	return successes, nil
+}
+
+// A NoMatchingVersionError indicates that Query found a module at the requested
+// path, but not at any versions satisfying the query string and allow-function.
+type NoMatchingVersionError struct {
+	query string
+}
+
+func (e *NoMatchingVersionError) Error() string {
+	return fmt.Sprintf("no matching versions for query %q", e.query)
+}
+
+// A packageNotInModuleError indicates that QueryPattern found a candidate
+// module at the requested version, but that module did not contain any packages
+// matching the requested pattern.
+type packageNotInModuleError struct {
+	mod     module.Version
+	query   string
+	pattern string
+}
+
+func (e *packageNotInModuleError) Error() string {
+	found := ""
+	if e.query != e.mod.Version {
+		found = fmt.Sprintf(" (%s)", e.mod.Version)
+	}
+
+	if strings.Contains(e.pattern, "...") {
+		return fmt.Sprintf("module %s@%s%s found, but does not contain packages matching %s", e.mod.Path, e.query, found, e.pattern)
+	}
+	return fmt.Sprintf("module %s@%s%s found, but does not contain package %s", e.mod.Path, e.query, found, e.pattern)
 }