Uuid::new_v4() compiles and then panics for wasm32-unknown-unknown target

zrzka commented

2018-11-26 05:49:33 -06:00

(Migrated from github.com)

Is your feature request related to a problem? Please describe.

I do use uuid crate in my crate, which can be used as a pure Rust library or as an isomorphic NPM package (node & browser). The only feature I do use is random UUID (v4) generation.

It panics:

panicked at 'could not initialize thread_rng: No entropy sources available (permanently unavailable)', /Users/robertvojta/.cargo/registry/src/github.com-1ecc6299db9ec823/rand-0.5.5/src/rngs/thread.rs:84:17

Example of what we do is in this issue. Our uuidv4() function source code.

Describe the solution you'd like

Generate UUID v4 even when compiled for the wasm32-unknown-unknown target.

Is it blocking?

Kind of. Not a high priority.

Describe alternatives you've considered

Thinking about using Math.random in my function if the target is wasm32-....

Other

Not sure if this is the uuid crate problem or it lies somehwere deeper inside this whole bleeding edge stuff.

**Is your feature request related to a problem? Please describe.** I do use `uuid` crate in my crate, which can be used as a pure Rust library or as an isomorphic NPM package (node & browser). The only feature I do use is random UUID (v4) generation. It panics: ``` panicked at 'could not initialize thread_rng: No entropy sources available (permanently unavailable)', /Users/robertvojta/.cargo/registry/src/github.com-1ecc6299db9ec823/rand-0.5.5/src/rngs/thread.rs:84:17 ``` Example of what we do is in [this issue](https://github.com/balena-io-modules/balena-temen/issues/35). Our `uuidv4()` function [source code](https://github.com/balena-io-modules/balena-temen/blob/master/src/builtin/function/uuidv4.rs#L9-L11). **Describe the solution you'd like** Generate UUID v4 even when compiled for the `wasm32-unknown-unknown` target. **Is it blocking?** Kind of. Not a high priority. **Describe alternatives you've considered** Thinking about using `Math.random` in my function if the target is `wasm32-...`. **Other** Not sure if this is the `uuid` crate problem or it lies somehwere deeper inside this whole bleeding edge stuff.

Dylan-DPC-zz commented

2018-11-26 06:35:46 -06:00

(Migrated from github.com)

Hi. Thanks for reporting this.

Rand added wasm support to 0.6, while we are still using 0.5. (We have a PR to upgrade it to 0.6 but haven't merged it yet).

You can test it by depending on the dependabot/cargo/rand-0.6 branch instead of master.

Hi. Thanks for reporting this. Rand added wasm support to 0.6, while we are still using 0.5. (We have [a PR](https://github.com/uuid-rs/uuid/pull/348) to upgrade it to 0.6 but haven't merged it yet). You can test it by [depending](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-git-repositories) on the `dependabot/cargo/rand-0.6` branch instead of master.

❤️ 1

zrzka commented

2018-11-26 06:50:22 -06:00

(Migrated from github.com)

@Dylan-DPC thanks, just tried ...

Browser

panicked at 'could not initialize thread_rng: No entropy sources available
  (permanently unavailable)', .../rand-0.6.1/src/rngs/thread.rs:82:17

Node

panicked at 'could not initialize thread_rng: No entropy sources available
  (permanently unavailable)', .../rand-0.6.1/src/rngs/thread.rs:82:17

Testing with:

uuid = { 
  features = ["v4"],
  git = "https://github.com/uuid-rs/uuid",
  branch = "dependabot/cargo/rand-0.6" 
}

@Dylan-DPC thanks, just tried ... Browser ``` panicked at 'could not initialize thread_rng: No entropy sources available (permanently unavailable)', .../rand-0.6.1/src/rngs/thread.rs:82:17 ``` Node ``` panicked at 'could not initialize thread_rng: No entropy sources available (permanently unavailable)', .../rand-0.6.1/src/rngs/thread.rs:82:17 ``` Testing with: ``` uuid = { features = ["v4"], git = "https://github.com/uuid-rs/uuid", branch = "dependabot/cargo/rand-0.6" } ```

zrzka commented

2018-11-26 07:45:05 -06:00

(Migrated from github.com)

Not sure if this comment is related to my issue somehow:

use with --target wasm32-unknown-unknown --features=stdweb

Will check tomorrow if it's gated behind this feature.

Not sure if [this](https://github.com/rust-random/rand/blob/master/Cargo.toml#L64) comment is related to my issue somehow: > use with `--target wasm32-unknown-unknown --features=stdweb` Will check tomorrow if it's gated behind this feature.

zrzka commented

2018-11-26 15:12:37 -06:00

(Migrated from github.com)

@Dylan-DPC it's not enough to update to 0.6. One has to use stdweb or wasm-bindgen feature with wasm32-unknown-unknown target. Even the error message says it:

No entropy sources available (permanently unavailable)

Just checked the rand source code. It works (node & browser) if I change uuid crate dependency to:

 [dependencies.rand]
 optional = true
-version = "0.5"
+version = "0.6"
+features = ["wasm-bindgen"]

One can use stdweb or wasm-bindgen.

@Dylan-DPC it's not enough to update to 0.6. One has to use `stdweb` or `wasm-bindgen` feature with `wasm32-unknown-unknown` target. Even the error message says it: ``` No entropy sources available (permanently unavailable) ``` Just checked the `rand` source code. It works (node & browser) if I change `uuid` crate dependency to: ``` [dependencies.rand] optional = true -version = "0.5" +version = "0.6" +features = ["wasm-bindgen"] ``` One can use `stdweb` or `wasm-bindgen`.

KodrAus commented

2018-11-26 19:05:13 -06:00

(Migrated from github.com)

Hmm, this doesn't seem ideal to me... We'd either have to pick one of the wasm-bindgen or stdweb features in the rand dependency unconditionally. I haven't been keeping up with the wasm space so am not sure which of these features would be the better one.

We don't currently claim any wasm support in uuid (although it might be nice to figure out) so a workaround until we have some idea of what that should look could be to depend on rand yourself with one of the features you want.

@zrzka Do you have any thoughts?

Hmm, this doesn't seem ideal to me... We'd either have to pick one of the `wasm-bindgen` or `stdweb` features in the `rand` dependency unconditionally. I haven't been keeping up with the wasm space so am not sure which of these features would be the better one. We don't currently claim any `wasm` support in `uuid` (although it might be nice to figure out) so a workaround until we have some idea of what that should look could be to depend on `rand` yourself with one of the features you want. @zrzka Do you have any thoughts?

zrzka commented

2018-11-27 00:38:54 -06:00

(Migrated from github.com)

@KodrAus there's one way. uuid crate can have same features (stdweb, wasm-bindgen) as rand and these features can be passed to uuid crate dependencies.

uuid crate Cargo.toml:

[dependencies.rand]
optional = true
version = "0.6"

[features]
wasm-bindgen = ["rand/wasm-bindgen"]
stdweb = ["rand/stdweb"]

My crate Cargo.toml:

[dependencies]
uuid = { features = ["v4"], path = "../uuid" }

[target.wasm32-unknown-unknown.dependencies]
uuid = { features = ["wasm-bindgen"], path = "../uuid" }

This works for me. And anyone can still use uuid as it is or with either stdweb or wasm-bindgen feature, which just enables this feature in the rand crate.

I also tried to add ...

[target.wasm32-unknown-unknown.features]
wasm-bindgen = ["rand/wasm-bindgen"]
stdweb = ["rand/stdweb"]

... to enable these features for wasm32 only, but it seems that it doesn't work with ...features, just ...dependencies.

Maybe @alexcrichton can sched a light on this, I mean, what's the recommended approach. IIRC I saw Alex in all these wasm32 commits in the rand crate.

Another option, just for me, is to disable v4 feature on the UUID, copy & paste new_v4() to my crate, add rand dependency and generate / crate UUID v4 by myself.

@KodrAus there's one way. `uuid` crate can have same features (`stdweb`, `wasm-bindgen`) as `rand` and these features can be passed to `uuid` crate dependencies. `uuid` crate Cargo.toml: ``` [dependencies.rand] optional = true version = "0.6" [features] wasm-bindgen = ["rand/wasm-bindgen"] stdweb = ["rand/stdweb"] ``` My crate Cargo.toml: ``` [dependencies] uuid = { features = ["v4"], path = "../uuid" } [target.wasm32-unknown-unknown.dependencies] uuid = { features = ["wasm-bindgen"], path = "../uuid" } ``` This works for me. And anyone can still use `uuid` as it is or with either `stdweb` or `wasm-bindgen` feature, which just enables this feature in the `rand` crate. I also tried to add ... ``` [target.wasm32-unknown-unknown.features] wasm-bindgen = ["rand/wasm-bindgen"] stdweb = ["rand/stdweb"] ``` ... to enable these features for `wasm32` only, but it seems that it doesn't work with `...features`, just `...dependencies`. Maybe @alexcrichton can sched a light on this, I mean, what's the recommended approach. IIRC I saw Alex in all these `wasm32` commits in the `rand` crate. Another option, just for me, is to disable `v4` feature on the UUID, copy & paste `new_v4()` to my crate, add `rand` dependency and generate / crate UUID v4 by myself.

Dylan-DPC-zz commented

2018-11-27 00:57:01 -06:00

(Migrated from github.com)

It is a good idea to add wasm support for UUID. @zrzka since you have already started digging into this, are you open to sending a PR that adds the feature? we will mentor you. If you don't have time, I'll look into adding it later. Thanks

👍 1

zrzka commented

2018-11-27 01:08:31 -06:00

(Migrated from github.com)

@Dylan-DPC yup, going to do it now.

👍 1

zrzka commented

2018-11-27 05:02:28 -06:00

(Migrated from github.com)

Did the PR, but not sure if it's the right thing to do. It works for me and I have a temporary fix for my issue. No need to hurry.

What bothers me is this wasm-bindgen feature. There's a crate with the same name and you can't have a feature and a dependency with the same name. If, and if, you'll want to create wasm-bindgen based NPM package from the uuid crate, it will fail because of the wasm-bindgen feature = name conflict. Not sure if it applies to rand crate as well, because wasm-bindgen feature is not in the Cargo.toml, it's in the code only. Just thinking aloud, hypothetical example ...

Did the PR, but not sure if it's the right thing to do. It works for me and I have a [temporary fix](https://github.com/balena-io-modules/balena-temen/pull/39/files) for my issue. No need to hurry. What bothers me is this `wasm-bindgen` feature. There's a crate with the same name and you can't have a feature and a dependency with the same name. If, and if, you'll want to create `wasm-bindgen` based NPM package from the `uuid` crate, it will fail because of the `wasm-bindgen` feature = name conflict. Not sure if it applies to `rand` crate as well, because `wasm-bindgen` feature is not in the `Cargo.toml`, it's in the code only. Just thinking aloud, hypothetical example ...

alexcrichton commented

2018-11-27 11:36:27 -06:00

(Migrated from github.com)

For now we don't have a great way to solve this, but the recommended approach would be to have an feature name like wasm-bindgen and such which is enabled by consumers (and transitively enables the feature in required dependencies). Which is to say I think you're already on the right track!

For now we don't have a great way to solve this, but the recommended approach would be to have an feature name like `wasm-bindgen` and such which is enabled by consumers (and transitively enables the feature in required dependencies). Which is to say I think you're already on the right track!

KodrAus commented

2018-11-30 04:33:00 -06:00

(Migrated from github.com)

@alexcrichton Hm, this isn't going to play particularly nicely with dependencies that are optional to begin with though, right? So with a wasm-bindgen feature we can't not depend on rand, because we have to enable it along with its wasm-bindgen feature.

But if this is the way we're recommending folks do this then we should follow it here, so at least uuid is in the same boat as everyone else if we try come up with a more scalable solution later.

@alexcrichton Hm, this isn't going to play particularly nicely with dependencies that are optional to begin with though, right? So with a `wasm-bindgen` feature we can't _not_ depend on `rand`, because we have to enable it along with its `wasm-bindgen` feature. But if this is the way we're recommending folks do this then we should follow it here, so at least `uuid` is in the same boat as everyone else if we try come up with a more scalable solution later.

zrzka commented

2018-11-30 04:51:49 -06:00

(Migrated from github.com)

@KodrAus there's another, hypothetical, but still, issue with this approach. Imagine that uuid crate will follow this path, so, ...

wasm-bindgen & stdweb features will be added
they just transitively enable same features in dependencies (rand crate here)

... and here's the hypothetical issue. What if uuid crate devs will decide to publish this crate as an NPM package as well (via wasm-pack & wasm-bindgen). Then they need to add wasm-bindgen dependency and there will be a conflict. Because dependencies & features must not have same name. @alexcrichton I wonder if this was discussed even for the rand crate?

An example in opposite direction. In our crate, we're depending on wasm-bindgen crate. And here, we're unable to add wasm-bindgen / stdweb features, because of the name conflict (feature vs dependency). It's hypothetical example as well, because we don't need it, but I can imagine it can happen sooner or later.

@KodrAus there's another, hypothetical, but still, issue with this approach. Imagine that `uuid` crate will follow this path, so, ... * `wasm-bindgen` & `stdweb` features will be added * they just transitively enable same features in dependencies (`rand` crate here) ... and here's the hypothetical issue. What if `uuid` crate devs will decide to publish this crate as an NPM package as well (via `wasm-pack` & `wasm-bindgen`). Then they need to add `wasm-bindgen` dependency and there will be a conflict. Because dependencies & features must not have same name. @alexcrichton I wonder if this was discussed even for the `rand` crate? An example in opposite direction. In [our crate](https://github.com/balena-io-modules/balena-temen/), we're depending on `wasm-bindgen` crate. And here, we're unable to add `wasm-bindgen` / `stdweb` features, because of the name conflict (feature vs dependency). It's hypothetical example as well, because we don't need it, but I can imagine it can happen sooner or later.

Dylan-DPC-zz commented

2018-11-30 05:56:35 -06:00

(Migrated from github.com)

@zrzka That's a good spot. Though having feature names that are named differently from their purpose might mislead some users. We don't need wasm-bindgen, so adding a dependency just for wasm-support feels like overkill. The current approach seems fine to me and we have no plans for npm support right now, so we can go the original way or you can rename them to something that relates to them.

zrzka commented

2018-11-30 06:01:46 -06:00

(Migrated from github.com)

@Dylan-DPC I agree, that's the reason why I wrote that this is a hypothetical example, because there're lot of UUID NPM packages and there's no need for another one :) It's just something to be aware of.

Dylan-DPC-zz commented

2018-11-30 06:12:29 -06:00

(Migrated from github.com)

Yeah thanks.

Also on the PR, you should get a "Restore branch" option and I can reopen the PR after that.

Yeah thanks. Also on the PR, you should get a "Restore branch" option and I can reopen the PR after that.

alexcrichton commented

2018-11-30 10:10:04 -06:00

(Migrated from github.com)

@KodrAus yeah it's not a great situation unfortunately :(. It's currently a local optimum we've figured out, but if there's better ideas I'm all ears!

zrzka commented

2018-12-01 11:10:48 -06:00

(Migrated from github.com)

Fixed in the #351 PR. Pending Dependency Update, see #352.

👍 1

KodrAus commented

2018-12-03 19:10:01 -06:00

(Migrated from github.com)

@alexcrichton Yeh, I guess something along the same lines could involve some fancier way to link up enabled cargo features on-the-fly:

[dependencies.rand]
optional = true

[dependencies.wasm-bindgen]
optional = true

[when.'cfg(all(feature = "wasm-bindgen", feature = "rand"))']
features = ["rand/wasm-bindgen"]

Actually, that might be nice for a bunch of other cases too, but is probably not quite like using [target.'cfg(...)'].

@alexcrichton Yeh, I guess something along the same lines could involve some fancier way to link up enabled cargo features on-the-fly: ```toml [dependencies.rand] optional = true [dependencies.wasm-bindgen] optional = true [when.'cfg(all(feature = "wasm-bindgen", feature = "rand"))'] features = ["rand/wasm-bindgen"] ``` Actually, that might be nice for a bunch of other cases too, but is probably not quite like using `[target.'cfg(...)']`.

alexcrichton commented

2018-12-04 08:39:54 -06:00

(Migrated from github.com)

@KodrAus ah yes indeed! I think that's this old RFC perhaps?

@KodrAus ah yes indeed! I think that's [this old RFC](https://github.com/rust-lang/rfcs/pull/1787) perhaps?

👍 1

Rows
Columns

Uuid::new_v4() compiles and then panics for wasm32-unknown-unknown target #350