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: Bump msrv to 1.70 #975

Closed
wants to merge 1 commit into from
Closed

Conversation

tottoto
Copy link
Contributor

@tottoto tottoto commented Jan 30, 2024

home 0.5.9 requires Rust 1.70.

@caspermeijn
Copy link
Collaborator

Can you give more context why you think this is needed?

For me, cargo msrv verify reports the MSRV is correct.

$ cargo msrv verify
Fetching index
Verifying the Minimum Supported Rust Version (MSRV) for toolchain x86_64-unknown-linux-gnu
Using check command cargo check
   Finished Satisfied MSRV check: 1.60.0

Copy link
Collaborator

@caspermeijn caspermeijn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking further into it, my previous command didn't account for the other projects in the workspace. The correct command to reproduce this problem is:

$ cargo +1.60 check --workspace --all-features
error: package `indexmap v2.2.2` cannot be built because it requires rustc 1.63 or newer, while the currently active rustc version is 1.60.0

@@ -12,7 +12,7 @@ documentation = "https://docs.rs/prost-build"
readme = "README.md"
description = "A Protocol Buffers implementation for the Rust Language."
edition = "2021"
rust-version = "1.60"
rust-version = "1.70"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are actually more than one rust-version definitions in the code base. Can you update the others as well?

@caspermeijn
Copy link
Collaborator

This PR can be closed as the issue is fixed in #982

@tottoto
Copy link
Contributor Author

tottoto commented Feb 16, 2024

Are there any reason to bump the msrv of the crates which do not depend on home?

@caspermeijn
Copy link
Collaborator

It feels like there is too little maintainer capacity, and I think a single MSRV for the whole repo is easier to maintain. This PR doesn't actually verify the MSRV of main prost crate in CI.

@tottoto
Copy link
Contributor Author

tottoto commented Feb 17, 2024

This PR doesn't actually verify the MSRV of main prost crate in CI.

Yes, and it has not been performed in this project as well so far.

Bumping the rust-version field make build fail the build lower than its version (without --ignore-rust-version option), and as prost is a widely used crate, it effect is relatively large.

@caspermeijn
Copy link
Collaborator

Let's continue this discussion in #983

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