From a19a4dcb987d010b5ed0cecc4b377382f42a6ecc Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" <bcmills@google.com>
Date: Fri, 30 Oct 2020 22:13:51 -0400
Subject: [PATCH] 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>
---
 src/cmd/go/internal/mvs/mvs.go      | 51 ++++++++++++++++++++---------
 src/cmd/go/internal/mvs/mvs_test.go |  4 +--
 2 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/src/cmd/go/internal/mvs/mvs.go b/src/cmd/go/internal/mvs/mvs.go
index fe6d14e9dc..b630b610f1 100644
--- a/src/cmd/go/internal/mvs/mvs.go
+++ b/src/cmd/go/internal/mvs/mvs.go
@@ -108,19 +108,21 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) (m
 		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 @@ func Upgrade(target module.Version, reqs Reqs, upgrade ...module.Version) ([]mod
 	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 af1bb216a7..721cd9635c 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) Max(v1, v2 string) string {
 }
 
 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
 		}
 	}