Consider supporting MSRV in rustup

[anp]

See comment below.

Original post:

Currently I commit rust-toolchain files for most of my projects so that I and contributors have less manual work to do (check README, run commands, etc). It works well in combination with the RLS too.

That said, the same way that exact semver bounds in Cargo.toml create many duplicated deps in builds, my observation of having had this habit for a while is that I end up with many more rust toolchains installed than I really need to work on all of my projects. Many of the older toolchains are pinned at a specific stable version -- not because I didn't want to support newer stable releases, but because I wanted to make sure that contributors are building with at least a particular minimum version.

My instinct here is to propose that adding a + to the end of a toolchain specifier will allow rustup to satisfy the toolchain requirement with the provided version or any newer version from the same release channel (nightly-foo-+ will only give newer nightlies, 1.28.0+ will only give newer stables).

I haven't discussed this with anyone before, so I'm very open to the idea that this proposal misses important needs and would love to hear feedback on the idea. A quick search of the issue tracker didn't turn up any previous discussions, please forgive me if I'm retreading old ground!

[rofrol]

This ^^^

[kinnison]

Hi @anp and @rofrol

I'm sorry nobody got back to you before now. Is this something you still would like?

if not, please could you close the issue. If it's still potentially useful then I'd like to understand your use cases so I can judge the best approach.

Thanks,

Daniel.

[anp]

Will rustup respect the minimum Rust version RFC that was recently approved? If so, I think that would address my main use case.

[kinnison]

I had not seen that RFC, no right now "no" but it looks very interesting. If you're sure that would make the most sense, then I think I'll retitle this issue "Consider supporting MSRV in rustup" so we can work out what, if anything, rustup can usefully do to make this possible.

[illicitonion]

Now that Cargo supports rust-version as an optional Cargo.toml field (since 1.56.0), and is starting to seriously consider factoring MSRV into resolution, it'd be really handy to start better supporting MSRV in rustup.

The problem I'd like to solve is: It's hard to ensure you're actually testing MSRV on CI. Currently, the best way seems to be to have a CI pipeline which hard-codes what the MSRV is, and either manually verify, or write custom automation, to verify that the CI pipeline's rust version is in sync with the crate's package.rust-version attribute. The fact that these can go out of sync is an annoying footgun.

For me, the goal here would be that CI can run cargo +msrv test and rustup will find the MSRV from the Cargo.toml file and use that as if it'd been specified on the command line.

So in this example:

[package]
rust-version = "1.54.0"
# ...

running cargo +msrv test would be the equivalent of running cargo +1.54.0 test.

A complication here is that workspaces may have different MSRV versions, and some crates in the workspace may not set an MSRV; if we take the serde repo as an example; as of commit d6de911855d1cc0ad13f87503a79d40dc4490442 it is a workspace with the following Cargo.toml files:

• Cargo.toml - just a virtual workspace file, doesn't have a package and accordingly no rust-version attribute
• serde/Cargo.toml - rust-version 1.19
• serde_derive/Cargo.toml - rust-version 1.56
• serde_derive_internals/Cargo.toml - rust-version 1.56
• serde_test/Cargo.toml - rust-version 1.19
• test_suite/Cargo.toml - no rust-version
• test_suite/no_std/Cargo.toml - no rust-version

Probably the correct thing to do here is for rustup to look in the current directory for a Cargo.toml file, and use its rust-version if one's set (and error if you try to use +msrv without having one set), and for Cargo.toml to support setting a rust-version for a workspace using one of a few possible mechanisms:

1. Delegating to a specific sub-crate - Cargo could allow you to say "serde is the main crate, inherit its rust-version"
2. Selecting the minimum or maximum rust-version specified in a crate in the workspace
3. Just explicitly specifying the MSRV if rustup happens to be running in that directory.

cc @epage who seems to be thinking about this at the moment.

[kinnison]

I am concerned that this mixes the concerns of Rustup and Cargo. To properly honour this, Rustup would need to understand how to parse the full workspace and to understand that field as Cargo would - this seems like it'd result in Rustup needing to use the cargo crate to achieve that; which is a huge huge new dependency for rustup.

I'm not saying this isn't possible, but I am wondering how sensible it would be to do it this way.

I'd also like to get @rbtcollins and @ehuss involved in this discussion as both are more immediately on the coal-face as it were.

[epage]

To expand on @kinnison's comment

• If rustup directly parses Cargo.toml (directly, which is complicated, or by depending on cargo), that means any workspace parsing changes need to be reflected in rustup and people need to upgrade rustup to parse the latest version of their manifests
  • While its not common, think of cargo adding workspace inheritance as an example of this
  • Another angle to this is cargo always produces warnings when there are fields that aren't understood.
• rustup could call into cargo metadata which is usually what we recommend but
  • This requires a version of cargo to be installed in the first place which gets into a chicken-and-egg problem
  • rustup has to decide which version of cargo to use if there are multiple

For me, the goal here would be that CI can run cargo +msrv test and rustup will find the MSRV from the Cargo.toml file and use that as if it'd been specified on the command line.

Keep in mind the solution would also need to include installing the MSRV toolchain before having the proxies reference it.

[rbtcollins]

I think this should be a cargo feature.

Resolve the toolchain needed for msrv testing then invoke cargo +version .... Itself.

[epage]

Doing this in cargo has its own challenges

• First, we'd need to decide if we'd want a cargo feature that only works if rustup is installed
• This would need to be exposed in a way that doesn't conflict with the rustup proxy
• Cargo would need to use the current version of cargo (and hope that works) to get far enough into processing to have found the right Cargo.tomls and parsed them to then stop re-launch with the new cargo with all of the original command-line flags (which, depending on where this is happening, might be its own challenge).
• This would need to be directly integrated into each subcommand (since it needs --manifest-path, --package, etc). This also means that third-party subcommands would not get it for free.