Skip to content

Commit 06642d8

Browse files
author
Jay Conrod
committed
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 golang#31942 Change-Id: Icaa75c5c93f73f18a400c22f18a8cc603aa4011a Reviewed-on: https://go-review.googlesource.com/c/go/+/177337 Run-TryBot: Jay Conrod <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]>
1 parent 703fb66 commit 06642d8

6 files changed

Lines changed: 102 additions & 21 deletions

File tree

‎src/cmd/go/internal/mvs/mvs.go‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"sync"
1414
"sync/atomic"
1515

16-
"cmd/go/internal/base"
1716
"cmd/go/internal/module"
1817
"cmd/go/internal/par"
1918
)
@@ -118,7 +117,7 @@ func BuildList(target module.Version, reqs Reqs) ([]module.Version, error) {
118117
return buildList(target, reqs, nil)
119118
}
120119

121-
func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) module.Version) ([]module.Version, error) {
120+
func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) (module.Version, error)) ([]module.Version, error) {
122121
// Explore work graph in parallel in case reqs.Required
123122
// does high-latency network operations.
124123
type modGraphNode struct {
@@ -133,6 +132,10 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) mo
133132
min = map[string]string{} // maps module path to minimum required version
134133
haveErr int32
135134
)
135+
setErr := func(n *modGraphNode, err error) {
136+
n.err = err
137+
atomic.StoreInt32(&haveErr, 1)
138+
}
136139

137140
var work par.Work
138141
work.Add(target)
@@ -149,8 +152,7 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) mo
149152

150153
required, err := reqs.Required(m)
151154
if err != nil {
152-
node.err = err
153-
atomic.StoreInt32(&haveErr, 1)
155+
setErr(node, err)
154156
return
155157
}
156158
node.required = required
@@ -159,9 +161,9 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) mo
159161
}
160162

