cmd/go/internal/modload: break more cycles in readModGraph

Before CL 471595, modload.readModGraph in module with graph pruning
enabled only ever chased down transitive dependencies of unpruned
roots, so pruned dependencies couldn't cause cycles and we didn't
need to dedup them in the loading queue.

However, in 'go get' we are now passing in a set of upgraded modules
to unprune, and those upgraded modules can potentially contain cycles,
leading to an infinite loop during loading.

We have two options for a fix: we could either drop the 'unprune'
check in the enqueue operation (and instead expand the 'unprune'
requirements in a separate pass, as we do in workspace mode), or we
could check for cycles for all modules (not just the ones that are
naturally unpruned). The latter option makes it clearer that this
process must terminate, so we choose that.

(It may be possible to clean up and simplify the workspace-mode case
now that we are passing in the 'unprune' map, but for now we're
looking for a minimal fix for the Go 1.21 release.)

Fixes #60490.

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

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

@@ -315,9 +315,13 @@ func readModGraph(ctx context.Context, pruning modPruning, roots []module.Versio
 		mg.g.Require(MainModules.mustGetSingleMainModule(), roots)
 	}
 
+	type dedupKey struct {
+		m       module.Version
+		pruning modPruning
+	}
 	var (
-		loadQueue       = par.NewQueue(runtime.GOMAXPROCS(0))
-		loadingUnpruned sync.Map // module.Version → nil; the set of modules that have been or are being loaded via roots that do not support pruning
+		loadQueue = par.NewQueue(runtime.GOMAXPROCS(0))
+		loading   sync.Map // dedupKey → nil; the set of modules that have been or are being loaded
 	)
 
 	// loadOne synchronously loads the explicit requirements for module m.
@@ -345,13 +349,11 @@ func readModGraph(ctx context.Context, pruning modPruning, roots []module.Versio
 			return
 		}
 
-		if pruning == unpruned {
-			if _, dup := loadingUnpruned.LoadOrStore(m, nil); dup {
-				// m has already been enqueued for loading. Since unpruned loading may
-				// follow cycles in the requirement graph, we need to return early
-				// to avoid making the load queue infinitely long.
-				return
-			}
+		if _, dup := loading.LoadOrStore(dedupKey{m, pruning}, nil); dup {
+			// m has already been enqueued for loading. Since unpruned loading may
+			// follow cycles in the requirement graph, we need to return early
+			// to avoid making the load queue infinitely long.
+			return
 		}
 
 		loadQueue.Add(func() {

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

@@ -0,0 +1,48 @@
+# Regression test for https://go.dev/issue/60490: 'go get' should not cause an
+# infinite loop for cycles introduced in the pruned module graph.
+
+go get example.net/c@v0.1.0
+
+-- go.mod --
+module example
+
+go 1.19
+
+require (
+	example.net/a v0.1.0
+	example.net/b v0.1.0
+)
+
+replace (
+	example.net/a v0.1.0 => ./a1
+	example.net/a v0.2.0 => ./a2
+	example.net/b v0.1.0 => ./b1
+	example.net/b v0.2.0 => ./b2
+	example.net/c v0.1.0 => ./c1
+)
+-- a1/go.mod --
+module example.net/a
+
+go 1.19
+-- a2/go.mod --
+module example.net/a
+
+go 1.19
+
+require example.net/b v0.2.0
+-- b1/go.mod --
+module example.net/b
+
+go 1.19
+-- b2/go.mod --
+module example.net/b
+
+go 1.19
+
+require example.net/a v0.2.0
+-- c1/go.mod --
+module example.net/c
+
+go 1.19
+
+require example.net/a v0.2.0