From cf77263f83269b334c2804960a7b43706b1932fd Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Fri, 9 Jul 2021 09:45:19 -0700 Subject: [PATCH] [dev] Fixing spec dependencies parsing bugs. (#1117) --- toolkit/tools/go.mod | 1 + toolkit/tools/go.sum | 2 + .../tools/internal/pkggraph/pkggraph_test.go | 2 +- toolkit/tools/internal/pkgjson/pkgjson.go | 174 +++++-- .../tools/internal/pkgjson/pkgjson_test.go | 444 +++++++++++++++++- .../internal/versioncompare/versioncompare.go | 36 +- toolkit/tools/specreader/specreader.go | 7 +- 7 files changed, 584 insertions(+), 82 deletions(-) diff --git a/toolkit/tools/go.mod b/toolkit/tools/go.mod index 4592783c4f..91301282da 100644 --- a/toolkit/tools/go.mod +++ b/toolkit/tools/go.mod @@ -8,6 +8,7 @@ require ( github.com/bendahl/uinput v1.4.0 github.com/cavaliercoder/go-cpio v0.0.0-20180626203310-925f9528c45e github.com/gdamore/tcell v1.3.0 + github.com/jinzhu/copier v0.3.2 github.com/klauspost/compress v1.10.5 // indirect github.com/klauspost/pgzip v1.2.3 github.com/muesli/crunchy v0.3.0 diff --git a/toolkit/tools/go.sum b/toolkit/tools/go.sum index ec8f8d1479..d2c26733ef 100644 --- a/toolkit/tools/go.sum +++ b/toolkit/tools/go.sum @@ -18,6 +18,8 @@ github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo github.com/gdamore/tcell v1.3.0 h1:r35w0JBADPZCVQijYebl6YMWWtHRqVEGt7kL2eBADRM= github.com/gdamore/tcell v1.3.0/go.mod h1:Hjvr+Ofd+gLglo7RYKxxnzCBmev3BzsS67MebKS4zMM= github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k= +github.com/jinzhu/copier v0.3.2 h1:QdBOCbaouLDYaIPFfi1bKv5F5tPpeTwXe4sD0jqtz5w= +github.com/jinzhu/copier v0.3.2/go.mod h1:24xnZezI2Yqac9J61UC6/dG/k76ttpq0DdJI3QmUvro= github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= github.com/klauspost/compress v1.10.5 h1:7q6vHIqubShURwQz8cQK6yIe/xC3IF0Vm7TGfqjewrc= github.com/klauspost/compress v1.10.5/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= diff --git a/toolkit/tools/internal/pkggraph/pkggraph_test.go b/toolkit/tools/internal/pkggraph/pkggraph_test.go index 98e95e1b2a..aca7c56f52 100644 --- a/toolkit/tools/internal/pkggraph/pkggraph_test.go +++ b/toolkit/tools/internal/pkggraph/pkggraph_test.go @@ -590,7 +590,7 @@ func TestConditionalLookupMulti(t *testing.T) { Version: "3", SCondition: ">", SVersion: "3"}) - assert.NoError(t, err) + assert.Error(t, err) assert.Nil(t, lu) } diff --git a/toolkit/tools/internal/pkgjson/pkgjson.go b/toolkit/tools/internal/pkgjson/pkgjson.go index efc7fa9499..91eb6cd7a9 100644 --- a/toolkit/tools/internal/pkgjson/pkgjson.go +++ b/toolkit/tools/internal/pkgjson/pkgjson.go @@ -13,6 +13,15 @@ import ( "microsoft.com/pkggen/internal/logger" ) +var knownConditions = map[string]bool{ + "": true, + "=": true, + "<": true, + "<=": true, + ">": true, + ">=": true, +} + // PackageRepo contains an array of SRPMs and relational dependencies type PackageRepo struct { Repo []*Package `json:"Repo"` @@ -77,13 +86,21 @@ func (pkgVer *PackageVer) IsImplicitPackage() bool { // structure considers valid. func (pkgVer *PackageVer) Interval() (interval PackageVerInterval, err error) { var ( - v1, v2 *versioncompare.TolerantVersion - c1, c2 string lowerBound, upperBound *versioncompare.TolerantVersion lowerCond, upperCond string lowerInclusive, upperInclusive bool ) + c1 := pkgVer.Condition + c2 := pkgVer.SCondition + v1 := versioncompare.New(pkgVer.Version) + v2 := versioncompare.New(pkgVer.SVersion) + + if err = pkgVer.validatedIntervals(); err != nil { + err = fmt.Errorf("invalid intervals for '%s': %v", pkgVer.Name, err) + return + } + switch { case pkgVer.Version == "" && pkgVer.SVersion == "": // No version information @@ -91,18 +108,13 @@ func (pkgVer *PackageVer) Interval() (interval PackageVerInterval, err error) { upperBound = versioncompare.NewMax() upperInclusive = true lowerInclusive = true - case pkgVer.Version == "" && pkgVer.SVersion != "": - fallthrough - case pkgVer.SVersion == "" && pkgVer.Version != "": - fallthrough - case pkgVer.Version == pkgVer.SVersion && pkgVer.Condition == pkgVer.SCondition: + case pkgVer.Version == "" && pkgVer.SVersion != "", + pkgVer.Version != "" && pkgVer.SVersion == "", + pkgVer.Version == pkgVer.SVersion && pkgVer.Condition == pkgVer.SCondition: // Only one version set, or duplicated version data - if pkgVer.Version != "" { - v1 = versioncompare.New(pkgVer.Version) - c1 = pkgVer.Condition - } else { - v1 = versioncompare.New(pkgVer.SVersion) - c1 = pkgVer.SCondition + if pkgVer.Version == "" { + v1 = v2 + c1 = c2 } switch c1 { @@ -110,35 +122,25 @@ func (pkgVer *PackageVer) Interval() (interval PackageVerInterval, err error) { lowerInclusive = true fallthrough case ">": - lowerBound, lowerCond = v1, c1 - upperBound, upperCond = versioncompare.NewMax(), "<=" + lowerBound = v1 + upperBound = versioncompare.NewMax() upperInclusive = true case "<=": upperInclusive = true fallthrough case "<": - lowerBound, lowerCond = versioncompare.NewMin(), ">=" - upperBound, upperCond = v1, c1 + lowerBound = versioncompare.NewMin() + upperBound = v1 lowerInclusive = true - case "": - fallthrough - case "=": - lowerBound, lowerCond = v1, "=" - upperBound, upperCond = v1, "=" + case "", "=": + lowerBound = v1 + upperBound = v1 lowerInclusive = true upperInclusive = true - default: - err = fmt.Errorf("can't handle single interval for %s", pkgVer) - return } case pkgVer.Version != "" && pkgVer.SVersion != "": // Explicit version information for both (duplicate version data is handled above) - v1 = versioncompare.New(pkgVer.Version) - c1 = pkgVer.Condition - v2 = versioncompare.New(pkgVer.SVersion) - c2 = pkgVer.SCondition - - if v1.Compare(v2) < 0 { + if v1.Compare(v2) == versioncompare.LessThan { lowerBound, lowerCond = v1, c1 upperBound, upperCond = v2, c2 } else { @@ -146,23 +148,31 @@ func (pkgVer *PackageVer) Interval() (interval PackageVerInterval, err error) { upperBound, upperCond = v1, c1 } - if !(upperCond == "<" || upperCond == "<=") { - err = fmt.Errorf("%s has invalid upper conditional for interval", pkgVer) - return - } - if !(lowerCond == ">" || lowerCond == ">=") { - err = fmt.Errorf("%s has invalid lower conditional for interval", pkgVer) - return - } - - if upperCond == "<=" { - upperInclusive = true - } - if lowerCond == ">=" { + switch { + case conditionEquals(lowerCond): lowerInclusive = true + upperInclusive = true + upperBound = lowerBound + case conditionEquals(upperCond): + lowerInclusive = true + upperInclusive = true + lowerBound = upperBound + case conditionUpperBound(lowerCond): + upperBound = lowerBound + lowerBound = versioncompare.NewMin() + upperInclusive = conditionCanEqual(lowerCond) + lowerInclusive = true + case conditionLowerBound(upperCond): + lowerBound = upperBound + upperBound = versioncompare.NewMax() + lowerInclusive = conditionCanEqual(upperCond) + upperInclusive = true + default: + upperInclusive = conditionCanEqual(upperCond) + lowerInclusive = conditionCanEqual(lowerCond) } default: - err = fmt.Errorf("unhandled interval state for %s", pkgVer) + err = fmt.Errorf("unexpected conditions interval: %s", pkgVer) return } @@ -176,6 +186,53 @@ func (pkgVer *PackageVer) Interval() (interval PackageVerInterval, err error) { return } +func (pkgVer *PackageVer) validatedIntervals() error { + c1 := pkgVer.Condition + c2 := pkgVer.SCondition + + if _, known := knownConditions[c1]; !known { + return fmt.Errorf("unknown condition (%s)", c1) + } + + if _, known := knownConditions[c2]; !known { + return fmt.Errorf("unknown condition (%s)", c2) + } + + if pkgVer.Version == "" && c1 != "" { + return fmt.Errorf("invalid empty version and condition (%s) combination", c1) + } + + if pkgVer.SVersion == "" && c2 != "" { + return fmt.Errorf("invalid empty version and condition (%s) combination", c2) + } + + if (pkgVer.Version == "" && c1 == "") || + (pkgVer.SVersion == "" && c2 == "") { + return nil + } + + sameDirection := conditionsHaveSameDirection(c1, c2) + if sameDirection { + if conditionEquals(c1) && pkgVer.Version != pkgVer.SVersion { + return fmt.Errorf("found contradicting package version requirements: %s", pkgVer) + } + + return nil + } + + v1 := versioncompare.New(pkgVer.Version) + v2 := versioncompare.New(pkgVer.SVersion) + + comparisonResult := v1.Compare(v2) + if (comparisonResult == versioncompare.LessThan && (conditionUpperBound(c1) || (conditionEquals(c1) && !conditionUpperBound(c2)))) || + (comparisonResult == versioncompare.EqualTo && (!conditionCanEqual(c1) || !conditionCanEqual(c2))) || + (comparisonResult == versioncompare.GreatherThan && (conditionUpperBound(c2) || (conditionEquals(c2) && !conditionUpperBound(c1)))) { + return fmt.Errorf("version bounds (%s) don't overlap", pkgVer) + } + + return nil +} + // String outputs an interval in interval notation func (interval *PackageVerInterval) String() (s string) { var ( @@ -317,3 +374,30 @@ func (interval *PackageVerInterval) Satisfies(queryInterval *PackageVerInterval) return queryUpperValid || queryLowerValid || superset } + +// conditionCanEqual checks if the input condition allows "equal to" versions. +func conditionCanEqual(condition string) bool { + return condition == "" || strings.Contains(condition, "=") +} + +// conditionEqual checks if the input condition "equal to" versions. +func conditionEquals(condition string) bool { + return condition == "" || condition == "=" +} + +// conditionsHaveSameDirection checks if both conditions are either the same +// or create the same boundary direction (greater, equal, or lesser). +func conditionsHaveSameDirection(firstCondition, secondCondition string) bool { + return (firstCondition == secondCondition) || + (firstCondition != "" && secondCondition != "" && firstCondition[0] == secondCondition[0]) +} + +// conditionLowerBound checks if the input condition is of the ">" or ">=" variation. +func conditionLowerBound(condition string) bool { + return strings.Contains(condition, ">") +} + +// conditionUpperBound checks if the input condition is of the "<" or "<=" variation. +func conditionUpperBound(condition string) bool { + return strings.Contains(condition, "<") +} diff --git a/toolkit/tools/internal/pkgjson/pkgjson_test.go b/toolkit/tools/internal/pkgjson/pkgjson_test.go index 0c7637eb72..f3cc15b743 100644 --- a/toolkit/tools/internal/pkgjson/pkgjson_test.go +++ b/toolkit/tools/internal/pkgjson/pkgjson_test.go @@ -41,7 +41,6 @@ func TestIntervalPrint(t *testing.T) { assert.Equal(t, "(1,MAX_VER]", i2.String()) assert.Equal(t, "[1,MAX_VER]", i3.String()) assert.Equal(t, "(1,2)", i4.String()) - } func TestBasicIntervalSVersion(t *testing.T) { @@ -279,21 +278,6 @@ func TestSubsetInvalid(t *testing.T) { assert.False(t, interval1.Contains(&interval3)) } -func TestInvalidRange(t *testing.T) { - p1 := &PackageVer{Version: "1", Condition: "<=", SVersion: "2", SCondition: "<="} - p2 := &PackageVer{Version: "1", Condition: ">=", SVersion: "2", SCondition: ">="} - p3 := &PackageVer{Version: "1", Condition: "=", SVersion: "2", SCondition: ">="} - p4 := &PackageVer{Version: "1", Condition: "=", SVersion: "2", SCondition: "<="} - _, err := p1.Interval() - assert.Error(t, err) - _, err = p2.Interval() - assert.Error(t, err) - _, err = p3.Interval() - assert.Error(t, err) - _, err = p4.Interval() - assert.Error(t, err) -} - func TestIntervalEquality(t *testing.T) { p1 := &PackageVer{Version: "1", Condition: ">=", SVersion: "2", SCondition: "<="} p2 := &PackageVer{Version: "2", Condition: "<=", SVersion: "1", SCondition: ">="} @@ -367,3 +351,431 @@ func TestIntervalCompareWithHigherExclusion(t *testing.T) { assert.Equal(t, -1, intervalLow.Compare(&intervalHigh)) assert.Equal(t, 1, intervalHigh.Compare(&intervalLow)) } + +func TestShouldPassEqualEqualConditionSameVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "1", SCondition: "="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, "1", interval.LowerBound.String()) +} + +func TestShouldPassLesserEqualEqualConditionSameVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "<=", SVersion: "1", SCondition: "="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, "1", interval.LowerBound.String()) +} + +func TestShouldPassGreaterEqualEqualConditionSameVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">=", SVersion: "1", SCondition: "="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, "1", interval.LowerBound.String()) +} + +func TestShouldPassEqualLesserEqualConditionSameVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "1", SCondition: "<="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, "1", interval.LowerBound.String()) +} + +func TestShouldPassEqualGreaterEqualConditionSameVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "1", SCondition: ">="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, "1", interval.LowerBound.String()) +} + +func TestShouldPassGreaterEqualGreaterEqualConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: ">=", SVersion: "1", SCondition: ">="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, versioncompare.NewMax(), interval.UpperBound) +} + +func TestShouldPassGreaterGreaterEqualConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: ">", SVersion: "1", SCondition: ">="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.False(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, versioncompare.NewMax(), interval.UpperBound) +} + +func TestShouldPassGreaterEqualGreaterConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: ">=", SVersion: "1", SCondition: ">"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, versioncompare.NewMax(), interval.UpperBound) +} + +func TestShouldPassGreaterGreaterConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: ">", SVersion: "1", SCondition: ">"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.False(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, versioncompare.NewMax(), interval.UpperBound) +} + +func TestShouldPassLesserEqualLesserEqualConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: "<=", SVersion: "1", SCondition: "<="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, versioncompare.NewMin(), interval.LowerBound) +} + +func TestShouldPassLesserEqualLesserConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: "<=", SVersion: "1", SCondition: "<"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.False(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, versioncompare.NewMin(), interval.LowerBound) +} + +func TestShouldPassLesserLesserEqualConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: "<", SVersion: "1", SCondition: "<="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, versioncompare.NewMin(), interval.LowerBound) +} + +func TestShouldPassLesserLesserConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: "<", SVersion: "1", SCondition: "<"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.False(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, versioncompare.NewMin(), interval.LowerBound) +} + +func TestShouldPassLesserEqualEqualConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: "<=", SVersion: "1", SCondition: "="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.LowerBound.String()) + assert.Equal(t, "1", interval.UpperBound.String()) +} + +func TestShouldPassLesserEqualConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: "<", SVersion: "1", SCondition: "="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.LowerBound.String()) + assert.Equal(t, "1", interval.UpperBound.String()) +} + +func TestShouldPassEqualGreaterEqualConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: "=", SVersion: "1", SCondition: ">="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, "2", interval.UpperBound.String()) +} + +func TestShouldPassEqualGreaterConditionDecreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: "=", SVersion: "1", SCondition: ">"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, "2", interval.UpperBound.String()) +} + +func TestShouldPassGreaterEqualGreaterEqualConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">=", SVersion: "2", SCondition: ">="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, versioncompare.NewMax(), interval.UpperBound) +} + +func TestShouldPassGreaterGreaterEqualConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">", SVersion: "2", SCondition: ">="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, versioncompare.NewMax(), interval.UpperBound) +} + +func TestShouldPassGreaterEqualGreaterConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">=", SVersion: "2", SCondition: ">"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.False(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, versioncompare.NewMax(), interval.UpperBound) +} + +func TestShouldPassGreaterGreaterConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">", SVersion: "2", SCondition: ">"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.False(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, versioncompare.NewMax(), interval.UpperBound) +} + +func TestShouldPassLesserEqualLesserEqualConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "<=", SVersion: "2", SCondition: "<="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, versioncompare.NewMin(), interval.LowerBound) +} + +func TestShouldPassLesserEqualLesserConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "<=", SVersion: "2", SCondition: "<"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, versioncompare.NewMin(), interval.LowerBound) +} + +func TestShouldPassLesserLesserEqualConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "<", SVersion: "2", SCondition: "<="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.False(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, versioncompare.NewMin(), interval.LowerBound) +} + +func TestShouldPassLesserLesserConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "<", SVersion: "2", SCondition: "<"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.False(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, versioncompare.NewMin(), interval.LowerBound) +} + +func TestShouldPassEqualLesserEqualConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "2", SCondition: "<="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.LowerBound.String()) + assert.Equal(t, "1", interval.UpperBound.String()) +} + +func TestShouldPassEqualLesserConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "2", SCondition: "<"} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "1", interval.LowerBound.String()) + assert.Equal(t, "1", interval.UpperBound.String()) +} + +func TestShouldPassGreaterEqualEqualConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">=", SVersion: "2", SCondition: "="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, "2", interval.UpperBound.String()) +} + +func TestShouldPassGreaterEqualConditionIncreasingVersionInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">", SVersion: "2", SCondition: "="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.LowerInclusive) + assert.True(t, interval.UpperInclusive) + assert.Equal(t, "2", interval.LowerBound.String()) + assert.Equal(t, "2", interval.UpperBound.String()) +} + +func TestShouldPassBarelyOverlappingDisjointConditionsInterval(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "<=", SVersion: "1", SCondition: ">="} + interval, err := packageVersion.Interval() + + assert.NoError(t, err) + assert.True(t, interval.UpperInclusive) + assert.True(t, interval.LowerInclusive) + assert.Equal(t, "1", interval.UpperBound.String()) + assert.Equal(t, "1", interval.LowerBound.String()) +} + +func TestShouldFailIntervalCreationForFirstSmallerLessEqualSecondLargerGreaterEqual(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "<=", SVersion: "2", SCondition: ">="} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationForFirstLargerGreaterEqualSecondSmallerLessEqual(t *testing.T) { + packageVersion := &PackageVer{Version: "2", Condition: ">=", SVersion: "1", SCondition: "<="} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationForFirstSameGreaterEqualSecondSameLess(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">=", SVersion: "1", SCondition: "<"} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationForFirstSameLessEqualSecondSameGreater(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "<=", SVersion: "1", SCondition: ">"} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationForFirstSmallerEqualSecondLargerGreaterEqual(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "2", SCondition: ">="} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationForFirstSameEqualSecondSameLess(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "1", SCondition: "<"} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationForFirstSameEqualSecondSameGreater(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "1", SCondition: ">"} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationForFirstEqualSecondLargerEqual(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "=", SVersion: "2", SCondition: "="} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationUnkownFirstCondition(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: "?", SVersion: "2", SCondition: "="} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationUnkownSecondCondition(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">", SVersion: "2", SCondition: "?"} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationFirstConditionWithoutVersion(t *testing.T) { + packageVersion := &PackageVer{Version: "", Condition: ">", SVersion: "2", SCondition: ">"} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationSecondConditionWithoutVersion(t *testing.T) { + packageVersion := &PackageVer{Version: "1", Condition: ">", SVersion: "", SCondition: ">"} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationFirstConditionEmptySecondConditionWithoutVersion(t *testing.T) { + packageVersion := &PackageVer{Version: "", Condition: "", SVersion: "", SCondition: ">"} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} + +func TestShouldFailIntervalCreationFirstConditionWithoutVersionSecondConditionEmpty(t *testing.T) { + packageVersion := &PackageVer{Version: "", Condition: ">", SVersion: "", SCondition: ""} + _, err := packageVersion.Interval() + + assert.Error(t, err) +} diff --git a/toolkit/tools/internal/versioncompare/versioncompare.go b/toolkit/tools/internal/versioncompare/versioncompare.go index a7e34917bd..cbec6d9767 100644 --- a/toolkit/tools/internal/versioncompare/versioncompare.go +++ b/toolkit/tools/internal/versioncompare/versioncompare.go @@ -10,6 +10,12 @@ import ( "strings" ) +const ( + LessThan = -1 + EqualTo = 0 + GreatherThan = 1 +) + var ( componentRegex = regexp.MustCompile(`(\d+|[a-z]+)`) epochComponentRegex = regexp.MustCompile(`^(\d+|[a-z])\:`) @@ -63,57 +69,51 @@ func (v *TolerantVersion) CompareWithConditional(condition string, b *TolerantVe // Compare compares this version and the argument version and returns 1 if the argument's version is higher, // -1 if argument's version is lower and 0 if they are equal (three-way comparison) func (v *TolerantVersion) Compare(other *TolerantVersion) int { - const ( - lessThan = -1 - equalTo = 0 - greatherThan = 1 - ) - switch { case v.isMaxVer && other.isMaxVer: fallthrough case v.isMinVer && other.isMinVer: - return equalTo + return EqualTo case v.isMaxVer || other.isMinVer: - return greatherThan + return GreatherThan case v.isMinVer || other.isMaxVer: - return lessThan + return LessThan } for i := range v.versionComponents { if i == len(other.versionComponents) { - return greatherThan + return GreatherThan } if v.versionComponents[i] < other.versionComponents[i] { - return lessThan + return LessThan } if v.versionComponents[i] > other.versionComponents[i] { - return greatherThan + return GreatherThan } } if len(v.versionComponents) < len(other.versionComponents) { - return lessThan + return LessThan } // Only check the release components if both versions request it. if len(v.releaseComponents) > 0 && len(other.releaseComponents) > 0 { for i := range v.releaseComponents { if i == len(other.releaseComponents) { - return greatherThan + return GreatherThan } if v.releaseComponents[i] < other.releaseComponents[i] { - return lessThan + return LessThan } if v.releaseComponents[i] > other.releaseComponents[i] { - return greatherThan + return GreatherThan } } if len(v.releaseComponents) < len(other.releaseComponents) { - return lessThan + return LessThan } } - return equalTo + return EqualTo } // String returns the original string representation of the version diff --git a/toolkit/tools/specreader/specreader.go b/toolkit/tools/specreader/specreader.go index 284a3aa8ec..1d0dc0c91f 100644 --- a/toolkit/tools/specreader/specreader.go +++ b/toolkit/tools/specreader/specreader.go @@ -20,6 +20,7 @@ import ( "microsoft.com/pkggen/internal/rpm" "microsoft.com/pkggen/internal/safechroot" + "github.com/jinzhu/copier" "gopkg.in/alecthomas/kingpin.v2" "microsoft.com/pkggen/internal/exe" "microsoft.com/pkggen/internal/logger" @@ -518,8 +519,10 @@ func condensePackageVersionArray(packagelist []*pkgjson.PackageVer, specfile str } } } - if nameMatch == false { - processedPkgList = append(processedPkgList, pkg) + if !nameMatch { + var processPkg pkgjson.PackageVer + copier.Copy(&processPkg, pkg) + processedPkgList = append(processedPkgList, &processPkg) } } return