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

Set a minimum rust-version #5518

Closed
wants to merge 1 commit into from

Conversation

vertesians
Copy link
Contributor

Objective

Trying to compile Bevy on Rust 1.61 or below causes compile errors because deriving 'Default' on enums is experimental, which is confusing to users who don't realize that they need to update their toolchain to fix it.

Solution

Set rust-version = 1.62 so that Cargo gives a clearer error.

@alice-i-cecile alice-i-cecile added A-Meta About the project itself X-Controversial There is active debate or serious implications around merging this PR C-Usability A simple quality-of-life change that makes Bevy easier to use labels Aug 1, 2022
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Aug 1, 2022

This is controversial because, as discussed in #288, we follow a "latest stable Rust" policy and this would need to be constantly manually updated. I've made a prominent note in the readme in #4274, but perhaps we could do better.

@vertesians
Copy link
Contributor Author

Understandable, but there's a reasonable amount of users who get into Rust and Bevy at the same time, so I think it makes sense to make it clearer for them. (That said, I only noticed a handful of users on Discord get confused by the error, so maybe it's not that common.)

... also, I completely missed that message in the README, whoops. 😅 I haven't had reason to look at it in a while.

@alice-i-cecile
Copy link
Member

Understandable, but there's a reasonable amount of users who get into Rust and Bevy at the same time

I agree, but a) they typically just download the latest Rust and b) they won't know to look for an MSRV :)

@mockersf
Copy link
Member

mockersf commented Aug 1, 2022

b) they won't know to look for an MSRV :)

Cargo refuses to run if the local Rust version is not recent enough, with a more explicit message than the usual "hey you're using nightly features on stable"

Maybe we could have the field... but I would like to avoid PR just updating that field because another PR that introduced a dependency on a newer version of Rust forgot to update it. That would mean adding a job to CI actually checking that version

@mockersf
Copy link
Member

mockersf commented Aug 1, 2022

Error message like in #5520 is so confusing for someone not used to them

@tim-blackbird
Copy link
Contributor

Is there really a need to worry about this on main?

I would expect this to affect people using bevy from crates.io the most and I don't see a downside to updating the rust-version to the latest stable before each Bevy release.
Maybe as a part of the automatic PR that bumps the version of all bevy crates.

It would be nice to add this to the 0.8.1 milestone.

@mockersf
Copy link
Member

mockersf commented Aug 1, 2022

If we add the field in the Cargo.toml, its value must be true. Updating it only on release means it would sometimes be false on main, which is not good.

@nicopap
Copy link
Contributor

nicopap commented Aug 5, 2022

It's still possible to remove the line after release during development and adding it back with a new rust version just before releasing a new version.

@alice-i-cecile
Copy link
Member

Closing in favor of #6852, which automates this further.

bors bot pushed a commit that referenced this pull request Jan 9, 2023
# Objective

- Fixes #6777, fixes #2998, replaces #5518
- Help avoid confusing error message when using an older version of Rust

## Solution

- Add the `rust-version` field to `Cargo.toml`
- Add a CI job checking the MSRV
- Add the job to bors
james7132 pushed a commit to james7132/bevy that referenced this pull request Jan 21, 2023
# Objective

- Fixes bevyengine#6777, fixes bevyengine#2998, replaces bevyengine#5518
- Help avoid confusing error message when using an older version of Rust

## Solution

- Add the `rust-version` field to `Cargo.toml`
- Add a CI job checking the MSRV
- Add the job to bors
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
# Objective

- Fixes bevyengine#6777, fixes bevyengine#2998, replaces bevyengine#5518
- Help avoid confusing error message when using an older version of Rust

## Solution

- Add the `rust-version` field to `Cargo.toml`
- Add a CI job checking the MSRV
- Add the job to bors
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Fixes bevyengine#6777, fixes bevyengine#2998, replaces bevyengine#5518
- Help avoid confusing error message when using an older version of Rust

## Solution

- Add the `rust-version` field to `Cargo.toml`
- Add a CI job checking the MSRV
- Add the job to bors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Meta About the project itself C-Usability A simple quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants