Stop forwarding getrandom's wasm features

[dhardy]

For rand 0.8, I suggest we stop forwarding these features in rand. 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.

This means that WASM users (who are using wasm32-unknown-unknown instead of the newer wasm32-wasi target) will need to depend on getrandom directly (but see also rust-random/getrandom#63).

Likely we would retain the getrandom feature: default → std → getrandom → rand_core/getrandom, though we could also drop that (default → std → rand_core/getrandom).

This is technically a breaking change, thus we can't do so before 0.8 (which likely won't be soon).

[newpavlov]

Meanwhile you can add a comment to Cargo.toml with a recommendation not to use this feature and instead enable it directly for getrandom.

Also maybe add a milestone for v0.8 so it will be easier to track such breaking changes?

[benmkw]

I used the rand wasm-bindgen feature and am not sure how to best achieve the same thing after you removed it. Maybe this is a limitation of my understanding of cargo. I see no option to enable the feature of getrandom which is a dependency of rand which is a depency of age which is a dependency my crate.

If my crate would use a different version of getrandom or one of my dependencies would use getrandom with incompatible feature flags to rand I'd have no way of activating the feature. Is this intended?

str4d/rage#97

As far as I see the situation cargo requires every app to forward all feture flags to the top level or there is an option to change features of dependencies of dependencies [sic] in Cargo.toml which I could not yet discover.

My example is basically this https://www.reddit.com/r/rust/comments/3s48re/avoiding_different_version_of_crates/cwu174a/ . Maybe in the case of getrandom this is unlikely so its not supported/ expected to break?

[burdges]

Ideally, cargo should add feature injection, ala features = "getrandom/wasm", since all features are exposed and additive anyways.

[newpavlov]

@benmkw
Library crates should not enable getrandom features, unless they target wasm-bindgen/stdweb exclusively. And application crates can simply add the following line to their Cargo.toml:

getrandom = { version = "0.1", features = ["wasm-bindgen"] }

In getrandom v0.2 those features will be replaced with "custom" crates, so applications will use the wasm-bindgen-getrandom crate instead. By adding this crate to your dependency tree you will overwrite the panicking implementation default for unsupported platforms. In other words with rand v0.8, your application crate can simply have the following line:

[target.wasm32-unknown-unknown.dependencies]
wasm-bindgen-getrandom = "0.1"

Farther in future getrandom may get integrated into std, making it similar to alloc.

[benmkw]

@newpavlov

Library crates should not enable getrandom features, unless they target wasm-bindgen/stdweb exclusively.

As far as I can see they would need to forward features of deps to make it possible to enable them reliably, because..

And application crates can simply add the following line to their Cargo.toml:

My point was that this approach does not seem to work (I mean that I have not figured out how) if I need a version of getrandom which is incomptible with the version the dep of a dep of me is using. (This is a hypothetical situation for me at the moment)

Splitting crates seems like a very rough approach to an issue which is basically what features are made to solve (I guess).

[newpavlov]

if I need a version of getrandom which is incomptible with the version the dep of a dep of me is using.

But there are no such versions right now and in the future version there will be no features to forward! During transition period when apps will have both getrandom v0.1 and getrandom v0.2 in their dependency tree, they would have to use both lines which I provided. It's certainly not ideal, but I think it should manageable.

rand v0.7 users may continue to use wasm-bindgen feature as usual, although I would recommend to use the approach in my previous message instead of feature forwarding.

Splitting crates seems like a very rough approach to an issue which is basically what features are made to solve (I guess).

The issue is not WASM specific, similar problems arise for other targets, such as embedded or UEFI. Custom crates is essentially our way to emulate alloc approach without language-level support. We couldn't find a better solution after lengthy pondering about this issue. With this approach users will be able to write their own custom crates without merging their code (which could be highly specialized) into getrandom, thus they will be able to use the rand ecosystem and crates which depend on it in their projects.

[benmkw]

Ok thanks for your consideration, I think at some point maybe cargo should support something like @burdges mentioned but I'm sure this approach has limitations/ other issues as well with regards to compatibility or else it would have already been considered.

I think in this case a similar issue was solved with confilicting features between "dev-deps" and "direct/ own deps" vs in my example "direct/ own deps" and "deps of deps". But in their case they already knew how to specify the features.
In my described case its not yet possible to specify the features without the "hack" of requiring a dependency which is only transitively required, just for the purpose of overwriting/ setting its features.
The reason why I describe this as a "hack" is because it does not allow setting the features of a dep of a dep if two incompatible versions of it are required in the dependency tree.
Its also not great because if the dep of a dep is dropped from the dep, I still require it without noticing that its now effectively unused.

My point was basically that rand should continue forwarding the feature flags of getrandom as it makes feature selection more robust but as you described, the "hack" with requiring getrandom on the top level and thus setting its feature flags is not likely to break in the future in a way I described so its fine.
I also learned that this setup can only happen if the dep of the dep is used in two semver incompatible versions which means their first part of the version number has to be different because otherwise cargo does not allow both versions to be used together so this is another reason why this will not happen with rand 0.8 specifically.

[burdges]

If you want this now, then you could do a build.rs script that checked the dependencies matched.

I suppose you'd need some dev dependencies of build.rs for reading the cargo.lock, which they become annoying dependencies for the library. lol

I think getrandom 0.2 uses #[no_mangle] which "should" break builds anytime you link two versions of getrandom. :)

[vks]

This was fixed with 0aa4617.