cmd/go/internal/vgo: change get -u to avoid downgrades, prereleases
Module requirements should typically be stated in terms of tagged
versions, like v1.2.3. But for both bootstrapping and debugging, it is
also possible to refer to specific commits using pseudoversions of the
form v0.0.0-yyyymmddhhmmss-abcdefabcdef, where abcdefabcdef is a
revision identifier (a commit hash for git repos).
These pseudoversions are valid semantic versions but chosen so that
they sort below any tagged version: if one part of a build wants a
tagged version and another part wants a pseudoversion, we explicitly
want the tagged version to take priority, so that a dependency deep in
your build cannot force it into compiling against a random commit
instead of a released (tagged) commit.
But this same fact, that pseudoversions sort before tagged versions,
confuses "vgo get -u". If the build is using a pseudoversion of a
commit from earlier today, it makes no sense for "vgo get -u" to
"update" to a latest tagged version that happens to be from last week.
This CL changes "vgo get -u" to avoid this kind of accidental downgrade.
But if the latest tagged version was committed after the current
pseudoversion, then it's OK to upgrade to that tagged version.
Now that we can make the effect of "vgo get -u" context-dependent,
treat prereleases similarly: never choose to upgrade to a prerelease,
but if we're using a prerelease newer than the latest tagged
non-prerelease, stay there instead of downgrading.
This fixes "vgo get -u" for many real repos, especially ones that
must use pseudoversions to get at, say, non-module v17.0.0
of a dependency. Without this logic, "vgo get -u" would rewind
to the latest v1, which might be much older.
Change-Id: Ie9656b78dc6cc326048921898288dcf588441bf5
Reviewed-on: https://go-review.googlesource.com/121000
Reviewed-by: Bryan C. Mills <bcmills@google.com>
diff --git a/vendor/cmd/go/internal/modfetch/cache.go b/vendor/cmd/go/internal/modfetch/cache.go
index 1f6551e..cc785ec 100644
--- a/vendor/cmd/go/internal/modfetch/cache.go
+++ b/vendor/cmd/go/internal/modfetch/cache.go
@@ -250,7 +250,7 @@
}
suffix := "-" + rev + ".info"
for _, name := range names {
- if strings.HasSuffix(name, suffix) && isPseudoVersion(strings.TrimSuffix(name, ".info")) {
+ if strings.HasSuffix(name, suffix) && IsPseudoVersion(strings.TrimSuffix(name, ".info")) {
return readDiskStat(path, strings.TrimSuffix(name, ".info"))
}
}
diff --git a/vendor/cmd/go/internal/modfetch/coderepo.go b/vendor/cmd/go/internal/modfetch/coderepo.go
index 9a60640..4c93e58 100644
--- a/vendor/cmd/go/internal/modfetch/coderepo.go
+++ b/vendor/cmd/go/internal/modfetch/coderepo.go
@@ -107,7 +107,7 @@
if r.codeDir != "" {
v = v[len(r.codeDir)+1:]
}
- if !semver.IsValid(v) || v != semver.Canonical(v) || isPseudoVersion(v) || !module.MatchPathMajor(v, r.pathMajor) {
+ if !semver.IsValid(v) || v != semver.Canonical(v) || IsPseudoVersion(v) || !module.MatchPathMajor(v, r.pathMajor) {
continue
}
list = append(list, v)
@@ -141,7 +141,7 @@
func (r *codeRepo) convert(info *codehost.RevInfo) (*RevInfo, error) {
versionOK := func(v string) bool {
- return semver.IsValid(v) && v == semver.Canonical(v) && !isPseudoVersion(v) && module.MatchPathMajor(v, r.pathMajor)
+ return semver.IsValid(v) && v == semver.Canonical(v) && !IsPseudoVersion(v) && module.MatchPathMajor(v, r.pathMajor)
}
v := info.Version
if r.codeDir == "" {
@@ -168,7 +168,7 @@
func (r *codeRepo) revToRev(rev string) string {
if semver.IsValid(rev) {
- if isPseudoVersion(rev) {
+ if IsPseudoVersion(rev) {
i := strings.Index(rev, "-")
j := strings.Index(rev[i+1:], "-")
return rev[i+1+j+1:]
@@ -526,6 +526,23 @@
var pseudoVersionRE = regexp.MustCompile(`^v[0-9]+\.0\.0-[0-9]{14}-[A-Za-z0-9]+$`)
-func isPseudoVersion(v string) bool {
+// IsPseudoVersion reports whether v is a pseudo-version.
+func IsPseudoVersion(v string) bool {
return pseudoVersionRE.MatchString(v)
}
+
+// PseudoVersionTime returns the time stamp of the pseudo-version v.
+// It returns an error if v is not a pseudo-version or if the time stamp
+// embedded in the pseudo-version is not a valid time.
+func PseudoVersionTime(v string) (time.Time, error) {
+ if !IsPseudoVersion(v) {
+ return time.Time{}, fmt.Errorf("not a pseudo-version")
+ }
+ i := strings.Index(v, "-") + 1
+ j := i + strings.Index(v[i:], "-")
+ t, err := time.Parse("20060102150405", v[i:j])
+ if err != nil {
+ return time.Time{}, fmt.Errorf("malformed pseudo-version %q", v)
+ }
+ return t, nil
+}
diff --git a/vendor/cmd/go/internal/modfetch/query.go b/vendor/cmd/go/internal/modfetch/query.go
index b5b8813..5902d5e 100644
--- a/vendor/cmd/go/internal/modfetch/query.go
+++ b/vendor/cmd/go/internal/modfetch/query.go
@@ -15,24 +15,25 @@
// The module must be a complete module path.
// The version must take one of the following forms:
//
-// - the literal string "latest", denoting the latest available tagged version
+// - the literal string "latest", denoting the latest available, allowed tagged version,
+// with non-prereleases preferred over prereleases
// - v1.2.3, a semantic version string
-// - v1 or v1.2, an abbreviated semantic version string completed by adding zeroes (v1.0.0 or v1.2.0);
-// - >v1.2.3, denoting the earliest available version after v1.2.3
-// - <v1.2.3, denoting the latest available version before v1.2.3
-// - an RFC 3339 time stamp, denoting the latest available version at that time
-// - a Unix time expressed as seconds since 1970, denoting the latest available version at that time
+// - v1 or v1.2, an abbreviated semantic version string completed by adding zeroes (v1.0.0 or v1.2.0)
+// - >v1.2.3, denoting the earliest available version after v1.2.3 (including prereleases)
+// - <v1.2.3, denoting the latest available version before v1.2.3 (including prereleases)
// - a repository commit identifier, denoting that version
//
-// The time stamps can be followed by an optional @branch suffix to limit the
-// result to revisions on a particular branch name.
+// If the allowed function is non-nil, Query excludes any versions for which allowed returns false.
//
func Query(path, vers string, allowed func(module.Version) bool) (*RevInfo, error) {
+ if allowed == nil {
+ allowed = func(module.Version) bool { return true }
+ }
if semver.IsValid(vers) {
// TODO: This turns query for "v2" into Stat "v2.0.0",
// but probably it should allow checking for a branch named "v2".
vers = semver.Canonical(vers)
- if allowed != nil && !allowed(module.Version{Path: path, Version: vers}) {
+ if !allowed(module.Version{Path: path, Version: vers}) {
return nil, fmt.Errorf("%s@%s excluded", path, vers)
}
@@ -68,27 +69,33 @@
return repo.Latest()
}
if vers == "latest" {
+ // Prefer a proper (non-prerelease) release.
for i := len(versions) - 1; i >= 0; i-- {
- if allowed == nil || allowed(module.Version{Path: path, Version: versions[i]}) {
+ if semver.Prerelease(versions[i]) == "" && allowed(module.Version{Path: path, Version: versions[i]}) {
+ return repo.Stat(versions[i])
+ }
+ }
+ // Fall back to pre-releases if that's all we have.
+ for i := len(versions) - 1; i >= 0; i-- {
+ if semver.Prerelease(versions[i]) != "" && allowed(module.Version{Path: path, Version: versions[i]}) {
return repo.Stat(versions[i])
}
}
} else if op == "<" {
for i := len(versions) - 1; i >= 0; i-- {
- if semver.Compare(versions[i], vers) < 0 && (allowed == nil || allowed(module.Version{Path: path, Version: versions[i]})) {
+ if semver.Compare(versions[i], vers) < 0 && allowed(module.Version{Path: path, Version: versions[i]}) {
return repo.Stat(versions[i])
}
}
} else {
for i := 0; i < len(versions); i++ {
- if semver.Compare(versions[i], vers) > 0 && (allowed == nil || allowed(module.Version{Path: path, Version: versions[i]})) {
+ if semver.Compare(versions[i], vers) > 0 && allowed(module.Version{Path: path, Version: versions[i]}) {
return repo.Stat(versions[i])
}
}
}
return nil, fmt.Errorf("no matching versions for %s%s", op, vers)
}
- // TODO: Time queries, maybe.
return repo.Stat(vers)
}
diff --git a/vendor/cmd/go/internal/mvs/mvs.go b/vendor/cmd/go/internal/mvs/mvs.go
index 0b53689..a16eec0 100644
--- a/vendor/cmd/go/internal/mvs/mvs.go
+++ b/vendor/cmd/go/internal/mvs/mvs.go
@@ -40,12 +40,18 @@
// and similarly v1 <= v2 can be written Max(v1, v2) == v2.
Max(v1, v2 string) string
- // Latest returns the latest known version of the module at path
- // (the one to use during UpgradeAll).
+ // Upgrade returns the upgraded version of m,
+ // for use during an UpgradeAll operation.
+ // If m should be kept as is, Upgrade returns m.
+ // If m is not yet used in the build, then m.Version will be "none".
+ // More typically, m.Version will be the version required
+ // by some other module in the build.
//
- // Latest never returns version "none": if no module exists at the given path,
- // it returns a non-nil error instead.
- Latest(path string) (module.Version, error)
+ // If no module version is available for the given path,
+ // Upgrade returns a non-nil error.
+ // TODO(rsc): Upgrade must be able to return errors,
+ // but should "no latest version" just return m instead?
+ Upgrade(m module.Version) (module.Version, error)
// Previous returns the version of m.Path immediately prior to m.Version,
// or "none" if no such version is known.
@@ -217,7 +223,7 @@
return target
}
- latest, err := reqs.Latest(m.Path)
+ latest, err := reqs.Upgrade(m)
if err != nil {
panic(err) // TODO
}
diff --git a/vendor/cmd/go/internal/mvs/mvs_test.go b/vendor/cmd/go/internal/mvs/mvs_test.go
index b6fb542..d52edd5 100644
--- a/vendor/cmd/go/internal/mvs/mvs_test.go
+++ b/vendor/cmd/go/internal/mvs/mvs_test.go
@@ -166,6 +166,30 @@
build A: A B1 C1 D1
upgrade* A: A B2 C2
+name: up1
+A: B1 C1
+B1:
+B2:
+B3:
+B4:
+B5.hidden:
+C2:
+C3:
+build A: A B1 C1
+upgrade* A: A B4 C3
+
+name: up2
+A: B5.hidden C1
+B1:
+B2:
+B3:
+B4:
+B5.hidden:
+C2:
+C3:
+build A: A B5.hidden C1
+upgrade* A: A B5.hidden C3
+
name: down1
A: B2
B1: C1
@@ -378,17 +402,17 @@
return v1
}
-func (r reqsMap) Latest(path string) (module.Version, error) {
- var m module.Version
+func (r reqsMap) Upgrade(m module.Version) (module.Version, error) {
+ var u module.Version
for k := range r {
- if k.Path == path && m.Version < k.Version {
- m = k
+ if k.Path == m.Path && u.Version < k.Version && !strings.HasSuffix(k.Version, ".hidden") {
+ u = k
}
}
- if m.Path == "" {
- return module.Version{}, &MissingModuleError{module.Version{Path: path, Version: ""}}
+ if u.Path == "" {
+ return module.Version{}, &MissingModuleError{module.Version{Path: m.Path, Version: ""}}
}
- return m, nil
+ return u, nil
}
func (r reqsMap) Previous(m module.Version) (module.Version, error) {
diff --git a/vendor/cmd/go/internal/semver/semver.go b/vendor/cmd/go/internal/semver/semver.go
index ecc5300..42760f7 100644
--- a/vendor/cmd/go/internal/semver/semver.go
+++ b/vendor/cmd/go/internal/semver/semver.go
@@ -69,6 +69,28 @@
return v[:1+len(pv.major)]
}
+// Prerelease returns the prerelease suffix of the semantic version v.
+// For example, Prerelease("v2.1.0-pre+meta") == "-pre".
+// If v is an invalid semantic version string, Prerelease returns the empty string.
+func Prerelease(v string) string {
+ pv, ok := parse(v)
+ if !ok {
+ return ""
+ }
+ return pv.prerelease
+}
+
+// Build returns the build suffix of the semantic version v.
+// For example, Build("v2.1.0+meta") == "+meta".
+// If v is an invalid semantic version string, Build returns the empty string.
+func Build(v string) string {
+ pv, ok := parse(v)
+ if !ok {
+ return ""
+ }
+ return pv.build
+}
+
// Compare returns an integer comparing two versions according to
// according to semantic version precedence.
// The result will be 0 if v == w, -1 if v < w, or +1 if v > w.
diff --git a/vendor/cmd/go/internal/semver/semver_test.go b/vendor/cmd/go/internal/semver/semver_test.go
index 7a697f6..6c9a735 100644
--- a/vendor/cmd/go/internal/semver/semver_test.go
+++ b/vendor/cmd/go/internal/semver/semver_test.go
@@ -42,6 +42,7 @@
{"v1.2.3-zzz", "v1.2.3-zzz"},
{"v1.2.3", "v1.2.3"},
{"v1.2.3+meta", "v1.2.3"},
+ {"v1.2.3+meta-pre", "v1.2.3"},
}
func TestIsValid(t *testing.T) {
@@ -75,6 +76,36 @@
}
}
+func TestPrerelease(t *testing.T) {
+ for _, tt := range tests {
+ pre := Prerelease(tt.in)
+ var want string
+ if tt.out != "" {
+ if i := strings.Index(tt.out, "-"); i >= 0 {
+ want = tt.out[i:]
+ }
+ }
+ if pre != want {
+ t.Errorf("Prerelease(%q) = %q, want %q", tt.in, pre, want)
+ }
+ }
+}
+
+func TestBuild(t *testing.T) {
+ for _, tt := range tests {
+ build := Build(tt.in)
+ var want string
+ if tt.out != "" {
+ if i := strings.Index(tt.in, "+"); i >= 0 {
+ want = tt.in[i:]
+ }
+ }
+ if build != want {
+ t.Errorf("Build(%q) = %q, want %q", tt.in, build, want)
+ }
+ }
+}
+
func TestCompare(t *testing.T) {
for i, ti := range tests {
for j, tj := range tests {
diff --git a/vendor/cmd/go/internal/vgo/load.go b/vendor/cmd/go/internal/vgo/load.go
index 65a9dc0..62d24bf 100644
--- a/vendor/cmd/go/internal/vgo/load.go
+++ b/vendor/cmd/go/internal/vgo/load.go
@@ -431,12 +431,6 @@
if err != nil {
return cached{nil, err}
}
- if *getU {
- for i := range list {
- list[i].Version = "none"
- }
- return cached{list, nil}
- }
for i, mv := range list {
for excluded[mv] {
mv1, err := r.next(mv)
@@ -517,6 +511,9 @@
return nil, fmt.Errorf("parsing downloaded go.mod: %v", err)
}
+ if f.Module == nil {
+ return nil, fmt.Errorf("%v@%v go.mod: missing module line", mod.Path, mod.Version)
+ }
if mpath := f.Module.Mod.Path; mpath != origPath && mpath != mod.Path {
return nil, fmt.Errorf("downloaded %q and got module %q", mod.Path, mpath)
}
@@ -541,21 +538,38 @@
return v1
}
-// Latest returns the latest tagged version of the module at path,
-// or the latest untagged version if no version is tagged.
-func (*mvsReqs) Latest(path string) (module.Version, error) {
+// Upgrade returns the desired upgrade for m.
+// If m is a tagged version, then Upgrade returns the latest tagged version.
+// If m is a pseudo-version, then Upgrade returns the latest tagged version
+// when that version has a time-stamp newer than m.
+// Otherwise Upgrade returns m (preserving the pseudo-version).
+// This special case prevents accidental downgrades
+// when already using a pseudo-version newer than the latest tagged version.
+func (*mvsReqs) Upgrade(m module.Version) (module.Version, error) {
// Note that query "latest" is not the same as
// using repo.Latest.
// The query only falls back to untagged versions
// if nothing is tagged. The Latest method
// only ever returns untagged versions,
// which is not what we want.
- fmt.Fprintf(os.Stderr, "vgo: finding %s latest\n", path)
- info, err := modfetch.Query(path, "latest", allowed)
+ fmt.Fprintf(os.Stderr, "vgo: finding %s latest\n", m.Path)
+ info, err := modfetch.Query(m.Path, "latest", allowed)
if err != nil {
return module.Version{}, err
}
- return module.Version{Path: path, Version: info.Version}, nil
+
+ // If we're on a later prerelease, keep using it,
+ // even though normally an Upgrade will ignore prereleases.
+ if semver.Compare(info.Version, m.Version) < 0 {
+ return m, nil
+ }
+
+ // If we're on a pseudo-version chronologically after the latest tagged version, keep using it.
+ // This avoids accidental downgrades.
+ if mTime, err := modfetch.PseudoVersionTime(m.Version); err == nil && info.Time.Before(mTime) {
+ return m, nil
+ }
+ return module.Version{Path: m.Path, Version: info.Version}, nil
}
func versions(path string) ([]string, error) {
diff --git a/vendor/cmd/go/vgo_test.go b/vendor/cmd/go/vgo_test.go
index e4b7065..3798883 100644
--- a/vendor/cmd/go/vgo_test.go
+++ b/vendor/cmd/go/vgo_test.go
@@ -374,6 +374,51 @@
tg.runFail("-vgo", "get", "-m", "golang.org/x/crypto/pbkdf2@7f39a6fea4fe9364")
}
+func TestGetModuleUpgrade(t *testing.T) {
+ testenv.MustHaveExternalNetwork(t)
+
+ tg := testgo(t)
+ defer tg.cleanup()
+ tg.makeTempdir()
+
+ tg.setenv(homeEnvName(), tg.path("home"))
+ tg.must(os.MkdirAll(tg.path("x"), 0777))
+ tg.cd(tg.path("x"))
+ tg.must(ioutil.WriteFile(tg.path("x/x.go"), []byte(`package x; import _ "rsc.io/quote"`), 0666))
+
+ tg.must(ioutil.WriteFile(tg.path("x/go.mod"), []byte(`
+ module x
+ require rsc.io/quote v1.5.1
+ `), 0666))
+
+ tg.run("-vgo", "get", "-x", "-u")
+ tg.run("-vgo", "list", "-m", "all")
+ tg.grepStdout(`quote v1.5.2$`, "should have upgraded only to v1.5.2")
+
+ tg.run("-vgo", "get", "-m", "rsc.io/quote@dd9747d")
+ tg.run("-vgo", "list", "-m", "all")
+ tg.grepStdout(`quote v0.0.0-20180628003336-dd9747d19b04$`, "should have moved to pseudo-commit")
+
+ tg.run("-vgo", "get", "-m", "-u")
+ tg.run("-vgo", "list", "-m", "all")
+ tg.grepStdout(`quote v0.0.0-20180628003336-dd9747d19b04$`, "should have stayed on pseudo-commit")
+
+ tg.run("-vgo", "get", "-m", "rsc.io/quote@23179ee8a")
+ tg.run("-vgo", "list", "-m", "all")
+ tg.grepStdout(`quote v0.0.0-20180214005840-23179ee8a569$`, "should have moved to new pseudo-commit")
+
+ tg.run("-vgo", "get", "-m", "-u")
+ tg.run("-vgo", "list", "-m", "all")
+ tg.grepStdout(`quote v1.5.2$`, "should have moved off pseudo-commit")
+
+ tg.must(ioutil.WriteFile(tg.path("x/go.mod"), []byte(`
+ module x
+ `), 0666))
+ tg.run("-vgo", "list")
+ tg.grepStderr(`adding rsc.io/quote v1.5.2`, "should have added quote v1.5.2")
+ tg.grepStderrNot(`v1.5.3-pre1`, "should not mention v1.5.3-pre1")
+}
+
func TestVgoBadDomain(t *testing.T) {
tg := testgo(t)
defer tg.cleanup()
