nix icon indicating copy to clipboard operation
nix copied to clipboard

Raise the MSRV to 1.51.0

Open asomers opened this issue 3 years ago • 9 comments
trafficstars

Nix's code hasn't changed. However, Serde accidentally raised its MSRV to 1.51.0 in a patch release, due to a Cargo bug. They don't plan to change it back. Nix does not depend on Serde, but it's used by cargo-hack, which we build as part of our CI process. So we need to either raise our MSRV, or else install a separate toolchain during CI just to build cargo-hack.

https://github.com/serde-rs/serde/issues/2255

asomers avatar Aug 11 '22 23:08 asomers

@asomers should we create a release with the current MSRV first?

rtzoeller avatar Aug 11 '22 23:08 rtzoeller

That would probably be good to do. It's about time anyway. We'll have to disable cargo-hack just to get through CI. Are there any other PRs that ought to be included?

asomers avatar Aug 11 '22 23:08 asomers

Are there any other PRs that ought to be included?

I would have liked to get #1744 in, but there's an open question regarding send.

I've been a bit short on bandwidth recently, but that should change soon. Maybe we can do a subsequent 0.26.0 release relatively soon after 0.25.0, with the new MSRV.

I don't see any PRs which I would hold the release for, given that we'll miss cargo hack in validating them.

rtzoeller avatar Aug 11 '22 23:08 rtzoeller

Perhaps #1785?

rtzoeller avatar Aug 11 '22 23:08 rtzoeller

Ok. Also, how would you feel about raising MSRV to 1.54.0? The CI failures are due to a bunch of overly aggressive Clippy lints that got fixed or downgraded by 1.54.0.

asomers avatar Aug 11 '22 23:08 asomers

Perhaps #1785?

Yeah. And a couple of other cleanups. I'll create a milestone.

asomers avatar Aug 11 '22 23:08 asomers

Ok. Also, how would you feel about raising MSRV to 1.54.0? The CI failures are due to a bunch of overly aggressive Clippy lints that got fixed or downgraded by 1.54.0.

I'd be fine with that, but would push for 1.56.0.

I was hoping https://github.com/rust-lang/libs-team/issues/72 would be resolved before we needed to bump our MSRV, so we could adopt it as our policy (barring a strong reason not to). https://github.com/rust-lang/libs-team/issues/72#issuecomment-1190171399 has some rough approximations on version coverage - given those numbers I would push for 1.56.0 over 1.54.0, as the coverage is nearly identical (and that was ~3 weeks ago).

Is there a reason to target 1.54.0 over 1.56.0?

rtzoeller avatar Aug 11 '22 23:08 rtzoeller

Ok. Also, how would you feel about raising MSRV to 1.54.0? The CI failures are due to a bunch of overly aggressive Clippy lints that got fixed or downgraded by 1.54.0.

I'd be fine with that, but would push for 1.56.0.

I was hoping rust-lang/libs-team#72 would be resolved before we needed to bump our MSRV, so we could adopt it as our policy (barring a strong reason not to). rust-lang/libs-team#72 (comment) has some rough approximations on version coverage - given those numbers I would push for 1.56.0 over 1.54.0, as the coverage is nearly identical (and that was ~3 weeks ago).

Is there a reason to target 1.54.0 over 1.56.0?

Oh boy, I'm eagerly watching that libc issue too. It's blocking me from fixing a kevent bug (https://github.com/rust-lang/libc/pull/2813 , and that will have a Nix component too). But I have no strong feelings about 1.54.0 vs 1.56.0. I do however have strong feelings about 1.63.0. That would allow us to implement #1750 . I'm not yet sure how much work that issue will require, however. So maybe we shouldn't jump all the way to 1.63.0 just yet. Plus, it's only hours old.

asomers avatar Aug 12 '22 00:08 asomers

But I have no strong feelings about 1.54.0 vs 1.56.0. I do however have strong feelings about 1.63.0. That would allow us to implement #1750 . I'm not yet sure how much work that issue will require, however. So maybe we shouldn't jump all the way to 1.63.0 just yet. Plus, it's only hours old.

I pulled a newer version of the same data as Alex's comment which I linked previously; here's the results for the last 30 days:

1.47.0: 97.2%
1.48.0: 96.9%
1.49.0: 96.6%
1.50.0: 96.5%
1.51.0: 96.5%
1.52.0: 96.4%
1.52.1: 96.2%
1.53.0: 96.0%
1.54.0: 95.9%
1.55.0: 95.9%
1.56.0: 95.9%
1.56.1: 95.8%
1.57.0: 95.5%
1.58.0: 95.4%
1.58.1: 95.1%
1.59.0: 94.4%
1.60.0: 93.8%
1.61.0: 92.5%
1.62.0: 87.8%
1.62.1: 52.4%
1.63.0: 0.01%

This is coming from pip installs, so the near-zero 1.63.0 value makes sense - it'd be people who have upgraded Rust and subsequently installed a Python package with a recent version of pip (I also pulled this data a few hours ago).

If trends hold, we're probably 2-3 months away from a MSRV of 1.63.0 having ~90% adoption (per this metric). Requiring a newer pip does mean we're biased towards newer versions of Rust. Perhaps we should look at the MSRVs of recently updated crates which depend on nix?

rtzoeller avatar Aug 12 '22 04:08 rtzoeller

Nix does not depend on Serde, but it's used by cargo-hack,

I would suggest using +stable for cargo-hack installation or installing cargo-hack from GitHub release (like https://github.com/crossbeam-rs/crossbeam/pull/761 or https://github.com/tokio-rs/bytes/pull/507) (cargo-hack is compatible with cargo 1.26 and later, regardless of the version used to compile it.)

taiki-e avatar Aug 12 '22 19:08 taiki-e

Nix does not depend on Serde, but it's used by cargo-hack,

I would suggest using +stable for cargo-hack installation or installing cargo-hack from GitHub release (like crossbeam-rs/crossbeam#761 or tokio-rs/bytes#507) (cargo-hack is compatible with cargo 1.26 and later, regardless of the version used to compile it.)

I thought about using +stable to build cargo-hack. But that would require installing two toolchains instead of one, slowing down CI. Installing the binary release isn't an option because they only publish releases for a few platforms.

asomers avatar Aug 12 '22 20:08 asomers

I thought about using +stable to build cargo-hack. But that would require installing two toolchains instead of one, slowing down CI.

Ah, sorry, I missed that Cirrus CI does not install rust by default.

Installing the binary release isn't an option because they only publish releases for a few platforms.

Hmm. Which platform prebuilt binaries are missing? I can probably publish prebuilt binaries for that platform if it is supported by cross.

taiki-e avatar Aug 12 '22 20:08 taiki-e

I thought about using +stable to build cargo-hack. But that would require installing two toolchains instead of one, slowing down CI.

Ah, sorry, I missed that Cirrus CI does not install rust by default.

Installing the binary release isn't an option because they only publish releases for a few platforms.

Hmm. Which platform prebuilt binaries are missing? I can probably publish prebuilt binaries for that platform if it is supported by cross.

We run CI on x86_64-unknown-freebsd, x86_64-apple-darwin, x86_64-unknown-linux-gnu, and aarch64-unknown-linux-gnu. The other targets we test by cross compiling.

asomers avatar Aug 12 '22 20:08 asomers

LGTM, but don't forget to update the PR description. It's currently out of date.

Are you sure? Press refresh.

bors=rtzoeller

asomers avatar Aug 14 '22 21:08 asomers

Uhh... bors?

bors r+

rtzoeller avatar Aug 16 '22 00:08 rtzoeller