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

Towards rand 1.0 #232

Closed
dhardy opened this issue Oct 9, 2021 · 10 comments
Closed

Towards rand 1.0 #232

dhardy opened this issue Oct 9, 2021 · 10 comments

Comments

@dhardy
Copy link
Member

dhardy commented Oct 9, 2021

We've had questions about when Rand will reach 1.0. We're not there yet, but it is still worth discussing the dependency on getrandom.

From memory & quick skim the only part of the API which is affected by getrandom is rand_core::Error, and only in two ways:

Potential solutions:

  1. Stabilise getrandom 1.0 soon? Seems unlikely?
  2. A separate crate just for Error — perhaps viable but definitely not desirable
  3. Use the promise above and deprecate that impl, forcing users to rely on getrandom::Error::code() -> NonZeroU32 and impl From<NonZeroU32> for Error

CC @vks @newpavlov

@gilescope
Copy link

That error code looks a little like an enum with non-exhaustive on it?

@vks
Copy link

vks commented Oct 10, 2021

I think the most important part of our API affected by getrandom is actually SeedableRng::from_entropy.

Why is stabilizing getrandom 1.0 unlikely?

@dhardy
Copy link
Member Author

dhardy commented Oct 11, 2021

from_entropy and OsRng are not re-exporting any getrandom symbols. They use it internally, but that's fine because bumping the getrandom version inside a patch-release of rand_core won't cause problems there — but it can with that From<getrandom::Error> impl.

That error code looks a little like an enum with non-exhaustive on it?

Essentially, yes. But one that's user-extensible (e.g. by a custom backend) without any crate dependencies. Go read #3.

@gilescope
Copy link

Thx for that history pointer. A stability promise on the existing constants seems pretty uncontroversial? New ones could still be added...

@josephlr
Copy link
Member

As one of the maintainers, I would be fine committing to stability for the numerical constants. Specifically something like "these constants will not change between now and 1.0", and we could put something like that in the README.

I think the most important part of our API affected by getrandom is actually SeedableRng::from_entropy.

While this is true, it should be possible to have that API be stabilized without getrandom necessarily being in 1.0. For example if getrandom's API changed (not that we really anticipate such a thing), you would just need a way to still implement from_entropy with the new API.

I think removing the impl From<getrandom::Error> for Error would be wise, as it seems redundant to have that impl and impl From<NonZeroU32> for Error

Why is stabilizing getrandom 1.0 unlikely?

Personally, I don't think it's too unlikely, I would just want to resolve some outstanding issues before doing so. Specifically:

@newpavlov
Copy link
Member

Also ideally before releasing 1.0 I think it's worth to have MSRV-dependent dependency version resolution implemented in Cargo, so it would be possible to bump MSRV without breaking builds on older toolchains.

@gilescope
Copy link

Great work that's being done pushing this all forward. Mouse's review seemed very positive. Let me know if I can help in any way.

@newpavlov
Copy link
Member

As proposed here, another potential API change for 1.0 is to use BorrowedBuf instead of &mut [MaybeUninit<u8>] for getrandom_uninit.

@briansmith
Copy link
Contributor

As proposed #485 (comment), another potential API change for 1.0 is to use BorrowedBuf instead of &mut [MaybeUninit] for getrandom_uninit.

I don't know if it would make sense to change the public API to use BorrowedBuf/Cursor. I was suggesting there it might make sense to change the internal ones like sys_fill_exact. Even then I'm not sure.

@newpavlov newpavlov removed this from the v0.3 milestone Oct 28, 2024
@newpavlov
Copy link
Member

I think we can close this issue for now since most of the discussion is no longer relevant.

Before considering v1.0 we need to get some experience with getrandom v0.3. There is also a possibility of std introducing a getrandom-like API (see rust-lang/rust#130703) which would make this crate obsolete.

When it will be the time for rand v1.0 release we can open a new issue to consider whether we should release getrandom v1.0 or not.

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

No branches or pull requests

6 participants