cmd/go/internal/modget: consolidate Load entrypoints

This change replaces ImportPaths, ImportPathsQuiet, LoadALL, and
LoadVendor with a single LoadPackages function, with a LoadOpts struct
that more clearly documents the variations in behavior.

It also eliminates the cmd/go/internal/load.ImportPaths function,
which was undocumented and had only one call site (within its own
package).

The modload.LoadTests global variable is subsumed by a field in the
new LoadOpts struct, and is no longer needed for callers that invoke
LoadPackages directly. It has been (temporarily) replaced with a
similar global variable, load.ModResolveTests, which can itself be
converted to an explicit, local argument.

For #37438
For #36460
Updates #40775
Fixes #26977

Change-Id: I4fb6086c01b04de829d98875db19cf0118d40f8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/255938
Trust: Bryan C. Mills <[email protected]>
Trust: Jay Conrod <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>

src/cmd/go/internal/list/list.go

@@ -325,7 +325,7 @@ var (
 var nl = []byte{'\n'}
 
 func runList(ctx context.Context, cmd *base.Command, args []string) {
-	modload.LoadTests = *listTest
+	load.ModResolveTests = *listTest
 	work.BuildInit()
 	out := newTrackingWriter(os.Stdout)
 	defer out.w.Flush()

src/cmd/go/internal/load/pkg.go

@@ -742,7 +742,7 @@ func loadPackageData(path, parentPath, parentDir, parentRoot string, parentIsStd
 	// For vendored imports, it is the expanded form.
 	//
 	// Note that when modules are enabled, local import paths are normally
-	// canonicalized by modload.ImportPaths before now. However, if there's an
+	// canonicalized by modload.LoadPackages before now. However, if there's an
 	// error resolving a local path, it will be returned untransformed
 	// so that 'go list -e' reports something useful.
 	importKey := importSpec{
@@ -885,7 +885,7 @@ var preloadWorkerCount = runtime.GOMAXPROCS(0)
 // because of global mutable state that cannot safely be read and written
 // concurrently. In particular, packageDataCache may be cleared by "go get"
 // in GOPATH mode, and modload.loaded (accessed via modload.Lookup) may be
-// modified by modload.ImportPaths.
+// modified by modload.LoadPackages.
 type preload struct {
 	cancel chan struct{}
 	sema   chan struct{}
@@ -2106,6 +2106,18 @@ func LoadImportWithFlags(path, srcDir string, parent *Package, stk *ImportStack,
 	return p
 }
 
+// ModResolveTests indicates whether calls to the module loader should also
+// resolve test dependencies of the requested packages.
+//
+// If ModResolveTests is true, then the module loader needs to resolve test
+// dependencies at the same time as packages; otherwise, the test dependencies
+// of those packages could be missing, and resolving those missing dependencies
+// could change the selected versions of modules that provide other packages.
+//
+// TODO(#40775): Change this from a global variable to an explicit function
+// argument where needed.
+var ModResolveTests bool
+
 // Packages returns the packages named by the
 // command line arguments 'args'. If a named package
 // cannot be loaded at all (for example, if the directory does not exist),
@@ -2147,7 +2159,18 @@ func PackagesAndErrors(ctx context.Context, patterns []string) []*Package {
 		}
 	}
 
-	matches := ImportPaths(ctx, patterns)
+	var matches []*search.Match
+	if modload.Init(); cfg.ModulesEnabled {
+		loadOpts := modload.PackageOpts{
+			ResolveMissingImports: true,
+			LoadTests:             ModResolveTests,
+			AllowErrors:           true,
+		}
+		matches, _ = modload.LoadPackages(ctx, loadOpts, patterns...)
+	} else {
+		matches = search.ImportPaths(patterns)
+	}
+
 	var (
 		pkgs    []*Package
 		stk     ImportStack
@@ -2217,13 +2240,6 @@ func setToolFlags(pkgs ...*Package) {
 	}
 }
 
-func ImportPaths(ctx context.Context, args []string) []*search.Match {
-	if modload.Init(); cfg.ModulesEnabled {
-		return modload.ImportPaths(ctx, args)
-	}
-	return search.ImportPaths(args)
-}
-
 // PackagesForBuild is like Packages but exits
 // if any of the packages or their dependencies have errors
 // (cannot be built).

src/cmd/go/internal/modcmd/tidy.go

@@ -9,6 +9,7 @@ package modcmd
 import (
 	"cmd/go/internal/base"
 	"cmd/go/internal/cfg"
+	"cmd/go/internal/imports"
 	"cmd/go/internal/modload"
 	"context"
 )
@@ -49,11 +50,15 @@ func runTidy(ctx context.Context, cmd *base.Command, args []string) {
 	// those packages. In order to make 'go test' reproducible for the packages
 	// that are in 'all' but outside of the main module, we must explicitly
 	// request that their test dependencies be included.
-	modload.LoadTests = true
 	modload.ForceUseModules = true
 	modload.RootMode = modload.NeedRoot
 
-	modload.LoadALL(ctx)
+	modload.LoadPackages(ctx, modload.PackageOpts{
+		Tags:                  imports.AnyTags(),
+		ResolveMissingImports: true,
+		LoadTests:             true,
+		AllowErrors:           false, // TODO(#26603): Make this a flag.
+	}, "all")
 	modload.TidyBuildList()
 	modload.TrimGoSum()
 	modload.WriteGoMod()

src/cmd/go/internal/modcmd/vendor.go

@@ -49,7 +49,13 @@ func runVendor(ctx context.Context, cmd *base.Command, args []string) {
 	}
 	modload.ForceUseModules = true
 	modload.RootMode = modload.NeedRoot
-	pkgs := modload.LoadVendor(ctx)
+
+	loadOpts := modload.PackageOpts{
+		Tags:                  imports.AnyTags(),
+		ResolveMissingImports: true,
+		UseVendorAll:          true,
+	}
+	_, pkgs := modload.LoadPackages(ctx, loadOpts, "all")
 
 	vdir := filepath.Join(modload.ModRoot(), "vendor")
 	if err := os.RemoveAll(vdir); err != nil {

src/cmd/go/internal/modcmd/why.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"cmd/go/internal/base"
+	"cmd/go/internal/imports"
 	"cmd/go/internal/modload"
 
 	"golang.org/x/mod/module"
@@ -63,12 +64,14 @@ func init() {
 func runWhy(ctx context.Context, cmd *base.Command, args []string) {
 	modload.ForceUseModules = true
 	modload.RootMode = modload.NeedRoot
-	loadALL := modload.LoadALL
-	if *whyVendor {
-		loadALL = modload.LoadVendor
-	} else {
-		modload.LoadTests = true
+
+	loadOpts := modload.PackageOpts{
+		Tags:         imports.AnyTags(),
+		LoadTests:    !*whyVendor,
+		AllowErrors:  true,
+		UseVendorAll: *whyVendor,
 	}
+
 	if *whyM {
 		listU := false
 		listVersions := false
@@ -80,7 +83,8 @@ func runWhy(ctx context.Context, cmd *base.Command, args []string) {
 		}
 		mods := modload.ListModules(ctx, args, listU, listVersions, listRetractions)
 		byModule := make(map[module.Version][]string)
-		for _, path := range loadALL(ctx) {
+		_, pkgs := modload.LoadPackages(ctx, loadOpts, "all")
+		for _, path := range pkgs {
 			m := modload.PackageModule(path)
 			if m.Path != "" {
 				byModule[m] = append(byModule[m], path)
@@ -109,8 +113,11 @@ func runWhy(ctx context.Context, cmd *base.Command, args []string) {
 			sep = "\n"
 		}
 	} else {
-		matches := modload.ImportPaths(ctx, args) // resolve to packages
-		loadALL(ctx)                              // rebuild graph, from main module (not from named packages)
+		// Resolve to packages.
+		matches, _ := modload.LoadPackages(ctx, loadOpts, args...)
+
+		modload.LoadPackages(ctx, loadOpts, "all") // rebuild graph, from main module (not from named packages)
+
 		sep := ""
 		for _, m := range matches {
 			for _, path := range m.Pkgs {

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

@@ -284,7 +284,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
 	if cfg.Insecure {
 		fmt.Fprintf(os.Stderr, "go get: -insecure flag is deprecated; see 'go help get' for details\n")
 	}
-	modload.LoadTests = *getT
+	load.ModResolveTests = *getT
 
 	// Do not allow any updating of go.mod until we've applied
 	// all the requested changes and checked that the result matches
@@ -314,7 +314,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
 
 	// Add missing modules to the build list.
 	// We call SetBuildList here and elsewhere, since newUpgrader,
-	// ImportPathsQuiet, and other functions read the global build list.
+	// LoadPackages, and other functions read the global build list.
 	for _, q := range queries {
 		if _, ok := selectedVersion[q.m.Path]; !ok && q.m.Version != "none" {
 			buildList = append(buildList, q.m)
@@ -400,9 +400,16 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
 
 		if len(pkgPatterns) > 0 {
 			// Don't load packages if pkgPatterns is empty. Both
-			// modload.ImportPathsQuiet and ModulePackages convert an empty list
+			// modload.LoadPackages and ModulePackages convert an empty list
 			// of patterns to []string{"."}, which is not what we want.
-			matches = modload.ImportPathsQuiet(ctx, pkgPatterns, imports.AnyTags())
+			loadOpts := modload.PackageOpts{
+				Tags:                     imports.AnyTags(),
+				ResolveMissingImports:    true, // dubious; see https://golang.org/issue/32567
+				LoadTests:                *getT,
+				AllowErrors:              true, // Errors may be fixed by subsequent upgrades or downgrades.
+				SilenceUnmatchedWarnings: true, // We will warn after iterating below.
+			}
+			matches, _ = modload.LoadPackages(ctx, loadOpts, pkgPatterns...)
 			seenPkgs = make(map[string]bool)
 			for i, match := range matches {
 				arg := pkgGets[i]

src/cmd/go/internal/modload/build.go

@@ -50,7 +50,7 @@ func findStandardImportPath(path string) string {
 // PackageModuleInfo returns information about the module that provides
 // a given package. If modules are not enabled or if the package is in the
 // standard library or if the package was not successfully loaded with
-// ImportPaths or a similar loading function, nil is returned.
+// LoadPackages or ImportFromFiles, nil is returned.
 func PackageModuleInfo(pkgpath string) *modinfo.ModulePublic {
 	if isStandardImportPath(pkgpath) || !Enabled() {
 		return nil
@@ -250,8 +250,7 @@ func moduleInfo(ctx context.Context, m module.Version, fromBuildList, listRetrac
 
 // PackageBuildInfo returns a string containing module version information
 // for modules providing packages named by path and deps. path and deps must
-// name packages that were resolved successfully with ImportPaths or one of
-// the Load functions.
+// name packages that were resolved successfully with LoadPackages.
 func PackageBuildInfo(path string, deps []string) string {
 	if isStandardImportPath(path) || !Enabled() {
 		return ""
@@ -321,9 +320,8 @@ func mustFindModule(target, path string) module.Version {
 }
 
 // findModule searches for the module that contains the package at path.
-// If the package was loaded with ImportPaths or one of the other loading
-// functions, its containing module and true are returned. Otherwise,
-// module.Version{} and false are returend.
+// If the package was loaded, its containing module and true are returned.
+// Otherwise, module.Version{} and false are returend.
 func findModule(path string) (module.Version, bool) {
 	if pkg, ok := loaded.pkgCache.Get(path).(*loadPkg); ok {
 		return pkg.mod, pkg.mod != module.Version{}

src/cmd/go/internal/modload/buildlist.go

@@ -17,8 +17,8 @@ import (
 )
 
 // buildList is the list of modules to use for building packages.
-// It is initialized by calling ImportPaths, ImportFromFiles,
-// LoadALL, or LoadBuildList, each of which uses loaded.load.
+// It is initialized by calling LoadPackages or ImportFromFiles,
+// each of which uses loaded.load.
 //
 // Ideally, exactly ONE of those functions would be called,
 // and exactly once. Most of the time, that's true.
@@ -31,8 +31,8 @@ var buildList []module.Version
 // module pattern, starting with the Target module and in a deterministic
 // (stable) order, without loading any packages.
 //
-// Modules are loaded automatically (and lazily) in ImportPaths:
-// LoadAllModules need only be called if ImportPaths is not,
+// Modules are loaded automatically (and lazily) in LoadPackages:
+// LoadAllModules need only be called if LoadPackages is not,
 // typically in commands that care about modules but no particular package.
 //
 // The caller must not modify the returned list.
@@ -44,7 +44,7 @@ func LoadAllModules(ctx context.Context) []module.Version {
 }
 
 // LoadedModules returns the list of module requirements loaded or set by a
-// previous call (typically LoadAllModules or ImportPaths), starting with the
+// previous call (typically LoadAllModules or LoadPackages), starting with the
 // Target module and in a deterministic (stable) order.
 //
 // The caller must not modify the returned list.
@@ -71,8 +71,8 @@ func ReloadBuildList() []module.Version {
 }
 
 // TidyBuildList trims the build list to the minimal requirements needed to
-// retain the same versions of all packages from the preceding Load* or
-// ImportPaths* call.
+// retain the same versions of all packages from the preceding call to
+// LoadPackages.
 func TidyBuildList() {
 	used := map[module.Version]bool{Target: true}
 	for _, pkg := range loaded.pkgs {

src/cmd/go/internal/modload/init.go

@@ -83,12 +83,12 @@ const (
 
 // ModFile returns the parsed go.mod file.
 //
-// Note that after calling ImportPaths or LoadBuildList,
+// Note that after calling LoadPackages or LoadAllModules,
 // the require statements in the modfile.File are no longer
 // the source of truth and will be ignored: edits made directly
 // will be lost at the next call to WriteGoMod.
 // To make permanent changes to the require statements
-// in go.mod, edit it before calling ImportPaths or LoadBuildList.
+// in go.mod, edit it before loading.
 func ModFile() *modfile.File {
 	Init()
 	if modFile == nil {
@@ -943,9 +943,9 @@ func WriteGoMod() {
 
 // keepSums returns a set of module sums to preserve in go.sum. The set
 // includes entries for all modules used to load packages (according to
-// the last load function like ImportPaths, LoadALL, etc.). It also contains
-// entries for go.mod files needed for MVS (the version of these entries
-// ends with "/go.mod").
+// the last load function such as LoadPackages or ImportFromFiles).
+// It also contains entries for go.mod files needed for MVS (the version
+// of these entries ends with "/go.mod").
 //
 // If addDirect is true, the set also includes sums for modules directly
 // required by go.mod, as represented by the index, with replacements applied.
@@ -977,8 +977,7 @@ func keepSums(addDirect bool) map[module.Version]bool {
 	}
 	walk(Target)
 
-	// Add entries for modules that provided packages loaded with ImportPaths,
-	// LoadALL, or similar functions.
+	// Add entries for modules from which packages were loaded.
 	if loaded != nil {
 		for _, pkg := range loaded.pkgs {
 			m := pkg.mod