Add `matches_prerelease` to allow update to prerelease version if it meets Semver compatible
What 's this PR want to solve?
The main purpose is to solve the upper bound semantic approved by rfc#precise-pre-release-cargo-update
it extends the matching mechanism of the pre-release version,
So before,
1.2.3 -> ^1.2.3 -> >=1.2.3, <2.0.0 (with implicit holes excluding pre-release versions)
would become
1.2.3 -> ^1.2.3 -> >=1.2.3, <2.0.0-0
Note that the old syntax implicitly excluded 2.0.0-<prerelease> which we have have to explicitly exclude by referencing the smallest possible pre-release version of -0.
If this PR could be merged, it maybe fixed the cargo update --precise <pre-release> upper bound semantic
Sorry, need to address the node test firstly
Oh, node test is a call to the node program to verify version compatibility, which is used for comparison testing.
In this case, I didn't know if there was a corresponding pre-release compatibility match test for node, so I bypassed the node test
Experimented with this branch, definitely found cases I was not expecting. For example the requirement >=0.24, <=0.25 does not matches_prerelease with version 0.24.1-PRE. Or ~0.34 not matching 0.34.7-p1.
Experimented with this branch, definitely found cases I was not expecting. For example the requirement
>=0.24, <=0.25does notmatches_prereleasewith version0.24.1-PRE. Or~0.34not matching0.34.7-p1.
Oops,the paritial-version doesn't take into consideration. Thank you for your feedback.
Thanks for the update! I tried again and found more examples that didn't match my expectations:
=4.1with4.1.1-rev.0^4.5with4.5.0-alpha.1~0with0.1.0-0=4.2with4.2.1(this one doesn't even involve prerelease)
These are of course just "my expectations" and the "correct behavior" may not match what's in my head at the moment.
For context I am generating these with https://github.com/pubgrub-rs/semver-pubgrub/tree/new-pre-release
and reviewing the cases where the two algorithms disagree to figure out which one looks right. There are definitely cases where that code is also getting the wrong answer. For example I have no idea what correct behavior is for ^4.4.11, <4.5 with 4.5.0-alpha.1.
My apologies. My Last commit have some regression and I am eager to correct it.
I test you example and use it to explain the match logic
let ref r = req("=4.1");
// Match >= 4.1.0, < 4.2.0-0
assert_prerelease_match_all(r, &["4.1.1-rev.0"]);
assert_prerelease_match_none(r, &["1.2.4", "4.1.0-rev.0", "4.2.0-0", "4.2.0"]);
let ref r = req("^4.5");
// Match >= 4.5.0, < 5.0.0-0
assert_prerelease_match_all(r, &["4.5.0", "4.6.0-0", "4.6.0"]);
assert_prerelease_match_none(r, &["1.2.4", "4.5.0-rev.0"]);
let ref r = req("~0");
// Match >= 0.0.0, < 1.0.0-0
assert_prerelease_match_all(r, &["0.1.0-0"]);
assert_prerelease_match_none(
r,
&["0.0.0-pre", "1.2.4", "4.1.0-rev.0", "4.2.0-0", "4.2.0"],
);
let ref r = req("=4.2");
// Match >= 4.2.0, < 4.3.0-0
assert_prerelease_match_all(r, &["4.2.0", "4.2.1"]);
assert_prerelease_match_none(r, &["1.2.4", "4.1.0-rev.0", "4.2.0-0", "4.3.0-0", "4.3.0"]);
let ref r = req("^4.4.11, <4.5");
// Match >= 4.4.11, < 4.5.0
assert_prerelease_match_all(r, &["4.5.0-alpha.1"]);
assert_prerelease_match_none(r, &["1.2.4", "4.1.0-rev.0", "4.2.0-0", "4.5.0"]);
Thanks for the update!
req(">= 4.1") == req(">= 4.1.0") not match "4.1.0-rev.0" feels really odd to me. Intuitively it looks like =4.1 should match something that starts 4.1. So perhaps we should pad one more zero so that it the sugars to 4.1.0-0. But if we do that, there is no way to specify "I'm broken on the prereleases and require a fix from the stable release" because =4.1.0 would be sugar to 4.1.0-0. What we think the "correct" behavior should be?
I tried again and found more examples:
<0.10with0.10.0-BP-beta.1(this is the flipside of the one we just discussed)=3.0.0-alpha.24with3.0.0-alpha.25(this I think is just wrong)
I am very appreciate your feedback.
=3.0.0-alpha.24 with 3.0.0-alpha.25
This had been corrected.
Then for comparison, I collated the versions that currently have pre-release comparison logic
# Op::Exact
- =I.J.K equivalent to >=I.J.K, <I.J.(K+1)-0
- =I.J equivalent to >=I.J.0, <I.(J+1).0-0
- =I equivalent to >=I.0.0, <(I+1).0.0-0
# Op::Greater
- >I.J.K
- >I.J equivalent to >=I.(J+1).0-0
- >I equivalent to >=(I+1).0.0-0
# Op::GreaterEq
- >=I.J.K
- >=I.J equivalent to >=I.J.0
- >=I equivalent to >=I.0.0
# Op::Less
- <I.J.K
- <I.J equivalent to <I.J.0
- <I equivalent to <I.0.0
# Op::LessEq
- <=I.J.K
- <=I.J equivalent to <I.(J+1).0-0
- <=I equivalent to <(I+1).0.0-0
# Op::Tilde
- ~I.J.K — equivalent to >=I.J.K, <I.(J+1).0-0
- ~I.J — equivalent to >=I.J.0, <I.(J+1).0-0
- ~I — >=I.0.0, <(I+1).0.0-0
# Op::Caret
- ^I.J.K (for I>0) — equivalent to >=I.J.K, <(I+1).0.0-0
- ^0.J.K (for J>0) — equivalent to >=0.J.K, <0.(J+1).0-0
- ^0.0.K — equivalent to >=0.0.K, <0.0.(K+1)-0
- ^I.J (for I>0 or J>0) — equivalent to >=I.J.0, <(I+1).0.0-0)
- ^0.0 — equivalent to >=0.0.0, <0.1.0-0
- ^I — equivalent to >=I.0.0, <(I+1).0.0-0
At present, I think this version is much better than the first commit, but I feel that there are still some corners that have not been sorted out, which may be discussed separately next time
Thank you for the prompt fixes. I only had a brief chance to look at this today, and it requires more thought than I had available. I will try and look at it again next week.
Should we be trialing this in Cargo under a nightly feature before trying to merge this into a stable API?
Should we be trialing this in Cargo under a nightly feature before trying to merge this into a stable API?
https://github.com/rust-lang/rfcs/pull/3493 is the only customer of this features now. And the matches_prerelease proposed semantic differs from the original matches in some ways. So this proposed semantic may be implemented in Cargo.
BTW, https://github.com/rust-lang/rfcs/pull/3493 doesn't give all scenes need to be considered, it only give a caret op example
So before,
1.2.3 -> ^1.2.3 -> >=1.2.3, <2.0.0 (with implicit holes excluding pre-release versions)
would become
1.2.3 -> ^1.2.3 -> >=1.2.3, <2.0.0-0
Note that the old syntax implicitly excluded 2.0.0-<prerelease> which we have have to explicitly exclude by referencing the smallest possible pre-release version of -0.
And I found I haven't read fully The semantic versioner for npm, who knows what inspiration this might bring
In short, it is better to carry this implementation into Cargo, and can be better combined with cargo design
By this commit, these stuffs have been added:
- Add
node semver compatibility(with includePrerelease=true) test. - According to the
node semver compatibilitytest, a mirror-version implemention comes out, which behind the featuremirror_node_matches_prerelease - rewrite
test_matches_prerelease.rsto better reflect current implementions.
I found the diffenence between node semver compatibility implemtation and the current-proposed semantic
- if the
VersionReqis partial, the former will padding the Prerelease Tag with 0 except 'Tilde Op'. - And for
Caret Op, it will also padding the Prerelease Tag with "0" if theMajoris 0.
If my assumption is correct, then the node semver compatibilit implemention also suitable for the https://github.com/rust-lang/rfcs/pull/3493, but these node semantic is non-default, you need to set includePrerelease=true.
Thank you for all the work you put into this! The details here are tricky and overwhelming and I really appreciate you sticking with it.
Compatibility with npm is nice but not required. Starting with their semantics was a very good idea for a place to start, someone else thought this was the right choice. But, we have every right to decide we want different semantics if that's what we decide we need.
I was able to reproduce the semantics as of https://github.com/dtolnay/semver/commit/3464fd136a180fd64c6b0a394ccb0479ccb8d967 in my project. The diff is mostly boilerplate. The only two exceptions were the lower bound for ^x|^x.y|^x.y.z and the upper bound for <x|<x.y|<x.y.z. I had to change from expanding to x.y.z-0 -> x.y.z. This rarely makes a difference, because requirements that don't mention prerelease are not allowed to match versions with prereleases, so it's extremely rare that which prereleases it matches to matters. In fact, there are no examples on crates.io where the difference matters. And the fuzzing tools tend to minimize two examples that seem irrelevant. <0, ^0.0.0-0 and ^0, ^0.0.0-0 matched on 0.0.0-0. There all 0 because of the minimizer, the pattern can be generalized to other numbers.
I've assumed that if matches then also matches_prerelease. If that is one of the axioms of our design then we need to fix these obscure cases. Either by changing the behavior of matches_prerelease to be less intuitive. Or by changing the behavior of matches to match. Which would technically be a breaking change. But on such obscure inputs that perhaps it's not worth making a fuss about.
Is there any plan to merge this?
I doubt there is in the short term. The Cargo Team is ~still~ intermittently trying to figure out what semantics we want. I doubt this will get merged in this repo until the semantic are pin down precisely.
The proposed semantics isn't determinate, welcome any ideas and feadbacks.
BTW, this proposal had merged into Cargo, if you like to test it, please refer to https://doc.rust-lang.org/cargo/reference/unstable.html?highlight=unstable#precise-pre-release.