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

cargo add takes package.rust-version into account when no version-req is specified #10653

Closed
epage opened this issue May 11, 2022 · 6 comments
Closed
Assignees
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-add

Comments

@epage
Copy link
Contributor

epage commented May 11, 2022


Problem

When running cargo add foo, we create a version-req out of the latest version in the index. If this has a higher rust-version, then that can break for people.

Proposed Solution

Constrain the candidate versions in the index by their rust-version being lesser or equal to the current crate's rust-version.

The main question is what to do when there are no matches due to rust-version.

  • Add anyways
  • Error with newest version's rust-version

Notes

No response

@epage epage added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-add labels May 11, 2022
@epage
Copy link
Contributor Author

epage commented May 11, 2022

See also killercup/cargo-edit#587

@epage
Copy link
Contributor Author

epage commented Jul 26, 2022

When rust-version gets added to the registry, we should inform Dependabot people on dependabot/dependabot-core#5423

bors added a commit that referenced this issue Apr 28, 2023
feat: Add `-Zmsrv-policy` feature flag

### What does this PR try to resolve?

Nothing noticeable....

The intent is to unblock experiments with different compatible MSRV policies like
- #9930
- #10653
- #10903

While I normally don't like PRs that do nothing on their own, this at least allows any one of those efforts to move forward with different people without juggling these base commits for whoever is first to include in their PR

While there isn't an RFC for this yet, this is intended to allow us to experiment to get a better idea of what we should put in an RFC.  In some cases, we first do an eRFC for this but I assumed this wouldn't be needed in this case as this builds on rust-lang/rfcs#2495 and, I'm assuming, will be more surgical in nature

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

The `Summary` changes are largely untested as they will be mostly tested through the future work that builds on this PR.  However, I wasn't too concerned about that because the code is relatively trivial.

### Additional information

I chose the name `msrv-policy` to distinguish this unstable feature from `rust-version`.  Though those appear in different places (`Cargo.toml` vs `-Z`), I can see them being confusing which was especially apparent when editing `unstable.md` which has an anchor for `rust-version`.
@cassaundra
Copy link
Contributor

@rustbot claim

cassaundra added a commit to cassaundra/cargo that referenced this issue May 2, 2023
When `-Zmsrv-policy` is enabled, try to select dependencies which satisfy the
target package's `rust-version` field (if present). If the selected version is
not also the latest, emit a warning to the user about this discrepancy.

Dependency versions without a `rust-version` are considered compatible by
default.

Implements rust-lang#10653.
cassaundra added a commit to cassaundra/cargo that referenced this issue May 3, 2023
When `-Zmsrv-policy` is enabled, try to select dependencies which satisfy the
target package's `rust-version` field (if present). If the selected version is
not also the latest, emit a warning to the user about this discrepancy.

Dependency versions without a `rust-version` are considered compatible by
default.

Implements rust-lang#10653.
cassaundra added a commit to cassaundra/cargo that referenced this issue May 4, 2023
When `-Zmsrv-policy` is enabled, try to select dependencies which satisfy the
target package's `rust-version` field (if present). If the selected version is
not also the latest, emit a warning to the user about this discrepancy.

Dependency versions without a `rust-version` are considered compatible by
default.

Implements rust-lang#10653.
cassaundra added a commit to cassaundra/cargo that referenced this issue May 15, 2023
When `-Zmsrv-policy` is enabled, try to select dependencies which satisfy the
target package's `rust-version` field (if present). If the selected version is
not also the latest, emit a warning to the user about this discrepancy.

Dependency versions without a `rust-version` are considered compatible by
default.

Implements rust-lang#10653.
cassaundra added a commit to cassaundra/cargo that referenced this issue May 18, 2023
When `-Zmsrv-policy` is enabled, try to select dependencies which satisfy the
target package's `rust-version` field (if present). If the selected version is
not also the latest, emit a warning to the user about this discrepancy.

Dependency versions without a `rust-version` are considered compatible by
default.

Implements rust-lang#10653.
cassaundra added a commit to cassaundra/cargo that referenced this issue May 23, 2023
When `-Zmsrv-policy` is enabled, try to select dependencies which satisfy the
target package's `rust-version` field (if present). If the selected version is
not also the latest, emit a warning to the user about this discrepancy.

Dependency versions without a `rust-version` are considered compatible by
default.

Implements rust-lang#10653.
bors added a commit that referenced this issue May 23, 2023
Consider rust-version when selecting packages for cargo add

When `-Zmsrv-policy` is enabled, try to select dependencies which satisfy the target package's `rust-version` field (if present). If the selected version is not also the latest, emit a warning to the user about this discrepancy.

Dependency versions without a `rust-version` are considered compatible by default.

One remaining question is whether we should go into more detail when explaining the discrepancy and ways to resolve it to the user. For example:

```
warning: selecting older version of `fancy-dep` to satisfy the minimum supported rust version
note: version 0.1.2 of `fancy-dep` has an MSRV of 1.72, which is greater than this package's MSRV of 1.69
```

Implements #10653.

r? `@epage`
@cassaundra
Copy link
Contributor

Implemented in #12078. Maybe close?

@epage
Copy link
Contributor Author

epage commented May 26, 2023

Implemented but not stable so this works as a place to track stabilization. I should update the issue to reflect that though.

@epage
Copy link
Contributor Author

epage commented Nov 30, 2023

I've moved stabilization tracking to #9930 as part of rust-lang/rfcs#3537

@epage epage closed this as completed Nov 30, 2023
bors added a commit that referenced this issue Mar 3, 2024
feat(add): Fallback to `rustc -v` when no MSRV is set

### What does this PR try to resolve?

#10653 made version-requirement selection respect MSRV as part of #9930.
This updates the implementation for the now-approved RFC specifies that we should respect `rustc -v` if there is no MSRV.

The messages also get a little bit of polish.

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

Tests are added in separate commits for easier viewing of behavior changes.

### Additional information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-add
Projects
None yet
Development

No branches or pull requests

2 participants