cargo icon indicating copy to clipboard operation
cargo copied to clipboard
verified publisher icon rust-lang

feat: Add matches_prerelease semantic

Open linyihai opened this issue 1 year ago • 6 comments

What does this PR try to resolve?

One implementaion for https://github.com/rust-lang/cargo/issues/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 2
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 1 <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.

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 https://github.com/dtolnay/semver/pull/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.

linyihai avatar Jul 26 '24 09:07 linyihai

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

rustbot avatar Jul 26 '24 09:07 rustbot

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

epage avatar Jul 27 '24 00:07 epage

For reference, matches_prerelease_mirror_node is yet another implementaion of Node, this is a little different from what we proposed, see below,

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 <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::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:

  • matches flows the limit, that says if the version has a prerelease tag then its MAJOR.MINOR.PATCH must be same with the VersionReq at least once and this VersionReq prerelase tag must speficy, ie. ^1.2.3-0 meets 1.2.3-1 but not ^1.2.4-1, ^1.2.3 will not meets any prerelase version.
  • matches_prerelease_mirror_node is a 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
  • the semantic decorated with Bold Italic between matches_prerelease and matches_prerelease_mirror_node is totally same
table source 
| 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***           | ***<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::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           |

linyihai avatar Jul 29 '24 08:07 linyihai

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.

Eh2406 avatar Aug 01 '24 02:08 Eh2406

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.

linyihai avatar Aug 20 '24 09:08 linyihai

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

bors avatar Aug 20 '24 16:08 bors

Thanks!

@bors r+

epage avatar Aug 23 '24 15:08 epage

:pushpin: Commit bdf19e054477bb2c7a1d0fc3c1a9b1e1fd814a9f has been approved by epage

It is now in the queue for this repository.

bors avatar Aug 23 '24 15:08 bors

:hourglass: Testing commit bdf19e054477bb2c7a1d0fc3c1a9b1e1fd814a9f with merge a0acc29f7bdbc6549f1682de3e8a9677b2f70641...

bors avatar Aug 23 '24 15:08 bors

:sunny: Test successful - checks-actions Approved by: epage Pushing a0acc29f7bdbc6549f1682de3e8a9677b2f70641 to master...

bors avatar Aug 23 '24 16:08 bors