rust-random / rand

A Rust library for random number generation.
https://crates.io/crates/rand
Other
1.64k stars 429 forks source link

Tracker: rand 0.9 #1165

Open vks opened 3 years ago

vks commented 3 years ago

Blocker:

Planned breaking changes for rand 0.9 are:

Possibly also:

Non-breaking TODOs:

dhardy commented 2 years ago

We should also conform to common Rust coding standards (generally expected of open projects now). I have argued against these in the past (some poor formatting and false-positive lints, verbosity of opt-outs), but the reasons not to use these are fewer (less active development) and reasons to do so greater (mature tools and very widespread usage):

I suggest only doing this late before v0.9 since both will cause significant merge conflicts and there is currently some on-going work.

vks commented 2 years ago

Fixing the clippy warnings is not a big deal, we are already doing pretty well. Also see #1197.

dhardy commented 2 years ago

We should consider bumping the MSRV to Rust 1.51.0 (March 2021), enabling usage of const generics by default.

dhardy commented 1 year ago

Question: do we make a breaking release of rand_core at the same time or stick with 0.6.x?

See my comment in #1269:

vks commented 1 year ago

What breaking changes would we want to release for rand_core?

dhardy commented 1 year ago

Pending: #1182, #1267.

Possible future changes: #1261, and @newpavlov mentioned somewhere using const generics to re-write RngCore with a method like fn generate(&mut self, result: &mut [u8; Self::LENGTH]). (I can't find his comment.)

newpavlov commented 1 year ago

I also would like to rework a bit the crypto traits. We have some issues with them in RustCrypto, see: https://github.com/RustCrypto/traits/issues/1148

coolreader18 commented 1 year ago

Not sure if this would be the place for it, but in terms of major version bumps it'd be good to switch feature specifications over to weak feature dependencies ("dep?/feature" syntax) once the MSRV >= 1.60. Otherwise there's no actual way to e.g. enable std without also enabling rand_chacha as a dependency cause it's std = [.. "rand_chacha/std", ..]

dhardy commented 1 year ago

I guess we could go all the way to 1.60 (April 2022). We already have a PR for 1.56: #1269.

dhardy commented 1 year ago

Lets go with a breaking release for rand_core since we have a few proposed changes and no objections.

@coolreader18 feel free to make a PR with your proposed changes after the 1.56 PR is merged. I'm not certain yet we'll use 1.60 but think it's unlikely there will be a significant reason not to.

SUPERCILEX commented 10 months ago

Is it worth making a release with what we currently have? Selfishly, I'd like to be able to use my PRs, but it's also worth noting that the last release was in February 2022. I tend to prefer the regular release cadence approach rather than the wait for everything to be ready approach. Thoughts?

dhardy commented 10 months ago

I would like to get a release out soon, if only because we've had a lot of changes since the last release. Will have to review (and dedicate a bit of time to this).

dhardy commented 7 months ago

Update: a pre-release is planned.

I'd like to get a couple more significant changes in before the actual release:

dhardy commented 5 months ago

We should also resolve this, one way or another:

vks commented 5 months ago

I don't think #1399 is a blocker, it's one of the caveats of using usize. I think we can fix it, and it would be a value-breaking change, but I would be fine with leaving this for Rand 0.10. If we just add gen_index, it would not even be a breaking change.

dhardy commented 5 months ago

Fair point, although we already have value-breaking changes here.

zerocopy may be our biggest blocker right now anyway (see update at the very top of this issue).

josephlr commented 4 months ago

We are looking at updating the getrandom implementation on Windows to match that in libstd: https://github.com/rust-random/getrandom/issues/414

The simplest way to implement this is to use raw-dylib which was only stabilized in Rust 1.71 (released July 2023). We were considering releasing a release of getrandom which bumped the MSRV to 1.71. Would that be too new for rand 0.9?

CC: @newpavlov

dhardy commented 4 months ago

At the rate we're going, that may still be a year old by the time 0.9 is ready (though I hope not).

I am not aware of any hard constraints here.

joshlf commented 4 months ago

Fair point, although we already have value-breaking changes here.

zerocopy may be our biggest blocker right now anyway (see update at the very top of this issue).

Is there a reason you can't use zerocopy 0.7.32? What 0.8 features do you need? Maybe we could backport things to 0.7 to unblock you?

joshlf commented 4 months ago

Update: I've submitted https://github.com/rust-random/rand/pull/1446; at least as of the current state of master, 0.7.33 works, which I hope can unblock you guys.

dhardy commented 4 months ago

It might be worth deciding whether to adopt #1424 before the next release. (Comments welcome.)

I was also hoping to see the migration to chacha20. Not sure exactly where that's at. @tarcieri?

tarcieri commented 4 months ago

We've merged https://github.com/RustCrypto/stream-ciphers/pull/333 which re-adds rand_core support.

