FieldReport: Upgrading from geo 0.31.x to 0.32.x breaks the wasm32 build

[martinfrances107]

I see this when building for the browser

The problems is side-effects from changes inside of the getrandom crate.

1. Here is a dependency graph

MyApp -> MyLib -> geo -> rand -> getrandom

getrandom The change here releates to the sourcing of randomness for "supported targets" Here is the relevant quote from the getrandom README

 "wasm32-unknown-unknown" is no longer supported by default. see
(webassembly-support)[<https://crates.io/crates/getrandom#webassembly-support>]

For this PR the summary is

rand must enable the "wasm-js" and the root crate ( MyApp in this example ) must set this enviromment variable

RUSTFLAGS='--cfg getrandom_backend="wasm_js"'

rand takes its own view on this matter

Here is the relevant quote form the rand README

"The WASI and Emscripten targets are directly supported. The wasm32-unknown-unknown target is not automatically supported. To enable support for this target, refer to the getrandom documentation for WebAssembly. Alternatively, the os_rng feature may be disabled."

The best we can do, in geo, I think, is to optionally remove the rand crate, for builds that target the web.

geo = { path ="0.32", features = [ "earcutr", "spade", "multithreading"], default-features = false }

I should empahsise this comes at the cost of removing the kmeans module and its associated functionality.

• [ x] I agree to follow the project's code of conduct.
• I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

---

[dabreegster]

As a workaround, you can add getrandom = { version = "0.2.16", features = ["js"] } to your crate. This was discussed a bit on Discord.

[martinfrances107]

WOW .. thank you, thank you

That works and fixes a problem I have been thinking about for weeks. 😍

Just so I understand you and for other people reading this

in the MyApp Cargo.toml file from above

MyApp -> MyLib -> geo -> rand -> getrandom

When I add this

# Create a shadow dependency on getrandom with the feature "js"
# In effect this will add the "js" feature flag to the real depedency
#  MyApp -> MyLib -> geo -> rand -> genrandom
getrandom = { version = "0.2.16", features = ["js"]}

My response is this is brittle but ok

It needs to be updated with the changing winds from getrandom, which makes me sad.
When I see crazy/sneaky/"Not at all obvious" that is when I document.

I will adjust this PR sometime in this evening... limiting it to just adding a section to to README

[dabreegster]

I think this PR is a nice solution, since it stops downstream users from needing this workaround. Opting into a feature to get kmeans seems much more typical. I'll let other maintainers chime in on the best way forward

[martinfrances107]

I have stripped down the PR to just documentation.

I see that the rand crate needs bumping to the latest version ...
one step at a time .. but just to be here clear here I am leaving a maintenance behind.

[martinfrances107]

Cross post .. I posted the stripped down PR before I saw the

Opting into a feature to get kmeans seems much more typical.

Depending on what other have to say .. I will happily restore to the first patch
"The one with the ability to drop "rand" by dropping the default behaviour ..

[kylebarron]

It feels like removing the kmeans feature by default avoids the problem while the docs solution handles the problem.

[michaelkirk]

Thanks for reporting - that seems annoying.

This seems like it'd be an issue with any crate which depends on random which is built for wasm32-unknown-unknown. Is that true? I wonder how other folks have dealt with it.

I'm all about having documentation, especially as a stop gap, but the whole point of package managers and feature flags is so you can just install the thing you actually need, and any dependencies should "magically" be taken care of for you.

[martinfrances107]

I would like to articulate the problem with some concrete examples

I am trying to form my thoughts on the question

"Where should we cut a linkage"

Case 1: Web App:

See

https://github.com/martinfrances107/rust_d3_geo_voronoi/tree/main/benchmark

The benchmark which acts a web app from the d3_geo_voronoi_rs

In the mindset of the WebApp developer .. "I need for the source of randomness comes from the top level only"

"web benchmark" -> d3_geo_voronoi_rs> |-> rust_d3_geo -> geo -> rand -> getrandom
|
|-> getrandom

rust_d3_geo does not use "kmean" and so the portion of the top line could be snipped at geo -
only to be picked up again by at the "web benchmark" top level

Case 2 Conventional App:

"profile_target" not a web app it is a binary used to test the d3_geo_voronoi_rs crate.
https://github.com/martinfrances107/rust_d3_geo_voronoi/tree/main/profile_target

The source of randomness comes from a different place, the "operating system"

"profile target" -> d3_geo_voronoi_rs> |-> rust_d3_geo -> geo -> rand -> getrandom
|
|-> getrandom

"getrandom" is required only at the top level and a "fake Phantom dependencies on getrandom" just get in the way

So my question from the top "Where to cut the linkage"
Others may have different opinions, but I want it cut as low as possible.

A criticism of the "Allow deviations from the default - and cut out kmeans." stratergy is to make the "rand" crate do the write thing
but hey this is where we are.

[michaelkirk]

As a workaround, you can add getrandom = { version = "0.2.16", features = ["js"] } to your crate.

It seems like adding this line to every application crate is the preferred resolution from the rand crate authors.

That's honestly disappointing, but I'm willing to believe they have their reasons.

See rust-random/rand#886 and a handful of other "wasm" issues for details (also https://github.com/rust-random/getrandom).

[kylebarron]

I don't know the upstream backstory, but yes, I think currently every downstream wasm-bindgen application must handle getrandom themselves. I always refer back to my prior work whenever I hit this. In particular, I had to use libraries that use both getrandom 0.2 and 0.3

[urschrei]

Urgh. I take it there are no obvious alternatives to rand that don't have this problem.

[martinfrances107]

Sorry, cross post I posted my last long reply before reading michaelkirk comment

This seems like it'd be an issue with any crate which depends on random which is built for wasm32-unknown-unknown. Is that true?

We can to the same thought ..."I wish the "random" crate has come up with a better response to the problem"
the one I quoted in my first entry to this PR

rand ....

"The WASI and Emscripten targets are directly supported. The wasm32-unknown-unknown target is not automatically supported. To enable support for this target, refer to the getrandom documentation for WebAssembly. Alternatively, the os_rng feature may be disabled."

here is a list of projects depending on rand

https://crates.io/crates/rand/reverse_dependencies

1. time depends on the standard library and is

"Mostly compatible with #![no_std]

2. base64 and http --- may end up in webApps

Wow time deployed on esp32 microprocessor ..

I will sleep on this tonight .... This PR is outdated ... in the morning I will also go back the first PR -- as I perceive the conversation has gone in favour of supplying a fix of some kind rather than just documenting a work around.

PS

Here is a final case study -- a binary, not a web app .. with no need for a source of randomness where a developer copying my example is quite right to ask ... Why should I care where my source of randomness is sourced...

https://github.com/martinfrances107/rust_d3_geo/tree/v3.x-dev/examples/globe/rotating_wgpu

[martinfrances107]

Just to add a final diversion before bed...

The modern version of getrandom version supports WASI targets
this would be how getrandom obtains it source of randomness ( it has a feature "wasm_js" )

Currently modern browser use a WASI shim... @bjorn3/browser_wasi_shim so I needed in the morning for take a view on if this is too bleeding edge to be something we should even think about...

[martinfrances107]

In the light of the recent bump [email protected]

This is my way of of responding to michaelkirk enqiry

I wonder how other folks have dealt with it.

So when I start with a blank piece of paper, and answer

What configurations should be support?

In order of decreasing performance here is my list.

1. "OS"(Default) - Use the operating systems source of randomnes.
2. "Web" - Use the WasmBindgen/WASI to get the sandboxes source of randomness.
3. "NoStd" - Software implementation of a LCG https://en.wikipedia.org/wiki/Linear_congruential_generator
4. "NoRand" - Stub off the kmean module.

I note the "rand" crate supports a multitude of options, related to performance. So (1) and (2) could split into many variants
which I don't want to deal with further. Unless someone comes forward with a use case.

The latest version of "rand" still provides no clean way forward regarding "web" support. So I just want to see that we maintain
the current status quo

I appreciate (3) and (4) could be merged into one. As following the series of fallbacks down to "NoStd" means the kmeans module never ACTUALLY needs to be stubbed out as the LCG will work in all instances.

But I want a (3) and (4) to be distinct options, as in the examples outlined developer working from "Web benchmark" and "profile_target" may need to employ the "Phantom Crate" trick to modify the features of getrandom ... and I want (4) because it too easy to step into configuration hell.

So in short I think the current PR should be merged.

[weiznich]

There are two other solution I don't see discussed here:

• Disable the default features for rand and only enable the std_rnd. This should prevent geo from depending on getrandom at all. This seems possible as the new code actually doesn't use any os dependent rand features at all.
• Change the API of the kmeans implementation to explicitly accept a &mut impl rand::Rand as argument and enable no default features at all. Again this removes the dependency to getrandom and would place the burden of which random number generator is compatible to the environment the user is running the code in to the user itself. They should be aware which generators work in their environment and which not.

I'm happy to provide a PR for both solutions if that would be welcome.

[urschrei]

Oh that's interesting. The second option would (obviously) be breaking but the API is very new.

[michaelkirk]

Disable the default features for rand and only enable the std_rnd. This should prevent geo from depending on getrandom at all. This seems possible as the new code actually doesn't use any os dependent rand features at all.

I confess, this is my first exploration of the rand crate, but how do you instantiate a RNG without an explicit seed or the os?

[weiznich]

I confess, this is my first exploration of the rand crate, but how do you instantiate a RNG without an explicit seed or the os?

Seems like I missed this part. So yes you either need a seed or the os-rand feature or you need another random number generator to initialize the seed.

Given that the second option sounds like a better solution, even if it's a breaking change.

[martinfrances107]

Disable the default features for rand and only enable the std_rnd.

When I try this idea out by modifying the proposed golden reference (#1480)

I note : -

A ) "alloc" needs to be included

-rand = "0.9.2"
+rand = { version = "0.9.2", features = ["alloc", "std_rng"], default-features=false }

B) This moves the active focus to .. "This explicitly is where the kmeans code calls out the operating system."

StdRnd::form_os_rng()

[INFO  wasm_pack::child] Running cd "/home/XXX/build/geo/geo/examples/web-app" && "cargo" "build" "--lib" "--target" "wasm32-unknown-unknown"
   Compiling geo v0.32.0 (/home/XXX/build/geo/geo)
error[E0599]: no function or associated item named `from_os_rng` found for struct `StdRng` in the current scope
   --> geo/src/algorithm/kmeans/mod.rs:577:25
    |
577 |         None => StdRng::from_os_rng(),
    |                         ^^^^^^^^^^^ function or associated item not found in `StdRng'

I value the discussion .. The reference design should be peerless. ( encapsulating all the rules ) So it is essential to develop a sound understanding of exactly what our requirements are.

but

In years to come there is going to be a corner case where my app falls over due to a buggy interaction between say an android phone and outdated browser. It may only affect 1% of the global install base. As a users of this crate I want to de-risk my concerns. That makes me want to have the option to drop the rand crate from all further consideration.

This is all this PR does, other considerations could be handled in other PRs.