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

chore: Bumping MSRV to 1.75 #233

Merged
merged 6 commits into from
Sep 19, 2024
Merged

Conversation

oestradiol
Copy link
Contributor

No description provided.

@oestradiol
Copy link
Contributor Author

Oh wow, that's interesting. It seems like changing the MSRV caused a small change on the formatting of a specific line?

@sugyan
Copy link
Owner

sugyan commented Sep 19, 2024

You are changing the toolchain version setting of the project, not the MSRV.
This has bumped rustfmt from 1.5.2 to rustfmt 1.7.0, and there seems to be a subtle behavioral change.

If you want to bump the MSRV, I think you need to change rust-version.

rust-version = "1.70"

ref: https://doc.rust-lang.org/cargo/reference/manifest.html#the-rust-version-field

@oestradiol
Copy link
Contributor Author

Ohh, mhm, I see. I thought by bumping the entire toolchain, it would also change the version of Rust.

I'm a little bit confused. Did I misunderstand what's written in the docs? Here are the exact parts that are confusing me:

The Channels chapter:

  • "rustup can also install specific versions of Rust, such as 1.45.2 or nightly-2020-07-27. See the Toolchains chapter for more information on installing different channels and releases."

The Toolchains chapter:

  • "Many rustup commands deal with toolchains, a single installation of the Rust compiler."
  • "(...) ‘channel’ is a named release channel, a major and minor version number such as 1.42, or a fully specified version number, such as 1.42.0 (...)"

Do you understand what's the difference between Cargo.toml's rust-version and rust-toolchain.toml's channel? If you do, I'd appreciate some clarification...

Either way, I think we need to bump the toolchain too, right? Since we will need Clippy to be up to date with our MSRV so that all the lints work properly? Please correct me if I'm wrong.

@oestradiol
Copy link
Contributor Author

oestradiol commented Sep 19, 2024

Ok, @sugyan! I've researched a bit more and I think I understand it now. Is this correct?

  1. rust-version in Cargo.toml specifies the minimum Rust version required to compile the project. It ensures compatibility by signaling that the codebase relies on features available from that version onward. This field is useful to prevent users from trying to compile with an unsupported Rust version;
  2. rust-toolchain.toml defines a fixed Rust toolchain version used in the development environment. This affects the versions of tools like clippy and rustfmt, and ensures consistent behavior across different systems by enforcing the exact compiler version used. This also automatically switches the Rust version in rustup when entering the project directory.

To achieve the goal of bumping the MSRV, I think both should be updated, which will ensure that clippy and other tools are using the correct version, and also anyone who tries to work on this codebase.

@sugyan
Copy link
Owner

sugyan commented Sep 19, 2024

Yes, I generally understand the same thing.
The rust-version represents MSRV, which is also shown in crates.io as Metadata as package information.
The rust-toolchain is to prevent developers from unintentionally breaking MSRV. For example, a project with MSRV 1.75 should be fixed at 1.75 to avoid writing code that uses 1.76 or later features.

As for verifying that it works with MSRV, maybe we should set up a matrix workflow in Actions in rust.yml to confirm that it can be built with 1.75 as well as stable.

.github/workflows/rust.yml Outdated Show resolved Hide resolved
@oestradiol
Copy link
Contributor Author

Also, I'll remove "Install Rust Problem Matcher". actions-rust-lang/setup-rust-toolchain@v1 does it by default if CARGO_INCREMENTAL == 0.

@sugyan sugyan merged commit 4b75e60 into sugyan:main Sep 19, 2024
10 checks passed
@github-actions github-actions bot mentioned this pull request Sep 19, 2024
@oestradiol oestradiol deleted the chore/msrv-1.75-bump branch September 19, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants