Use getrandom crate for uuid #447
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ammgws
requested review from
Dylan-DPC-zz,
kinggoesgaming and
KodrAus
as code owners
kinggoesgaming
requested changes
Jan 18, 2020
ammgws
changed the title
ammgws
force-pushed
the
getrand
branch
6 times, most recently
from
97e9bcf
to
bcc2fb1
Compare
ammgws
changed the title
Reduces the number of dependencies.
kinggoesgaming
requested changes
Jan 21, 2020
| /// This uses the [`rand`] crate's default task RNG as the source of random | ||
| /// numbers. If you'd like to use a custom generator, don't use this | ||
| /// method: use the `rand::Rand trait`'s `rand()` method instead. | ||
| /// This uses the [`getrandom`] crate to utilise the operating system's RNG |
There was a problem hiding this comment.
@KodrAus should we expose the fact that we are using the getrandom crate implementation in the docs?
|
Thanks @ammgws for working on this :) |
Co-Authored-By: Hunar Roop Kahlon <hunar.roop@gmail.com>
Co-Authored-By: Hunar Roop Kahlon <hunar.roop@gmail.com>
Co-Authored-By: Hunar Roop Kahlon <hunar.roop@gmail.com>
|
Shall I rebase this? |
kinggoesgaming
approved these changes
Jan 28, 2020
|
bors r+ |
bors bot
added a commit
that referenced
this pull request
Jan 28, 2020
447: Use getrandom crate for uuid r=kinggoesgaming a=ammgws
<!--
If this PR is a breaking change, ensure that you are opening it against
the `breaking` branch. If the pull request is incomplete, prepend the Title with WIP:
-->
**I'm submitting a(n)** other
# Description
Use `getrandom` crate directly instead of `rand` for generating UUIDs using v4.
# Motivation
Reduce the number of dependencies used by the crate.
Running the example in the README:
- Before (10): c2-chacha, cfg-if, getrandom, libc, ppv-lite86, rand, rand_chacha, rand_core, rand_hc, wasi
- After (4): cfg-if, getrandom, libc, wasi
# Tests
<!-- How are these changes tested? -->
`cargo test` locally and test usage in a simple program.
# Related Issue(s)
# Other
Wasn't sure whether I should bump the version here or not.
Co-authored-by: Jason Nader <jason@kayoway.com>
Co-authored-by: Jason <ammgws@users.noreply.github.com>
bors bot
added a commit
that referenced
this pull request
Dec 21, 2020
503: Un-breaking-change Uuid::new_v4 r=KodrAus a=CAD97
<!--
If this PR is a breaking change, ensure that you are opening it against
the `breaking` branch. If the pull request is incomplete, prepend the Title with WIP:
-->
**I'm submitting a(n)** refactor
# Description
#447 changed from using `rand::thread_rng` to using `getrandom` in `Uuid::new_v4`. This also changed the return type from `Uuid` to `Result<Uuid, getrandom::Error>`. This PR reverts the signature to the simpler `new_v4() -> Uuid`.
# Motivation
This signature is much simpler to use, and avoids a breaking change. `getrandom` is _highly_ unlikely to fail, and previously we used `thread_rng` here, which also panics if it fails to initialize from the OS entropy source. If `getrandom` fails, it is highly unlikely that any program creating v4 UUIDs has any reasonable recovery other than to abort, as the system is in a broken state.
If users absolutely need to recover in this situation, they can call `getrandom` first themselves to make sure that their system is working, or generate the bytes themselves and create the UUID from those bytes.
Additionally, actually wrapping `getrandom::Error` in `uuid::Error` comes with its own fun set of problems, described in #502.
# Tests
N/A
# Related Issue(s)
#475: this undoes the breaking change to `Uuid::new_v4`, thus making the requested publish a patch update
Closes #502: alternate approach to the same TODO
Co-authored-by: CAD97 <cad97@cad97.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm submitting a(n) other
Description
Use
getrandomcrate directly instead ofrandfor generating UUIDs using v4.Motivation
Reduce the number of dependencies used by the crate.
Running the example in the README:
Tests
cargo testlocally and test usage in a simple program.Related Issue(s)
Other
Wasn't sure whether I should bump the version here or not.