cmd/go/internal: factor out modload.QueryPackage and use in in modget

modload.Import contains a loop that looks for the module containing a package.
Because we overload Import to locate both packages and modules, that loop
contains a bunch of special-cases for modules with empty roots.

In this change, we factor out the loop into a new function (QueryPackage) and
use that directly in modget.getQuery. That restores the invariant that
the paths passed to modload.Import must be importable packages, and fixes 'go
get' lookups for packages that have moved between a module and submodules with
the same path prefix.

Updates #26602.

Change-Id: I8bc8340c17f2df062d03ce720f4dc18b2ba406b2
Reviewed-on: https://go-review.googlesource.com/128136
Reviewed-by: Russ Cox <[email protected]>

Author: Bryan C. Mills

src/cmd/go/internal/modget/get.go

@@ -528,7 +528,7 @@ func runGet(cmd *base.Command, args []string) {
 					// current directory, even if it is not tho module root.
 					continue
 				}
-				if strings.HasPrefix(p.Error.Err, "no Go files") && modload.ModuleInfo(p.ImportPath) != nil {
+				if strings.Contains(p.Error.Err, "cannot find module providing") && modload.ModuleInfo(p.ImportPath) != nil {
 					// Explicitly-requested module, but it doesn't contain a package at the
 					// module root.
 					continue
@@ -551,13 +551,6 @@ func runGet(cmd *base.Command, args []string) {
 // If forceModulePath is set, getQuery must interpret path
 // as a module path.
 func getQuery(path, vers string, forceModulePath bool) (module.Version, error) {
-	if path == modload.Target.Path {
-		if vers != "" {
-			return module.Version{}, fmt.Errorf("cannot update main module to explicit version")
-		}
-		return modload.Target, nil
-	}
-
 	if vers == "" {
 		vers = "latest"
 	}
@@ -569,36 +562,14 @@ func getQuery(path, vers string, forceModulePath bool) (module.Version, error) {
 		return module.Version{Path: path, Version: info.Version}, nil
 	}
 
-	// Even if the query fails, if the path is (or must be) a real module, then report the query error.
-	if forceModulePath || *getM || isModulePath(path) {
-		return module.Version{}, err
-	}
-
-	// Otherwise, interpret the package path as an import
-	// and determine what module that import would address
-	// if found in the current source code.
-	// Then apply the version to that module.
-	m, _, err := modload.Import(path)
-	if e, ok := err.(*modload.ImportMissingError); ok && e.Module.Path != "" {
-		m = e.Module
-	} else if err != nil {
+	// Even if the query fails, if the path must be a real module, then report the query error.
+	if forceModulePath || *getM {
 		return module.Version{}, err
 	}
-	if m.Path == "" {
-		return module.Version{}, fmt.Errorf("package %q is not in a module", path)
-	}
-	info, err = modload.Query(m.Path, vers, modload.Allowed)
-	if err != nil {
-		return module.Version{}, err
-	}
-	return module.Version{Path: m.Path, Version: info.Version}, nil
-}
 
-// isModulePath reports whether path names an actual module,
-// defined as one with an accessible latest version.
-func isModulePath(path string) bool {
-	_, err := modload.Query(path, "latest", modload.Allowed)
-	return err == nil
+	// Otherwise, try a package path.
+	m, _, err := modload.QueryPackage(path, vers, modload.Allowed)
+	return m, err
 }
 
 // An upgrader adapts an underlying mvs.Reqs to apply an

src/cmd/go/internal/modload/import.go

@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"go/build"
 	"os"
-	pathpkg "path"
 	"path/filepath"
 	"strings"
 
@@ -124,24 +123,6 @@ func Import(path string) (m module.Version, dir string, err error) {
 		return module.Version{}, "", errors.New(buf.String())
 	}
 
-	// Special case: if the path matches a module path,
-	// and we haven't found code in any module on the build list
-	// (since we haven't returned yet),
-	// force the use of the current module instead of
-	// looking for an alternate one.
-	// This helps "go get golang.org/x/net" even though
-	// there is no code in x/net.
-	for _, m := range buildList {
-		if m.Path == path {
-			root, isLocal, err := fetch(m)
-			if err != nil {
-				return module.Version{}, "", err
-			}
-			dir, _ := dirInModule(path, m.Path, root, isLocal)
-			return m, dir, nil
-		}
-	}
-
 	// Not on build list.
 
 	// Look up module containing the package, for addition to the build list.
@@ -150,43 +131,11 @@ func Import(path string) (m module.Version, dir string, err error) {
 		return module.Version{}, "", fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod)
 	}
 
-	for p := path; p != "."; p = pathpkg.Dir(p) {
-		// We can't upgrade the main module.
-		// Note that this loop does consider upgrading other modules on the build list.
-		// If that's too aggressive we can skip all paths already on the build list,
-		// not just Target.Path, but for now let's try being aggressive.
-		if p == Target.Path {
-			// Can't move to a new version of main module.
-			continue
-		}
-
-		info, err := Query(p, "latest", Allowed)
-		if err != nil {
-			continue
-		}
-		m := module.Version{Path: p, Version: info.Version}
-		root, isLocal, err := fetch(m)
-		if err != nil {
-			continue
-		}
-		_, ok := dirInModule(path, m.Path, root, isLocal)
-		if ok {
-			return module.Version{}, "", &ImportMissingError{ImportPath: path, Module: m}
-		}
-
-		// Special case matching the one above:
-		// if m.Path matches path, assume adding it to the build list
-		// will either add the right code or the right code doesn't exist.
-		if m.Path == path {
-			return module.Version{}, "", &ImportMissingError{ImportPath: path, Module: m}
-		}
+	m, _, err = QueryPackage(path, "latest", Allowed)
+	if err != nil {
+		return module.Version{}, "", &ImportMissingError{ImportPath: path}
 	}
-
-	// Did not resolve import to any module.
-	// TODO(rsc): It would be nice to return a specific error encountered
-	// during the loop above if possible, but it's not clear how to pick
-	// out the right one.
-	return module.Version{}, "", &ImportMissingError{ImportPath: path}
+	return m, "", &ImportMissingError{ImportPath: path, Module: m}
 }
 
 // maybeInModule reports whether, syntactically,

src/cmd/go/internal/modload/query.go

@@ -9,6 +9,7 @@ import (
 	"cmd/go/internal/module"
 	"cmd/go/internal/semver"
 	"fmt"
+	pathpkg "path"
 	"strings"
 )
 
@@ -29,6 +30,8 @@ import (
 //
 // If the allowed function is non-nil, Query excludes any versions for which allowed returns false.
 //
+// If path is the path of the main module and the query is "latest",
+// Query returns Target.Version as the version.
 func Query(path, query string, allowed func(module.Version) bool) (*modfetch.RevInfo, error) {
 	if allowed == nil {
 		allowed = func(module.Version) bool { return true }
@@ -117,6 +120,16 @@ func Query(path, query string, allowed func(module.Version) bool) (*modfetch.Rev
 		return info, nil
 	}
 
+	if path == Target.Path {
+		if query != "latest" {
+			return nil, fmt.Errorf("can't query specific version (%q) for the main module (%s)", query, path)
+		}
+		if !allowed(Target) {
+			return nil, fmt.Errorf("internal error: main module version is not allowed")
+		}
+		return &modfetch.RevInfo{Version: Target.Version}, nil
+	}
+
 	// Load versions and execute query.
 	repo, err := modfetch.Lookup(path)
 	if err != nil {
@@ -187,3 +200,44 @@ func isSemverPrefix(v string) bool {
 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.
+//
+// If multiple modules with revisions matching the query provide the requested
+// package, QueryPackage picks the one with the longest module path.
+//
+// If the path is in the 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) {
+	if _, ok := dirInModule(path, Target.Path, ModRoot, true); ok {
+		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)
+		}
+		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 Target, &modfetch.RevInfo{Version: Target.Version}, nil
+	}
+
+	finalErr := errMissing
+	for p := path; p != "."; p = pathpkg.Dir(p) {
+		info, err := Query(p, query, allowed)
+		if err != nil {
+			if finalErr == errMissing {
+				finalErr = err
+			}
+			continue
+		}
+		m := module.Version{Path: p, Version: info.Version}
+		root, isLocal, err := fetch(m)
+		if err != nil {
+			return module.Version{}, nil, err
+		}
+		_, ok := dirInModule(path, m.Path, root, isLocal)
+		if ok {
+			return m, info, nil
+		}
+	}
+
+	return module.Version{}, nil, finalErr
+}

src/cmd/go/testdata/mod/example.com_join_subpkg_v1.0.0.txt

@@ -0,0 +1,9 @@
+Written by hand.
+Test case for package moved into a parent module.
+
+-- .mod --
+module example.com/join/subpkg
+-- .info --
+{"Version": "v1.0.0"}
+-- x.go --
+package subpkg

src/cmd/go/testdata/mod/example.com_join_subpkg_v1.1.0.txt

@@ -0,0 +1,9 @@
+Written by hand.
+Test case for package moved into a parent module.
+
+-- .mod --
+module example.com/join/subpkg
+
+require example.com/join v1.1.0
+-- .info --
+{"Version": "v1.1.0"}

src/cmd/go/testdata/mod/example.com_join_v1.0.0.txt

@@ -0,0 +1,7 @@
+Written by hand.
+Test case for package moved into a parent module.
+
+-- .mod --
+module example.com/join
+-- .info --
+{"Version": "v1.0.0"}

src/cmd/go/testdata/mod/example.com_join_v1.1.0.txt

@@ -0,0 +1,9 @@
+Written by hand.
+Test case for package moved into a parent module.
+
+-- .mod --
+module example.com/join
+-- .info --
+{"Version": "v1.1.0"}
+-- subpkg/x.go --
+package subpkg

src/cmd/go/testdata/mod/example.com_split_subpkg_v1.1.0.txt

@@ -0,0 +1,11 @@
+Written by hand.
+Test case for getting a package that has been moved to a different module.
+
+-- .mod --
+module example.com/split/subpkg
+
+require example.com/split v1.1.0
+-- .info --
+{"Version": "v1.1.0"}
+-- x.go --
+package subpkg

src/cmd/go/testdata/mod/example.com_split_v1.0.0.txt

@@ -0,0 +1,9 @@
+Written by hand.
+Test case for getting a package that has been moved to a different module.
+
+-- .mod --
+module example.com/split
+-- .info --
+{"Version": "v1.0.0"}
+-- subpkg/x.go --
+package subpkg

src/cmd/go/testdata/mod/example.com_split_v1.1.0.txt

@@ -0,0 +1,9 @@
+Written by hand.
+Test case for getting a package that has been moved to a different module.
+
+-- .mod --
+module example.com/split
+
+require example.com/split/subpkg v1.1.0
+-- .info --
+{"Version": "v1.1.0"}