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 fe6d14e9dcb9aacf1a450907bf63405fbd07c388..b630b610f10e03641362acf7b385f34781e58cf5 100644 (file)
--- 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 af1bb216a7f9c59fd56cb5e0ae5548e2dca7773a..721cd9635c8d84dabead835817dfa9f817b97fc2 100644 (file)
--- 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
                }
        }