`getrandom` feature should enable `rand_core/getrandom`

[ghost]

Hello, I'm currently using this in a no_std environment and after adding my own getrandom implementation I enabled the getrandom feature of your crate but alas I got a compile error in your rand_hack when rand is not enabled.

Compiling schnorrkel v0.9.2
error[E0425]: cannot find value `OsRng` in crate `rand_core`
   --> /home/becominginsane/.cargo/registry/src/github.com-1ecc6299db9ec823/schnorrkel-0.9.2/src/lib.rs:233:18
    |
233 |     ::rand_core::OsRng
    |                  ^^^^^ not found in `rand_core`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0425`.
error: could not compile `schnorrkel`

I believe schnorrkel should enable rand_core/getrandom when the feature is enabled, as this scheme currently requires the user to add the feature manually

[burdges]

Did this work in 0.9.1? We've maybe broken something but not obviously..

I agree our getrandom feature should activate rand_core's getrandom feature, but..

I've the error "feature names may not contain slashes: rand_core/getrandom" if I try

[dependencies.getrandom]
version = "0.2.2"
default-features = false
optional = true
features = ["rand_core/getrandom"]

I've the error "features and dependencies cannot have the same name: getrandom" if I try

[features]
...
getrandom = ["rand_core/getrandom"]

Yet, this works if I then remove the wasm-bindgen feature. I'd rather make wasm-bindgen if that's the only option.

It's asinine if cargo prevents a dependency chain from depending passing features to a common optional dependency, so let me know if anyone knows a better way.

Any chance this creates issues for our usage of wasm-bindgen anyplace @thiolliere ?

cc #64

[ghost]

The solution is to change getrandom dependency:

[features]
getrandom = ["getrandom_", "rand_core/getrandom"]

[dependencies.getrandom_]
version = "0.2.2"
default-features = false
optional = true

and either change all occurences of getrandom in the crate to getrandom_, or pub use getrandom_ as getrandom and refer to it as crate::getrandom... Either way you need to change all occurences of getrandom

[burdges]

Funny. :) We'd getrandom purely to forward features to dependencies, not direct usage. Anyone who wants wasm-bindgen can depend upon getrandom themselves and exploit cargo features being additive And strange dependency forwarding hacks make little sense if rand_core never bothers either: https://github.com/rust-random/rand/blob/master/rand_core/Cargo.toml

It's possible @jacogr knows more about this part.

[ghost]

well the thing is, if your crate has only the getrandom feature activated, and not rand, it relies on ::rand_core::OsRng in rand_hack function, and OsRng is only activated if rand_core's getrandom feature is enabled, thus making your crate depend on such feature when you enable getrandom... Since you don't have any direct usage the hack I gave you earlier would suffice

[burdges]

There is no reason to depend directly upon getrandom here, except to forward the wasm-bindgen feature, which sounds fairly niche. It's less niche for this crate's audience, but why obscure details of getrandom, which is one of the most used crate?

There is unstable cargo support for doing this in https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#namespaced-features via https://github.com/rust-lang/cargo/issues/9210 although I'm still not sure my previous wasm-bindgen feature made sense.

I'll publish 10.1 if the current master works for everyone.

[ghost]

Oh I see what you did now. This would solve the issue that I had in the first place, but I'm not sure how your other changes affect anything else... But I think removing a feature requires a MINOR bump, not PATCH...

[burdges]

We went to 0.10.0 yesterday and it'll get yanked, so this semver break does not bother me.

[burdges]

Published

[gui1117]

I see master, it looks good to me, and ppl should activate "wasm-bindgen/get_random" themself if they need it.