Skip to content

Conversation

@hanyuone
Copy link

@hanyuone hanyuone commented Sep 8, 2025

This PR implements the "first PR" outlined here in #2341:

First PR: Vendor the getrandom wasm_js.rs backend along with wrappers in lib.rs into rand/getrandom/{lib.rs,backends.rs,wasm_js.rs}. Compile and use the vendored implementation only if the target is wasm32 and wasm32_unknown_unknown_js is enabled. Use getrandom 0.3 otherwise.

I needed to add error.rs and utils.rs as well, since the Error struct and some utility functions were required by the other files.

Currently, running ring on wasm32-unknown-unknown requires the getrandom_backend config variable to be set in order for getrandom to compile properly, but making the ergonomics smoother is outside the scope of PR.

@hanyuone hanyuone changed the title Update getrandom Vendor getrandom 0.3 Sep 8, 2025
@hanyuone hanyuone mentioned this pull request Sep 10, 2025
Copy link
Owner

@briansmith briansmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on. Please post a new version with these changes and squash them all into one commit with a good commit message. Then I will review the updated version by diffing it with upstream. Also, please use an upstream commit that corresponds to a crates.io release of getrandom.

@hanyuone
Copy link
Author

Sorry for getting back to this late - all the points should be addressed, and I'm using the upstream commit corresponding to v0.3.3 on crates.io. Once you've confirmed everything is good, I'll squash everything down to one commit.

With ring, are we certain that we'll only ever be using getrandom_backend="wasm_js"? If so, then I can get rid of the getrandom_msan optional code in lib as well.

With v0.3.4, the getrandom_backend config flag is now default - would that change our approach to updating getrandom?

@hanyuone
Copy link
Author

@briansmith Have you had a chance to look at this yet?

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

Successfully merging this pull request may close these issues.

2 participants