]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: save checksums for go.mod files needed for go version lines
authorBryan C. Mills <bcmills@google.com>
Wed, 26 Apr 2023 05:43:20 +0000 (01:43 -0400)
committerGopher Robot <gobot@golang.org>
Fri, 5 May 2023 15:42:09 +0000 (15:42 +0000)
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' in older
module versions, but fixes it for 'go mod tidy' in modules at go 1.21
or higher and for 'go get -t' at all versions.

Fixes #56222.

Change-Id: Icd6acce348907621ae0b02dbeac04fb180353dcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/489075
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>

16 files changed:
src/cmd/go/internal/list/list.go
src/cmd/go/internal/modload/import.go
src/cmd/go/internal/modload/init.go
src/cmd/go/internal/modload/modfile.go
src/cmd/go/testdata/script/list_parse_err.txt
src/cmd/go/testdata/script/list_test_cycle.txt
src/cmd/go/testdata/script/mod_install_pkg_version.txt
src/cmd/go/testdata/script/mod_list_sums.txt
src/cmd/go/testdata/script/mod_run_pkg_version.txt
src/cmd/go/testdata/script/mod_sum_issue56222.txt [new file with mode: 0644]
src/cmd/go/testdata/script/mod_sum_readonly.txt
src/cmd/go/testdata/script/mod_tidy_compat.txt
src/cmd/go/testdata/script/mod_tidy_compat_ambiguous.txt
src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt
src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt
src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt

index be2dd60dff4ec1720aa0c6e261cc5baca261e7f4..4a45a2157d4ff9a7f9d808e4da69301f6b884f99 100644 (file)
@@ -649,7 +649,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.Fatalf("can't load test package: %s", err)
+                                               base.Fatalf("go: can't load test package: %s", err)
                                        }
                                }
                                testPackages = append(testPackages, testPackageSet{p, pmain, ptest, pxtest})
index a8b4a2d21fbfc9d8669a9e3ac221eb2b4214c429..ec1632b1751938d4efff058e65d30641314229de 100644 (file)
@@ -435,6 +435,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 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
                }
 
index c25b25f15c42a536e1bef95efc05f3f30603bdcd..bb458b791e33d2e56ef3b49864d1f2c3e21884e9 100644 (file)
@@ -1591,7 +1591,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,
@@ -1613,6 +1614,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
@@ -1668,6 +1679,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
                        }
                }
index 0e42292c015621f6c10b7c0a83513d57e936b56a..79ec530caad05cf054ca7913a9f298e27528cc5c 100644 (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
@@ -583,10 +590,10 @@ func goModSummary(m module.Version) (*modFileSummary, error) {
        }
 
        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})
                }
        }
index 3c5345801a233e12890ba61bd3c118071031f696..0a3eb02c045977f0b059e042ca88bd43b9bf85d5 100644 (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.
index 2ab8528926021e5fe9824110b11e999cbc4ed3c1..ea6379200787bf01dd552fbe7c481eb17f4adc0a 100644 (file)
@@ -14,7 +14,7 @@ cmp stderr wanterr.txt
 cmp stderr wanterr.txt
 
 -- wanterr.txt --
-can't load test package: package example/p
+go: can't load test package: package example/p
        imports example/r
        imports example/q: import cycle not allowed in test
 -- go.mod --
index e3f59fc15289d9999e3c8a0f1c9057ecfc14eb2a..53c3e4134b292b38e0a5169af89ccca427730c4e 100644 (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
index 6c2f57c2b2d88de231fdd05407f3d2daf5308b59..cf1c18b575f9299f918bcbb611fd5578493bfdd1 100644 (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$'
index c3a218d553d5ca19f8f8556d5f3ce71e9148b5d2..969852c1eee8e4706ed6e1b059e717c6649e2324 100644 (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
diff --git a/src/cmd/go/testdata/script/mod_sum_issue56222.txt b/src/cmd/go/testdata/script/mod_sum_issue56222.txt
new file mode 100644 (file)
index 0000000..aaffc7d
--- /dev/null
@@ -0,0 +1,74 @@
+# 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.
+
+# 'go mod tidy' in a module at go 1.21 or higher should preserve
+# checksums needed to run 'go test all'.
+cd m1
+go mod tidy
+
+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 all
+
+cat go.sum
+replace 'example.com/generics v1.0.0/go.mod' 'example.com/notgenerics v1.0.0/go.mod' go.sum
+
+! go list -f '{{if eq .ImportPath "example.com/generics"}}{{.Module.GoVersion}}{{end}}' -deps -test example.com/m2/q
+stderr '^go: can''t load test package: \.\.'${/}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
+stdout '"GoModSum":'
+go list -f '{{if eq .ImportPath "example.com/generics"}}{{.Module.GoVersion}}{{end}}' -deps -test example.com/m2/q
+stdout 1.18
+
+
+# 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).
+
+go mod tidy -go=1.20
+! go test -o $devnull -c all
+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
+
+
+# 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 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.21
+
+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.21
+
+require example.com/generics v1.0.0
+-- m2/q/q.go --
+package q
+-- m2/q/q_test.go --
+package q
+
+import _ "example.com/generics"
index 57c5bbeefdf0203c43f28b50d8294500b6dff7a7..c4260739e278de0017bacaece25650eeabed397b 100644 (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
index 18b297da60e1a2cb5d3b1ed1a6866f073947d539..724c83e14eeb582e6b3a84a27ba719dcd180d30b 100644 (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 --
index a45de5ad8cdcb8133d9bca070be5168707a31ba9..8f29f74875aaf3a57f79ea87cf4ec741380167ca 100644 (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 --
index 186a3f8e6720f5654bc34cda9ec4e04ba23ccd1b..8b5869780c2e77dd4d5a5ef5f29a1700f3cf7016 100644 (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...
index 11313f144c3a340f35da2cb037d3512ee39680dd..1fef4b629c6602d460dc3f0194bde3307eb7f221 100644 (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.
index 7c22fca6c0f4d15b0e27f1c0f11706efe7b11398..59926d06d69afcc7b5fc399b24a110cd8f999276 100644 (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 --