cmd/go/internal/modget: resolve paths at the requested versions

Previously, we resolved each argument to 'go get' to a package path or
module path based on what was in the build list at existing versions,
even if the argument specified a different version explicitly. That
resulted in bugs like #37438, in which we variously resolved the wrong
version or guessed the wrong argument type for what is unambiguously a
package argument at the requested version.

We were also using a two-step upgrade/downgrade algorithm, which could
not only upgrade more that is strictly necessary, but could also
unintentionally upgrade *above* the requested versions during the
downgrade step.

This change instead uses an iterative approach, with an explicit
disambiguation step for the (rare) cases where an argument could match
the same package path in multiple modules. We use a hook in the
package loader to halt package loading as soon as an incorrect version
is found — preventing over-resolving — and verify that the result
after applying downgrades successfully obtained the requested versions
of all modules.

Making 'go get' be correct and usable is especially important now that
we are defaulting to read-only mode (#40728), for which we are
recommending 'go get' more heavily.

While I'm in here refactoring, I'm also reworking the API boundary
between the modget and modload packages. Previously, the modget
package edited the build list directly, and the modload package
accepted the edited build list without validation. For lazy loading
(#36460), the modload package will need to maintain additional
metadata about the requirement graph, so it needs tighter control over
the changes to the build list.

As of this change, modget no longer invokes MVS directly, but instead
goes through the modload package. The resulting API gives clearer
reasons in case of updates, which we can use to emit more useful
errors.

Fixes #37438
Updates #36460
Updates #40728

Change-Id: I596f0020f3795870dec258147e6fc26a3292c93a
Reviewed-on: https://go-review.googlesource.com/c/go/+/263267
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

1 files changed, 45 insertions, 9 deletions

diff --git a/src/cmd/go/internal/modload/query.go b/src/cmd/go/internal/modload/query.go
index 99cbac1aa7..d4a1e85041 100644
--- a/src/cmd/go/internal/modload/query.go
+++ b/src/cmd/go/internal/modload/query.go
@@ -47,8 +47,9 @@ import (
 //   with non-prereleases preferred over prereleases.
 // - a repository commit identifier or tag, denoting that commit.
 //
-// current denotes the current version of the module; it may be "" if the
-// current version is unknown or should not be considered. If query is
+// current denotes the currently-selected version of the module; it may be
+// "none" if no version is currently selected, or "" if the currently-selected
+// version is unknown or should not be considered. If query is
 // "upgrade" or "patch", current will be returned if it is a newer
 // semantic version or a chronologically later pseudo-version than the
 // version that would otherwise be chosen. This prevents accidental downgrades
@@ -98,7 +99,7 @@ func queryProxy(ctx context.Context, proxy, path, query, current string, allowed
 	ctx, span := trace.StartSpan(ctx, "modload.queryProxy "+path+" "+query)
 	defer span.Done()
 
-	if current != "" && !semver.IsValid(current) {
+	if current != "" && current != "none" && !semver.IsValid(current) {
 		return nil, fmt.Errorf("invalid previous version %q", current)
 	}
 	if cfg.BuildMod == "vendor" {
@@ -109,7 +110,7 @@ func queryProxy(ctx context.Context, proxy, path, query, current string, allowed
 	}
 
 	if path == Target.Path {
-		if query != "latest" && query != "upgrade" && query != "patch" {
+		if query != "upgrade" && query != "patch" {
 			return nil, &QueryMatchesMainModuleError{Pattern: path, Query: query}
 		}
 		if err := allowed(ctx, Target); err != nil {
@@ -236,7 +237,7 @@ func queryProxy(ctx context.Context, proxy, path, query, current string, allowed
 		}
 	}
 
-	if (query == "upgrade" || query == "patch") && current != "" {
+	if (query == "upgrade" || query == "patch") && current != "" && current != "none" {
 		// "upgrade" and "patch" may stay on the current version if allowed.
 		if err := allowed(ctx, module.Version{Path: path, Version: current}); errors.Is(err, ErrDisallowed) {
 			return nil, err
@@ -323,7 +324,7 @@ func newQueryMatcher(path string, query, current string, allowed AllowedFunc) (*
 		qm.mayUseLatest = true
 
 	case query == "upgrade":
-		if current == "" {
+		if current == "" || current == "none" {
 			qm.mayUseLatest = true
 		} else {
 			qm.mayUseLatest = modfetch.IsPseudoVersion(current)
@@ -331,6 +332,9 @@ func newQueryMatcher(path string, query, current string, allowed AllowedFunc) (*
 		}
 
 	case query == "patch":
+		if current == "none" {
+			return nil, &NoPatchBaseError{path}
+		}
 		if current == "" {
 			qm.mayUseLatest = true
 		} else {
@@ -554,6 +558,9 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
 
 	if i := strings.Index(pattern, "..."); i >= 0 {
 		base = pathpkg.Dir(pattern[:i+3])
+		if base == "." {
+			return nil, nil, &WildcardInFirstElementError{Pattern: pattern, Query: query}
+		}
 		match = func(mod module.Version, root string, isLocal bool) *search.Match {
 			m := search.NewMatch(pattern)
 			matchPackages(ctx, m, imports.AnyTags(), omitStd, []module.Version{mod})
@@ -578,7 +585,7 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
 	if HasModRoot() {
 		m := match(Target, modRoot, true)
 		if len(m.Pkgs) > 0 {
-			if query != "latest" && query != "upgrade" && query != "patch" {
+			if query != "upgrade" && query != "patch" {
 				return nil, nil, &QueryMatchesPackagesInMainModuleError{
 					Pattern:  pattern,
 					Query:    query,
@@ -598,7 +605,7 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
 			return nil, nil, err
 		}
 
-		if query != "latest" && query != "upgrade" && query != "patch" && matchPattern(Target.Path) {
+		if query != "upgrade" && query != "patch" && matchPattern(Target.Path) {
 			if err := allowed(ctx, Target); err == nil {
 				modOnly = &QueryResult{
 					Mod: Target,
@@ -724,6 +731,7 @@ func queryPrefixModules(ctx context.Context, candidateModules []string, queryMod
 	var (
 		noPackage   *PackageNotInModuleError
 		noVersion   *NoMatchingVersionError
+		noPatchBase *NoPatchBaseError
 		notExistErr error
 	)
 	for _, r := range results {
@@ -740,6 +748,10 @@ func queryPrefixModules(ctx context.Context, candidateModules []string, queryMod
 			if noVersion == nil {
 				noVersion = rErr
 			}
+		case *NoPatchBaseError:
+			if noPatchBase == nil {
+				noPatchBase = rErr
+			}
 		default:
 			if errors.Is(rErr, fs.ErrNotExist) {
 				if notExistErr == nil {
@@ -771,6 +783,8 @@ func queryPrefixModules(ctx context.Context, candidateModules []string, queryMod
 			err = noPackage
 		case noVersion != nil:
 			err = noVersion
+		case noPatchBase != nil:
+			err = noPatchBase
 		case notExistErr != nil:
 			err = notExistErr
 		default:
@@ -795,12 +809,34 @@ type NoMatchingVersionError struct {
 
 func (e *NoMatchingVersionError) Error() string {
 	currentSuffix := ""
-	if (e.query == "upgrade" || e.query == "patch") && e.current != "" {
+	if (e.query == "upgrade" || e.query == "patch") && e.current != "" && e.current != "none" {
 		currentSuffix = fmt.Sprintf(" (current version is %s)", e.current)
 	}
 	return fmt.Sprintf("no matching versions for query %q", e.query) + currentSuffix
 }
 
+// A NoPatchBaseError indicates that Query was called with the query "patch"
+// but with a current version of "" or "none".
+type NoPatchBaseError struct {
+	path string
+}
+
+func (e *NoPatchBaseError) Error() string {
+	return fmt.Sprintf(`can't query version "patch" of module %s: no existing version is required`, e.path)
+}
+
+// A WildcardInFirstElementError indicates that a pattern passed to QueryPattern
+// had a wildcard in its first path element, and therefore had no pattern-prefix
+// modules to search in.
+type WildcardInFirstElementError struct {
+	Pattern string
+	Query   string
+}
+
+func (e *WildcardInFirstElementError) Error() string {
+	return fmt.Sprintf("no modules to query for %s@%s because first path element contains a wildcard", e.Pattern, 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.