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

Release v1.0.106 bumped MSRV in a patch release #1144

Closed
tnull opened this issue Jul 8, 2024 · 19 comments · Fixed by #1158
Closed

Release v1.0.106 bumped MSRV in a patch release #1144

tnull opened this issue Jul 8, 2024 · 19 comments · Fixed by #1158

Comments

@tnull
Copy link

tnull commented Jul 8, 2024

It seems the just-released v1.0.106 bumped the MSRV from 1.63 to 1.67 in a patch release.

Unfortunately this breaks our downstream MSRV builds, which depend on rustc 1.63 (currently shipped with Debian Bookworm).

I want to ask you to consider reverting the MSRV bump as it introduces issues for users downstream that need to support 'older' Rust version, especially since raising the MSRV seems to have happened only for a mere convenience feature in #1137

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jul 8, 2024

Hello, sorry to hear that it breaks your build.

Is it possible for you to pin the cc version via Cargo.lock?

BTW cargo is working on a msrv-aware resolution feature to help this.

@TheBlueMatt
Copy link
Contributor

Is it possible for you to pin the cc version via Cargo.lock?

For a library to support an MSRV, sadly, that's not an option. We could pin cc explicitly in the Cargo.toml, but that really sucks as it means any new features needed for new platforms cant work either.

There's also the issue of -Zbuild-std, which doesn't support Cargo.lock and is needed when building targeting apple platforms from Linux (which is, in turn, needed for practical reproducible builds, or at least I've never gotten them to work otherwise).

@ChrisDenton
Copy link
Member

I don't think it's currently tenable for us to consistently maintain a 3+ year MSRV.

If using the Debian system rustc is absolutely necessary, note that they do also maintain a cc-rs package for this purpose.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jul 8, 2024

There's also the issue of -Zbuild-std, which doesn't support Cargo.lock

stdlib/rustc doesn't pin the version?

I was under the impression that stdlib/rustc pin to a specific cc version...

But if you are using -Zbuild-version, then it should be nightly compiler, unless you compiled the rustc with some special options to enable nightly options

@TheBlueMatt
Copy link
Contributor

I don't think it's currently tenable for us to consistently maintain a 3+ year MSRV.

I mean that may well be the case, but (a) we could at least only change the MSRV if there's a compelling feature that cannot (practically) be shipped without changing the MSRV, and/or (b) change the minor (or major) version when that point is reached. At this point it seems like cc did just fine with a very old MSRV for a long time and until now hadn't found a feature that was work increasing it for even with a very low bar for doing so. Bumping that bar a bit would be much appreciated.

stdlib/rustc doesn't pin the version?

Sadly -Zbuild-std doesn't (to my knowledge) currently pin crate versions and pulls them from crates.io. Its a known limitation, AFAIU.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jul 8, 2024

If it is for x-lto, is there another way?

I.e. compile in docker, where the rustc 1.67 and clang uses the same LLVM version?

@ChrisDenton
Copy link
Member

change the minor (or major) version when that point is reached

Others have tried this approach (treating it as a semver breaking change) but found it isn't really tenable. E.g. see the discussion on the API guidelines repo rust-lang/api-guidelines#231 (comment)

@TheBlueMatt
Copy link
Contributor

Others have tried this approach (treating it as a semver breaking change) but found it isn't really tenable.

Major version maybe not but many other projects have no problem changing the minor version for MSRV changes? eg tokio does it and it lets tokio support the MSRV-limited versions for some time with security and important updates without being forced into a low MSRV forever.

If it is for x-lto, is there another way? I.e. compile in docker, where the rustc 1.67 and clang uses the same LLVM version?

I mean in principle yes, but that takes building an application from source from "just clone this repo, install rust and build" to "first build rust, including fetching a full version of docker or whatever, also build llvm and clang from source, etc", which is totally untenable.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jul 8, 2024

I'm ok with a 1.1.0 release, that sounds very reasonable

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jul 8, 2024

I mean in principle yes, but that takes building an application from source from "just clone this repo, install rust and build" to "first build rust, including fetching a full version of docker or whatever, also build llvm and clang from source, etc", which is totally untenable.

Yeah that makes sense, for project that needs x-lto, I guess either you depend on debian stable, or maintain a Dockerfile officially with the same llvm version

@Noratrieb
Copy link
Member

How does updating it in a minor release vs a patch release help you? I don't see a significant difference there.

@TheBlueMatt
Copy link
Contributor

Its not a huge difference, no, either way we're stuck pinning cc in Cargo.toml explicitly which sucks, the only difference is assuming cc takes a stance like tokio where it can/does backport security and important bugfixes (or, for cc, possibly even new platform support where it doesn't require breaking MSRV for existing platforms) to the prior minor version. Of course my preferred outcome would be at least a policy of "dont bump MSRV unless you kinda have to", but I was just raising other options.

@NobodyXu
Copy link
Collaborator

A cyclical compilation error is found in #1146 , we would need to bump msrv to 1.70 to use std::sync::OnceLock to fix it.

@workingjubilee
Copy link
Member

workingjubilee commented Jul 12, 2024

Its not a huge difference, no, either way we're stuck pinning cc in Cargo.toml explicitly which sucks, the only difference is assuming cc takes a stance like tokio where it can/does backport security and important bugfixes (or, for cc, possibly even new platform support where it doesn't require breaking MSRV for existing platforms) to the prior minor version.

I don't think that's happening for cc-rs. aiui tokio has professional support explicitly paid to work on tokio and adjacent problems.

@TheBlueMatt
Copy link
Contributor

I don't think that's happening for cc-rs. aiui tokio has professional support explicitly paid to work on tokio and adjacent problems.

Of course I'm not purporting to mandate that some open source project takes on the same work as a funded one, but funding isn't the issue here - just go look at how often Tokio has to backport anything (and that's for a runtime network-facing dependency, not a compile-time one), it's very little of their total work volume, arguably almost nothing.

@NobodyXu
Copy link
Collaborator

@tnull @TheBlueMatt msrv is reverted to 1.63 in v1.1.3

@tnull
Copy link
Author

tnull commented Jul 15, 2024

@tnull @TheBlueMatt msrv is reverted to 1.63 in v1.1.3

Thank you for the swift reaction, much appreciated!

@TheBlueMatt
Copy link
Contributor

Thank you so much!

@NobodyXu
Copy link
Collaborator

BTW, it looks like we will have to refrain MSRV bump until next debian stable release, which is probably going to happen in 2025 Aug-Oct with rust 1.79

Finger crossed it will come soonner

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.

6 participants