However, it will probably still be some time before we cut a release, and one of the potential blockers on such a release is if we should upgrade to a prospective rand_core v0.7 first

dhardy commented 2 months ago

I'd like to get #1299 solved one way (#1301) or another (#1462 — assuming I can convince people that the differences between open and closed ranges are unimportant for floats), then I think we can drop a Beta release.

SUPERCILEX commented 2 months ago

It sounds like numpy allows returning the exclusive bound of a range, so doing #1462 seems reasonable especially since it probably improves performance.

WarrenWeckesser commented 2 months ago

@dhardy, sorry for disappearing for so long. I can take another look at the uniform changes this weekend (probably Sunday), but don't wait for me if you want to get the release out sooner!

dhardy commented 1 month ago

Our to-do list is now done (excepting replacing ChaCha code with the chacha20 implementation, for which it's been requested we release the next rand version first).

One last thing I'd like to solve, however, is usize / isize non-portability (#1399). To that end I have opened #1471, though I release that approach may be unpopular. Opinions on that PR please.

I'm away on holiday from Thursday and would love to make a beta release of v0.9 before then.

dhardy commented 1 month ago

I have published an alpha: https://github.com/rust-random/rand/tree/0.9.0-alpha.2 (master + #1471 + #1480 + a couple of tweaks to Cargo.toml).

dhardy commented 1 month ago

Alpha anouncement: https://www.reddit.com/r/rust/comments/1egixd9/rand_v09_alpha_usizeisize_support/

SUPERCILEX commented 1 month ago

Can https://github.com/rust-random/rngs get alpha releases too? Pretty sure it's not possible to upgrade if use something like rand_xoshiro. Then again might be a bit of a pain, so could wait for the full release.

hsn10 commented 1 month ago

Create MSRV policy and hardcode dependencies, otherwise you can lose MSRV if dependency upgrades its MSRV in minor version.

I am happy with any MSRV <= 1.60. Usually 1.56.0 is used for simple crates because its lowest 2021 edition.

newpavlov commented 1 month ago

Pinning dependencies is very restrictive and can easily break downstream, so we are highly unlikely to do that. The most practical choice is to rely on the Nightly MSRV-aware resolver to generate an appropriate Cargo.lock file.

hsn10 commented 1 month ago

Nightly MSRV resolver is broken and it won't be fixed to resolve dependency trees correctly. It will work only if all crates in tree have MSRV information because author don't have courage to implement proper tree walking strategy like other package managers (npm, apt, dnf).

It don't and won't support case when you have tree MSRV crate -> crate without msrv -> msrv crate. So you can't count on it to do much good.

newpavlov commented 1 month ago

All rand dependencies specify rust-version, so it does not matter in our case. If some of your project dependencies do not do that, it's better to fix them.

hsn10 commented 1 month ago

getrandom and bincode doesnt

newpavlov commented 1 month ago

cargo update -Zminimal-versions works without issues (I think it uses edition to estimate MSRV in such cases). IIUC both getrandom and bincode have stricter MSRV than we target for rand v0.9 and have no plans to bump it. getrandom will use rust-version in v0.3 (which likely will be released before rand v0.9) and bincode v1 is no longer developed.

Either way, it does not change the outcome: we will not be pinning dependency versions since it's too disruptive.

tarcieri commented 1 month ago

@hsn10 we (also @newpavlov) experimented with pinning dependencies in library crates as you're suggesting in the @RustCrypto ecosystem to preserve MSRV, and it ran into huge ecosystem incompatibilities and breakages which are still ongoing years later. In 20/20 hindsight, such pinning was very much a mistake.

While it might seem like a nice solution at first it creates incompatibilities that don't need to exist and valid crate combinations being rejected. Where crates upgrading their MSRV might be a minor annoyance, incompatible dependency versions create an unsolvable problem (or rather, a problem that can only be solved by removing the overly restrictive bounds).

The only people who should be pinning in their Cargo.toml to preserve MSRV are authors of [bin] crates (and even then, Cargo.lock can largely take care of it for you).

epage commented 1 month ago

In 20/20 hindsight, such pinning was very much a mistake.

For added emphasis, Cargo published official guidelines on this based on the negative experiences caused by using version requirements for MSRV, see https://doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html#multiple-requirements

cargo update -Zminimal-versions works without issues (I think it uses edition to estimate MSRV in such cases).

-Zminimal-versions is exactly what it says on the surface. I prefers lower compatible version numbers over higher ones. It does not take MSRV into account. For example, I dropped the MSRV on some of my packages recently, so -Zminimal-versions would only work for MSRV testing by first bumping the relevant version requirements.

I need to post an update on the tracking issue. We discussed it in a recent cargo team meeting and are considering dropping -Zminimal-versions