Skip to content

Commit 337f922

Browse files
committed
go/version,internal/gover: fix bug that mistreat prerelease patch as 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 #68634
1 parent 8b51146 commit 337f922

File tree

4 files changed

+80
-17
lines changed

4 files changed

+80
-17
lines changed

src/go/version/version.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,31 @@ func Lang(x string) string {
5252
// The versions x and y must begin with a "go" prefix: "go1.21" not "1.21".
5353
// Invalid versions, including the empty string, compare less than
5454
// valid versions and equal to each other.
55-
// The language version "go1.21" compares less than the
56-
// release candidate and eventual releases "go1.21rc1" and "go1.21.0".
55+
// After go1.21, the language version is less than specific release versions
56+
// or other prerelease versions.
57+
// For example:
58+
//
59+
// Compare("go1.21rc1", "go1.21") = 1
60+
// Compare("go1.21rc1", "go1.21.0") = -1
61+
// Compare("go1.22rc1", "go1.22") = 1
62+
// Compare("go1.22rc1", "go1.22.0") = -1
63+
//
64+
// However, When the language version is below go1.21, the situation is quite different,
65+
// because the initial release version was 1.N, not 1.N.0.
66+
// For example:
67+
//
68+
// Compare("go1.20rc1", "go1.21") = -1
69+
// Compare("go1.19rc1", "go1.19") = -1
70+
// Compare("go1.18", "go1.18rc1") = 1
71+
// Compare("go1.18", "go1.18rc1") = 1
72+
//
73+
// This situation also happens to prerelease for some old patch versions, such as "go1.8.5rc5, "go1.9.2rc2"
74+
// For example:
75+
//
76+
// Compare("go1.8.5rc4", "go1.8.5rc5") = -1
77+
// Compare("go1.8.5rc5", "go1.8.5") = -1
78+
// Compare("go1.9.2rc2", "go1.9.2") = -1
79+
// Compare("go1.9.2rc2", "go1.9") = 1
5780
func Compare(x, y string) int {
5881
return gover.Compare(stripGo(x), stripGo(y))
5982
}

src/go/version/version_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ var compareTests = []testCase2[string, string, int]{
4040
{"go1.19alpha3", "go1.19beta2", -1},
4141
{"go1.19beta2", "go1.19rc1", -1},
4242
{"go1.1", "go1.99999999999999998", -1},
43+
{"go1.9.2rc2", "go1.9.2", -1},
44+
{"go1.9.2rc2", "go1.9.2rc3", -1},
45+
{"go1.9.2beta2", "go1.9.2rc3", -1},
46+
{"go1.9.2alpha1", "go1.9.2beta2", -1},
4347
{"go1.99999999999999998", "go1.99999999999999999", -1},
4448
}
4549

@@ -52,6 +56,7 @@ var langTests = []testCase1[string, string]{
5256
{"go1.2", "go1.2"},
5357
{"go1", "go1"},
5458
{"go222", "go222.0"},
59+
{"go1.8.2rc", "go1.8"},
5560
{"go1.999testmod", "go1.999"},
5661
}
5762

@@ -73,6 +78,8 @@ var isValidTests = []testCase1[string, bool]{
7378
{"go1.19", true},
7479
{"go1.3", true},
7580
{"go1.2", true},
81+
{"go1.9.2rc2", true},
82+
{"go1.9.2+rc2", false},
7683
{"go1", true},
7784
}
7885

src/internal/gover/gover.go

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ func Compare(x, y string) int {
4747
return c
4848
}
4949
if c := cmp.Compare(vx.Kind, vy.Kind); c != 0 { // "" < alpha < beta < rc
50+
// for patch release, alpha < beta < rc < ""
51+
if vx.Patch != "" {
52+
if vx.Kind == "" {
53+
c = 1
54+
} else if vy.Kind == "" {
55+
c = -1
56+
}
57+
}
5058
return c
5159
}
5260
if c := CmpInt(vx.Pre, vy.Pre); c != 0 {
@@ -133,39 +141,49 @@ func Parse(x string) Version {
133141
// Parse patch if present.
134142
if x[0] == '.' {
135143
v.Patch, x, ok = cutInt(x[1:])
136-
if !ok || x != "" {
137-
// Note that we are disallowing prereleases (alpha, beta, rc) for patch releases here (x != "").
138-
// Allowing them would be a bit confusing because we already have:
139-
// 1.21 < 1.21rc1
140-
// But a prerelease of a patch would have the opposite effect:
141-
// 1.21.3rc1 < 1.21.3
142-
// We've never needed them before, so let's not start now.
144+
if !ok {
143145
return Version{}
144146
}
147+
148+
// If there has prerelease for patch releases.
149+
if x != "" {
150+
v.Kind, v.Pre, ok = parsePreRelease(x)
151+
if !ok {
152+
return Version{}
153+
}
154+
}
155+
145156
return v
146157
}
147158

148159
// Parse prerelease.
160+
v.Kind, v.Pre, ok = parsePreRelease(x)
161+
if !ok {
162+
return Version{}
163+
}
164+
return v
165+
}
166+
167+
func parsePreRelease(x string) (kind, pre string, ok bool) {
149168
i := 0
150169
for i < len(x) && (x[i] < '0' || '9' < x[i]) {
151170
if x[i] < 'a' || 'z' < x[i] {
152-
return Version{}
171+
return "", "", false
153172
}
154173
i++
155174
}
156175
if i == 0 {
157-
return Version{}
176+
return "", "", false
158177
}
159-
v.Kind, x = x[:i], x[i:]
178+
kind, x = x[:i], x[i:]
160179
if x == "" {
161-
return v
180+
return kind, "", true
162181
}
163-
v.Pre, x, ok = cutInt(x)
182+
pre, x, ok = cutInt(x)
164183
if !ok || x != "" {
165-
return Version{}
184+
return "", "", false
166185
}
167-
168-
return v
186+
return kind, pre, true
169187
}
170188

171189
// cutInt scans the leading decimal number at the start of x to an integer

src/internal/gover/gover_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ var compareTests = []testCase2[string, string, int]{
3535
{"1.19rc1", "1.19.0", -1},
3636
{"1.19alpha3", "1.19beta2", -1},
3737
{"1.19beta2", "1.19rc1", -1},
38+
{"1.9.2rc2", "1.9.2", -1},
39+
{"1.9.2rc2", "1.9.2rc3", -1},
40+
{"1.9.2beta2", "1.9.2rc3", -1},
41+
{"1.9.2alpha1", "1.9.2beta2", -1},
3842
{"1.1", "1.99999999999999998", -1},
3943
{"1.99999999999999998", "1.99999999999999999", -1},
4044
}
@@ -53,6 +57,9 @@ var parseTests = []testCase1[string, Version]{
5357
{"1.24", Version{"1", "24", "", "", ""}},
5458
{"1.24rc3", Version{"1", "24", "", "rc", "3"}},
5559
{"1.24.0", Version{"1", "24", "0", "", ""}},
60+
{"1.9.2rc2", Version{"1", "9", "2", "rc", "2"}},
61+
{"1.8.5rc4", Version{"1", "8", "5", "rc", "4"}},
62+
{"1.8.2beta2", Version{"1", "8", "2", "beta", "2"}},
5663
{"1.999testmod", Version{"1", "999", "", "testmod", ""}},
5764
{"1.99999999999999999", Version{"1", "99999999999999999", "", "", ""}},
5865
}
@@ -64,6 +71,8 @@ var langTests = []testCase1[string, string]{
6471
{"1.2.3", "1.2"},
6572
{"1.2", "1.2"},
6673
{"1", "1"},
74+
{"1.9.2rc2", "1.9"},
75+
{"1.8.5rc4", "1.8"},
6776
{"1.999testmod", "1.999"},
6877
}
6978

@@ -72,6 +81,8 @@ func TestIsLang(t *testing.T) { test1(t, isLangTests, "IsLang", IsLang) }
7281
var isLangTests = []testCase1[string, bool]{
7382
{"1.2rc3", false},
7483
{"1.2.3", false},
84+
{"1.9.2rc2", false},
85+
{"1.8.5rc5", false},
7586
{"1.999testmod", false},
7687
{"1.22", true},
7788
{"1.21", true},
@@ -96,6 +107,10 @@ var isValidTests = []testCase1[string, bool]{
96107
{"1.20.0", true},
97108
{"1.20", true},
98109
{"1.19", true},
110+
{"1.8.5rc5", true},
111+
{"1.9.2rc2", true},
112+
{"1.9.2beta1", true},
113+
{"1.9.2alpha", true},
99114
{"1.3", true},
100115
{"1.2", true},
101116
{"1", true},

0 commit comments

Comments
 (0)