-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Add rust-toolchain file #22040
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
base: main
Are you sure you want to change the base?
Add rust-toolchain file #22040
Conversation
|
You added a new example but didn't add metadata for it. Please update the root Cargo.toml file. |
|
could you set up dependabot to update it? https://github.blog/changelog/2025-08-19-dependabot-now-supports-rust-toolchain-updates/ |
5b57d59 to
2076ebe
Compare
|
I added a daily dependabot job for the rust-toolchain. Not sure if that's the best schedule. I wouldn't be surprised if a human will always end up doing it first since a rust release is hard to miss for anyone that tracks them |
| @@ -0,0 +1,2 @@ | |||
| [toolchain] | |||
| channel = "1.91.1" | |||
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.
I see the value in being exact, but before adding semi-redundant "things", can you verify that the minimum Rust version in Cargo.toml does not meet your needs?
[package]
rust-version = "1.88.0"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.
Ah, I didn't realize we even had a rust version specified.
Are you saying I should set the toolchain value to match that 1.88 or are you saying that the toolchain file is redundant?
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.
I'm saying it is partially redundant. It is a piece of metadata that we already bump periodically, that is an attestation that Bevy requires features from that version.
The big tradeoff is that we aren't actually running our CI for that version (we use latest stable).
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.
The question is ... is that enough of a solve for your usecase, given that the proposal adds more complexity to our process.
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.
I admit that being aware of the rust version does change things a little bit, but in my case the reason I want it is for the ability to git bisect and having cargo automatically detect the toolchain it should be using makes it simpler. With that said, yeah, I guess the rust-version field solves the "guessing the compatible rust version" part. Either it didn't exist until recently or that field is essentially ignored by cargo though, because me and many people I've talked to on discord have had issues with invalid rust versions when running git bisect.
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.
The big tradeoff is that we aren't actually running our CI for that version (we use latest stable).
We don't run all the CI on it, but Bevy is built in CI with the rust version from rust-version in this job: https://github.com/bevyengine/bevy/blob/main/.github/workflows/ci.yml#L456
it didn't exist until recently
It was (only) added 3 years ago
I think the issue is maybe with lints (builtin or clippy), with every rust version having new lints that Bevy fails. Or it did happen a few times that new versions of Rust don't compile Bevy, this is fixed usually well enough in advance by the Rust team with PR to Bevy, but if you go back in history with a modern version of rust it will fail to compile.
Objective
Solution
Testing