Without `getrandom` feature flag forwarding, wasm users builds could break on minor `rand` update

[michaelkirk]

My wasm32-unknown-unknown build is working by following the documented suggestion to add an explicit dependency on getrandom in my application crates. Something like this:

[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies]
getrandom = { version = "0.3.4", features = ["wasm_js"] }

This is unsatisfying in a couple ways (but maybe it's the least worst?):

Firstly, since our crate has some browser usage, we want to be sure our lib crate builds on wasm32-unknown-unknown. Since it's a lib crate, not an application crate, the least worst solution I could find in the spirit of the official documentation would be to introduce a binary (application) crate for the sole purpose of testing our libraries wasm32 compatibility. That's doable, but a little annoying. Is there a better way?

Secondly, since getrandom is a private dependency, rand can now do a major (in the semver sense) update of getrandom without needing a corresponding major semver bump in rand. This was one of the motivations mentioned when making getrandom non-public in #1537

So one day when getrandom releases a major update (say 0.4.0) and rand updates internally and does a (semver) minor release (say to 0.8.x), our wasm32 users builds will break again, until they update their getrandom dependency in Cargo.toml to something like:

  [target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies]
- getrandom = { version = "0.3.4", features = ["wasm_js"] }
+ getrandom = { version = "0.4.0", features = ["wasm_js"] } # assuming the getrandom feature flag doesn't change

It's a bit rough, right? (I assume you have your reasons!)

One alternative would be to restore something like the pass-through flag getrandom?/js_wasm feature, which was intentionally removed in #886

The reason originally listed for its removal.

Since this is currently the only reason we depend on the getrandom crate directly, this allows us to remove the getrandom_package rename hack, thus also clearing up #881.

If I'm understanding correctly, the rename hack has meanwhile been solved by Rust 1.60 dep:foo syntax, released in April 2022.

Elsewhere I've seen concerns about Cargo.lock bloat and broken builds. I'm not sure if the situation there has improved. I would guess Cargo bloat has not improved. I'm not aware of what the broken builds were, so I have no idea if the situation has improved there.

At the moment I'm considering "solving" this for our users, by pinning our own lib crate to a major version of getrandom:

[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies]
getrandom = { version = "0.4.0", features = ["wasm_js"] } # assuming the getrandom feature flag doesn't change

   let mut rng = match seed {
        Some(s) => StdRng::seed_from_u64(s),
        #[cfg(all(target_arch = "wasm32", target_os = "unknown"))]
        None => {
            let mut seed = [0u8; 32];
            getrandom::fill(&mut seed)?;
            StdRng::from_seed(seed)
        },
        #[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))]
        None => StdRng::from_os_rng(),
    };

That way if rand bumps its internal private usage of getrandom, we don't have to worry about it breaking our users builds. Do you see any concerns with that approach?

[dhardy]

the least worst solution I could find in the spirit of the official documentation would be to introduce a binary (application) crate for the sole purpose of testing our libraries wasm32 compatibility.

All tests run on libraries will use a test binary of some sort. This could go under an examples or tests sub-dir (using its own Cargo.toml if necessary). This type of test case for libraries is not uncommon in my experience.

Secondly, since getrandom is a private dependency, rand can now do a major (in the semver sense) release of getrandom without needing a corresponding major semver bump in rand. This was one of the motivations mentioned when making getrandom non-public in #1537

Good point. I'm not sure yet whether getrandom will be a public dep in the next rand version. In the current rand version it effectively is, so there isn't currently a risk of us bumping getrandom in a breaking way.

It's a bit rough, right? (I assume you have your reasons!)

Yes. wasm32-unknown-unknown is a half-baked target which doesn't specify itself enough to know what kind of getrandom backend is available, and getandom is not yet available as a std library feature. Believe me, we also find the situation rather frustrating. Eventually the solution should be WASI (see wasm32-wasip3) but that's not stable yet.

Another way you could look at it is that WASM-on-the-browser is an experiment not yet ready for wide-scale adoption. I have spoken to some of the core library maintainers on the issue, whose take is that even if getrandom was supported through the std library it would not support the wasm32-unknown-unknown target. So I guess we could just follow suit and not support wasm32-unknown-unknown at all; we've had more complaints than thanks for this work.

It may be worth reintroducing the wasm_js feature in rand. The next release will have a direct dependency on getrandom anyway.

• Consider reintroduce wasm_js feature (if getrandom is not a pub dep of rand)

---

   let mut rng = match seed {
        Some(s) => StdRng::seed_from_u64(s),

Out of curiosity, why are you supporting optional-seeding of a non-portable PRNG? The next rand release will re-export ChaCha*Rng and Xoshiro*PlusPlus from rand::rngs, one of which might be a better pick here. (In the current version you are supposed to depend on another crate for this.)

[newpavlov]

Reintroduce wasm_js feature

I will put it as mildly as I can... I am strongly against it. Such action would yet again encourage proliferation of this feature in downstream crates, which I find abhorrent.

This is why I argued that we should remove WASM JS support from getrandom completely in favor of the custom-fallback feature and make it wasm-bindgen's headache. The v0.4 release would be a perfect opportunity for this...

[newpavlov]

@michaelkirk

This is unsatisfying in a couple ways (but maybe it's the least worst?)

Here is a litmus test which I recommend: if your library crate unconditionally depends on wasm-bindgen or js-sys on the wasm32-unknown target, then it's fine to enable the wasm_js feature for getrandom like in you snippet. If not, then you SHOULD NOT do it and getrandom configuration should be responsibility of downstream users, especially considering that they may have several getradom versions (e.g. v0.3 and v0.4) in their dependency tree.

[dhardy]

I will put it as mildly as I can... I am strongly against it.

Considering that rand will (soon) depend directly on getrandom, I don't see anything wrong with making getrandom a public dependency, in which case we don't need this. I'll edit the above.

I do agree with you that — in an ideal world — it would be preferable to have as little WASM-JS specific features baked into rand & getrandom as possible. However, we already decided on the getrandom side (for good reasons) and I don't wish to revisit that.

[newpavlov]

I don't see anything wrong with making getrandom a public dependency

It's not about making getrandom public dependency or not. It's about bubbling the wasm_js feature in library crates. We explicitly documented that library crates should not do this and now you want to ignore it in rand?!

[dhardy]

avoid unconditionally enabling it in library crates

Did you skip over the word 'unconditionally'?

It's not about making getrandom public dependency or not.

But it is. As noted in the opening comment above, the previous move to make getrandom not a public dependency is the a reason that such a feature might be wanted.

I apologise that my first reply above was not clear about my intention: namely to bear this in mind when we update rand for the recent rand_core & getrandom changes.

[newpavlov]

Did you skip over the word 'unconditionally'?

You are technically correct, but the intent was for the feature to be enabled only in "application" crates (including tests and benchmarks), with the only potential exception being library crates already dependent on wasm-bindgen. It's worth to update the wording to clarify it more explicitly.

[Bluefinger]

I kinda want to mention that for all the potential problems the current setup has, it is still the least painful for the ecosystem at large to handle Web WASM, and is less brittle with particular setups. getrandom 0.3 initially proved to be painful to handle for Bevy, and there was also plenty of projects just vendoring getrandom code to get round the bad WASM ergonomics.

Yes, the problem exists because wasm32-unknown-unknown is a dead albatross wrung around Rust's neck, but if we have to go through another round of bad WASM ergonomics, we will just see more projects not deal with getrandom and do it their own way, which might not actually be secure, or just duplicate a lot of code.

[michaelkirk]

dhardy: All tests run on libraries will use a test binary of some sort. This could go under an examples or tests sub-dir
(using its own Cargo.toml if necessary). This type of test case for libraries is not uncommon in my experience.

Admittedly this is a small point, but it's something I'd prefer to avoid. It's not a huge burden to do it for one crate. Just one more thing to remember to update. It would be a burden if we had to do this for many crates. Maybe rand is a special case that deserves special treatment.

michaelkirk: since getrandom is a private dependency, rand can now do a major (in the semver sense) release of getrandom without needing a corresponding major semver bump in rand.

dhardy: In the current rand version it effectively is, so there isn't currently a risk of us bumping getrandom in a breaking way.

This was the bigger concern. I'm (somewhat 😉) relieved by your saying there (currently 😉) isn't a risk of bumping getrandom in this way.

dhardy: Out of curiosity, why are you supporting optional-seeding of a non-portable PRNG?

Oops - thanks! Our current usage of rand isn't very sensitive, but I'd guess that was unintentional.

newpavlov: Here is a litmus test which I recommend: if your library crate unconditionally depends on wasm-bindgen or js-sys on the wasm32-unknown target, then it's fine to enable the wasm_js feature for getrandom like in you snippet. If not, then you SHOULD NOT do it and getrandom configuration should be responsibility of downstream users, especially considering that they may have several getradom versions (e.g. v0.3 and v0.4) in their dependency tree.

Thank you for the clear recommendation. There are many issues here and in getrandom's tracker similarly explaining what to do, but I haven't yet found the discussion/rationale for why its useful to break our wasm32-unknown users build, rather than make a change that would cause things to "just work" for them.

I'm assuming there is some worse downside for them if we go down the path of adding this to our lib crate's Cargo.toml:

[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies]
getrandom = { version = "0.3.4", features = ["wasm_js"] }

There was a reference to Cargo.lock bloat and also to confusing build failures. Is that the bad thing we risk if we include something like this in the geo (lib) crate's Cargo.toml, or was it something else? Any reference to an old issue or configuration that's known to encounter this would be appreciated.

I know this is a long running topic for which you've expended a lot of effort and received a lot of frustrating scrutiny. I really do appreciate your in-depth responses so far.

[dhardy]

Admittedly this is a small point, but it's something I'd prefer to avoid. It's not a huge burden to do it for one crate. Just one more thing to remember to update. It would be a burden if we had to do this for many crates. Maybe rand is a special case that deserves special treatment.

If you want the test to use non-default feature flags you could just pass --features when running it, though that might be a nuisance. You might also use dev-dependencies. But often using a new crate (probably a workspace member) is the best way to test a library with non-default features and extra dependencies.

but I haven't yet found the discussion/rationale for why its useful to break our wasm32-unknown users build, rather than make a change that would cause things to "just work" for them.

Because there are some non-web applications using wasm32-unknown-unknown which are broken by the wasm_js feature. Also because the wasm_js feature adds a lot of bloat to Cargo.lock. But if you don't care about that, well, up to you.

For reading material you could start here.

[dhardy]

It may be worth reintroducing the wasm_js feature in rand. The next release will have a direct dependency on getrandom anyway.

I think this is the only outstanding item here: do we add a "wasm_js" feature flag to rand? @newpavlov is clearly against this and I am not particularly in favour either.

rand = "0.10.0"

[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies]
getrandom = { version = "0.4.1", features = ["wasm_js"] }

While ugly, this would have approximately the same effect as:

rand = { version = "0.10.0", features = ["wasm_js"] }

The problem (aside from scope) is that "wasm_js" is a non-additive feature: it can break builds. We did however decide to enable it in getrandom v0.4 anyway, because as @Bluefinger said above:

for all the potential problems the current setup has, it is still the least painful for the ecosystem at large to handle Web WASM, and is less brittle with particular setups.

I wouldn't be too surprised if a future getrandom version (and thus also a future rand version) replaces this with something else (in an API-breaking manner), but such is life.