cmd/go/internal/mvs: in Upgrade, pass upgrades to buildList as upgrades

This has no impact on the resulting build list, but provides clearer
diagnostics if reqs.Required returns an error for one of the upgraded
modules.

For #37438

Change-Id: I5cd8f72a9b7b9a0b185e1a728f46fefbd2f09b4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/266897
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/src/cmd/go/internal/mvs/mvs.go b/src/cmd/go/internal/mvs/mvs.go
index fe6d14e..b630b61 100644
--- a/src/cmd/go/internal/mvs/mvs.go
+++ b/src/cmd/go/internal/mvs/mvs.go
@@ -108,19 +108,21 @@
 		node := &modGraphNode{m: m}
 		mu.Lock()
 		modGraph[m] = node
-		if v, ok := min[m.Path]; !ok || reqs.Max(v, m.Version) != v {
-			min[m.Path] = m.Version
+		if m.Version != "none" {
+			if v, ok := min[m.Path]; !ok || reqs.Max(v, m.Version) != v {
+				min[m.Path] = m.Version
+			}
 		}
 		mu.Unlock()
 
-		required, err := reqs.Required(m)
-		if err != nil {
-			setErr(node, err)
-			return
-		}
-		node.required = required
-		for _, r := range node.required {
-			if r.Version != "none" {
+		if m.Version != "none" {
+			required, err := reqs.Required(m)
+			if err != nil {
+				setErr(node, err)
+				return
+			}
+			node.required = required
+			for _, r := range node.required {
 				work.Add(r)
 			}
 		}
@@ -333,12 +335,31 @@
 	if err != nil {
 		return nil, err
 	}
-	// TODO: Maybe if an error is given,
-	// rerun with BuildList(upgrade[0], reqs) etc
-	// to find which ones are the buggy ones.
+
+	pathInList := make(map[string]bool, len(list))
+	for _, m := range list {
+		pathInList[m.Path] = true
+	}
 	list = append([]module.Version(nil), list...)
-	list = append(list, upgrade...)
-	return BuildList(target, &override{target, list, reqs})
+
+	upgradeTo := make(map[string]string, len(upgrade))
+	for _, u := range upgrade {
+		if !pathInList[u.Path] {
+			list = append(list, module.Version{Path: u.Path, Version: "none"})
+		}
+		if prev, dup := upgradeTo[u.Path]; dup {
+			upgradeTo[u.Path] = reqs.Max(prev, u.Version)
+		} else {
+			upgradeTo[u.Path] = u.Version
+		}
+	}
+
+	return buildList(target, &override{target, list, reqs}, func(m module.Version) (module.Version, error) {
+		if v, ok := upgradeTo[m.Path]; ok {
+			return module.Version{Path: m.Path, Version: v}, nil
+		}
+		return m, nil
+	})
 }
 
 // Downgrade returns a build list for the target module
diff --git a/src/cmd/go/internal/mvs/mvs_test.go b/src/cmd/go/internal/mvs/mvs_test.go
index af1bb21..721cd96 100644
--- a/src/cmd/go/internal/mvs/mvs_test.go
+++ b/src/cmd/go/internal/mvs/mvs_test.go
@@ -491,9 +491,9 @@
 }
 
 func (r reqsMap) Upgrade(m module.Version) (module.Version, error) {
-	var u module.Version
+	u := module.Version{Version: "none"}
 	for k := range r {
-		if k.Path == m.Path && u.Version < k.Version && !strings.HasSuffix(k.Version, ".hidden") {
+		if k.Path == m.Path && r.Max(u.Version, k.Version) == k.Version && !strings.HasSuffix(k.Version, ".hidden") {
 			u = k
 		}
 	}