search

rust-random / getrandom

A small cross-platform library for retrieving random data from (operating) system source
Apache License 2.0
264 stars 166 forks source link

Release 0.3? #433

Open josephlr opened 1 month ago

josephlr commented 1 month ago

We have a few changes coming up (#425, #414 , #432) which will make the following changes to the compatibility of this crate:

The Rust Semver Compatibility Guide does not specify if such actions would be breaking changes. Both:

are listed as "Possibly-breaking" changes.

My personal preference would be to release these changes as 0.2.16 (not consider this a breaking change) because:

However, if we do decide to have this be a breaking change, we should discuss if there are any other semver-breaking changes we wish to make (see #346 and #230). Another option would just be to get consensus on #232 and release these changes as 1.0

josephlr commented 1 month ago

CC: @dhardy @vks for how this might affect rand

newpavlov commented 1 month ago

We are not making any changes to the API of this crate

There was a proposal to rename getrandom(_uninit) to fill(_uninit), plus there is #365, but I do not have a concrete proposal for it now. Associated Error constants could also use a bit of work, e.g. I am not sure it's worth to expose constants like VXWORKS_RAND_SECURE on all platforms. We also can use this release to reconsider our cfgs (e.g. #230 and #346). It's also worth to tweak __getrandom_custom's signature to remove this hack.

Moreover, we can use this release to relax our MSRV policy in the vein suggested by @briansmith, but this should be done together with rand.

getrandom is widely used throughout the Rust ecosystem so updating all callers can be somewhat tedious.

I think it's rarely used directly, usually it's done through rand crates.

It's not clear how a user can have a custom implementation work with both versions of getrandom.

It's possible to do this:

[dependencies]
getrandom02 = { version = "0.2", package = "getrandom" }
getrandom = "0.3"

And call the registering macros from each crate (assuming extern functions are called differently).

dhardy commented 1 month ago

As @josephlr says, pushing these changes to a 0.2.x release does not break any formal stability guarantees and likely won't cause a large number of issues, though it's impossible to be certain.

I have a slight preference for bumping the version to 0.3 since the drawbacks are small:

briansmith commented 1 month ago
  • No other dependants will be affected. Some build trees may end up using multiple getrandom versions, but this hardly matters.

I understand what you're saying, but in practice it does matter. Many people have their CI set up to reject multiple versions of a dependency by default. so they have to either make an exception to their CI rules, and/or they submit PRs upstream to try to upgrade everybody to the new version, which is extra work for everybody. Then those upstream projects (such as some I maintain) end up having to choose to drop support for 0.2 to help these people, or spend time trying to explain to them (over and over, indefinitely) why it "hardly matters." Either way it ends up being disproportionate amount of effort.

If we release 0.3:

briansmith commented 1 month ago

There was a proposal to rename getrandom(_uninit) to fill(_uninit),

It would be nice but (a) it can be done in a backward-compatible way by renaming it to fill and providing a wrapper named getrandom[_uninit] that is optionally marked #[deprecated].

https://github.com/rust-random/getrandom/issues/365

I think nonblocking getrandom should be a separate API and so it could be added in a SemVer-capable manner. And/or we could implement the old API in terms of the new API, optionally eventually marking the old API #[deprecated].

Associated Error constants could also use a bit of work, e.g. I am not sure it's worth to expose constants like VXWORKS_RAND_SECURE on all platforms.

Better to discuss it in its own issue to reach a decision.

We also can use this release to reconsider our cfgs (e.g. https://github.com/rust-random/getrandom/issues/230 and https://github.com/rust-random/getrandom/issues/346). It's also worth to tweak __getrandom_custom's signature to remove this hack.

Yes, these are basically bugs that we've deferred fixes for until we make a SemVer-breaking version bump, so we should do this for 0.3.

josephlr commented 1 month ago

I opened #438 and #439 to discuss breaking changes to the API and feature flags respectively. I'm convinced by the arguments above, we should release these changes as a breaking change.

  • What will we cap the MSRV to for the 0.3 series?

Personally, I think that we should cap the MSRV for the upcoming release at 1.57 (1.56 is Rust 2021, and 1.57 gives us panicking in const which is super nice for static checks). My preference would actually be to release these upcoming changes as 1.0. I know @newpavlov wants to wait for MSRV-aware dependency resolution, but I'm not sure why we should block releasing 1.0 on that feature.

For 1.0 and onward, we can just say that removing platform support and increasing the MSRV can be done in a minor version, and that:

That way folks can put getrandom = "1.0" in their dependency file if they are fine with an MSRV of "2 years ago" and if they really want to for-sure not be broken, they can do getrandom = "~1.0"

  • When we add new OS support for 0.3, will we backport those to 0.2? In general will we keep maintaining 0.2? Depending on the MSRV policy for 0.3, there is likely going to be pressure to maintain 0.2.

I really wouldn't want to bother back-porting changes. I think we could have backports be "contributions welcome". I'd be fine reviewing backports, but I don't want to do the backporting (especially if nobody's actually using it).

newpavlov commented 1 month ago

I don't think we should release v1.0, not only because of the MSRV-aware resolver, but also because we will introduce new APIs. There are also potential for new features like "global crate features", which can be quite useful for us. Remember that we always can use the semver trick to release v0.3-compatible v1.0, assuming the APIs stay stable enough.

I really wouldn't want to bother back-porting changes. I think we could have backports be "contributions welcome". I'd be fine reviewing backports, but I don't want to do the backporting (especially if nobody's actually using it).

I agree with this, IIRC we acted similarly with v0.1 backports.

briansmith commented 1 month ago

The main problem with trying to make the next release be 1.0: The release will be long-delayed because there's too much perfecting and too many potential compatibility-breaking changes.

I think we need to accept the MSRV increases for 0.2 and do a 0.2 release soon, so that we don't get into a state where we can't do a release because master isn't good enough to be 1.0.