cmd/go/internal/mvs: recompute build list in Reqs before minimizing

modload.MinReqs was passing modload.buildList to mvs.Reqs explicitly,
apparently as an optimization. However, we do not always have the
invariant that modload.buildList is complete: in particular, 'go mod
tidy' begins by reducing modload.buildList to only the set of modules
that provide packages to the build, which may be substantially smaller
than the final build list.

Other operations, such as 'go mod graph', do not load the entire
import graph, and therefore call Reqs with the unreduced build list.

Since Reqs retains modules according to a post-order traversal of the
list, an incomplete list may produce a different traversal order — and
therefore a different minimal solution, when multiple minimal
solutions exist. That caused 'go mod tidy' to produce different output
from other 'go' subcommands when certain patterns of dependencies are
present.

Since passing in the build list is only an optimization anyway, remove
the parameter and recompute the actual (complete) list at the
beginning of mvs.Reqs itself. That way, it is guaranteed to be
complete and in canonical order.

Fixes #34086

Change-Id: I3101bb81a1853c4a5e773010da3e44d2d90a570c
Reviewed-on: https://go-review.googlesource.com/c/go/+/193397
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

1 files changed, 9 insertions, 4 deletions

diff --git a/src/cmd/go/internal/mvs/mvs.go b/src/cmd/go/internal/mvs/mvs.go
index 4e7a828c24..8855d44f21 100644
--- a/src/cmd/go/internal/mvs/mvs.go
+++ b/src/cmd/go/internal/mvs/mvs.go
@@ -250,10 +250,15 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) (m
 	return list, nil
 }
 
-// Req returns the minimal requirement list for the target module
-// that results in the given build list, with the constraint that all
-// module paths listed in base must appear in the returned list.
-func Req(target module.Version, list []module.Version, base []string, reqs Reqs) ([]module.Version, error) {
+// Req returns the minimal requirement list for the target module,
+// with the constraint that all module paths listed in base must
+// appear in the returned list.
+func Req(target module.Version, base []string, reqs Reqs) ([]module.Version, error) {
+	list, err := BuildList(target, reqs)
+	if err != nil {
+		return nil, err
+	}
+
 	// Note: Not running in parallel because we assume
 	// that list came from a previous operation that paged
 	// in all the requirements, so there's no I/O to overlap now.