Repository navigation
Closed
Milestone
Description
In the process of addressing #94, we realized that some of the changes we want to do would require breaking backwards compatibility. If we are going to release a version 0.2, we should consider other changes to getrandom.
A potential list is below. @dhardy and @newpavlov feel free to edit this to include your own ideas.
- Only expose the
stdtraits if thestdfeature is enabled (see trait impl disappeared without bumping major version #94 and Add back in std-only trait definitions #96)- This is needed for Use getrandom crate rust-lang/rust#62082
- Think about having default features
- What should these be?
-
Rename features to allow for grouping of dependencieswasm-bindgen -> bindgen, would make it much easier to usejs_sysorweb_sys.- Moot, now that Add support for Custom RNGs #109 removed those features.
- The default behavior for
wasm32-unknown-unknown(with no explicit features). Options are:- Always fail (current implementation)
- Compile error (current 0.2 behavior, see Feature for using CPU-based RNG #84)
- Default to
bindgen
- Cleanup
wasm32error codes- Implemented in wasm32: Unify error codes and implementations #146
- Custom Randomness (embedded source compatibility #4)
- Can we make a solution that does not require unsafe code for users?
- Feature for CPU randomness (Feature for using CPU-based RNG #84)
- If we add this, then it might make sense to remove explicit support for
hermit,l4re, anduefiand just use this feature instead, as these platforms (especially UEFI) have multiple ways to get randomness.
- If we add this, then it might make sense to remove explicit support for
- Minimum Rust Version
- Minor Cleanups for 0.2 #136 increases the MSRV to 1.33
- Do we want to increase this further? Bumping to 1.34 would let us use
TryInto, which could be useful. - Update MSRV to 1.34 #159 Bumped to 1.34
- Documentation
- Implementation at Update docs for 0.2 Release #135
If you have suggestions, please comment!
Metadata
Metadata
Assignees
Labels
No labels
Activity
newpavlov commentedon Aug 25, 2019
Unfortunately we probably still can't use compilation error by default for
wasm32-unknown-unknown, since it would require bumpingrand/rand_coreversions (i.e. breaking change leaks downstream in this case).I like the idea of replacing support for
hermit,l4re, anduefiwithrdrandflag, but I am bit unsure about implications forrand, since again strictly speaking it can be considered a breaking change. :/I am still not sure why would we want to use
web_sysinstead ofwasm-bindgendirectly.josephlr commentedon Aug 25, 2019
Hmm, that would be true. We could consider dividing the issues into "things that would require
rand 0.8" and "things we could make work withrand 0.7".I mean we could always cop out and say those platforms weren't officially supported by
randyet and so it's not a breaking change (as all of these are new-ish targets, it might be fine).A fair point, I guess what I meant here is that if we want to do something like this, we should rename the feature. I'll try to see if the code would be any better using
js_sys. I think this would be easier to talk about with actual code.dhardy commentedon Aug 25, 2019
We could use a feature-flag to re-enable (perhaps
dummy-wasm32-unknown). But I don't recall any reason we shouldn't keep this behaviour as the default forwasm32-unknown-unknown; as @newpavlov has said a couple of times it's a not-quite-standardised target that can hopefully be dropped at some point.IMO this is not an urgent issue (at least, I've seen very little demand for it). There is already a safe alternative: a Cargo patch to replace the crate.
There are multiple aspects to this:
josephlr commentedon Sep 19, 2019
I've created a 0.2 branch to track breaking changes to
getrandom. PRs that make breaking changes should be submitted/merged against that branch.newpavlov commentedon Oct 28, 2019
Maybe we also should remove the
logfeature? IIRC it was mostly useful for WASM backends and I haven't seen anyone to enable this feature directly and it's not cascaded from therandcrates.CryZe commentedon Nov 19, 2019
Both cargo and the wasm target are still way too broken to remove the dummy implementation for rand / getrandom's wasm32-unknown-unknown support.
josephlr commentedon Jan 6, 2020
@CryZe what do you mean? Our plan (see #109) is to give users of
rand/getrandoma way to set a custom RNG source (similar to how Custom Allocators work) on unsupported targets (which would includewasm32-unknown-unknown). Is there a reason that wouldn't work for your use-case?Having the dummy impl makes debugging issues very hard, as the dummy impl fails at runtime as opposed to compile time.
CryZe commentedon Jan 6, 2020
Ah, I think that would work then.
josephlr commentedon Jan 6, 2020
@newpavlov I think dropping the
logfeature is also a good idea. We should focus on getting any relevant error info into theErrorstructure itself.newpavlov commentedon Jan 10, 2020
@josephlr
Maybe it's worth to update the OP list?
josephlr commentedon Feb 26, 2020
@newpavlov @dhardy I updated the list. I think we can release
0.2after #136 and #135 are reviewed an merged.vks commentedon Jul 1, 2020
I think we should consider bumping MSRV to 1.36, see rust-random/rand#987.
newpavlov commentedon Jul 7, 2020
getrandomis not dependent onalloc, so there is no reason to bump MSRV to 1.36 (but I would prefer ifrand v0.8andrand-distr v0.3had MSRV equal to 1.36). I don't think we have to keep MSRV synchronized across allrandcrates.Lokathor commentedon Aug 26, 2020
Increasing from 1.33 to 1.34 is entirely reasonable, since that's current Debian Stable.