From 337f922303a0682701494ab6e09785bc0c14293d Mon Sep 17 00:00:00 2001 From: 246859 <2633565580@qq.com> Date: Wed, 31 Jul 2024 22:52:17 +0800 Subject: [PATCH] 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 --- src/go/version/version.go | 27 ++++++++++++++++-- src/go/version/version_test.go | 7 +++++ src/internal/gover/gover.go | 48 ++++++++++++++++++++++---------- src/internal/gover/gover_test.go | 15 ++++++++++ 4 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/go/version/version.go b/src/go/version/version.go index 6b8ee674423..cc4588ba643 100644 --- a/src/go/version/version.go +++ b/src/go/version/version.go @@ -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)) } diff --git a/src/go/version/version_test.go b/src/go/version/version_test.go index ad83a258614..1634975e09c 100644 --- a/src/go/version/version_test.go +++ b/src/go/version/version_test.go @@ -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}, } @@ -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"}, } @@ -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}, } diff --git a/src/internal/gover/gover.go b/src/internal/gover/gover.go index 2ad068464dd..28541d7489c 100644 --- a/src/internal/gover/gover.go +++ b/src/internal/gover/gover.go @@ -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 { @@ -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 diff --git a/src/internal/gover/gover_test.go b/src/internal/gover/gover_test.go index 0edfb1f47df..98c4e84f0ae 100644 --- a/src/internal/gover/gover_test.go +++ b/src/internal/gover/gover_test.go @@ -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}, } @@ -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", "", "", ""}}, } @@ -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"}, } @@ -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}, @@ -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},