Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking Issue for update --precise with pre-release #13290

Open
2 tasks
ehuss opened this issue Jan 13, 2024 · 9 comments
Open
2 tasks

Tracking Issue for update --precise with pre-release #13290

ehuss opened this issue Jan 13, 2024 · 9 comments
Labels
C-tracking-issue Category: A tracking issue for something unstable. Command-update S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns.

Comments

@ehuss
Copy link
Contributor

ehuss commented Jan 13, 2024

Summary

RFC: #3493
Original issue: #12579
Implementation: #13626
Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#precise-pre-release

Extends cargo update --precise to allow pre-release versions, even if the dependency declarations do not specify a pre-release.

Unresolved Issues

  • Upstream pre-release match logic to semver crate
  • Upper bound semantic. See the unresolved questions in the RFC.

Future Extensions

No response

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

@ehuss ehuss added Command-update S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. C-tracking-issue Category: A tracking issue for something unstable. labels Jan 13, 2024
@ehuss ehuss moved this to Big Projects, no backers in Cargo Roadmap Jan 13, 2024
@ehuss ehuss added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. labels Jan 13, 2024
eopb added a commit to eopb/cargo that referenced this issue Jan 14, 2024
Tracking Issue: [rust-lang#13290](rust-lang/rust#13290)

This change introduces the feature but does not yet attempt an implementation.
The actual implementation will happen in future PRs

r? @epage
bors added a commit that referenced this issue Jan 15, 2024
Introduce `-Zprecise-pre-release` unstable flag

Tracking Issue: [#13290](#13290)

This change introduces the feature but does not yet attempt an implementation. The actual implementation will happen in future PRs.

r? `@epage`
@epage epage changed the title Tracking Issue for precise pre-release update Tracking Issue for update --precise with pre-release Feb 8, 2024
@linyihai
Copy link
Contributor

linyihai commented Mar 4, 2024

Since #13296 had merged, i think i can lend a hand to continue.

Just in case I forgot, I was gonna @rustbot claim.

@epage
Copy link
Contributor

epage commented Mar 4, 2024

I believe @eopb was planning on working on this. I'd recommend coordinating with them

@eopb
Copy link
Contributor

eopb commented Mar 6, 2024

Thanks Ed. Yes, I'm still intending to work on this. If you'd like to help @linyihai, please reach out to me on zulip (under the same username as here).

I'm sure we can work together. The last thing I'd want to do is to slow things down by trying to hog this tracking issue.

@linyihai linyihai removed their assignment Mar 7, 2024
@linyihai
Copy link
Contributor

linyihai commented Mar 7, 2024

Yes, I'm still intending to work on this. I'm sure we can work together. The last thing I'd want to do is to slow things down by trying to hog this tracking issue.

I'm sorry that I started the claim without further understanding of the requirement. You're the one who came up with this RFC, so I'm sure you know more details than I do. If you think it's better to do it yourself, that's fine. If you need help, break it down into smaller issues so that others see it and can help @eopb

please reach out to me on zulip (under the same username as here).

I'll be in touch if I need anything. Thank you @eopb

@linyihai
Copy link
Contributor

FYI,I recently tried to implement an MVP based on RFC content.

bors added a commit that referenced this issue Apr 3, 2024
Allow precise update to prerelease.

### What does this PR try to resolve?

This is a feature that attempts to support updates to pre-release versions via `cargo update --precise`.

when `precise-pre-release` used, the prerelase version will be taking consider as compatible version.  That said, we can update to any compatible pre-release version. The logic of checking the compatibility of pre-release versions is currently tentative and does not take many conditions into account, this part of the logic makes more sense when implemented in semver.

Use `-Zunstable-options`  instead of  `-Zprecise-pre-release`.

### How should we test and review this PR?

### Additional information
Part of #13290
@weihanglo
Copy link
Member

#14013 shows the current implementation is a bit buggy, and there are upper bound semantic issues not yet resolved. See these test cases:

let cases = [
//
("1.2.3", "1.2.3-0", true), // bug, must be false
("1.2.3", "1.2.3-1", true), // bug, must be false
("1.2.3", "1.2.4-0", true),
//
(">=1.2.3", "1.2.3-0", true), // bug, must be false
(">=1.2.3", "1.2.3-1", true), // bug, must be false
(">=1.2.3", "1.2.4-0", true),
//
(">1.2.3", "1.2.3-0", false),
(">1.2.3", "1.2.3-1", false),
(">1.2.3", "1.2.4-0", true),
//
(">1.2.3, <1.2.4", "1.2.3-0", false),
(">1.2.3, <1.2.4", "1.2.3-1", false),
(">1.2.3, <1.2.4", "1.2.4-0", false), // upper bound semantic
//
(">=1.2.3, <1.2.4", "1.2.3-0", true), // bug, must be false
(">=1.2.3, <1.2.4", "1.2.3-1", true), // bug, must be false
(">=1.2.3, <1.2.4", "1.2.4-0", false), // upper bound semantic
//
(">1.2.3, <=1.2.4", "1.2.3-0", false),
(">1.2.3, <=1.2.4", "1.2.3-1", false),
(">1.2.3, <=1.2.4", "1.2.4-0", true),
//
(">=1.2.3-0, <1.2.3", "1.2.3-0", false), // upper bound semantic
(">=1.2.3-0, <1.2.3", "1.2.3-1", false), // upper bound semantic
(">=1.2.3-0, <1.2.3", "1.2.4-0", false),
];

It is technically possible to fix them inside cargo. However to do that we'll need to duplicate the logic in semver crate. I am in the team of upstreaming things to semver.

@linyihai
Copy link
Contributor

linyihai commented Jun 5, 2024

Yes, The semver crate only supports to compare prerelease with prerelease if I remember correct,
https://docs.rs/semver/latest/src/semver/eval.rs.html#14-21

 // If a version has a prerelease tag (for example, 1.2.3-alpha.3) then it
    // will only be allowed to satisfy req if at least one comparator with the
    // same major.minor.patch also has a prerelease tag.
    for cmp in &req.comparators {
        if pre_is_compatible(cmp, ver) {
            return true;
        }
    }

So we need to add the pre tag to the version_req in order to use the semver crate, like RFC writes:

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

@linyihai
Copy link
Contributor

linyihai commented Jul 9, 2024

I have drafted a semver side PR to address upper bound semantic, but it stucks on node tests.

Also, my changes to cargo can be viewed in detail in this branch, which seems to solve the problem

@linyihai
Copy link
Contributor

After some experiments on semver PR, I found something interesting,

  • the semver crate matches implementaion may be based on node semver compatibility. the semver matches follow the limit if the version has a prerelease tag then its MAJOR.MINOR.PATCH must be same with the VersionReq at least once, and another issuse is it can't handle partial VersionReq
  • By removing the limit and handle the partial VersionReq, the first proposed semantic came out. The prerelease tag padding logic has some controversial, like '<4.2', we chose pading it to '<4.2.0' not '<4.2.0-0' .

By look into the node semver compatibility semantic, it offers includePrerelease=true to remove the limit, then I add the test and try to implement it in semver, see dtolnay/semver#321 (comment). The result is the node semver compatibility implement ation also covers the semantic proposed by rust-lang/rfcs#3493.

bors added a commit that referenced this issue Aug 23, 2024
feat: Add matches_prerelease semantic

### 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 [<sub>2<sub>](#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 [<sub>1<sub>](#op-less) | <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:

<div id="op-less"></div>

- `<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`.

<div id="mirror-node"></div>

- [matches_prerelease_mirror_node](dtolnay/semver@3464fd1) is yet another  implementation of [node semver compatibility](https://github.com/npm/node-semver?tab=readme-ov-file#semver1----the-semantic-versioner-for-npm) (with [includePrerelease=true](https://github.com/npm/node-semver?tab=readme-ov-file#functions)) 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 ](dtolnay/semver#321 (comment)), 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](#13290 (comment)).
@weihanglo weihanglo added the S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. label Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for something unstable. Command-update S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns.
Projects
Status: Big Projects, no backers
Development

No branches or pull requests

5 participants