[release-branch.go1.19] 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 #60000.
Updates #56222.

Change-Id: Icd6acce348907621ae0b02dbeac04fb180353dcf
(cherry picked from CL 489075 and CL 492741)
Reviewed-on: https://go-review.googlesource.com/c/go/+/492983
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Bypass: Bryan Mills <bcmills@google.com>
This commit is contained in:
Bryan C. Mills 2023-04-26 01:43:20 -04:00 committed by Michael Knyszek
parent 65cc8e6ad8
commit 29f34697a4
17 changed files with 162 additions and 27 deletions

View File

@ -643,7 +643,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 {

View File

@ -248,7 +248,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,
@ -412,6 +418,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
}

View File

@ -1571,7 +1571,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,
@ -1593,6 +1594,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
@ -1648,6 +1659,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
}
}

View File

@ -842,6 +842,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.171.20).
skipImportModFiles bool
work *par.Queue
// reset on each iteration
@ -1022,6 +1026,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 {
@ -1402,7 +1410,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
}
@ -1613,7 +1621,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 {
@ -1701,7 +1709,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
}
@ -1960,7 +1968,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}

View File

@ -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,8 +583,6 @@ 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
@ -586,7 +593,7 @@ func goModSummary(m module.Version) (*modFileSummary, error) {
if HasModRoot() && cfg.BuildMod == "readonly" && !inWorkspaceMode() && 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})
}
}

View File

@ -0,0 +1,21 @@
example.com/generics v1.0.0
written by hand
-- .mod --
module example.com/generics
go 1.18
-- .info --
{"Version":"v1.0.0"}
-- go.mod --
module example.com/generics
go 1.18
-- generics.go --
package generics
type Int interface {
~int
}
func Bar() {}

View File

@ -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.

View File

@ -16,7 +16,8 @@ env GO111MODULE=auto
cd m
cp go.mod go.mod.orig
! go list -m all
stderr '^go: example.com/cmd@v1.1.0-doesnotexist: missing go.sum entry; to add it:\n\tgo mod download example.com/cmd$'
stderr '^go: example.com/cmd@v1.1.0-doesnotexist: 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/cmd@v1.1.0-doesnotexist: 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

View File

@ -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$'

View File

@ -21,7 +21,8 @@ env GO111MODULE=on
cd m
cp go.mod go.mod.orig
! go list -m all
stderr '^go: example.com/cmd@v1.1.0-doesnotexist: missing go.sum entry; to add it:\n\tgo mod download example.com/cmd$'
stderr '^go: example.com/cmd@v1.1.0-doesnotexist: 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/cmd@v1.1.0-doesnotexist: missing go.sum entry for go.mod file; to add it:\n\tgo mod download example.com/cmd$'
go run example.com/cmd/a@v1.0.0
stdout '^a@v1.0.0$'
cmp go.mod go.mod.orig

View File

@ -0,0 +1,67 @@
# 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.19 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/generics@v1.0.0: 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.19 or earlier, 'go mod tidy' shouldn't need go.mod files or
# checksums that it won't record.
go mod tidy -go=1.19
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.19
go list -m all
! stdout example.com/generics
go get -t example.com/m2/q@v1.0.0
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.19
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.19
require example.com/generics v1.0.0
-- m2/q/q.go --
package q
-- m2/q/q_test.go --
package q
import _ "example.com/generics"

View File

@ -4,7 +4,7 @@ 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/quote@v1.5.2: missing go.sum entry; to add it:\n\tgo mod download rsc.io/quote$'
stderr '^go: rsc.io/quote@v1.5.2: 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
@ -12,7 +12,7 @@ stderr '^go: rsc.io/quote@v1.5.2: missing go.sum entry; to add it:\n\tgo mod dow
# we should see the same error.
cp go.sum.h2only go.sum
! go list -m all
stderr '^go: rsc.io/quote@v1.5.2: missing go.sum entry; to add it:\n\tgo mod download rsc.io/quote$'
stderr '^go: rsc.io/quote@v1.5.2: 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/quote@v1.5.2=rsc.io/quote@v1.5.1
! go list -m all
stderr '^go: rsc.io/quote@v1.5.2 \(replaced by 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.2 \(replaced by rsc.io/quote@v1.5.1\): 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

View File

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

View File

@ -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 --

View File

@ -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...

View File

@ -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.

View File

@ -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/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$'
-- go.mod --