Allow pre-releases when the range uses them #184
Conversation
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 Masterminds#177 Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs
force-pushed
the
fix-constraint-range-with-prerelease
branch
from
68dfa87
to
6b3ebcf
Compare
There was a problem hiding this comment.
What happens if someone does a range like 1.0.0-alpha.1 - 1.0.0 <= 1.0.0? That last part isn't part of the range. This is 1.0.0-alpha.1 - 1.0.0 and <= 1.0.0. Is this user error for specifying a second part that doesn't handle a prerelease or should it handle a prerelease?
If we break it down based on each rule, the <= 1.0.0 should not handle a prerelease as it's not part of the rule that includes a prerelease. Reading, I see the rule I wrote should match nothing.
With this PR, passing in 1.0.0-beta.2 would pass as the <= 1.0.0 would allow prereleases.
I found this when I took the PR and added more test cases to it.
Thoughts?
|
Note, I have ideas how to modify this work so it only checks prereleases on the prerelease ranges. It involves some deeper cutting changes. If you want, I can pick up the work from here and try to change it. |
|
Ope, sorry for the slow turnaround, I was on vacation for a couple weeks there. I'm back now and have a bit of a start on handling the case you added. |
|
After looking at it more, I'm not sure how best to go about only applying the pre-release to a single range, since the range is immediately removed and converted into another representation when the constraint is parsed. If you have an idea for how to make this work well, please have at it! 👍 |
|
Any updates on this request? |
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 <me@carolynvanslyck.com>
Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
|
@mattfarina Sorry for the really long delay getting a fix for the test case you brought up. Please take another look? |
Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
|
I've fixed the merge conflict and this is ready to review. |
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