cmd/go: don't attempt to downgrade to incompatible versions

When we downgrade a module (using 'go get m@none' or similar), we
exclude versions of other modules that depend on it. We'll try
previous versions (in the "versions" list returned by the proxy or in
codeRepo.Versions for vcs) until we find a version that doesn't
require an excluded module version.

If older versions of a module are broken for some reason,
mvs.Downgrade currently panics. With this change, we ignore versions
with errors during downgrade.

A frequent cause of this is incompatible v2+ versions. These are
common if a repository tagged v2.0.0 before migrating to modules, then
tagged v2.0.1 with a go.mod file later. v2.0.0 is incorrectly
considered part of the v2 module.

Fixes #31942

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

diff --git a/src/cmd/go/internal/mvs/mvs.go b/src/cmd/go/internal/mvs/mvs.go
index 90f8f269b571f4eb1ca3559b36ccd821c759f018..04273e733c69aa9168856e1a87fd559378f58e98 100644 (file)
--- a/src/cmd/go/internal/mvs/mvs.go
+++ b/src/cmd/go/internal/mvs/mvs.go
@@ -13,7 +13,6 @@ import (
        "sync"
        "sync/atomic"
 
-       "cmd/go/internal/base"
        "cmd/go/internal/module"
        "cmd/go/internal/par"
 )
@@ -118,7 +117,7 @@ func BuildList(target module.Version, reqs Reqs) ([]module.Version, error) {
        return buildList(target, reqs, nil)
 }
 
-func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) module.Version) ([]module.Version, error) {
+func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) (module.Version, error)) ([]module.Version, error) {
        // Explore work graph in parallel in case reqs.Required
        // does high-latency network operations.
        type modGraphNode struct {
@@ -133,6 +132,10 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) mo
                min      = map[string]string{} // maps module path to minimum required version
                haveErr  int32
        )
+       setErr := func(n *modGraphNode, err error) {
+               n.err = err
+               atomic.StoreInt32(&haveErr, 1)
+       }
 
        var work par.Work
        work.Add(target)
@@ -149,8 +152,7 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) mo
 
                required, err := reqs.Required(m)
                if err != nil {
-                       node.err = err
-                       atomic.StoreInt32(&haveErr, 1)
+                       setErr(node, err)
                        return
                }
                node.required = required
