cmd/go/internal/modload: fix up main-module checks from CL 334932

Some critical Version == "" checks were missing in mvs.go, causing
mvs.Req to fail to retain requirements provided by older versions of
main modules.

A few checks also ought to be rotated to put the less expensive
string-equality checks before the more expensive map lookups.

Fixes #48511

Change-Id: Ib8de9d49a6413660792c003866bfcf9ab7f82ee2
Reviewed-on: https://go-review.googlesource.com/c/go/+/368136
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index 2c48c3c44473f485450b1fa681eac5c1cab6633f..893cc92e39990be85e847c54b3aeeb2cfe29a651 100644 (file)
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -1124,7 +1124,7 @@ func (r *resolver) loadPackages(ctx context.Context, patterns []string, findPack
        }
 
        opts.AllowPackage = func(ctx context.Context, path string, m module.Version) error {
-               if m.Path == "" || m.Version == "" && modload.MainModules.Contains(m.Path) {
+               if m.Path == "" || m.Version == "" {
                        // Packages in the standard library and main modules are already at their
                        // latest (and only) available versions.
                        return nil

diff --git a/src/cmd/go/internal/modload/buildlist.go b/src/cmd/go/internal/modload/buildlist.go
index f4c1311af5067bd6826522741ac9318ae62110c2..4ce71fef5b833d5fbd27aafb44fdc856b05a5e5a 100644 (file)
--- a/src/cmd/go/internal/modload/buildlist.go
+++ b/src/cmd/go/internal/modload/buildlist.go
@@ -45,7 +45,7 @@ type Requirements struct {
        pruning modPruning
 
        // rootModules is the set of root modules of the graph, sorted and capped to
-       // length. It may contain duplicates, and may  contain multiple versions for a
+       // length. It may contain duplicates, and may contain multiple versions for a
        // given module path. The root modules of the groph are the set of main
        // modules in workspace mode, and the main module's direct requirements
        // outside workspace mode.

diff --git a/src/cmd/go/internal/modload/edit.go b/src/cmd/go/internal/modload/edit.go
index 023983caed47d1de9a2339373c72fb4a5f254192..0f37e3b2e94773e983fc70f161ea2c768d759833 100644 (file)
--- a/src/cmd/go/internal/modload/edit.go
+++ b/src/cmd/go/internal/modload/edit.go
@@ -76,7 +76,7 @@ func editRequirements(ctx context.Context, rs *Requirements, tryUpgrade, mustSel
                // requirements.
                var rootPaths []string
                for _, m := range mustSelect {
-                       if !MainModules.Contains(m.Path) && m.Version != "none" {
+                       if m.Version != "none" && !MainModules.Contains(m.Path) {
                                rootPaths = append(rootPaths, m.Path)
                        }
                }
@@ -370,7 +370,7 @@ func selectPotentiallyImportedModules(ctx context.Context, limiter *versionLimit
                if err != nil {
                        return nil, false, err
                }
-               initial = mg.BuildList()[1:]
+               initial = mg.BuildList()[MainModules.Len():]
        } else {
                initial = rs.rootModules
        }

diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index 27bbfb783284c755166ac92359bb7cac70ec6096..5e7075da4ecb11a28c0346ac2a38c10042a41e1e 100644 (file)
--- a/src/cmd/go/internal/modload/load.go
+++ b/src/cmd/go/internal/modload/load.go
@@ -1091,7 +1091,7 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader {
                        break
                }
                if changed {
-                       // Don't resolve missing imports until the module graph have stabilized.
+                       // Don't resolve missing imports until the module graph has stabilized.
                        // If the roots are still changing, they may turn out to specify a
                        // requirement on the missing package(s), and we would rather use a
                        // version specified by a new root than add a new dependency on an

diff --git a/src/cmd/go/internal/modload/mvs.go b/src/cmd/go/internal/modload/mvs.go
index 40224d534b1b129c3990ef9155aa9400c028c22d..588bcf4bdc26453b41e53d2562ecbe66088ee0f3 100644 (file)
--- a/src/cmd/go/internal/modload/mvs.go
+++ b/src/cmd/go/internal/modload/mvs.go
@@ -42,7 +42,7 @@ type mvsReqs struct {
 }
 
 func (r *mvsReqs) Required(mod module.Version) ([]module.Version, error) {
-       if MainModules.Contains(mod.Path) {
+       if mod.Version == "" && MainModules.Contains(mod.Path) {
                // Use the build list as it existed when r was constructed, not the current
                // global build list.
                return r.roots, nil
@@ -108,12 +108,12 @@ func versions(ctx context.Context, path string, allowed AllowedFunc) ([]string,
 // previousVersion returns the tagged version of m.Path immediately prior to
 // m.Version, or version "none" if no prior version is tagged.
 //
-// Since the version of Target is not found in the version list,
+// Since the version of a main module is not found in the version list,
 // it has no previous version.
 func previousVersion(m module.Version) (module.Version, error) {
        // TODO(golang.org/issue/38714): thread tracing context through MVS.
 
-       if MainModules.Contains(m.Path) {
+       if m.Version == "" && MainModules.Contains(m.Path) {
                return module.Version{Path: m.Path, Version: "none"}, nil
        }
 

diff --git a/src/cmd/go/testdata/script/mod_get_issue48511.txt b/src/cmd/go/testdata/script/mod_get_issue48511.txt
new file mode 100644 (file)
index 0000000..0ba486d
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_issue48511.txt
@@ -0,0 +1,68 @@
+# Regression test for https://golang.org/issue/48511:
+# requirement minimization was accidentally replacing previous
+# versions of the main module, causing dependencies to be
+# spuriously dropping during requirement minimization and
+# leading to an infinite loop.
+
+cp go.mod go.mod.orig
+go mod tidy
+cmp go.mod go.mod.orig
+
+go get -u=patch ./...
+cmp go.mod go.mod.want
+
+-- go.mod --
+module example.net/m
+
+go 1.16
+
+replace (
+       example.net/a v0.1.0 => ./a
+       example.net/b v0.1.0 => ./b
+       example.net/b v0.1.1 => ./b
+       example.net/m v0.1.0 => ./m1
+)
+
+require example.net/a v0.1.0
+-- go.mod.want --
+module example.net/m
+
+go 1.16
+
+replace (
+       example.net/a v0.1.0 => ./a
+       example.net/b v0.1.0 => ./b
+       example.net/b v0.1.1 => ./b
+       example.net/m v0.1.0 => ./m1
+)
+
+require (
+       example.net/a v0.1.0
+       example.net/b v0.1.1 // indirect
+)
+-- m.go --
+package m
+
+import "example.net/a"
+-- m1/go.mod --
+module example.net/m
+
+go 1.16
+
+require example.net/b v0.1.0
+-- a/go.mod --
+module example.net/a
+
+go 1.16
+
+require example.net/m v0.1.0
+-- a/a.go --
+package a
+
+import "example.net/b"
+-- b/go.mod --
+module example.net/b
+
+go 1.16
+-- b/b.go --
+package b