Skip to content

Use the rust-version field in Cargo.toml to set minimum Rust version#506

Closed
bradjc wants to merge 1 commit intomasterfrom
msrv-v2
Closed

Use the rust-version field in Cargo.toml to set minimum Rust version#506
bradjc wants to merge 1 commit intomasterfrom
msrv-v2

Conversation

@bradjc
Copy link
Copy Markdown
Contributor

@bradjc bradjc commented Aug 22, 2023

This encodes the minimum version so that the tooling will verify the user has a new enough Rust.

Set based on the needs from #498.

Fixes #394. Supersedes #499.

Comment thread Cargo.toml Outdated
@lschuermann
Copy link
Copy Markdown
Member

How does this relate to rust-toolchain? Would we eventually want to switch away from this entirely in favor of a MSRV?

@lschuermann lschuermann dismissed their stale review August 22, 2023 20:17

Changes addressed

@lschuermann
Copy link
Copy Markdown
Member

Seems like this needs #481.

@bradjc
Copy link
Copy Markdown
Contributor Author

bradjc commented Aug 22, 2023

Ok yeah this applies on top of https://rust-lang.github.io/rustup/overrides.html

This is confusing, because either rust-toolchain meets this requirement or nothing will build. But, since we also use stable for tests, this is useful for making sure the tests compile with a new enough rust.

@bradjc
Copy link
Copy Markdown
Contributor Author

bradjc commented Aug 22, 2023

Why don't we use stable by default and nightly only for miri?

@jrvanwhy
Copy link
Copy Markdown
Collaborator

Why don't we use stable by default and nightly only for miri?

I first introduced the check (#315) when only a subset of libtock-rs' crates built on stable, then later expanded the set (#390) to all crates. At the time, I had a few ideas for things other than Miri that required nightly, but I haven't had the time to implement them so at the moment we only need nightly for Miri.

I think it's fine to flip that around, and use the MSRV by default (e.g. via rust-toolchain and invoke our nightly version only for specific commands (at the moment, the Miri invocations).

Comment thread Cargo.toml
version = "0.1.0"
# Minimum required Rust compiler version for libtock-rs.
# Aug 2023: Set to support `Result::is_ok_and()`.
rust-version = "1.70"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be set workspace-wide and inherited.

... which requires a newer nightly toolchain, because the current toolchain is too old to support workspace inheritance.

Copy link
Copy Markdown
Member

@lschuermann lschuermann Aug 22, 2023

Choose a reason for hiding this comment

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

Seems like someone should really be looking into updating the nightly toolchain, eh? 😆
I can attempt that tonight, if adding sptr as an external dependency is uncontroversial enough. All of the other proposals in #481 (especially mine!) seem ... less than suboptimal.

Comment thread Cargo.toml
version = "0.1.0"
# Minimum required Rust compiler version for libtock-rs.
# Aug 2023: Set to support `Result::is_ok_and()`.
rust-version = "1.70"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We need to test with this MSRV in Makefile rather than using +stable, otherwise this will bitrot.

@bradjc
Copy link
Copy Markdown
Contributor Author

bradjc commented Aug 24, 2023

Done in #513

@bradjc bradjc closed this Aug 24, 2023
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.

"make test" fails

3 participants