[release-branch.go1.20] cmd/go: save checksums for go.mod files needed for go version lines

When we load a package from a module, we need the go version line from
that module's go.mod file to know what language semantics to use for
the package. We need to save a checksum for the go.mod file even if
the module's requirements are pruned out of the module graph.
Previously, we were missing checksums for test dependencies of
packages in 'all' and packages passed to 'go get -t'.

This change preserves the existing bug for 'go mod tidy',
but fixes it for 'go get -t' and flags the missing checksum
with a clearer error in other cases.

Fixes #60001.
Updates #56222.

Change-Id: Icd6acce348907621ae0b02dbeac04fb180353dcf
(cherry picked from CL 489075 and CL 492741)
Reviewed-on: https://go-review.googlesource.com/c/go/+/493015
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>

Author: Bryan C. Mills

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

@@ -646,7 +646,7 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
 				} else {
 					pmain, ptest, pxtest, err = load.TestPackagesFor(ctx, pkgOpts, p, nil)
 					if err != nil {
-						base.Errorf("can't load test package: %s", err)
+						base.Errorf("go: can't load test package: %s", err)
 					}
 				}
 				if pmain != nil {

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

@@ -256,7 +256,13 @@ func (e *invalidImportError) Unwrap() error {
 // If the package is present in exactly one module, importFromModules will
 // return the module, its root directory, and a list of other modules that
 // lexically could have provided the package but did not.
-func importFromModules(ctx context.Context, path string, rs *Requirements, mg *ModuleGraph) (m module.Version, modroot, dir string, altMods []module.Version, err error) {
+//
+// If skipModFile is true, the go.mod file for the package is not loaded. This
+// allows 'go mod tidy' to preserve a minor checksum-preservation bug
+// (https://go.dev/issue/56222) for modules with 'go' versions between 1.17 and
+// 1.20, preventing unnecessary go.sum churn and network access in those
+// modules.
+func importFromModules(ctx context.Context, path string, rs *Requirements, mg *ModuleGraph, skipModFile bool) (m module.Version, modroot, dir string, altMods []module.Version, err error) {
 	invalidf := func(format string, args ...interface{}) (module.Version, string, string, []module.Version, error) {
 		return module.Version{}, "", "", nil, &invalidImportError{
 			importPath: path,
@@ -435,6 +441,18 @@ func importFromModules(ctx context.Context, path string, rs *Requirements, mg *M
 		}
 
 		if len(mods) == 1 {
+			// We've found the unique module containing the package.
+			// However, in order to actually compile it we need to know what
+			// Go language version to use, which requires its go.mod file.
+			//
+			// If the module graph is pruned and this is a test-only dependency
+			// of a package in "all", we didn't necessarily load that file
+			// when we read the module graph, so do it now to be sure.
+			if !skipModFile && cfg.BuildMod != "vendor" && mods[0].Path != "" && !MainModules.Contains(mods[0].Path) {
+				if _, err := goModSummary(mods[0]); err != nil {
+					return module.Version{}, "", "", nil, err
+				}
+			}
 			return mods[0], roots[0], dirs[0], altMods, nil
 		}
 

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

@@ -1583,7 +1583,8 @@ func commitRequirements(ctx context.Context) (err error) {
 // keepSums returns the set of modules (and go.mod file entries) for which
 // checksums would be needed in order to reload the same set of packages
 // loaded by the most recent call to LoadPackages or ImportFromFiles,
-// including any go.mod files needed to reconstruct the MVS result,
+// including any go.mod files needed to reconstruct the MVS result
+// or identify go versions,
 // in addition to the checksums for every module in keepMods.
 func keepSums(ctx context.Context, ld *loader, rs *Requirements, which whichSums) map[module.Version]bool {
 	// Every module in the full module graph contributes its requirements,
@@ -1605,6 +1606,16 @@ func keepSums(ctx context.Context, ld *loader, rs *Requirements, which whichSums
 				continue
 			}
 
+			// We need the checksum for the go.mod file for pkg.mod
+			// so that we know what Go version to use to compile pkg.
+			// However, we didn't do so before Go 1.21, and the bug is relatively
+			// minor, so we maintain the previous (buggy) behavior in 'go mod tidy' to
+			// avoid introducing unnecessary churn.
+			if !ld.Tidy || semver.Compare("v"+ld.GoVersion, tidyGoModSumVersionV) >= 0 {
+				r := resolveReplacement(pkg.mod)
+				keep[modkey(r)] = true
+			}
+
 			if rs.pruning == pruned && pkg.mod.Path != "" {
 				if v, ok := rs.rootSelected(pkg.mod.Path); ok && v == pkg.mod.Version {
 					// pkg was loaded from a root module, and because the main module has
@@ -1660,6 +1671,7 @@ func keepSums(ctx context.Context, ld *loader, rs *Requirements, which whichSums
 		if which == addBuildListZipSums {
 			for _, m := range mg.BuildList() {
 				r := resolveReplacement(m)
+				keep[modkey(r)] = true // we need the go version from the go.mod file to do anything useful with the zipfile
 				keep[r] = true
 			}
 		}

src/cmd/go/internal/modload/load.go

@@ -823,6 +823,10 @@ type loader struct {
 	// transitively *imported by* the packages and tests in the main module.)
 	allClosesOverTests bool
 
+	// skipImportModFiles indicates whether we may skip loading go.mod files
+	// for imported packages (as in 'go mod tidy' in Go 1.17–1.20).
+	skipImportModFiles bool
+
 	work *par.Queue
 
 	// reset on each iteration
@@ -1003,6 +1007,10 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader {
 			// version higher than the go.mod version adds nothing.
 			ld.TidyCompatibleVersion = ld.GoVersion
 		}
+
+		if semver.Compare("v"+ld.GoVersion, tidyGoModSumVersionV) < 0 {
+			ld.skipImportModFiles = true
+		}
 	}
 
 	if semver.Compare("v"+ld.GoVersion, narrowAllVersionV) < 0 && !ld.UseVendorAll {
@@ -1398,7 +1406,7 @@ func (ld *loader) updateRequirements(ctx context.Context) (changed bool, err err
 				//
 				// In some sense, we can think of this as ‘upgraded the module providing
 				// pkg.path from "none" to a version higher than "none"’.
-				if _, _, _, _, err = importFromModules(ctx, pkg.path, rs, nil); err == nil {
+				if _, _, _, _, err = importFromModules(ctx, pkg.path, rs, nil, ld.skipImportModFiles); err == nil {
 					changed = true
 					break
 				}
@@ -1609,7 +1617,7 @@ func (ld *loader) preloadRootModules(ctx context.Context, rootPkgs []string) (ch
 			// If the main module is tidy and the package is in "all" — or if we're
 			// lucky — we can identify all of its imports without actually loading the
 			// full module graph.
-			m, _, _, _, err := importFromModules(ctx, path, ld.requirements, nil)
+			m, _, _, _, err := importFromModules(ctx, path, ld.requirements, nil, ld.skipImportModFiles)
 			if err != nil {
 				var missing *ImportMissingError
 				if errors.As(err, &missing) && ld.ResolveMissingImports {
@@ -1697,7 +1705,7 @@ func (ld *loader) load(ctx context.Context, pkg *loadPkg) {
 	}
 
 	var modroot string
-	pkg.mod, modroot, pkg.dir, pkg.altMods, pkg.err = importFromModules(ctx, pkg.path, ld.requirements, mg)
+	pkg.mod, modroot, pkg.dir, pkg.altMods, pkg.err = importFromModules(ctx, pkg.path, ld.requirements, mg, ld.skipImportModFiles)
 	if pkg.dir == "" {
 		return
 	}
@@ -1956,7 +1964,7 @@ func (ld *loader) checkTidyCompatibility(ctx context.Context, rs *Requirements)
 
 		pkg := pkg
 		ld.work.Add(func() {
-			mod, _, _, _, err := importFromModules(ctx, pkg.path, rs, mg)
+			mod, _, _, _, err := importFromModules(ctx, pkg.path, rs, mg, ld.skipImportModFiles)
 			if mod != pkg.mod {
 				mismatches := <-mismatchMu
 				mismatches[pkg] = mismatch{mod: mod, err: err}

src/cmd/go/internal/modload/modfile.go

@@ -45,6 +45,13 @@ const (
 	// "// indirect" dependencies are added in a block separate from the direct
 	// ones. See https://golang.org/issue/45965.
 	separateIndirectVersionV = "v1.17"
+
+	// tidyGoModSumVersionV is the Go version (plus leading "v") at which
+	// 'go mod tidy' preserves go.mod checksums needed to build test dependencies
+	// of packages in "all", so that 'go test all' can be run without checksum
+	// errors.
+	// See https://go.dev/issue/56222.
+	tidyGoModSumVersionV = "v1.21"
 )
 
 // ReadModFile reads and parses the mod file at gomod. ReadModFile properly applies the
@@ -566,6 +573,8 @@ func goModSummary(m module.Version) (*modFileSummary, error) {
 		summary := &modFileSummary{
 			module: module.Version{Path: m.Path},
 		}
+
+		readVendorList(MainModules.mustGetSingleMainModule())
 		if vendorVersion[m.Path] != m.Version {
 			// This module is not vendored, so packages cannot be loaded from it and
 			// it cannot be relevant to the build.
@@ -574,19 +583,17 @@ func goModSummary(m module.Version) (*modFileSummary, error) {
 
 		// For every module other than the target,
 		// return the full list of modules from modules.txt.
-		readVendorList(MainModules.mustGetSingleMainModule())
-
 		// We don't know what versions the vendored module actually relies on,
 		// so assume that it requires everything.
 		summary.require = vendorList
 		return summary, nil
 	}
 
 	actual := resolveReplacement(m)
-	if HasModRoot() && cfg.BuildMod == "readonly" && !inWorkspaceMode() && actual.Version != "" {
+	if mustHaveSums() && actual.Version != "" {
 		key := module.Version{Path: actual.Path, Version: actual.Version + "/go.mod"}
 		if !modfetch.HaveSum(key) {
-			suggestion := fmt.Sprintf("; to add it:\n\tgo mod download %s", m.Path)
+			suggestion := fmt.Sprintf(" for go.mod file; to add it:\n\tgo mod download %s", m.Path)
 			return nil, module.VersionError(actual, &sumMissingError{suggestion: suggestion})
 		}
 	}

src/cmd/go/testdata/script/list_parse_err.txt

@@ -4,9 +4,9 @@ stderr '^p[/\\]b.go:2:2: expected ''package'', found ''EOF''$'
 ! go list -f '{{range .Imports}}{{.}} {{end}}' ./p
 stderr '^p[/\\]b.go:2:2: expected ''package'', found ''EOF''$'
 ! go list -test ./t
-stderr '^can''t load test package: t[/\\]t_test.go:8:1: expected declaration, found ʕ'
+stderr '^go: can''t load test package: t[/\\]t_test.go:8:1: expected declaration, found ʕ'
 ! go list -test -f '{{range .Imports}}{{.}} {{end}}' ./t
-stderr '^can''t load test package: t[/\\]t_test.go:8:1: expected declaration, found ʕ'
+stderr '^go: can''t load test package: t[/\\]t_test.go:8:1: expected declaration, found ʕ'
 
 # 'go list -e' should report imports, even if some files have parse errors
 # before the import block.

src/cmd/go/testdata/script/mod_install_pkg_version.txt

@@ -16,7 +16,8 @@ env GO111MODULE=auto
 cd m
 cp go.mod go.mod.orig
 ! go list -m all
-stderr '^go: example.com/[email protected]: missing go.sum entry; to add it:\n\tgo mod download example.com/cmd$'
+stderr '^go: example.com/[email protected]: reading http.*/mod/example.com/cmd/@v/v1.1.0-doesnotexist.info: 404 Not Found\n\tserver response: 404 page not found$'
+stderr '^go: example.com/[email protected]: missing go.sum entry for go.mod file; to add it:\n\tgo mod download example.com/cmd$'
 go install example.com/cmd/a@latest
 cmp go.mod go.mod.orig
 exists $GOPATH/bin/a$GOEXE

src/cmd/go/testdata/script/mod_list_sums.txt

@@ -29,4 +29,4 @@ stderr '^go: updates to go.sum needed, disabled by -mod=readonly$'
 #
 # TODO(#41297): This should not be an error either.
 ! go list -m -mod=readonly -versions rsc.io/sampler
-stderr '^go: rsc\.io/quote@v1\.5\.1: missing go\.sum entry; to add it:\n\tgo mod download rsc\.io/quote$'
+stderr '^go: rsc\.io/quote@v1\.5\.1: missing go\.sum entry for go.mod file; to add it:\n\tgo mod download rsc\.io/quote$'

src/cmd/go/testdata/script/mod_run_pkg_version.txt

@@ -21,7 +21,8 @@ env GO111MODULE=on
 cd m
 cp go.mod go.mod.orig
 ! go list -m all
-stderr '^go: example.com/[email protected]: missing go.sum entry; to add it:\n\tgo mod download example.com/cmd$'
+stderr '^go: example.com/[email protected]: reading http.*/mod/example\.com/cmd/@v/v1.1.0-doesnotexist.info: 404 Not Found\n\tserver response: 404 page not found$'
+stderr '^go: example.com/[email protected]: missing go.sum entry for go.mod file; to add it:\n\tgo mod download example.com/cmd$'
 go run example.com/cmd/[email protected]
 stdout '^[email protected]$'
 cmp go.mod go.mod.orig

src/cmd/go/testdata/script/mod_sum_issue56222.txt

@@ -0,0 +1,68 @@
+# Regression test for #56222: 'go get -t' and 'go mod tidy'
+# should save enough checksums to run 'go test' on the named
+# packages or any package in "all" respectively.
+
+
+# At go 1.20 or earlier, 'go mod tidy' should preserve the historical go.sum
+# contents, but 'go test' should flag the missing checksums (instead of trying
+# to build the test dependency with the wrong language version).
+
+cd m1
+go mod tidy
+! go test -o $devnull -c example.com/m2/q
+stderr '^# example.com/m2/q\n'..${/}m2${/}q${/}'q_test.go:3:8: example.com/[email protected]: missing go.sum entry for go.mod file; to add it:\n\tgo mod download example.com/generics$'
+
+go mod download -json example.com/generics
+go list -f '{{if eq .ImportPath "example.com/generics"}}{{.Module.GoVersion}}{{end}}' -deps -test example.com/m2/q
+stdout 1.18
+
+
+# Even at go 1.20 or earlier, 'go mod tidy' shouldn't need go.mod files or
+# checksums that it won't record.
+
+go mod tidy -go=1.20
+go clean -modcache  # Remove checksums from the module cache, so that only go.sum is used.
+
+env OLDSUMDB=$GOSUMDB
+env GOSUMDB=bad
+go mod tidy
+
+env GOSUMDB=$OLDSUMDB
+
+
+# Regardless of the go version in go.mod, 'go get -t' should fetch
+# enough checksums to run 'go test' on the named package.
+
+rm p
+go mod tidy -go=1.20
+go list -m all
+! stdout example.com/generics
+go get -t example.com/m2/[email protected]
+go list -f '{{if eq .ImportPath "example.com/generics"}}{{.Module.GoVersion}}{{end}}' -deps -test example.com/m2/q
+stdout 1.18
+[!short] go test -o $devnull -c example.com/m2/q
+
+
+-- m1/go.mod --
+module example.com/m1
+
+go 1.20
+
+require example.com/m2 v1.0.0
+replace example.com/m2 => ../m2
+-- m1/p/p.go --
+package p
+
+import _ "example.com/m2/q"
+-- m2/go.mod --
+module example.com/m2
+
+go 1.20
+
+require example.com/generics v1.0.0
+-- m2/q/q.go --
+package q
+-- m2/q/q_test.go --
+package q
+
+import _ "example.com/generics"

src/cmd/go/testdata/script/mod_sum_readonly.txt

@@ -4,15 +4,15 @@ env GO111MODULE=on
 # When a sum is needed to load the build list, we get an error for the
 # specific module. The .mod file is not downloaded, and go.sum is not written.
 ! go list -m all
-stderr '^go: rsc.io/[email protected]: missing go.sum entry; to add it:\n\tgo mod download rsc.io/quote$'
+stderr '^go: rsc.io/[email protected]: missing go.sum entry for go.mod file; to add it:\n\tgo mod download rsc.io/quote$'
 ! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.mod
 ! exists go.sum
 
 # If go.sum exists but contains hashes from an algorithm we don't know about,
 # we should see the same error.
 cp go.sum.h2only go.sum
 ! go list -m all
-stderr '^go: rsc.io/[email protected]: missing go.sum entry; to add it:\n\tgo mod download rsc.io/quote$'
+stderr '^go: rsc.io/[email protected]: missing go.sum entry for go.mod file; to add it:\n\tgo mod download rsc.io/quote$'
 ! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.mod
 cmp go.sum go.sum.h2only
 rm go.sum
@@ -21,7 +21,7 @@ rm go.sum
 cp go.mod go.mod.orig
 go mod edit -replace rsc.io/[email protected]=rsc.io/[email protected]
 ! go list -m all
-stderr '^go: rsc.io/[email protected] \(replaced by rsc.io/[email protected]\): missing go.sum entry; to add it:\n\tgo mod download rsc.io/quote$'
+stderr '^go: rsc.io/[email protected] \(replaced by rsc.io/[email protected]\): missing go.sum entry for go.mod file; to add it:\n\tgo mod download rsc.io/quote$'
 ! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.1.mod
 ! exists go.sum
 cp go.mod.orig go.mod

src/cmd/go/testdata/script/mod_tidy_compat.txt

@@ -50,7 +50,7 @@ cmp stdout m_all.txt
 
 go mod edit -go=1.16
 ! go list -m all
-stderr '^go: example.net/[email protected] requires\n\texample.com/[email protected]: missing go.sum entry; to add it:\n\tgo mod download example.com/version$'
+stderr '^go: example.net/[email protected] requires\n\texample.com/[email protected]: missing go.sum entry for go.mod file; to add it:\n\tgo mod download example.com/version$'
 
 
 -- go.mod --

src/cmd/go/testdata/script/mod_tidy_compat_ambiguous.txt

@@ -62,7 +62,7 @@ cmp stdout all-m.txt
 
 go mod edit -go=1.16
 ! go list -m all
-stderr '^go: example\.net/indirect@v0\.1\.0 requires\n\texample\.net/ambiguous@v0\.1\.0: missing go\.sum entry; to add it:\n\tgo mod download example\.net/ambiguous\n'
+stderr '^go: example\.net/indirect@v0\.1\.0 requires\n\texample\.net/ambiguous@v0\.1\.0: missing go\.sum entry for go\.mod file; to add it:\n\tgo mod download example\.net/ambiguous\n'
 
 
 -- go.mod --

src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt

@@ -45,14 +45,14 @@ go mod tidy -compat=1.17
 ! stderr .
 cmp go.mod go.mod.orig
 
-go list -deps -test -f $MODFMT all
-stdout '^example\.com/retract/incompatible v1\.0\.0$'
+go list -deps -test -f $MODFMT ./...
+stdout '^example.net/lazy v0.1.0$'
 
 go mod edit -go=1.16
-! go list -deps -test -f $MODFMT all
+! go list -deps -test -f $MODFMT ./...
 
 	# TODO(#46160): -count=1 instead of -count=2.
-stderr -count=2 '^go: example\.net/lazy@v0\.1\.0 requires\n\texample\.com/retract/incompatible@v1\.0\.0: missing go\.sum entry; to add it:\n\tgo mod download example\.com/retract/incompatible$'
+stderr -count=2 '^go: example\.net/lazy@v0\.1\.0 requires\n\texample\.com/retract/incompatible@v1\.0\.0: missing go\.sum entry for go\.mod file; to add it:\n\tgo mod download example\.com/retract/incompatible$'
 
 
 # If we combine a Go 1.16 go.sum file...
@@ -63,7 +63,7 @@ cp go.mod.orig go.mod
 
 # ...then Go 1.17 no longer works. 😞
 ! go list -deps -test -f $MODFMT all
-stderr -count=1 '^can''t load test package: lazy[/\\]lazy_test.go:3:8: missing go\.sum entry for module providing package example\.com/retract/incompatible \(imported by example\.net/lazy\); to add:\n\tgo get -t example.net/lazy@v0\.1\.0$'
+stderr -count=1 '^go: can''t load test package: lazy[/\\]lazy_test.go:3:8: missing go\.sum entry for module providing package example\.com/retract/incompatible \(imported by example\.net/lazy\); to add:\n\tgo get -t example.net/lazy@v0\.1\.0$'
 
 
 # However, if we take the union of the go.sum files...

src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt

@@ -49,7 +49,7 @@ cmp go.mod go.mod.orig
 go mod edit -go=1.16
 ! go list -f $MODFMT -deps ./...
 	# TODO(#46160): -count=1 instead of -count=2.
-stderr -count=2 '^go: example\.net/lazy@v0\.1\.0 requires\n\texample\.net/requireincompatible@v0\.1\.0 requires\n\texample\.com/retract/incompatible@v2\.0\.0\+incompatible: missing go.sum entry; to add it:\n\tgo mod download example.com/retract/incompatible$'
+stderr -count=2 '^go: example\.net/lazy@v0\.1\.0 requires\n\texample\.net/requireincompatible@v0\.1\.0 requires\n\texample\.com/retract/incompatible@v2\.0\.0\+incompatible: missing go.sum entry for go.mod file; to add it:\n\tgo mod download example.com/retract/incompatible$'
 
 
 # There are two ways for the module author to bring the two into alignment.

src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt

@@ -48,7 +48,7 @@ cmp stdout out-117.txt
 go mod edit -go=1.16
 ! go list -deps -test -f $MODFMT all
 	# TODO(#46160): -count=1 instead of -count=2.
-stderr -count=2 '^go: example.net/[email protected] requires\n\texample.com/retract/[email protected]: missing go.sum entry; to add it:\n\tgo mod download example.com/retract/incompatible$'
+stderr -count=2 '^go: example.net/[email protected] requires\n\texample.com/retract/[email protected]: missing go.sum entry for go.mod file; to add it:\n\tgo mod download example.com/retract/incompatible$'
 
 
 -- go.mod --