161163
if upgrade != nil {
162-
u := upgrade(m)
163-
if u.Path == "" {
164-
base.Errorf("Upgrade(%v) returned zero module", m)
164+
u, err := upgrade(m)
165+
if err != nil {
166+
setErr(node, err)
165167
return
166168
}
167169
if u != m {
@@ -332,17 +334,12 @@ func Req(target module.Version, list []module.Version, base []string, reqs Reqs)
332334
// UpgradeAll returns a build list for the target module
333335
// in which every module is upgraded to its latest version.
334336
func UpgradeAll(target module.Version, reqs Reqs) ([]module.Version, error) {
335-
return buildList(target, reqs, func(m module.Version) module.Version {
337+
return buildList(target, reqs, func(m module.Version) (module.Version, error) {
336338
if m.Path == target.Path {
337-
return target
339+
return target, nil
338340
}
339341

340-
latest, err := reqs.Upgrade(m)
341-
if err != nil {
342-
panic(err) // TODO
343-
}
344-
m.Version = latest.Version
345-
return m
342+
return reqs.Upgrade(m)
346343
})
347344
}
348345

@@ -351,7 +348,7 @@ func UpgradeAll(target module.Version, reqs Reqs) ([]module.Version, error) {
351348
func Upgrade(target module.Version, reqs Reqs, upgrade ...module.Version) ([]module.Version, error) {
352349
list, err := reqs.Required(target)
353350
if err != nil {
354-
panic(err) // TODO
351+
return nil, err
355352
}
356353
// TODO: Maybe if an error is given,
357354
// rerun with BuildList(upgrade[0], reqs) etc
@@ -370,7 +367,7 @@ func Upgrade(target module.Version, reqs Reqs, upgrade ...module.Version) ([]mod
370367
func Downgrade(target module.Version, reqs Reqs, downgrade ...module.Version) ([]module.Version, error) {
371368
list, err := reqs.Required(target)
372369
if err != nil {
373-
panic(err) // TODO
370+
return nil, err
374371
}
375372
max := make(map[string]string)
376373
for _, r := range list {
@@ -409,7 +406,17 @@ func Downgrade(target module.Version, reqs Reqs, downgrade ...module.Version) ([
409406
}
410407
list, err := reqs.Required(m)
411408
if err != nil {
412-
panic(err) // TODO
409+
// If we can't load the requirements, we couldn't load the go.mod file.
410+
// There are a number of reasons this can happen, but this usually
411+
// means an older version of the module had a missing or invalid
412+
// go.mod file. For example, if example.com/mod released v2.0.0 before
413+
// migrating to modules (v2.0.0+incompatible), then added a valid go.mod
414+
// in v2.0.1, downgrading from v2.0.1 would cause this error.
415+
//
416+
// TODO(golang.org/issue/31730, golang.org/issue/30134): if the error
417+
// is transient (we couldn't download go.mod), return the error from
418+
// Downgrade. Currently, we can't tell what kind of error it is.
419+
exclude(m)
413420
}
414421
for _, r := range list {
415422
add(r)
@@ -429,7 +436,12 @@ List:
429436
for excluded[r] {
430437
p, err := reqs.Previous(r)
431438
if err != nil {
432-
return nil, err // TODO
439+
// This is likely a transient error reaching the repository,
440+
// rather than a permanent error with the retrieved version.
441+
//
442+
// TODO(golang.org/issue/31730, golang.org/issue/30134):
443+
// decode what to do based on the actual error.
444+
return nil, err
433445
}
434446
// If the target version is a pseudo-version, it may not be
435447
// included when iterating over prior versions using reqs.Previous.

‎src/cmd/go/testdata/mod/example.com_downgrade_v2.0.0.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
example.com/downgrade v2.0.0
2+
written by hand
3+
4+
-- .mod --
5+
module example.com/downgrade
6+
7+
require rsc.io/quote v1.5.2
8+
-- .info --
9+
{"Version":"v2.0.0"}

‎src/cmd/go/testdata/mod/example.com_downgrade_v2_v2.0.1.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
example.com/downgrade/v2 v2.0.1
2+
written by hand
3+
4+
-- .mod --
5+
module example.com/downgrade/v2
6+
7+
require rsc.io/quote v1.5.2
8+
-- .info --
9+
{"Version":"v2.0.1"}
10+
-- go.mod --
11+
module example.com/downgrade/v2
12+
13+
require rsc.io/quote v1.5.2

‎src/cmd/go/testdata/mod/example.com_latemigrate_v2_v2.0.0.txt‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
example.com/latemigrate/v2 v2.0.0
2+
written by hand
3+
4+
This repository migrated to modules in v2.0.1 after v2.0.0 was already tagged.
5+
All versions require rsc.io/quote so we can test downgrades.
6+
7+
v2.0.0 is technically part of example.com/latemigrate as v2.0.0+incompatible.
8+
Proxies may serve it as part of the version list for example.com/latemigrate/v2.
9+
'go get' must be able to ignore these versions.
10+
11+
-- .mod --
12+
module example.com/latemigrate
13+
-- .info --
14+
{"Version":"v2.0.0"}

‎src/cmd/go/testdata/mod/example.com_latemigrate_v2_v2.0.1.txt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
example.com/latemigrate/v2 v2.0.1
2+
written by hand
3+
4+
This repository migrated to modules in v2.0.1 after v2.0.0 was already tagged.
5+
All versions require rsc.io/quote so we can test downgrades.
6+
7+
v2.0.1 belongs to example.com/latemigrate/v2.
8+
9+
-- .mod --
10+
module example.com/latemigrate/v2
11+
12+
require rsc.io/quote v1.3.0
13+
-- .info --
14+
{"Version":"v2.0.1"}
15+
-- go.mod --
16+
module example.com/latemigrate/v2
17+
18+
require rsc.io/quote v1.3.0
19+
-- late.go --
20+
package late

‎src/cmd/go/testdata/script/mod_get_downgrade.txt‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ env GO111MODULE=on
22
[short] skip
33

44
# downgrade sampler should downgrade quote
5+
cp go.mod.orig go.mod
56
go get rsc.io/[email protected]
67
go list -m all
78
stdout 'rsc.io/quote v1.4.0'
@@ -31,9 +32,21 @@ stdout 'rsc.io/quote v1.4.0'
3132
stdout 'rsc.io/sampler v1.0.0'
3233
! stdout golang.org/x/text
3334

34-
-- go.mod --
35+
# downgrading away quote should also downgrade away latemigrate/v2,
36+
# since there are no older versions. v2.0.0 is incompatible.
37+
cp go.mod.orig go.mod
38+
go list -m -versions example.com/latemigrate/v2
39+
stdout v2.0.0 # proxy may serve incompatible versions
40+
go get rsc.io/quote@none
41+
go list -m all
42+
! stdout 'example.com/latemigrate/v2'
43+
44+
-- go.mod.orig --
3545
module x
36-
require rsc.io/quote v1.5.1
46+
require (
47+
rsc.io/quote v1.5.1
48+
example.com/latemigrate/v2 v2.0.1
49+
)
3750
-- go.mod.empty --
3851
module x
3952
-- x.go --

0 commit comments

Comments
 (0)