feat: Add matches_prerelease semantic #14305

linyihai · 2024-07-26T09:33:15Z

What does this PR try to resolve?

One implementaion for #13290

Thanks to @Eh2406 feedback, a working version came out, and I think it should be able to test under the nightly feature.

This PR proposes a matches_prerelease semantic.

req	matches	matches_prerelease	matches_prerelease_mirror_node ₂
`Op::Exact`
=I.J.K	=I.J.K	>=I.J.K, <I.J.(K+1)-0	>=I.J.K, <I.J.(K+1)-0
=I.J	>=I.J.0, <I.(J+1).0	>=I.J.0, <I.(J+1).0-0	>=I.J.0-0, <I.(J+1).0-0
=I	>=I.0.0, <(I+1).0.0	>=I.0.0, <(I+1).0.0-0	>=I.0.0-0, <(I+1).0.0-0
`Op::Greater`
>I.J.K	>I.J.K	>I.J.K	>I.J.K
>I.J	>=I.(J+1).0	>=I.(J+1).0-0	>=I.(J+1).0-0
>I	>=(I+1).0.0	>=(I+1).0.0-0	>=(I+1).0.0-0
`Op::GreaterEq`
>=I.J.K	>=I.J.K	>=I.J.K	>=I.J.K
>=I.J	>=I.J.0	>=I.J.0	>=I.J.0-0
>=I	>=I.0.0	>=I.0.0	>=I.0.0-0
`Op::Less`
<I.J.K	<I.J.K	<I.J.K or I.J.K-0 depends ₁	<I.J.K
<I.J	<I.J.0	<I.J.0-0	<I.J.0-0
<I	<I.0.0	<I.0.0-0	<I.0.0-0
`Op::LessEq`
<=I.J.K	<=I.J.K	<=I.J.K	<=I.J.K
<=I.J	<I.(J+1).0	<I.(J+1).0-0	<I.(J+1).0-0
<=I	<(I+1).0.0	<(I+1).0.0-0	<(I+1).0.0-0
`Op::Tilde`
~I.J.K	>=I.J.K, <I.(J+1).0	>=I.J.K, <I.(J+1).0-0	>=I.J.K, <I.(J+1).0-0
~I.J	=I.J	>=I.J.0, <I.(J+1).0-0	>=I.J.0, <I.(J+1).0-0
~I	=I	>=I.0.0, <(I+1).0.0-0	>=I.0.0, <(I+1).0.0-0
`Op::Caret`
^I.J.K (for I>0)	>=I.J.K, <(I+1).0.0	>=I.J.K, <(I+1).0.0-0	>=I.J.K, <(I+1).0.0-0
^0.J.K (for J>0)	>=0.J.K, <0.(J+1).0	>=0.J.K, <0.(J+1).0-0	>=0.J.K-0, <0.(J+1).0-0
^0.0.K	=0.0.K	>=0.0.K, <0.0.(K+1)-0	>=0.J.K-0, <0.(J+1).0-0
^I.J	^I.J.0	>=I.J.0, <(I+1).0.0-0	>=I.J.0-0, <(I+1).0.0-0
^0.0	=0.0	>=0.0.0, <0.1.0-0	>=0.0.0-0, <0.1.0-0
^I	=I	>=I.0.0, <(I+1).0.0-0	>=I.0.0-0, <(I+1).0.0-0
`Op::Wildcard`
`I.J.*`	=I.J	>=I.J.0, <I.(J+1).0-0	>=I.J.0-0, <I.(J+1).0-0
`I.` or `I..*`	=I	>=I.0.0, <(I+1).0.0-0	>=I.0.0-0, <(I+1).0.0-0

Notes:

<I.J.K: This is equivalent to <I.J.K-0 if no lower bound or the lower bound isn't pre-release, otherwise this is equivalent to <I.J.K.

matches_prerelease_mirror_node is yet another implementation of node semver compatibility (with includePrerelease=true) test. This is extrapolated from the test cases and is not necessarily the same as the node implementation

Besides, the proposed semtantic left a unsolved issuse , I don't have clear idea to handle it.

How should we test and review this PR?

The tests in src/cargo/util/semver_eval_ext.rs are designed to reflect current semantic.

Additional information

Migrated from dtolnay/semver#321

TBO, this feature was not conceived in advance plus the semantic is unclear at first. I need to experiment step by step and find out what I think makes sense. My experiment can be seen this comment.

