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

Call for Testing: MSRV-aware resolver #13873

Open
epage opened this issue May 7, 2024 · 21 comments
Open

Call for Testing: MSRV-aware resolver #13873

epage opened this issue May 7, 2024 · 21 comments
Labels
A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns.

Comments

@epage
Copy link
Contributor

epage commented May 7, 2024

Call for testing

The MSRV-aware resolver part of this RFC is implemented and we are wanting to collect feedback in preparation for stabilization.

The goal is to allow you to manage your dependencies without having to manually ensure they are compatible with your rust-version

  • When stabilized, this will be opt-in. When upgrading to the 2024 Edition, it will be the opt-out.
  • It uses package.rust-version if set, otherwise rustc --version
  • It won't fail if there isn't a MSRV-compatible version but instead it will pick a compatible version

Basic steps:

Requirements:

At minimum, run

$ CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=fallback cargo +nightly -Zmsrv-policy generate-lockfile

Feel free to run any other commands and workflows with

  • CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=fallback (proposed behavior) or CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=allow (existing behavior) with +nightly -Zmsrv-policy
    • You could also set them via .cargo/config.toml
  • workspace.resolver = "3" or package.edition = "2024" with cargo-features = ["edition2024"] in your Cargo.toml

See also Documentation

Known issues

Changes between call for testing

Please leave feedback on this issue

@epage epage added A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. labels May 7, 2024
@Nemo157
Copy link
Member

Nemo157 commented May 8, 2024

