search

proptest-rs / proptest

Hypothesis-like property testing for Rust
Apache License 2.0
1.69k stars 158 forks source link

New release (v1.1.0) changes MSRV to 1.60 #300

Closed Karrq closed 10 months ago

Karrq commented 1 year ago

The new minor release changes the MSRV to 1.60 due to the use of unarray which uses MaybeUninit::assume_init_drop, stabilized on 1.60.

I know it might not make sense to support a compiler "that" old, but ultimately this broke my ci :( I tried searching in the repo other than that issue and it doesn't seem explicit if the MSRV bump is supposed to be breaking or not...

Thank you for your time!

matthew-russo commented 1 year ago

Thanks for reporting this and apologies that it caused some issues in your CI. There are differing opinions on whether MSRV bumps are breaking changes and a lot of crates have different approaches. From briefly reading through some discussions, it seems like many are of the opinion that MSRV bumps don't necessitate a major version bump, with a fair number of people settling that this can be deserving of a minor version bump. Some of the things I was looking at include:

which i just picked up from a quick google search.

I don't think we want to be so strict as to pin our MSRV for an entire major version but the very least we can do is formalize what our policy is regarding MSRV and document it clearly in the create README/documentation. Many crates have a policy that says any change that supports the last n rust versions is non-breaking which seems like a reasonable approach. During releases we should also clearly document any changes in MSRV and any known fallout from it. I'll bring this up with the other maintainers as well in case there are differing opinions.

If there are any specific things that block you from upgrading the rust version used in CI or any insight in to your cadence of rust version upgrades, that'd be insightful to help us come up with a balanced policy

Karrq commented 1 year ago

Thank you for the thoughtful answer! I guess it's on a project by project basis, but I wanted to point out the issue mostly so that it can be made explicit and avoid surprises in the future!

I can always just put =1.0.0 if ever I can't change the compiler version after all!

cameron1024 commented 1 year ago

It's for sure a tricky issue. I've had a look at some other crates, and there isn't much consensus in the ecosystem about what the "standard MSRV" should be. It's also complicated by the fact that it doesn't really matter what "most" crates do, it only matters what "the most aggressive crate in your dep graph" does. Looks like that was us this time :sweat_smile:

So I'd be curious to know "how broken" your CI was. If the fix was as simple as bumping the number in rust-toolchain.toml, then it's not a huge issue to me. But if it legitimately blocks you from upgrading (or makes upgrading a chore, rather than a simple cargo upgrade), I'd be keen to hear more about your situation.

We're currently reviewing what our policy should be, so any insight from users is much appreciated :pray:

But I agree, whatever we agree on, we should put it in the readme so people don't have to hunt through the code to find the version they need to get it to build

Karrq commented 1 year ago

Honestly I've had semi regular breakage in regards to MSRV, sometimes it's one crate sometimes another 😆

In my case I have a docker image built with a specific version of the compiler, and that image is used in CI, so upgrading version would mean upgrading the image, and that upgrades the compiler across multiple projects.

The compiler isn't pinned to a specific version for stability, but rather to make the CI faster, so upgrading isn't a blocking issue, but as you said it's a chore more than just cargo upgrade...

Personally I think all library crates should document their policy, but yeah the ecosystem isn't clear...serde has MSRV bumps as patches, for example.

Even cargo-semver-checks hasn't defined yet what MSRV bump should entail...

obi1kenobi commented 1 year ago

Even cargo-semver-checks hasn't defined yet what MSRV bump should entail...

We try to implement lints only after community consensus has been reached 😅 And indeed, MSRV hasn't achieved a clear consensus, though the majority seems to be on the "not major" side as far as I can tell.

Btw, if the maintainers of this repo are interested in adopting cargo-semver-checks, I'd be happy to help!

matthew-russo commented 10 months ago

I'm going to close this issue out as there aren't actionable items for us. If you run in to any problems in the future please open a new issue