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

specify a Minimum Supported Rust Version (MSRV) for #198

Closed
wucke13 opened this issue Aug 14, 2024 · 6 comments · Fixed by #204
Closed

specify a Minimum Supported Rust Version (MSRV) for #198

wucke13 opened this issue Aug 14, 2024 · 6 comments · Fixed by #204

Comments

@wucke13
Copy link
Contributor

wucke13 commented Aug 14, 2024

I can't build 1.4.0 with rustc 1.77.2 due to a lifetime issue. With a newer rustc version it works. I think it would be best to just clearly document this in the Cargo.toml and to use a tool like cargo-hack or cargo-msrv to check that the claim from the Cargo.toml holds true.

@Ivan-Velickovic
Copy link
Collaborator

Would specifying a rust-toolchain.toml be enough for your use case? (as well as documenting the minimum version)

There is a PR #159 to add that.

Also, this PR #199 makes that lifetime compile error go away on less recent versions of Rust if that's helpful.

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 14, 2024

In principal, the rust-toolchain.toml would not help with my use-case. I'd much prefer if a not-so-specific tool (such as the microkit binary) does not specify its own custom toolchain file. I see the use-case for these more for super niche embedded targets, lets say someone would have hacked together seL4 kernel support for Rust but requires a couple of nightly features and a custom target. Then I think having rust-toolchain.toml is justifiable.

But not for some small-ish CLI tool.

The second PR helps most definitely, but still I think it's good to at least know the MSRV, and to specify it in the Cargo.toml.

@Ivan-Velickovic
Copy link
Collaborator

Makes sense.

The main benefit that I see of the toolchain config file is that it will automatically grab the toolchain that is known to work which means that everyone ends up building the tool with the same Rust version. We have people who aren't familiar with Rust building the SDK from source so it would help them as well if their Rust version installed doesn't match what the tool needs.

Anyways, that's a separate issue. I've made #200 specifying 1.79.0 as the minimum version since that's what I've used to compile and test the SDK.

Let me know if that works for you and I can close the issue.

@Ivan-Velickovic
Copy link
Collaborator

If you can't upgrade to 1.79.0 though I can try 1.77 and drop the minimum version to that if everything works.

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 14, 2024

I highly recommend to also verify the MSRV in the CI, see https://doc.rust-lang.org/cargo/guide/continuous-integration.html#verifying-rust-version for an example on how to do that.

If you can't upgrade to 1.79.0 though I can try 1.77 and drop the minimum version to that if everything works.

That would be very very kind of you, I don't want to ask for this but it would help me if you'd did that.

The main benefit that I see of the toolchain config file is that it will automatically grab the toolchain that is known to work which means that everyone ends up building the tool with the same Rust version

Only for those using rustup though, but there are alternative ways of distributing Rust (e.g. nixpkgs) which may or may not be compatible with a toolchain file. It certainly is a good way of providing a very specific toolchain requirement, but its not a catch-all solution, and one should carefully weigh whether to be that specific on the toolchain. A use-case that I can foresee would be a company requiring the microkit tool to be compiled with a safety certified rust compiler such as from the Ferrocene project, where not all minor release of the Rust compiler are available. TL;DR, a toolchain file implies a tight coupling, tighter than necessary most likely, and tight coupling has sometimes painful implications on integration aspects.

We have people who aren't familiar with Rust building the SDK from source so it would help them as well if their Rust version installed doesn't match what the tool needs.

At least the promise from the Rust advertisement is: no existing code will break, unless one upgrades the edition of the crate. There are some very few counterexamples, but in general you should not have to worry about your code breaking with a newer toolchain. A lot of effort on the Rust project is spent on assuring this promise. W/r/t to running a toolchain that is is too old, well there the MSRV as specified in the Cargo.toml quite clearly indicates to the user why things aren't working.

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 15, 2024

Two notes:

  • with the changes from tool: minor internal cleanup #199 , compilation of the microkit 1.4.0 release on rustc 1.73.0 works fine.
  • I also ran cargo-msrv on current main (d1ea5bc), and it spit out 1.67.1 as the MSRV.

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 a pull request may close this issue.

2 participants