[Ignore this] Implement min-rust-version support (in cargo build, resolver support).

[moxian]

This implements RFC 2495 (rust-lang/rfcs#2495)
cc tracking issue rust-lang/rust#65262

Manifest and crate index get min-rust-version key,
Resolver ignores the packages (summaries) with min-rust-version below the current one.
Error messages tell what went wrong.

Several important details:

On naming

I strongly believe that calling the manifest field rust is way too ambiguous, and calling it msrv is way too cryptic.
Hence, min-rust-version as the name. It's the same amount of characters as dev_dependencies. There's no need to try and shorten it.

On unmatched version behavior

I strongly believe that it's fundamentally wrong to implement RFC as written.
Specifically, RFC says

If you build a crate with a dependency which has MSRV higher than the current version of your toolchain, cargo will return a compilation error stating the dependency and its MSRV.
Implementing the feature that way would only introduce a time-bomb, and each MSRV bump would mean a breaking semver change.

Consider the following situation: I have a package foo that depends on package bar = "^1.0". bar version 1.0.0 builds on rustc 1.45.0. Now bar owners decide to release new semver-compatible bar version 1.1.0, utilizing features from newly released rustc-1.72.0, and properly tag it with min-rust-version = 1.72. Now, when attempting to build my foo crate, cargo hard-fails, since it tries to select latest available bar version, finds 1.1.0 and fails min-rust-version check.
I'm assuming this suggestion was provide in the RFC only as way to minimize the amount of work for the original implementation, and make it more attractive to implementors.
So, I'm choosing to ignore this particular part of RFC, and replace it its "Future work" section of "Influencing version resolution".
With this PR, cargo selects the highest dependency version of those that pass min-rust-version check.
This is the reason why the MSRV requirements need to be stored in the crate index.

On nightly

Both the RFC and the tracking issue are thinking of ways to specify required nightly release into the mix.
I believe that this is a wrong approach, for several reasons:

• "this requires a specific release channel" is orthogonal to "this requires a release made after specific date". I'd be happy to see requires-nightly = true field in the manifest, but this should not be bundled with min-rust-version

and the following is several different ways to say "just update your nightly, duh":

• nightly being nightly gives no promises whatsoever that crate building on nightly today would continue to do so tomorrow
• I doubt nightly crate owners would bother updating the nightly version manually each time. It's often easier to just say "requires latest nightly"
• people using nightly should have no trouble updating to latest nightly. Linux distros a-la debian do not provide nightly via their package manager (due to the glacier pace of their relase), so the "bumping MSRV might be a breaking change" should not even be a concern here.
• By the time a crate requiring nightly of 2020-01-01 is published, and users discover it, users are very likely to have already updated nightly anyway, so the comparison is no-op.
• Comparing user's nightly against a crate targeting non-nightly rust version (say 1.40.0) is extremely likely to succeed. Having 6-months old nightly toolchains installed is weird.

So I did not implement any of this, and instead added a warning to users trying to write min-rust-version = "1.41.0-nightly"

On version comparison

Tracking issue has discussion on whether the min-rust-version specification should allow semver operators (e.g. "^1.2.3" vs ">=1.2.0" vs...) alluding to the fact that it would probably easy to implement.
Well, it turned that plain >= comparison is much easier to implement (read: code is very slightly more performant, and/or very slightly more easy to follow) than the semver one, so that's what I implemented for now.
I agree that semver comparison ("^") would probably be better, and can be convinced to change the code to use that instead.
I don't agree that user should be able to specify custom version comparison (=)

On deployment

I couldn't think of a way to deploy this better than just commiting this without any stabilization shenanigans.
Introducing a --honor-min-rust-version flag is weird, since it would only be enable on nightlies, which are almost by definition newer than any package out there. I agree that escape-hatch flag of "please disable min-rust-version check" is needed, but see below.

For this feature to fully work, the publishing side, and crates-io index generation would need to be updated. I haven't done any work on that yet, but it should be relatively straightforward.

On backwards compatibility

Old cargo version would ignore min_rust_version field both in the index and in the manifest.
For simplicity's sake this means that the version of the package that introduces min-rust-version field should be treated as breaking change (provided package owners consider MSRV bump a breaking change), and all subsequent ones to be treated as normal.
(More specifically: Provided package owners treat MSRV bump as breaking change: Addition of min-rust-version by itself is not a breaking change, but the first actual MSRV bump after and including the min-rust-version-introducting package is still breaking. The subsequent ones aren't).
I believe this should be worded more nicely than I am capable of, and put into docs.. somewhere..

On crate index format

Cargo crate index serves a lot of packages, and it seems that we try to optimize it as much as we can. Right now this PR adds a new min_rust_version key to the JSON. I wonder if maybe I should shrink it here to msrv instead? I'm not sure it would net enough benefit to be worth it though.

Future work

• add checks/support to cargo publish
• add support (and checks?) to crates-io

I think that's all the controversial parts.

I would appreciate input on:

• name of the escape-hatch flag. It's currently --ignore-min-rust-version, but that feels too long for a flag (it would fine as a config option though)
  • would it maybe be better to have a --assume-rust-version 1.72.2 instead of plain skipping the check? But then how does one express "no, just skip the check completely" with that?
• error messages
• additional tests (do note, the "current rustc version" that the tests see is the version that the tests have been compiled with. I.e. a moving target)

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

Thanks for the PR!

It looks like there's a lot of changes here related to the resolver, but I think that the RFC was initially accepted with this explicitly as future work? Although have I perhaps missed discussion in the meantime?

[moxian]

Does "explicitly as future work" mean this needs to go through RFC first?

As mentioned in my original comment, I believe it would be actively harmful to have compilation fail on MSRV mismatch, while not having resolver fall back to some other package that passes said check.
However I don't have energy or human language skills to get this argument through RFC.

Would you suggest starting from the other end, and getting MSRV information through cargo publish into the crates index first, and only then make cargo build use that information (either as initially proposed in RFC ("just fail the build") or with this resolver cleverness ("find a version that fits the constraints, if the obvious choice doesn't work"))? And if so, would you suggest closing this PR in the meanwhile?

[alexcrichton]

This is a very thorny question to tackle and we spent quite some time debating it amongst the Cargo team, and our conclusion was that we wanted to go with something quite minimal for now which leaves us room to become more ambitious in the future. I'm personally under the impression that's the pretense in which the RFC was accepted, and I would personally have some more objections, for example, to integrate it more deeply into Cargo at this time without having some more dicussion.

[moxian]

This is fair. I guess this topic is a bit more controversial than I assumed it be.
I'll close this PR then, and try and craft something more minimal instead.
Thanks!