From 6b3ebcf4d5a68d37cd74addcb6586d29823d4111 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Tue, 9 Aug 2022 15:05:01 -0500 Subject: [PATCH 1/3] Allow pre-releases when the range uses them Currently, when a range uses a pre-release, such as 1.0.0-alpha.1 - 1.1.0, all pre-release versions are being rejected when the constraint is checked because the upper part of the range does not include a pre-release. For example, 1.0.0-alpha.2 should match that range, but it is rejected right now. I have updated how we evaluate constraints so that it checks whether a constraint part uses pre-releases OR the larger constraint range that it is part of uses a pre-release. When either part of a range uses a pre-release, the entire range should allow pre-release versions. Fixes #177 Signed-off-by: Carolyn Van Slyck --- README.md | 3 ++- constraints.go | 33 ++++++++++++++++++++++++++++----- constraints_test.go | 3 +++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index d8f54dc..04beb94 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,8 @@ differences to notes between these two methods of comparison. different set of rules that are common for ranges with tools like npm/js and Rust/Cargo. This includes considering prereleases to be invalid if the ranges does not include one. If you want to have it include pre-releases a - simple solution is to include `-0` in your range. + simple solution is to include `-0` in your range. For example, `~1.2.3-0` or + `1.2.3-0 - 1.5.0`. 3. Constraint ranges can have some complex rules including the shorthand use of ~ and ^. For more details on those see the options below. diff --git a/constraints.go b/constraints.go index 547613f..10d9ca4 100644 --- a/constraints.go +++ b/constraints.go @@ -32,6 +32,10 @@ func NewConstraint(c string) (*Constraints, error) { return nil, fmt.Errorf("improper constraint: %s", v) } + // If any part of the range uses a pre-release, + // then allow pre-release versions to match when the range is checked. + var rangeUsesPrerelease bool + cs := findConstraintRegex.FindAllString(v, -1) if cs == nil { cs = append(cs, v) @@ -44,7 +48,21 @@ func NewConstraint(c string) (*Constraints, error) { } result[i] = pc + + // Check if the range uses a pre-release + if !rangeUsesPrerelease && pc.con.Prerelease() != "" { + rangeUsesPrerelease = true + } } + + if rangeUsesPrerelease { + // If any part of the range used a pre-release, + // all versions in the range should allow pre-releases + for _, pc := range result { + pc.allowPrerelease = true + } + } + or[k] = result } @@ -202,6 +220,10 @@ type constraint struct { minorDirty bool dirty bool patchDirty bool + + // allowPrerelease indicates if the constraint should allow pre-releases, + // such as when part of a range constraint that uses pre-releases + allowPrerelease bool } // Check if a version meets the constraint @@ -257,6 +279,7 @@ func parseConstraint(c string) (*constraint, error) { cs.minorDirty = minorDirty cs.patchDirty = patchDirty cs.dirty = dirty + cs.allowPrerelease = cs.con.Prerelease() != "" return cs, nil } @@ -389,7 +412,7 @@ func constraintGreaterThanEqual(v *Version, c *constraint) (bool, error) { // If there is a pre-release on the version but the constraint isn't looking // for them assume that pre-releases are not compatible. See issue 21 for // more details. - if v.Prerelease() != "" && c.con.Prerelease() == "" { + if v.Prerelease() != "" && !c.allowPrerelease { return false, fmt.Errorf("%s is a prerelease version and the constraint is only looking for release versions", v) } @@ -404,7 +427,7 @@ func constraintLessThanEqual(v *Version, c *constraint) (bool, error) { // If there is a pre-release on the version but the constraint isn't looking // for them assume that pre-releases are not compatible. See issue 21 for // more details. - if v.Prerelease() != "" && c.con.Prerelease() == "" { + if v.Prerelease() != "" && !c.allowPrerelease { return false, fmt.Errorf("%s is a prerelease version and the constraint is only looking for release versions", v) } @@ -437,7 +460,7 @@ func constraintTilde(v *Version, c *constraint) (bool, error) { // If there is a pre-release on the version but the constraint isn't looking // for them assume that pre-releases are not compatible. See issue 21 for // more details. - if v.Prerelease() != "" && c.con.Prerelease() == "" { + if v.Prerelease() != "" && !c.allowPrerelease { return false, fmt.Errorf("%s is a prerelease version and the constraint is only looking for release versions", v) } @@ -469,7 +492,7 @@ func constraintTildeOrEqual(v *Version, c *constraint) (bool, error) { // If there is a pre-release on the version but the constraint isn't looking // for them assume that pre-releases are not compatible. See issue 21 for // more details. - if v.Prerelease() != "" && c.con.Prerelease() == "" { + if v.Prerelease() != "" && !c.allowPrerelease { return false, fmt.Errorf("%s is a prerelease version and the constraint is only looking for release versions", v) } @@ -498,7 +521,7 @@ func constraintCaret(v *Version, c *constraint) (bool, error) { // If there is a pre-release on the version but the constraint isn't looking // for them assume that pre-releases are not compatible. See issue 21 for // more details. - if v.Prerelease() != "" && c.con.Prerelease() == "" { + if v.Prerelease() != "" && !c.allowPrerelease { return false, fmt.Errorf("%s is a prerelease version and the constraint is only looking for release versions", v) } diff --git a/constraints_test.go b/constraints_test.go index 0504399..3eb4062 100644 --- a/constraints_test.go +++ b/constraints_test.go @@ -374,6 +374,9 @@ func TestConstraintsCheck(t *testing.T) { {"~1.1-alpha", "1.1.1-beta", true}, {"~1.1.1-beta", "1.1.1-alpha", false}, {"~1.1.1-beta", "1.1.1", true}, + {constraint: "1.0.0-alpha.1 - 1.0.0", version: "1.0.0-beta.2", check: true}, + {constraint: "~1.2.3 || 1.2.3-beta.1", version: "1.2.3-beta.1", check: true}, + {constraint: "~1.2.3 || 1.2.3-beta.1", version: "1.2.3-beta.2", check: false}, {"~1.2.3", "1.2.5", true}, {"~1.2.3", "1.2.2", false}, {"~1.2.3", "1.3.2", false}, From e74be44bb20348802784f89d5a2a70d9ead1099b Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Fri, 17 Mar 2023 09:59:08 -0500 Subject: [PATCH 2/3] Limit prerelease detection to individual ranges Previously, I was detecting if any range in a larger constraint set contained a prerelease, and allowed prereleases for any constraint in the set. This incorrectly allowed prereleases on constraints that did not participate in a range that had prereleases. When rewriteRange is called, we lose key information about if a constraint was originally part of a range: 1.0.0-alpha.1 - 1.0.0 becomes >=1.0.0-alpha.1, <=1.0.0 I have updated rewriteRange so that it also returns any constraints that were originally part of a range that used a prerelease. Then when we are parsing constraints, allowPrerelease on that constraint is enabled if it was originally detected as participating in a range with prereleases. This ensures that only the individual ranges have allowPrerelease enabled. Signed-off-by: Carolyn Van Slyck --- constraints.go | 42 +++++++++++++++++++++--------------------- constraints_test.go | 13 +++++++++---- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/constraints.go b/constraints.go index 10d9ca4..85d2480 100644 --- a/constraints.go +++ b/constraints.go @@ -19,7 +19,7 @@ type Constraints struct { func NewConstraint(c string) (*Constraints, error) { // Rewrite - ranges into a comparison operation. - c = rewriteRange(c) + c, rangesWithPrerelease := rewriteRange(c) ors := strings.Split(c, "||") or := make([][]*constraint, len(ors)) @@ -32,10 +32,6 @@ func NewConstraint(c string) (*Constraints, error) { return nil, fmt.Errorf("improper constraint: %s", v) } - // If any part of the range uses a pre-release, - // then allow pre-release versions to match when the range is checked. - var rangeUsesPrerelease bool - cs := findConstraintRegex.FindAllString(v, -1) if cs == nil { cs = append(cs, v) @@ -47,22 +43,15 @@ func NewConstraint(c string) (*Constraints, error) { return nil, err } - result[i] = pc - - // Check if the range uses a pre-release - if !rangeUsesPrerelease && pc.con.Prerelease() != "" { - rangeUsesPrerelease = true - } - } - - if rangeUsesPrerelease { - // If any part of the range used a pre-release, - // all versions in the range should allow pre-releases - for _, pc := range result { + // When we parse a constraint we lose information about if it was part of a range + // Use rangesWithPrerelease to check if the range originally used a pre-release + pcStr := pc.string() + if _, ok := rangesWithPrerelease[pcStr]; ok { pc.allowPrerelease = true } - } + result[i] = pc + } or[k] = result } @@ -576,16 +565,27 @@ func isX(x string) bool { } } -func rewriteRange(i string) string { +func rewriteRange(i string) (string, map[string]struct{}) { m := constraintRangeRegex.FindAllStringSubmatch(i, -1) if m == nil { - return i + return i, nil } o := i + allowPrerelease := make(map[string]struct{}, 0) for _, v := range m { t := fmt.Sprintf(">= %s, <= %s", v[1], v[11]) o = strings.Replace(o, v[0], t, 1) + + // Check if any part of the range uses a pre-release + rangeUsesPrerelease := v[5] != "" || v[15] != "" + if rangeUsesPrerelease { + // do not use spaces, so that we can compare this later against a parsed constraint + lower := fmt.Sprintf(">=%s", v[1]) + upper := fmt.Sprintf("<=%s", v[11]) + allowPrerelease[lower] = struct{}{} + allowPrerelease[upper] = struct{}{} + } } - return o + return o, allowPrerelease } diff --git a/constraints_test.go b/constraints_test.go index 3eb4062..730b209 100644 --- a/constraints_test.go +++ b/constraints_test.go @@ -374,14 +374,19 @@ func TestConstraintsCheck(t *testing.T) { {"~1.1-alpha", "1.1.1-beta", true}, {"~1.1.1-beta", "1.1.1-alpha", false}, {"~1.1.1-beta", "1.1.1", true}, - {constraint: "1.0.0-alpha.1 - 1.0.0", version: "1.0.0-beta.2", check: true}, - {constraint: "~1.2.3 || 1.2.3-beta.1", version: "1.2.3-beta.1", check: true}, - {constraint: "~1.2.3 || 1.2.3-beta.1", version: "1.2.3-beta.2", check: false}, {"~1.2.3", "1.2.5", true}, {"~1.2.3", "1.2.2", false}, {"~1.2.3", "1.3.2", false}, {"~1.1", "1.2.3", false}, {"~1.3", "2.4.5", false}, + // Should fail because 1.5.0 is AND'd with the range and since it does not allow prereleases, the entire constraint fails + {"1.0.0-alpha.1 - 1.0.0, <= 1.5.0", "1.0.0-beta.2", false}, + // Should fail because only the first range should allow prereleases (detects if we are allowing prereleases for more than the affected range + {"1.0.0-alpha.1 - 1.0.0 || <= 1.5.0", "1.5.0-beta.2", false}, + // When the lower part of the range allows prereleases, the upper should also allow prereleases + {"1.0.0-alpha.1 - 1.0.0", "1.0.0-beta.2", true}, + // When the upper part of the range allows prereleases, the lower should also allow prereleases + {"1.0.0 - 2.0.0-beta.3", "2.0.0-beta.2", true}, } for _, tc := range tests { @@ -415,7 +420,7 @@ func TestRewriteRange(t *testing.T) { } for _, tc := range tests { - o := rewriteRange(tc.c) + o, _ := rewriteRange(tc.c) if o != tc.nc { t.Errorf("Range %s rewritten incorrectly as '%s'", tc.c, o) From 17277ec64a8a292ebdc02158b6ea14012696e502 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Wed, 5 Apr 2023 12:21:22 -0500 Subject: [PATCH 3/3] Run goimports Signed-off-by: Carolyn Van Slyck --- constraints_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/constraints_test.go b/constraints_test.go index 80009ec..b761b63 100644 --- a/constraints_test.go +++ b/constraints_test.go @@ -400,7 +400,7 @@ func TestConstraintsCheck(t *testing.T) { // Ranges should work in conjunction with other constraints anded together. {"1.0.0 - 2.0.0 <=2.0.0", "1.5.0", true}, {"1.0.0 - 2.0.0, <=2.0.0", "1.5.0", true}, - + // Should fail because 1.5.0 is AND'd with the range and since it does not allow prereleases, the entire constraint fails {"1.0.0-alpha.1 - 1.0.0, <= 1.5.0", "1.0.0-beta.2", false}, // Should fail because only the first range should allow prereleases (detects if we are allowing prereleases for more than the affected range