@@ -159,9 +161,9 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) mo
                }
 
                if upgrade != nil {
-                       u := upgrade(m)
-                       if u.Path == "" {
-                               base.Errorf("Upgrade(%v) returned zero module", m)
+                       u, err := upgrade(m)
+                       if err != nil {
+                               setErr(node, err)
                                return
                        }
                        if u != m {
@@ -332,17 +334,12 @@ func Req(target module.Version, list []module.Version, base []string, reqs Reqs)
 // UpgradeAll returns a build list for the target module
 // in which every module is upgraded to its latest version.
 func UpgradeAll(target module.Version, reqs Reqs) ([]module.Version, error) {
-       return buildList(target, reqs, func(m module.Version) module.Version {
+       return buildList(target, reqs, func(m module.Version) (module.Version, error) {
                if m.Path == target.Path {
-                       return target
+                       return target, nil
                }
 
-               latest, err := reqs.Upgrade(m)
-               if err != nil {
-                       panic(err) // TODO
-               }
-               m.Version = latest.Version
-               return m
+               return reqs.Upgrade(m)
        })
 }
 
@@ -351,7 +348,7 @@ func UpgradeAll(target module.Version, reqs Reqs) ([]module.Version, error) {
 func Upgrade(target module.Version, reqs Reqs, upgrade ...module.Version) ([]module.Version, error) {
        list, err := reqs.Required(target)
        if err != nil {
-               panic(err) // TODO
+               return nil, err
        }
        // TODO: Maybe if an error is given,
        // rerun with BuildList(upgrade[0], reqs) etc
@@ -370,7 +367,7 @@ func Upgrade(target module.Version, reqs Reqs, upgrade ...module.Version) ([]mod
 func Downgrade(target module.Version, reqs Reqs, downgrade ...module.Version) ([]module.Version, error) {
        list, err := reqs.Required(target)
        if err != nil {
-               panic(err) // TODO
+               return nil, err
        }
        max := make(map[string]string)
        for _, r := range list {
@@ -409,7 +406,17 @@ func Downgrade(target module.Version, reqs Reqs, downgrade ...module.Version) ([
                }
                list, err := reqs.Required(m)
                if err != nil {
-                       panic(err) // TODO
+                       // If we can't load the requirements, we couldn't load the go.mod file.
+                       // There are a number of reasons this can happen, but this usually
+                       // means an older version of the module had a missing or invalid
+                       // go.mod file. For example, if example.com/mod released v2.0.0 before
+                       // migrating to modules (v2.0.0+incompatible), then added a valid go.mod
+                       // in v2.0.1, downgrading from v2.0.1 would cause this error.
+                       //
+                       // TODO(golang.org/issue/31730, golang.org/issue/30134): if the error
+                       // is transient (we couldn't download go.mod), return the error from
+                       // Downgrade. Currently, we can't tell what kind of error it is.
+                       exclude(m)
                }
                for _, r := range list {
                        add(r)
@@ -429,7 +436,12 @@ List:
                for excluded[r] {
                        p, err := reqs.Previous(r)
                        if err != nil {
-                               return nil, err // TODO
+                               // This is likely a transient error reaching the repository,
+                               // rather than a permanent error with the retrieved version.
+                               //
+                               // TODO(golang.org/issue/31730, golang.org/issue/30134):
+                               // decode what to do based on the actual error.
+                               return nil, err
                        }
                        // If the target version is a pseudo-version, it may not be
                        // included when iterating over prior versions using reqs.Previous.

diff --git a/src/cmd/go/testdata/mod/example.com_downgrade_v2.0.0.txt b/src/cmd/go/testdata/mod/example.com_downgrade_v2.0.0.txt
new file mode 100644 (file)
index 0000000..88d50e5
--- /dev/null
+++ b/src/cmd/go/testdata/mod/example.com_downgrade_v2.0.0.txt
@@ -0,0 +1,9 @@
+example.com/downgrade v2.0.0
+written by hand
+
+-- .mod --
+module example.com/downgrade
+
+require rsc.io/quote v1.5.2
+-- .info --
+{"Version":"v2.0.0"}

diff --git a/src/cmd/go/testdata/mod/example.com_downgrade_v2_v2.0.1.txt b/src/cmd/go/testdata/mod/example.com_downgrade_v2_v2.0.1.txt
new file mode 100644 (file)
index 0000000..a4d665f
--- /dev/null
+++ b/src/cmd/go/testdata/mod/example.com_downgrade_v2_v2.0.1.txt
@@ -0,0 +1,13 @@
+example.com/downgrade/v2 v2.0.1
+written by hand
+
+-- .mod --
+module example.com/downgrade/v2
+
+require rsc.io/quote v1.5.2
+-- .info --
+{"Version":"v2.0.1"}
+-- go.mod --
+module example.com/downgrade/v2
+
+require rsc.io/quote v1.5.2

diff --git a/src/cmd/go/testdata/mod/example.com_latemigrate_v2_v2.0.0.txt b/src/cmd/go/testdata/mod/example.com_latemigrate_v2_v2.0.0.txt
new file mode 100644 (file)
index 0000000..25bd3d9
--- /dev/null
+++ b/src/cmd/go/testdata/mod/example.com_latemigrate_v2_v2.0.0.txt
@@ -0,0 +1,14 @@
+example.com/latemigrate/v2 v2.0.0
+written by hand
+
+This repository migrated to modules in v2.0.1 after v2.0.0 was already tagged.
+All versions require rsc.io/quote so we can test downgrades.
+
+v2.0.0 is technically part of example.com/latemigrate as v2.0.0+incompatible.
+Proxies may serve it as part of the version list for example.com/latemigrate/v2.
+'go get' must be able to ignore these versions.
+
+-- .mod --
+module example.com/latemigrate
+-- .info --
+{"Version":"v2.0.0"}

diff --git a/src/cmd/go/testdata/mod/example.com_latemigrate_v2_v2.0.1.txt b/src/cmd/go/testdata/mod/example.com_latemigrate_v2_v2.0.1.txt
new file mode 100644 (file)
index 0000000..be427a3
--- /dev/null
+++ b/src/cmd/go/testdata/mod/example.com_latemigrate_v2_v2.0.1.txt
@@ -0,0 +1,20 @@
+example.com/latemigrate/v2 v2.0.1
+written by hand
+
+This repository migrated to modules in v2.0.1 after v2.0.0 was already tagged.
+All versions require rsc.io/quote so we can test downgrades.
+
+v2.0.1 belongs to example.com/latemigrate/v2.
+
+-- .mod --
+module example.com/latemigrate/v2
+
+require rsc.io/quote v1.3.0
+-- .info --
+{"Version":"v2.0.1"}
+-- go.mod --
+module example.com/latemigrate/v2
+
+require rsc.io/quote v1.3.0
+-- late.go --
+package late

diff --git a/src/cmd/go/testdata/script/mod_get_downgrade.txt b/src/cmd/go/testdata/script/mod_get_downgrade.txt
index 00cd93e59884e9b40230800a806917c1163a85c7..ee9ac964751fe6b648fa77c879ce81089ec63a84 100644 (file)
--- a/src/cmd/go/testdata/script/mod_get_downgrade.txt
+++ b/src/cmd/go/testdata/script/mod_get_downgrade.txt
@@ -2,6 +2,7 @@ env GO111MODULE=on
 [short] skip
 
 # downgrade sampler should downgrade quote
+cp go.mod.orig go.mod
 go get rsc.io/sampler@v1.0.0
 go list -m all
 stdout 'rsc.io/quote v1.4.0'
@@ -31,9 +32,21 @@ stdout 'rsc.io/quote v1.4.0'
 stdout 'rsc.io/sampler v1.0.0'
 ! stdout golang.org/x/text
 
--- go.mod --
+# downgrading away quote should also downgrade away latemigrate/v2,
+# since there are no older versions. v2.0.0 is incompatible.
+cp go.mod.orig go.mod
+go list -m -versions example.com/latemigrate/v2
+stdout v2.0.0 # proxy may serve incompatible versions
+go get rsc.io/quote@none
+go list -m all
+! stdout 'example.com/latemigrate/v2'
+
+-- go.mod.orig --
 module x
-require rsc.io/quote v1.5.1
+require (
+       rsc.io/quote v1.5.1
+       example.com/latemigrate/v2 v2.0.1
+)
 -- go.mod.empty --
 module x
 -- x.go --