rustbot · 2024-07-26T09:33:20Z

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

tests/testsuite/precise_pre_release.rs

src/cargo/util/semver_ext.rs

epage · 2024-07-27T00:47:45Z

I've not gone into a lot of the details yet, more focusing on the high level

weihanglo

Really appreciate your efforts on prerelease!

src/cargo/util/semver_ext.rs

weihanglo · 2024-07-28T13:20:14Z

src/cargo/util/semver_eval_ext.rs

@@ -0,0 +1,520 @@
+use semver::{Comparator, Op, Prerelease, Version, VersionReq};


Should we have some mod-level comments explaining what have been copied over from semver and why?

No problems.

Eh2406 · 2024-08-01T02:21:43Z

Thank you all for doing work on this. I am going on vacation and will not be able to look at or think about this for 2 weeks.

One open design question is whether "matches implies matches_prerelease" is an axiom of our design. If it is then there are all kinds of (obscure) corner cases we need to track down and decide how to deal with. In general we should modify the new thing to meet our axioms. But given how obscure the cases are we should also consider modifying the existing behavior. Breakage will probably be only theoretical.

Please do not let my absence slow down this effort. When people feel it's ready, let's merge this PR. Unstable features are cheap to change if we change our mind.

linyihai · 2024-08-20T09:08:11Z

The forced update commits just updated the comments of the code .

And I had updated the semantic in the PR description, exspecially for <I.J.K.

src/cargo/util/semver_eval_ext.rs

src/cargo/util/semver_ext.rs

tests/testsuite/precise_pre_release.rs

bors · 2024-08-20T16:54:31Z

