-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.toml: Add rust-version field #10732
Conversation
Argument-capturing format strings are used in some places, which definitely places the minimum at 1.58. [0] - https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1580-2022-01-13
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon. Please see the contribution instructions for more information. |
@@ -7,6 +7,7 @@ homepage = "https://crates.io" | |||
repository = "https://github.com/rust-lang/cargo" | |||
documentation = "https://docs.rs/cargo" | |||
readme = "README.md" | |||
rust-version = "1.58" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to dependencies, our MSRV is 1.59
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, so we can wait until it’s 1.60 and then people will have to recurse yet again through GitHub issues to find their specific error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, cargo is developing with stable channel. And it is released under the same release model of Rust compiler. That is, for cargo lib, generally the MSRV lags behind stable by two minor versions.
readme = "README.md" | ||
rust-version = "1.58" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless we automate the verification of our rust-version
, I'm not a fan of setting it as its likely to become inaccurate, like it is in this PR at the time of my writing.
iirc it will automatically be set in the published crate to the version used when publishing (which will be valid if the user doesn't pass --no-verify
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it’s supposed to be set at the moment, it isn’t, as I stepped on a giant rake just today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And that’s the thing, I created this PR to get feedback, because I can’t possibly go through everything and pinpoint the exact version. It failed with 1.57.0 because of a specific change, so that’s what I put.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Automating should be trivial, get the version, use rustup to install the appropriate toolchain and build. But, again, you’re the reviewer and you should know if that’s feasible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc it will automatically be set in the published crate to the version used when publishing (which will be valid if the user doesn't pass --no-verify).
Huh, I don't know where I had gotten that impression from. I'm not seeing it set in the .crate
and it isn't listed in the RFC
Argument-capturing format strings are used in some places, which
definitely places the minimum at 1.58.
[0] - https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1580-2022-01-13