Skip to content

Commit

Permalink
go/version,internal/gover: fix bug that mistreat prerelease patch as …
Browse files Browse the repository at this point in the history
…invalid version

go/version.IsValid do not consider prerelease patch as valid version, but some prerelease patches actually do exist in history versions, such as go1.8.5rc5, go1.8.5rc4, go1.9.2rc2 .The version go1.9.2rc2 even could be found from https://go.dev/dl/?mode=json&include=all, and downloaded from https://dl.google.com/go/go1.9.2rc2.linux-amd64.tar.gz. It's unreasonable to treat an existing version as an invalid version, so it should be fixed.

Fixes golang#68634
  • Loading branch information
246859 committed Jul 31, 2024
1 parent 8b51146 commit 337f922
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 17 deletions.
27 changes: 25 additions & 2 deletions src/go/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,31 @@ func Lang(x string) string {
// The versions x and y must begin with a "go" prefix: "go1.21" not "1.21".
// Invalid versions, including the empty string, compare less than
// valid versions and equal to each other.
// The language version "go1.21" compares less than the
// release candidate and eventual releases "go1.21rc1" and "go1.21.0".
// After go1.21, the language version is less than specific release versions
// or other prerelease versions.
// For example:
//
// Compare("go1.21rc1", "go1.21") = 1
// Compare("go1.21rc1", "go1.21.0") = -1
// Compare("go1.22rc1", "go1.22") = 1
// Compare("go1.22rc1", "go1.22.0") = -1
//
// However, When the language version is below go1.21, the situation is quite different,
// because the initial release version was 1.N, not 1.N.0.
// For example:
//
// Compare("go1.20rc1", "go1.21") = -1
// Compare("go1.19rc1", "go1.19") = -1
// Compare("go1.18", "go1.18rc1") = 1
// Compare("go1.18", "go1.18rc1") = 1
//
// This situation also happens to prerelease for some old patch versions, such as "go1.8.5rc5, "go1.9.2rc2"
// For example:
//
// Compare("go1.8.5rc4", "go1.8.5rc5") = -1
// Compare("go1.8.5rc5", "go1.8.5") = -1
// Compare("go1.9.2rc2", "go1.9.2") = -1
// Compare("go1.9.2rc2", "go1.9") = 1
func Compare(x, y string) int {
return gover.Compare(stripGo(x), stripGo(y))
}
Expand Down
7 changes: 7 additions & 0 deletions src/go/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ var compareTests = []testCase2[string, string, int]{
{"go1.19alpha3", "go1.19beta2", -1},
{"go1.19beta2", "go1.19rc1", -1},
{"go1.1", "go1.99999999999999998", -1},
{"go1.9.2rc2", "go1.9.2", -1},
{"go1.9.2rc2", "go1.9.2rc3", -1},
{"go1.9.2beta2", "go1.9.2rc3", -1},
{"go1.9.2alpha1", "go1.9.2beta2", -1},
{"go1.99999999999999998", "go1.99999999999999999", -1},
}

Expand All @@ -52,6 +56,7 @@ var langTests = []testCase1[string, string]{
{"go1.2", "go1.2"},
{"go1", "go1"},
{"go222", "go222.0"},
{"go1.8.2rc", "go1.8"},
{"go1.999testmod", "go1.999"},
}

Expand All @@ -73,6 +78,8 @@ var isValidTests = []testCase1[string, bool]{
{"go1.19", true},
{"go1.3", true},
{"go1.2", true},
{"go1.9.2rc2", true},
{"go1.9.2+rc2", false},
{"go1", true},
}

Expand Down
48 changes: 33 additions & 15 deletions src/internal/gover/gover.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ func Compare(x, y string) int {
return c
}
if c := cmp.Compare(vx.Kind, vy.Kind); c != 0 { // "" < alpha < beta < rc
// for patch release, alpha < beta < rc < ""
if vx.Patch != "" {
if vx.Kind == "" {
c = 1
} else if vy.Kind == "" {
c = -1
}
}
return c
}
if c := CmpInt(vx.Pre, vy.Pre); c != 0 {
Expand Down Expand Up @@ -133,39 +141,49 @@ func Parse(x string) Version {
// Parse patch if present.
if x[0] == '.' {
v.Patch, x, ok = cutInt(x[1:])
if !ok || x != "" {
// Note that we are disallowing prereleases (alpha, beta, rc) for patch releases here (x != "").
// Allowing them would be a bit confusing because we already have:
// 1.21 < 1.21rc1
// But a prerelease of a patch would have the opposite effect:
// 1.21.3rc1 < 1.21.3
// We've never needed them before, so let's not start now.
if !ok {
return Version{}
}

// If there has prerelease for patch releases.
if x != "" {
v.Kind, v.Pre, ok = parsePreRelease(x)
if !ok {
return Version{}
}
}

return v
}

// Parse prerelease.
v.Kind, v.Pre, ok = parsePreRelease(x)
if !ok {
return Version{}
}
return v
}

func parsePreRelease(x string) (kind, pre string, ok bool) {
i := 0
for i < len(x) && (x[i] < '0' || '9' < x[i]) {
if x[i] < 'a' || 'z' < x[i] {
return Version{}
return "", "", false
}
i++
}
if i == 0 {
return Version{}
return "", "", false
}
v.Kind, x = x[:i], x[i:]
kind, x = x[:i], x[i:]
if x == "" {
return v
return kind, "", true
}
v.Pre, x, ok = cutInt(x)
pre, x, ok = cutInt(x)
if !ok || x != "" {
return Version{}
return "", "", false
}

return v
return kind, pre, true
}

// cutInt scans the leading decimal number at the start of x to an integer
Expand Down
15 changes: 15 additions & 0 deletions src/internal/gover/gover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ var compareTests = []testCase2[string, string, int]{
{"1.19rc1", "1.19.0", -1},
{"1.19alpha3", "1.19beta2", -1},
{"1.19beta2", "1.19rc1", -1},
{"1.9.2rc2", "1.9.2", -1},
{"1.9.2rc2", "1.9.2rc3", -1},
{"1.9.2beta2", "1.9.2rc3", -1},
{"1.9.2alpha1", "1.9.2beta2", -1},
{"1.1", "1.99999999999999998", -1},
{"1.99999999999999998", "1.99999999999999999", -1},
}
Expand All @@ -53,6 +57,9 @@ var parseTests = []testCase1[string, Version]{
{"1.24", Version{"1", "24", "", "", ""}},
{"1.24rc3", Version{"1", "24", "", "rc", "3"}},
{"1.24.0", Version{"1", "24", "0", "", ""}},
{"1.9.2rc2", Version{"1", "9", "2", "rc", "2"}},
{"1.8.5rc4", Version{"1", "8", "5", "rc", "4"}},
{"1.8.2beta2", Version{"1", "8", "2", "beta", "2"}},
{"1.999testmod", Version{"1", "999", "", "testmod", ""}},
{"1.99999999999999999", Version{"1", "99999999999999999", "", "", ""}},
}
Expand All @@ -64,6 +71,8 @@ var langTests = []testCase1[string, string]{
{"1.2.3", "1.2"},
{"1.2", "1.2"},
{"1", "1"},
{"1.9.2rc2", "1.9"},
{"1.8.5rc4", "1.8"},
{"1.999testmod", "1.999"},
}

Expand All @@ -72,6 +81,8 @@ func TestIsLang(t *testing.T) { test1(t, isLangTests, "IsLang", IsLang) }
var isLangTests = []testCase1[string, bool]{
{"1.2rc3", false},
{"1.2.3", false},
{"1.9.2rc2", false},
{"1.8.5rc5", false},
{"1.999testmod", false},
{"1.22", true},
{"1.21", true},
Expand All @@ -96,6 +107,10 @@ var isValidTests = []testCase1[string, bool]{
{"1.20.0", true},
{"1.20", true},
{"1.19", true},
{"1.8.5rc5", true},
{"1.9.2rc2", true},
{"1.9.2beta1", true},
{"1.9.2alpha", true},
{"1.3", true},
{"1.2", true},
{"1", true},
Expand Down

0 comments on commit 337f922

Please sign in to comment.