☔ The latest upstream changes (presumably #14412) made this pull request unmergeable. Please resolve the merge conflicts.

epage · 2024-08-22T18:45:51Z

src/cargo/util/semver_eval_ext.rs

@@ -75,7 +75,7 @@ fn matches_exact(cmp: &Comparator, ver: &Version) -> bool {
 }

 // See https://github.com/dtolnay/semver/blob/69efd3cc770ead273a06ad1788477b3092996d29/src/eval.rs#L64-L88
-fn matches_greater(cmp: &Comparator, ver: &Version) -> bool {
+pub(crate) fn matches_greater(cmp: &Comparator, ver: &Version) -> bool {


Unsure if this commit is worth it. In theory, semver_eval_ext will be going away and we'll have to undo this.

Well, this commit is not very important for this PR. We can decide what to do with it later.

epage · 2024-08-23T15:58:27Z

Thanks!

@bors r+

bors · 2024-08-23T15:58:29Z

📌 Commit bdf19e0 has been approved by epage

It is now in the queue for this repository.

bors · 2024-08-23T15:59:37Z

⌛ Testing commit bdf19e0 with merge a0acc29...

bors · 2024-08-23T16:30:18Z

☀️ Test successful - checks-actions
Approved by: epage
Pushing a0acc29 to master...

Update cargo 9 commits in 8f40fc59fb0c8df91c97405785197f3c630304ea..c1fa840a85eca53818895901a53fae34247448b2 2024-08-21 22:37:06 +0000 to 2024-08-29 21:03:53 +0000 - fix(resolve): With `latest` message, differentiate actionable updates (rust-lang/cargo#14461) - fix(pkgid): Allow open namespaces in PackageIdSpec's (rust-lang/cargo#14467) - feat(resolve): Report incompatible-with-rustc when MSRV-resolver is disabled (rust-lang/cargo#14459) - fix(resolve): Report incompatible packages with precise Rust version (rust-lang/cargo#14457) - fix(resolve): Dont show locking workspace members (rust-lang/cargo#14445) - Log details of failure if no errors were seen (rust-lang/cargo#14453) - More helpful missing feature error message (rust-lang/cargo#14436) - feat: Add matches_prerelease semantic (rust-lang/cargo#14305) - refactor(update): Prepare for smarter update messages (rust-lang/cargo#14440) r? ghost

rustbot assigned weihanglo Jul 26, 2024

rustbot added A-semver Area: semver specifications, version matching, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2024

linyihai commented Jul 26, 2024

View reviewed changes

tests/testsuite/precise_pre_release.rs Show resolved Hide resolved

epage reviewed Jul 27, 2024

View reviewed changes

src/cargo/util/semver_ext.rs Outdated Show resolved Hide resolved

epage reviewed Jul 27, 2024

View reviewed changes

src/cargo/util/semver_ext.rs Outdated Show resolved Hide resolved

epage reviewed Jul 27, 2024

View reviewed changes

src/cargo/util/semver_ext.rs Outdated Show resolved Hide resolved

weihanglo reviewed Jul 28, 2024

View reviewed changes

linyihai force-pushed the matches-prerelease-semantic branch 2 times, most recently from 1586c33 to 3e47ef8 Compare July 29, 2024 02:46

This comment was marked as outdated.

Sign in to view

linyihai force-pushed the matches-prerelease-semantic branch from dbec425 to d2c4453 Compare August 2, 2024 03:13

Eh2406 mentioned this pull request Aug 14, 2024

Compatibility with "cargo update --precise" pubgrub-rs/semver-pubgrub#5

Open

epage mentioned this pull request Aug 19, 2024

fix: Limiting pre-release match semantics to use only on OptVersionReq::Req #14412

Merged

linyihai force-pushed the matches-prerelease-semantic branch from d2c4453 to dbe0ace Compare August 20, 2024 07:54

epage reviewed Aug 20, 2024

View reviewed changes

src/cargo/util/semver_eval_ext.rs Outdated Show resolved Hide resolved

epage reviewed Aug 20, 2024

View reviewed changes

src/cargo/util/semver_ext.rs Outdated Show resolved Hide resolved

epage reviewed Aug 20, 2024

View reviewed changes

src/cargo/util/semver_ext.rs Show resolved Hide resolved

epage reviewed Aug 20, 2024

View reviewed changes

tests/testsuite/precise_pre_release.rs Outdated Show resolved Hide resolved

linyihai added 4 commits August 22, 2024 10:08

test: add more test for pre-release matches semantic

e62c271

feat: Add matches_prerelease semantic

7c4236e

doc: Add mod level documents

c4d7d74

feat: add upper bound semantic for Less Op

bdf19e0

linyihai force-pushed the matches-prerelease-semantic branch from dbe0ace to de78ea3 Compare August 22, 2024 06:45

rustbot added the A-manifest Area: Cargo.toml issues label Aug 22, 2024

epage reviewed Aug 22, 2024

View reviewed changes

linyihai force-pushed the matches-prerelease-semantic branch from de78ea3 to bdf19e0 Compare August 23, 2024 03:16

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 23, 2024

bors merged commit a0acc29 into rust-lang:master Aug 23, 2024
22 checks passed

linyihai deleted the matches-prerelease-semantic branch August 24, 2024 14:33

weihanglo mentioned this pull request Aug 31, 2024

Update cargo rust-lang/rust#129798

Merged

rustbot added this to the 1.83.0 milestone Sep 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add matches_prerelease semantic #14305

feat: Add matches_prerelease semantic #14305

linyihai commented Jul 26, 2024 •

edited

Loading

rustbot commented Jul 26, 2024

epage commented Jul 27, 2024 •

edited

Loading

weihanglo left a comment

weihanglo Jul 28, 2024

linyihai Jul 29, 2024

This comment was marked as outdated.

Eh2406 commented Aug 1, 2024

linyihai commented Aug 20, 2024

bors commented Aug 20, 2024

epage Aug 22, 2024

linyihai Aug 23, 2024

epage commented Aug 23, 2024

bors commented Aug 23, 2024

bors commented Aug 23, 2024

bors commented Aug 23, 2024

		@@ -0,0 +1,520 @@
		use semver::{Comparator, Op, Prerelease, Version, VersionReq};

feat: Add matches_prerelease semantic #14305

feat: Add matches_prerelease semantic #14305

Conversation

linyihai commented Jul 26, 2024 • edited Loading

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Jul 26, 2024

epage commented Jul 27, 2024 • edited Loading

weihanglo left a comment

Choose a reason for hiding this comment

weihanglo Jul 28, 2024

Choose a reason for hiding this comment

linyihai Jul 29, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

Eh2406 commented Aug 1, 2024

linyihai commented Aug 20, 2024

bors commented Aug 20, 2024

epage Aug 22, 2024

Choose a reason for hiding this comment

linyihai Aug 23, 2024

Choose a reason for hiding this comment

epage commented Aug 23, 2024

bors commented Aug 23, 2024

bors commented Aug 23, 2024

bors commented Aug 23, 2024

linyihai commented Jul 26, 2024 •

edited

Loading

epage commented Jul 27, 2024 •

edited

Loading