From 699756b7d0024a79735abd967d22af05f63d0eb6 Mon Sep 17 00:00:00 2001 From: Alexander Kanevskiy Date: Tue, 26 Sep 2017 17:35:20 +0300 Subject: [PATCH] Fix version comparison for versions with preRelease components Improve unit testing, so reverse comparison of versions also works Fixes #53055 --- pkg/util/version/version.go | 39 ++++++++++++++++++++++++-------- pkg/util/version/version_test.go | 20 ++++++++++++++-- 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/pkg/util/version/version.go b/pkg/util/version/version.go index e8cd0cecfa..24724e50ac 100644 --- a/pkg/util/version/version.go +++ b/pkg/util/version/version.go @@ -181,12 +181,11 @@ func (v *Version) String() string { // compareInternal returns -1 if v is less than other, 1 if it is greater than other, or 0 // if they are equal func (v *Version) compareInternal(other *Version) int { - for i := range v.components { + + vLen := len(v.components) + oLen := len(other.components) + for i := 0; i < vLen && i < oLen; i++ { switch { - case i >= len(other.components): - if v.components[i] != 0 { - return 1 - } case other.components[i] < v.components[i]: return 1 case other.components[i] > v.components[i]: @@ -194,6 +193,14 @@ func (v *Version) compareInternal(other *Version) int { } } + // If components are common but one has more items and they are not zeros, it is bigger + switch { + case oLen < vLen && !onlyZeros(v.components[oLen:]): + return 1 + case oLen > vLen && !onlyZeros(other.components[vLen:]): + return -1 + } + if !v.semver || !other.semver { return 0 } @@ -209,10 +216,7 @@ func (v *Version) compareInternal(other *Version) int { vPR := strings.Split(v.preRelease, ".") oPR := strings.Split(other.preRelease, ".") - for i := range vPR { - if i >= len(oPR) { - return 1 - } + for i := 0; i < len(vPR) && i < len(oPR); i++ { vNum, err := strconv.ParseUint(vPR[i], 10, 0) if err == nil { oNum, err := strconv.ParseUint(oPR[i], 10, 0) @@ -234,9 +238,26 @@ func (v *Version) compareInternal(other *Version) int { } } + switch { + case len(oPR) < len(vPR): + return 1 + case len(oPR) > len(vPR): + return -1 + } + return 0 } +// returns false if array contain any non-zero element +func onlyZeros(array []uint) bool { + for _, num := range array { + if num != 0 { + return false + } + } + return true +} + // AtLeast tests if a version is at least equal to a given minimum version. If both // Versions are Semantic Versions, this will use the Semantic Version comparison // algorithm. Otherwise, it will compare only the numeric components, with non-present diff --git a/pkg/util/version/version_test.go b/pkg/util/version/version_test.go index eb231b1e2b..aa0675f7f1 100644 --- a/pkg/util/version/version_test.go +++ b/pkg/util/version/version_test.go @@ -45,13 +45,24 @@ func testOne(v *Version, item, prev testItem) error { if err != nil { return fmt.Errorf("unexpected parse error: %v", err) } + rv, err := parse(prev.version, v.semver) + if err != nil { + return fmt.Errorf("unexpected parse error: %v", err) + } + rcmp, err := rv.Compare(item.version) + if err != nil { + return fmt.Errorf("unexpected parse error: %v", err) + } + switch { case cmp == -1: return fmt.Errorf("unexpected ordering %q < %q", item.version, prev.version) case cmp == 0 && !item.equalsPrev: - return fmt.Errorf("unexpected comparison %q == %q", item.version, item.version) + return fmt.Errorf("unexpected comparison %q == %q", item.version, prev.version) case cmp == 1 && item.equalsPrev: - return fmt.Errorf("unexpected comparison %q != %q", item.version, item.version) + return fmt.Errorf("unexpected comparison %q != %q", item.version, prev.version) + case cmp != -rcmp: + return fmt.Errorf("unexpected reverse comparison %q <=> %q %v %v %v %v", item.version, prev.version, cmp, rcmp, v.Components(), rv.Components()) } } @@ -76,6 +87,8 @@ func TestSemanticVersions(t *testing.T) { {version: "1.0.0-x.7.z.92"}, {version: "1.0.0"}, {version: "1.0.0+20130313144700", equalsPrev: true}, + {version: "1.8.0-alpha.3"}, + {version: "1.8.0-alpha.3.673+73326ef01d2d7c"}, {version: "1.9.0"}, {version: "1.10.0"}, {version: "1.11.0"}, @@ -187,6 +200,7 @@ func TestGenericVersions(t *testing.T) { {version: "1.2", unparsed: "1.2"}, {version: "1.2a.3", unparsed: "1.2", equalsPrev: true}, {version: "1.2.3", unparsed: "1.2.3"}, + {version: "1.2.3.0", unparsed: "1.2.3.0", equalsPrev: true}, {version: "1.2.3a", unparsed: "1.2.3", equalsPrev: true}, {version: "1.2.3-foo.", unparsed: "1.2.3", equalsPrev: true}, {version: "1.2.3-.foo", unparsed: "1.2.3", equalsPrev: true}, @@ -201,8 +215,10 @@ func TestGenericVersions(t *testing.T) { {version: "1.2.3.4b3", unparsed: "1.2.3.4", equalsPrev: true}, {version: "1.2.3.4.5", unparsed: "1.2.3.4.5"}, {version: "1.9.0", unparsed: "1.9.0"}, + {version: "1.9.0.0.0.0.0.0", unparsed: "1.9.0.0.0.0.0.0", equalsPrev: true}, {version: "1.10.0", unparsed: "1.10.0"}, {version: "1.11.0", unparsed: "1.11.0"}, + {version: "1.11.0.0.5", unparsed: "1.11.0.0.5"}, {version: "2.0.0", unparsed: "2.0.0"}, {version: "2.1.0", unparsed: "2.1.0"}, {version: "2.1.1", unparsed: "2.1.1"},