Initial impressions, when I ran this command in a project (https://github.com/Nemo157/u2f-touch-detector @ e12096fe3571ac3b7c501dca9f9d452b2b9f0e4c) it emitted what looked like a bunch of warnings to me:

> CARGO_RESOLVER_SOMETHING_LIKE_PRECEDENCE=something-like-rust-version cargo -Zmsrv-policy generate-lockfile
    Updating crates.io index
     Locking 101 packages to latest Rust 1.79.0 compatible versions
      Adding addr2line v0.21.0 (latest: v0.22.0)
      Adding gimli v0.28.1 (latest: v0.29.0)
      Adding io-lifetimes v1.0.11 (latest: v2.0.3)
      Adding linux-raw-sys v0.4.13 (latest: v0.6.4)
      Adding nix v0.27.1 (latest: v0.28.0)
      Adding nu-ansi-term v0.46.0 (latest: v0.50.0)
      Adding object v0.32.2 (latest: v0.35.0)
      Adding owo-colors v3.5.0 (latest: v4.0.0)
      Adding regex-automata v0.1.10 (latest: v0.4.6)
      Adding regex-syntax v0.6.29 (latest: v0.8.3)
      Adding windows-sys v0.48.0 (latest: v0.52.0)
      Adding windows-targets v0.48.5 (latest: v0.52.5)
      Adding windows_aarch64_gnullvm v0.48.5 (latest: v0.52.5)
      Adding windows_aarch64_msvc v0.48.5 (latest: v0.52.5)
      Adding windows_i686_gnu v0.48.5 (latest: v0.52.5)
      Adding windows_i686_msvc v0.48.5 (latest: v0.52.5)
      Adding windows_x86_64_gnu v0.48.5 (latest: v0.52.5)
      Adding windows_x86_64_gnullvm v0.48.5 (latest: v0.52.5)
      Adding windows_x86_64_msvc v0.48.5 (latest: v0.52.5)

Looking into the lockfile this had nothing to do with the MSRV clause though, this is just the transitive version requirements of my dependencies, so the same messages are emitted without the msrv-policy. It seems like there should be some distinction in these messages about why the latest isn't being selected.

I tried to force a MSRV conflict by extracting eyre@0.6.12 into a local directory, adding a patch.crates-io pointing to it, and bumping it to have version = "0.6.13" and rust-version = "1.80.0". Running the above command seemed to ignore that and still chose to use the local 0.6.13 version instead of the remote 0.6.12; but maybe there's something specifically about patch/path dependencies causing that.

@Nemo157
Copy link
Member

Nemo157 commented May 9, 2024

I came up with a different scheme for testing a current-rustc MSRV conflict, I overlaid a nightly cargo version on top of a stable 1.72 toolchain (I have no idea if something like this is possible with rustup, but it's easy to do with oxalica/rust-overlay with nix):

> nix shell pkgs#rust-bin.stable.'"1.72.0"'.minimal
> nix shell pkgs#rust-bin.nightly.latest.cargo
> cargo --version && rustc --version
cargo 1.80.0-nightly (0ca60e940 2024-05-08)
rustc 1.72.0 (5680fa18f 2023-08-23)

Regenerating the lockfile seemed to happen just as before, but correctly detected 1.72:

> CARGO_RESOLVER_SOMETHING_LIKE_PRECEDENCE=something-like-rust-version cargo -Zmsrv-policy generate-lockfile
    Updating crates.io index
     Locking 101 packages to latest Rust 1.72.0 compatible versions
      ... same as before ...

Trying to build then failed

> cargo check
error: rustc 1.72.0 is not supported by the following packages:
  clap@4.5.4 requires rustc 1.74
  clap_builder@4.5.2 requires rustc 1.74
  clap_derive@4.5.4 requires rustc 1.74
  clap_lex@0.7.0 requires rustc 1.74
Either upgrade rustc or select compatible dependency versions with
`cargo update <name>@<current-ver> --precise <compatible-ver>`
where `<compatible-ver>` is the latest version supporting rustc 1.72.0

I would have expected a warning about this when generating the lockfile (maybe even mentioning the latest compatible version so I didn't have to go searching the repository for when clap's MSRV was last bumped).

Setting the minimum requirement on clap back to a compatible version, regenerating the lockfile and building worked, but as I mentioned above there's no distinction that the reason the latest wasn't chosen was MSRV related rather than version constraint related

> cargo add clap@4.4.18
    Updating crates.io index
      Adding clap v4.4.18 to dependencies
             Features:
             ...

> CARGO_RESOLVER_SOMETHING_LIKE_PRECEDENCE=something-like-rust-version cargo -Zmsrv-policy generate-lockfile
    Updating crates.io index
     Locking 101 packages to latest Rust 1.72.0 compatible versions
      Adding addr2line v0.21.0 (latest: v0.22.0)
      Adding clap v4.4.18 (latest: v4.5.4)
      Adding clap_builder v4.4.18 (latest: v4.5.2)
      Adding clap_derive v4.4.7 (latest: v4.5.4)
      Adding clap_lex v0.6.0 (latest: v0.7.0)
      ... rest same as before ...

> cargo check
    ...
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 12.62s

@epage
Copy link
Contributor Author

epage commented May 9, 2024

We have two challenges with the reporting

As for having too high of version requirements, I wonder how often that will be happening in practice. Users get their version requirements from

  • cargo add: will auto-select a compatible version
  • crates.io: shows what the MSRV is

Hopefully this isn't a high traffic workflow that we have to worry about. Instead, we also expect people to see these messages when they are intentionally not using the MSRV-aware resolver where there is nothing actionable from the messages and they will likely annoy people.

I think I'd be ok with experimenting with adding to Adding line a requires Rust 1.74.0 but also saying what an alternative is might be a bit too noisy.

@Xaeroxe
Copy link

Xaeroxe commented May 10, 2024

This works great for the which crate. We've previously struggled with this issue, so thanks a bunch for devoting attention to it!

$ CARGO_RESOLVER_SOMETHING_LIKE_PRECEDENCE=something-like-rust-version cargo +nightly -Zmsrv-policy generate-lockfile
    Updating crates.io index
     Locking 36 packages to latest Rust 1.63 compatible versions
      Adding home v0.5.5 (latest: v0.5.9)
      Adding linux-raw-sys v0.4.13 (latest: v0.6.4)
      Adding regex v1.9.6 (latest: v1.10.4)
      Adding regex-automata v0.3.9 (latest: v0.4.6)
      Adding regex-syntax v0.7.5 (latest: v0.8.3)
      Adding windows-sys v0.48.0 (latest: v0.52.0)
      Adding windows-targets v0.48.5 (latest: v0.52.5)
      Adding windows_aarch64_gnullvm v0.48.5 (latest: v0.52.5)
      Adding windows_aarch64_msvc v0.48.5 (latest: v0.52.5)
      Adding windows_i686_gnu v0.48.5 (latest: v0.52.5)
      Adding windows_i686_msvc v0.48.5 (latest: v0.52.5)
      Adding windows_x86_64_gnu v0.48.5 (latest: v0.52.5)
      Adding windows_x86_64_gnullvm v0.48.5 (latest: v0.52.5)
      Adding windows_x86_64_msvc v0.48.5 (latest: v0.52.5)
$ cargo +1.69.0 build
   Compiling rustix v0.38.34
   Compiling bitflags v2.5.0
   Compiling linux-raw-sys v0.4.13
   Compiling home v0.5.5
   Compiling either v1.11.0
   Compiling which v5.0.0 (~/Documents/source_code/which-rs)
    Finished dev [unoptimized + debuginfo] target(s) in 1.00s

@Darksonn
Copy link

What is the syntax for configuring this using .cargo/config.toml? I could not figure it out.

Is there an option for using dependencies compatible with whichever rustc the user happens to be using?

@weihanglo
Copy link
Member

What is the syntax for configuring this using .cargo/config.toml? I could not figure it out.

Is there an option for using dependencies compatible with whichever rustc the user happens to be using?

This should work.

[unstable]
msrv-policy = true # or put this in CLI `-Zmsrv-policy`

[resolver]
something-like-precedence = 'something-like-rust-version'

@savente93

This comment was marked as resolved.

@weihanglo

This comment was marked as resolved.

@savente93

This comment was marked as resolved.

@kornelski
Copy link
Contributor

There's been a case where MSRV-aware resolver didn't help:

bitstream-io, an indirect dependency of the image crate, has started using a 1.79-only feature in v2.4.0 without specifying any rust-version. They've added rust-version = 1.79 in v2.4.1, but due to lack of rust-version set in previous release, the MSRV-aware Cargo falls back to the incompatible 2.4.0, instead of 2.3.0.

I'm not sure if Cargo can do anything about it.

  • Maybe if a dependency version fails to compile due to an unstable feature, add it to some temporary local blocklist, and try resolving deps again, falling back to an older version?

  • crates.io could allow owners to set rust-version manually on already-published crates?

  • Upgrade clippy::incompatible-msrv to a default-warn lint, and make it aware of language features, not just standard library annotations?

@epage
Copy link
Contributor Author

epage commented Jun 21, 2024

Yes, I've run into that as well, ignore being one example.

This was recognized as a problem in the RFC that we punted on.

crates.io could allow owners to set rust-version manually on already-published crates?

This is likely the smallest in scope implementation-wise. I think Cargo would happen to not get confused from the manifest and index not matching (granted, this is an implementation detail that might change). If this idea interests you, it might be worth reaching out to the crates.io team to see how open they are to it.

@epage
Copy link
Contributor Author

epage commented Aug 8, 2024

Note: I've updated the testing instructions to take into account the new config names as of #14296 and started a new round of call-for-testing in prep for stabilization.

shepmaster added a commit to integer32llc/registry-conformance that referenced this issue Aug 9, 2024
@VorpalBlade
Copy link

As posted on the ULRO call for testing (but duplicating it here so it doesn't get lost):


Slight gotcha (that I didn't expect) is that in a workspace it will pick the oldest MSRV of all the crates. Even if that crate doesn't depend on whatever it is that is downgraded. Eg. in the following virtual workspace:

crates/a (MSRV 1.80.0, depends on tokio)
crates/b (MSRV 1.65.0, does not depend on tokio, even indirectly)
Cargo.lock
Cargo.toml

It will pick a version of tokio that works for 1.65.0 (even though this isn't needed when publishing to crates.io)

That seems a bit broken?

@Darksonn
Copy link

Darksonn commented Aug 11, 2024

A similar example. Consider a workspace with these two crates:

[package]
name = "abc"
version = "0.1.0"
edition = "2021"
rust-version = "1.63.0"

[dependencies]
tokio = "1.30.0"

and

[package]
name = "def"
version = "0.1.0"
edition = "2021"
rust-version = "1.60.0"

[dependencies]

In this case, the following commands:

$ cd abc
$ CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=fallback cargo +nightly -Zmsrv-policy generate-lockfile
    Updating crates.io index
     Locking 15 packages to latest Rust 1.60.0 compatible versions
      Adding addr2line v0.21.0 (latest: v0.24.1)
      Adding backtrace v0.3.69 (latest: v0.3.73)
      Adding cc v1.0.94 (latest: v1.1.10)
      Adding gimli v0.28.1 (latest: v0.31.0)
      Adding memchr v2.6.2 (latest: v2.7.4)
      Adding miniz_oxide v0.7.4 (latest: v0.8.0)
      Adding object v0.32.2 (latest: v0.36.3)
$ cargo +1.63 build
error: package `tokio v1.39.2` cannot be built because it requires rustc 1.70 or newer, while the currently active rustc version is 1.63.0

We have conditions exactly like this on some of Tokio's LTS branches. For example, the tokio-1.32.x branch containing a Tokio version with an MSRV of 1.63 also happens to have some other crates that still specify rust-version = 1.56, so the tool will pick 1.56 instead of 1.63.

Is there a way to specify a specific rustc version to use for resolving deps? CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS does not seem to accept specific version numbers.

As an aside, the large amount of output about crates needing major version bumps makes it difficult to tell whether the resolver actually made the decision to downgrade anything or not.

@epage
Copy link
Contributor Author

epage commented Aug 12, 2024

Slight gotcha (that I didn't expect) is that in a workspace it will pick the oldest MSRV of all the crates. Even if that crate doesn't depend on whatever it is that is downgraded. Eg. in the following virtual workspace:

cargo updates output tries to communicate this by saying Locking 15 packages to latest Rust 1.65.0 compatible versions. There is also a lot of other output, so its understandable if it gets lost.

From what I understand of the resolver, this is an expected limitation of the fallback mechanism, related to the lack of backtracking.

When we are evaluating a version requirement, we don't know what all version requirements might apply. We filter and sort to find a version we pick and move on. If a chosen version is filtered out for another version req, we backtrack. fallback applies to the sorting step, not the filtering step, and never gets re-evaluated unless something else causes enough of a backtrack. If we sorted per version requirement, what you got would be dependent on the order the dependency tree was walked.

Originally we had considered incompatible-rust-versions = "deny" as the policy. The quality of the error messages from the current resolver is a blocker to that policy and stopped us from looking any further into this. In discussing fallback, other workflows came up that wouldn't work with deny, like no-MSRV-dev-dependencies or per-feature MSRVs.

The fallback mechanism was a compromise to get something that works now.

Once we have the PugGrub resolver, we'll also need to figure out how to map package versions back to workspace members back to know which MSRVs apply.

Is there a way to specify a specific rustc version to use for resolving deps? CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS does not seem to accept specific version numbers.

This is left as a future possibility.

As an aside, the large amount of output about crates needing major version bumps makes it difficult to tell whether the resolver actually made the decision to downgrade anything or not.

See also #13908

We should probably highlight when an MSRV-incompatible version is chosen.

@VorpalBlade
Copy link

This is left as a future possibility.

Hm, is the plan to make the MSRV aware resolver with this limitation default behaviour? If so that could break a lot of workspaces, where you don't want to force everything to be on the same MSRV. I have the luxury of being able to update everything to N-2 or even N (depending on the project). Some projects have promised far longer support windows and don't have that option.

If this is instead opt-in at this point I see no issues.

@epage
Copy link
Contributor Author

epage commented Aug 12, 2024

Hm, is the plan to make the MSRV aware resolver with this limitation default behaviour? If so that could break a lot of workspaces, where you don't want to force everything to be on the same MSRV

...

If this is instead opt-in at this point I see no issues.

What is "breaking" about this? That if people choose to re-generate their lockfile, bugs in their version requirements may be exposed? That seems out of the scope of us changing how things get resoled in the future.

Yes, this feature won't automatically take care of all problems for all people. We also acknowledge that when an MSRV isn't set or wasn't set but now is. I'm most projects are not in a mixed-MSRV scenario and those that are, are likely for more experienced users who are already doing strange things to handle this.

@VorpalBlade
Copy link

So, my use case is that I have several crates for a project (usually a command line program or embedded thing). Some of those crates can be generally useful (maybe a parser for a specific file format or things of that nature), and as such I publish them to crates.io as more polished standalone things. But it is easier for me to maintain it all in one workspace, especially while making breaking changes.

I don't see a need to bump MSRV unless there is a feature I need, and as such I tend to leave MSRV alone unless something forces it (either a dependency or me wanting to use a newer feature, yeah the bar for actually doing a bump according to my policy is low, I just don't like doing the bump needlessly).

Now this puts me in a strange position for such a workspace where I have to bump the more stable crates MSRV just because I need the resolver to be sensible for other crates with more active development. Especially annoying since most of the time those more stable crates are more foundational and probably only depends on winnow or something of that nature (and as such should really not be affected by tokio or whatever needing a newer MSRV).

That is the part I'm a bit annoyed at. With the new resolver, what would the recommended workflow for this use case be?

@epage
Copy link
Contributor Author

epage commented Aug 12, 2024

So if I'm understanding correctly, you have

  • libs with a low MSRV
    • version reqs match your low MSRV
  • bin with a high MSRV
    • version reqs match your high MSRV

When a change causes bin's dep to change, something compatible with your libs' low MSRV isn't found and instead the absolute latest is picked, despite being incompatible with your bin's high MSRV?

Mind opening an issue on that scenario so we can have a more dedicated conversation around it and explore what the potential options are?

@VorpalBlade
Copy link

Its late here now, but I'll open an issue tomorrow after work.

epage added a commit to epage/cargo that referenced this issue Aug 14, 2024
…cted

In discussin this in rust-lang#13873, it highlighted that we need to make sure we
tell people when we get in this state.

I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise.  I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.

I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
- red for "required rust version", yellow for "latest"
- yellow for "required rust version", nothing for "latest"

There is also more discussion on how to format "latest" at rust-lang#13908.
epage added a commit to epage/cargo that referenced this issue Aug 14, 2024
…cted

In discussin this in rust-lang#13873, it highlighted that we need to make sure we
tell people when we get in this state.

I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise.  I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.

I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
- red for "required rust version", yellow for "latest"
- yellow for "required rust version", nothing for "latest"

There is also more discussion on how to format "latest" at rust-lang#13908.
epage added a commit to epage/cargo that referenced this issue Aug 15, 2024
…cted

In discussin this in rust-lang#13873, it highlighted that we need to make sure we
tell people when we get in this state.

I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise.  I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.

I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
- red for "required rust version", yellow for "latest"
- yellow for "required rust version", nothing for "latest"

There is also more discussion on how to format "latest" at rust-lang#13908.
@VorpalBlade
Copy link

@epage I opened a new issue at #14414 (sorry it took a few days before I had time and energy left over to do so)

I don't fully understand your summary of my comment, I suspect we might have been talking past each other a bit (it sounds backwards to me)

epage added a commit to epage/cargo that referenced this issue Aug 16, 2024
…cted

In discussin this in rust-lang#13873, it highlighted that we need to make sure we
tell people when we get in this state.

I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise.  I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.

I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
- red for "required rust version", yellow for "latest"
- yellow for "required rust version", nothing for "latest"

There is also more discussion on how to format "latest" at rust-lang#13908.
bors added a commit that referenced this issue Aug 16, 2024
feat(update): Report when incompatible-rust-version packages are selected

### What does this PR try to resolve?

In discussin this in #13873, it highlighted that we need to make sure we
tell people when incompatible-rust-version packages are selected.

I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise.  I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.

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

### Additional information

I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
- red for "required rust version", yellow for "latest"
- yellow for "required rust version", nothing for "latest"

There is also more discussion on how to format "latest" at #13908.
bors added a commit that referenced this issue Aug 16, 2024
feat(update): Report when incompatible-rust-version packages are selected

### What does this PR try to resolve?

In discussin this in #13873, it highlighted that we need to make sure we
tell people when incompatible-rust-version packages are selected.

I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise.  I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.

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

### Additional information

I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
- red for "required rust version", yellow for "latest"
- yellow for "required rust version", nothing for "latest"

There is also more discussion on how to format "latest" at #13908.
bors added a commit that referenced this issue Aug 16, 2024
feat(update): Report when incompatible-rust-version packages are selected

### What does this PR try to resolve?

In discussin this in #13873, it highlighted that we need to make sure we
tell people when incompatible-rust-version packages are selected.

I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise.  I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.

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

### Additional information

I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
- red for "required rust version", yellow for "latest"
- yellow for "required rust version", nothing for "latest"

There is also more discussion on how to format "latest" at #13908.
antoniospg pushed a commit to antoniospg/cargo that referenced this issue Sep 8, 2024
…cted

In discussin this in rust-lang#13873, it highlighted that we need to make sure we
tell people when we get in this state.

I decided to keep "latest" and "required rust version" messages mutually
exclusive to avoid too much noise.  I gave "required rust version"
higher precedence as its the more critical to operation and, if you are
using an MSRV-incompatible package, it likely is "latest" already.

I was tempted to change colors to make "required rust version" stand out
from "latest" but was unsure what direction to go, so I held off.
Options included
- red for "required rust version", yellow for "latest"
- yellow for "required rust version", nothing for "latest"

There is also more discussion on how to format "latest" at rust-lang#13908.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns.
Projects
None yet
Development

No branches or pull requests

8 participants