Move wasm32 Custom RNG impls back to main crate

[josephlr]

Right now getrandom 0.2 has two custom RNG crates: stdweb-getrandom and wasm-bindgen-getrandom. This is because the Rust community has two main ways to call JavaScript from Rust code on wasm32-unknown-unknown (wasm-bindgen and cargo-web), so we support both. There's also the issue that wasm32-unknown-unknown targets aren't necessarily running in a JavaScript environment.

However, cargo web v0.6.24 added support for proper conditional compilation. This means we don't actually need two distinct crates, we can just "do the right thing" at compile time and only have a single crate.

Furthermore, it also no longer makes sense to have the JavaScript implementation on wasm32-unknown-unknown be it's own crate. So we move the implementations back into getrandom proper, under the opt-in "js" feature.

Issues to resolve:

• Now that we can detect most of the ambiguity at compile time, does it even make sense to have this as an external RNG?
  • We could just have a "js" feature that essentially says: "if I'm building on wasm32-unknown-unknown assume I also have JavaScript"
  • Alternatively, we could just unconditionally support wasm32-unknown-unknown and just assume that all users of that target are either using wasm-bindgen or cargo-web.
• Does the documentation I added look good?
  • It can be previewed with: cargo doc --no-deps --open --package getrandom-js --target=wasm32-unknown-unknown
• If we keep this as an external RNG, do we like the getrandom-js name?

@dhardy @newpavlov I can merge this if you don't have time to review, but let me know what you think and if you have any feedback.

@zer0x64 @Pauan @alexcrichton @koute I'd also like to have your thoughts on the above issues and which approach you would like best.

[newpavlov]

I am not closely familiar with Rust WASM ecosystem, so I can't comment on universality of such solution (e.g. should we care about users of older cargo web? or what about alternative tools?), so I hope others will comment on that.

Now that we can detect most of the ambiguity at compile time, does it even make sense to have this as an external RNG?

Personally I believe we should keep it as an external RNG. Supporting wasm32-unknown-unknown in getrandom is wrong in a certain sense, since the target triplet does not tell anything about environment. Plus don't forget about the potential integration of getrandom into std (I hope to revive the PR after v0.2 release).

If we keep this as an external RNG, do we like the getrandom-js name?

Sounds good to me.

[josephlr]

Personally I believe we should keep it as an external RNG. Supporting wasm32-unknown-unknown in getrandom is wrong in a certain sense, since the target triplet does not tell anything about environment.

This was my thought as well, however it might be worth considering the alternative of having an (opt-in) "js" cargo feature. Both the feature and the external crate do the same thing, they say "my wasm32 target has JS". The feature would make things sightly easier for users. It's also similar to the "cpu" feature. Is there a reason to support one of these approaches over another?

I agree that unconditionally assuming JS on wasm32 unknown is probably not the best way forward.

EDIT: Having an opt-in "js" feature essentially makes wasm32-unknown-unknown behave like x86_64-unknown-uefi with the "cpu" feature. In both these cases, we're not certain which RNG to use, but there's a solution that will work for the vast majority of uses (RDRAND for UEFI, JS for wasm32).

[newpavlov]

I don't think there is a big difference between adding getrandom = { version="0.2", features = ["js"] } and getrandom-js = "0.2" into a project's Cargo.toml. The second option is shorter, albeit probably less familiar. Also I think it's easier to enable js feature by mistake, than add non-dev getrandom-js dependency.

As for the cpu feature, we can infer the necessary capabilities from target triplets, so situation is different from strict PoV (also I am not 100% sure if CPU randomness should be implemented as a feature-gated functionality and not as a custom crate).

[josephlr]

As for the cpu feature, we can infer the necessary capabilities from target triplets, so situation is different from strict PoV

I think this is a good point. Even if a CPU's RNG isn't available, we can always generate code to check for this availability. However, for wasm32, even our code to check that the APIs are there requires knowing the type of environment we are in.

[josephlr]

I don't think there is a big difference between adding getrandom = { version="0.2", features = ["js"] } and getrandom-js = "0.2" into a project's Cargo.toml. The second option is shorter, albeit probably less familiar.

The only (minor) downside to the second approach is that the user also has to write use getrandom_js as _; in their crate. A minor benefit to the second approach is that it prevents poluting the Cargo.lock unless you explicitly opt-in to getrandom-js. See briansmith/ring#904

Plus don't forget about the potential integration of getrandom into std

EDIT: I think that whatever we do here will not affect what we do for libstd, as we will only use getrandom on non-wasm32-unknown-unknown platforms.

[newpavlov]

However, for wasm32, even our code to check that the APIs are there requires knowing the type of environment we are in.

I am not sure if I fully understood you here, but I think you forgot about the wasm32-wasi target. :) Ideally we would have something like wasm32-web, but it does not look like it will happen in the near future.

The only (minor) downside to the second approach is that the user also has to write use getrandom_js as _; in their crate.

Oh, you are right. I didn't know that cargo may not link crate depending on source code even though it compiles it. I wonder if we can work around it somehow, e.g. via build scripts in the custom crate...

I think that whatever we do here will not affect what we do for libstd, as we will only use getrandom on non-wasm32-unknown-unknown platforms.

It was mostly about the "unconditional support" variant. One minor drawback of moving from the custom crate is that it would require whitelisting (unused) WASM crates, which is not ideal (i.e. as you said it would pollute std's lock file).

[alexcrichton]

This seems reasonable, but for how to interpret wasm32-unknown-unknown, I don't think there's really a great answer at this point. I think today it's almost exclusively used for browser use cases with wasm32-wasi dominating the out-of-browser use case, but it's unlikely that either of these remains true until the end of time. The ecosystem is still developing that I think there's basically not a great answer here.

You could by-default assume JS with wasm32-unknown-unknown, but it's almost guaranteed to be an incorrect assumption eventually. You could also require an opt-in, but this will be unergonomic for practically all current users of wasm32-unknown-unknown (and into the future as well).

[Pauan]

We could just have a "js" feature that essentially says: "if I'm building on wasm32-unknown-unknown assume I also have JavaScript"

I think it's better to have two features, "stdweb" and "wasm-bindgen", because the types, tools, and APIs vary between them. Even if they might be compatible right now, that's not guaranteed in the future.

Alternatively, we could just unconditionally support wasm32-unknown-unknown and just assume that all users of that target are either using wasm-bindgen or cargo-web.

I think this is fundamentally wrong, and it also isn't very flexible: even with wasm-bindgen you might want different behavior on Node or the web. Or you might want to support other uses of Wasm, such as crypto or Amazon AWS. Targets just aren't flexible enough to handle the Wasm ecosystem, whereas Cargo features work well.

Instead, I think the correct approach is to use Cargo features (e.g. chrono has a "wasmbind" feature, and futures-timer has a "wasm-bindgen" feature). It is flexible, and Cargo features are transitive, which is generally what you want.

Having a separate crate doesn't work as good, because it doesn't work with transitive dependencies.

[newpavlov]

It is flexible, and Cargo features are transitive, which is generally what you want.

I don't think so. We can't define mutually exclusive features, so if one of your dependencies wrongly enables such feature (be it for convenience sake or as a mistake), it will infect all downstream crates. In other words, the more crates in your dependency tree the more likely situation that both wasm-bingen and stdweb features will get enabled, thus removing an ability for you to choose between them. Also it endorses proxying features, which I believe results in unnecessary churn in Cargo.toml.

Having a separate crate doesn't work as good, because it doesn't work with transitive dependencies.

Can you elaborate? The idea behind the custom crates, is that they get included only in application crates or as a dev dependency for benches and tests. It's essentially similar to how alloc works, but without language support, which leads to the minor use churn.

[Pauan]

@newpavlov We can't define mutually exclusive features, so if one of your dependencies wrongly enables such feature (be it for convenience sake or as a mistake), it will infect all downstream crates.

It is possible to use compile_error! to give a compile time error if conflicting features are enabled.

And in any case, that also happens with multiple crates: what if one crate uses getrandom-stdweb and another crate uses getrandom-wasm-bindgen?

The idea behind the custom crates, is that they get included only in application crates or as a dev dependency for benches and tests.

If there is a library foo which uses getrandom, and I want to use foo with my Wasm application, how would I do that without Cargo features?

Even if my application installs getrandom-wasm-bindgen, that won't affect foo, so that doesn't work.

[newpavlov]

what if one crate uses getrandom-stdweb and another crate uses getrandom-wasm-bindgen?

It will result in a linking error with the following description rust-lld: error: duplicate symbol: __getrandom_custom. Either way, using custom crate as a non-dev dependency in a library crate is a 100% bug (although our documentation could use improvement here), which I think is harder to make compared to an unintended features screw-up.

Even if my application installs getrandom-wasm-bindgen, that won't affect foo, so that doesn't work.

No, it will be affected. I think you misunderstand how custom crates work. With an enabled custom feature, getrandom uses an extern function on unsupported targets. Custom crates enable this feature and define the function via the register_custom_getrandom macro. So when application crate adds a custom crate into its dependency tree, it will influence all users of getrandom simultaneously (but note that only behavior on unsupported targets will be affected). So again, think about it as of a library-space emulation of the alloc functionality.

[Pauan]

@newpavlov No, it will be affected. I think you misunderstand how custom crates work.

Ah, sorry, I had assumed that it was still using the 0.1 behavior.

However, that approach won't work for all crates, and it's also not zero cost. Which is why other crates use Cargo features. So I think it's good to use the same system throughout the ecosystem, rather than creating ad-hoc systems.

It will result in a linking error with the following description rust-lld: error: duplicate symbol: __getrandom_custom.

That seems strictly worse than using compile_error!, since you can create excellent custom error messages with compile_error!.

which I think is harder to make compared to an unintended features screw-up.

I don't really understand that argument: enabling a feature is an explicit opt-in change which people only do when they need to enable that feature.

And it often requires changing the syntax from foo = "1.0.0" to foo = { version = "1.0.0", features = ["bar"] }. This change is annoying to make, so in my experience features are only enabled when they need to be enabled.

So how is slapping a dependency into your crate harder than enabling a feature? At worst they seem equally hard to me.

[josephlr]

@newpavlov @Pauan @alexcrichton thanks for the feedback here. I started playing around with things and I've noticed some usability issues with having getrandom-js as a separate crate.

Bad error messages

Due to how Custom RNGs are implemented (via extern functions) we cannot check for the existence of an RNG implementation until link time. This means that on unsupported targets we need to either use compile_error with a nice message or get a bad error: duplicate symbol: __getrandom_custom linker error. To solve this, we have a "custom" cargo feature that, when enabled, calls the extern function, otherwise we emit a compile_error. The idea here is that only Custom RNG implementations enable this feature, so either you use the custom implementation, or you get a good compiler error.

This works great if you are only compiling for one target. However, Cargo features enabled on any target apply to all targets. This means that if you use a Custom RNG on any target, the "custom" cargo feature will be enabled on all targets. This means if you just use getrandom-js on wasm32-unknown-unknown you will get the bad linker error on all other unsupported targets. If we used an opt-in "js" feature, we would not have these issues.

Transitive Stability Issues

Imagine we have a crate foo that was using getrandom v0.1 and was forwarding the wasm-bindgen feature. Let's say foo wants to upgrade to getrandom v0.2 without themselves having a breaking change. The wasm-bindgen feature is gone in v0.2, but if getrandom-js is it's own crate, foo can have getrandom-js as an optional dependency, and have the wasm-bigndgen feature depend on getrandom-js. For example, the Cargo.toml would have:

[dependencies]
getrandom = "0.2"
getrandom-js = { version = "0.2", optional = true}

[features]
wasm-bindgen = ["getrandom-js"]

and src/lib.rs would have:

#[cfg(feature = "wasm-bindgen")]
use getrandom_js as _;

But now say a crate bar wants to do the exact same thing. If they do, then we will get a duplicate definition linker error. This then forces crates to have to choose between (a) making a breaking change to update getrandom or (b) being incompatible with other crates that use getrandom. This is bad, and no such issues occur if we use a "js" Cargo feature.

A similar compatibility issue occurs if a crate just wants to only support wasm32-unknown-unknown environments that have JS. For instance, this is what ring does. While getrandom itself shouldn't assume that this target implies the presence of JS, other crates might choose to. This is reasonable, given that virtually all users of this target are for some kind of JS environment (as @alexcrichton noted, non-JS users seem to use wasm32-wasi). Crates that depend on getrandom may support fewer targets/environments than getrandom and that's OK.

If getrandom-js is its own crate, then two crates cannot both assume that wasm32-unknown-unknown == has_js. If we use a "js" feature, there is no issue. Both crates just unconditionally enable the feature.

Proposal

Until something like rust-lang/cargo#7914 fixes the per-target Cargo feature issue, using Custom RNGs will be possible, but awkward in practice (in that they will only work if defined in the root binary crate). I don't think that we should block releasing v0.2 on that getting resolved.

We should put the wasm32-unknown-unknown functionality behind an opt-in "js" feature. This implementation should take priority over any Custom RNGs (at least for now) to prevent issues like #150.

The final docs for v0.2 (see #135) should then say that register_custom_getrandom!() should only be used directly from the root crate. This then avoids any issues around needing unintuitive use statements in order to get the compiler to link correctly. A user's root src/main.rs just contains:

fn my_rng(&mut buf) -> Result<(), getrandom::Error> {
    // ...
}
register_custom_getrandom!(my_rng);

This makes register_custom_getrandom! look much closer to things like #[global_allocator] and #[panic_handler]. The user then depends on getrandom in their root crate like:

[dependencies]
getrandom = { version = "0.2", features = ["custom"]}

Let me know your thoughts on the proposal, I will update this PR to reflect what I am proposing here.

[josephlr]

Some other comments on stuff in this thread:

However, that approach won't work for all crates, and it's also not zero cost.

@Pauan, while I agree that the JS stuff shouldn't be done via a custom crate, I'd like to know what sorts of use-cases are incompatible with the above register_custom_getrandom!() approach. I agree it's not "zero-cost" (as you have to call an extern "C" function in a way that prevents some, but not all, types of inlining); however, I think the perf impact here is negligible.

I think it's better to have two features, "stdweb" and "wasm-bindgen", because the types, tools, and APIs vary between them. Even if they might be compatible right now, that's not guaranteed in the future.

I don't think having two features here is necessary. The "types, tools, and APIs" are also very different between all the other targets getrandom supports, but it's our job to present a uniform interface to the user. Our detection mechanism is taken directly from how stdweb whether to depend on wasm-bindgen, so I'm not super concerned about future incompatibility. We may need to update getrandom to work with future versions of tooling, but that seems fine.

[newpavlov]

@Pauan

that approach won't work for all crates

Can you elaborate? As for zero-cost, firstly the cost is really negligible, especially when compared to the cost of calling into JS world. And secondly, if I am not mistaken LTO should be able to optimize such code.

That seems strictly worse than using compile_error!

Yes, it's less convenient and clear, but it could be worked around. (see below)

enabling a feature is an explicit opt-in change which people only do when they need to enable that feature.

I think it's easier to wrongly enable feature for convenience sake (e.g. if you test you library crate on WASM targets) and since it will not affect most users it can easily go undetected. Another problem is that feature can be enabled anywhere in your dependency tree and it could be quite hard to find the source of the problem, while finding the crate which has added a custom crate is much easier. Also there is a problem of feature unification between dev and non-dev builds, granted it only affects crates in the same workspace (if I remember correctly), but still it could be quite annoying.

Another problem is potential breaking changes in getrandom. You could have both 0.2 and 0.3 crates in your dependency tree and you would have to enable the feature for both of them, while with the custom crate approach it will work with the both versions simultaneously (assuming the extern function signature is not changed).

@josephlr
Don't forget that users will really rarely depend on getrandom directly, usually it will be buried deep in a dependency tree. The example with Cargo.toml provided by you is a clear misuse of the crate and it should be indicated in docs as such. I agree that the use and the linking errors issues are certainly drawbacks of the approach, but I think they can be worked around.

First, about the use issue. I like your registering proposal, but I think it would work better with a custom crate. Code in an application crate could call getrandom_js::register!();, which would define the extern function and it could emit a compilation error on non-wasm32-unknown-unknown targets, thus promoting correct cfg-ing of the registration calls. It's a bit closer to how global allocators get registered, so it may be a bit more familiar to users.

Second, about the error issue. We could add special features to getrandom (e.g. custom_js or custom_wasmbindgen/custom_stdweb, as well as custom_cpu), specific to some common custom crates, which would be used only for checking configuration correctness. If more than one of the custom features will get enabled, we would emit a compile_error!. This way the compilation error will happen before the linking error.

[josephlr]

Another problem is potential breaking changes in getrandom. You could have both 0.2 and 0.3 crates in your dependency tree and you would have to enable the feature for both of them, while with the custom crate approach it will work with the both versions simultaneously (assuming the extern function signature is not changed).

I fairly sure that regardless of what approach we take here, we can fix these issues with semver tricks (assuming not too much has changed).

Don't forget that users will really rarely depend on getrandom directly, usually it will be buried deep in a dependency tree. The example with Cargo.toml provided by you is a clear misuse of the crate and it should be indicated in docs as such.

I'm aware that crates usually won't directly depend on getrandom, but there's still that fact that we will have users (either direct or indirect) who either:

• Want to go from getrandom v0.1 to v0.2 without having to release a smver breaking change themselves (example: schnorrkel, or potentially rand)
• Want to just assume that wasm32-unknown-unknown has JS, and not expose any additional complexity/features to their users (example: ring)

Both of these approaches are already done by many crates in the ecosystem, and we should support them. I can't figure out how to support these two use cases with getrandom-js, but I know we can support them with a "js" feature. While this is a less "pure" solution, I think it's reasonable to give users a massive shortcut here, as they will almost always have JS on the wasm32-unknown-unknown target.

First, about the use issue. I like your registering proposal, but I think it would work better with a custom crate. Code in an application crate could call getrandom_js::register!();, which would define the extern function and it could emit a compilation error on non-wasm32-unknown-unknown targets, thus promoting correct cfg-ing of the registration calls. It's a bit closer to how global allocators get registered, so it may be a bit more familiar to users.

I agree that we should try to mimic global allocators here. For example, jemallocator defines a struct implementing GlobalAlloc, and then the user of the crate registers this struct with #[global_allocator] in their root crate.

We could take a similar approach. External crates would define a function (with signature fn(&mut [u8]) -> Result<(), getrandom::Error>) and then users would use that function (w/ register_custom_getrandom!()) in their root crate. Custom RNG crates then don't need to enable any weird features, the user of those crates enables the "custom" feature, exposing register_custom_getrandom!().

[newpavlov]

I really dislike the fact that adding js feature results in 53 dependencies being added to Cargo.lock (47 if we exclude wasm-bindgen-test), which is not a small number. I understand the desire to assume JS environment for convenience sake, but I don't think it's a good long-term solution. I guess as a compromise we could add the js feature, which would use the custom crate under the hood and would emit a deprecation notice with a link to docs explaining how to use custom crates. It will not save us from the pollution problem, but would guide users to migration without breaking their builds. Removal of this feature can be postponed until v0.3/v1.0 release.

then users would use that function (w/ register_custom_getrandom!()) in their root crate

If the registration macro will originate in getrandom, then users would have to add both getrandom and the custom crate to their Cargo.toml (so alloc is different in this regard), which is suboptimal. Since the registration macro is quite simple, I don't see why it has to be included into getrandom.

[josephlr]

I really dislike the fact that adding js feature results in 53 dependencies being added to Cargo.lock (47 if we exclude wasm-bindgen-test), which is not a small number.

Ya, I was initially concerned about this, but the fact that Cargo.lock includes dev dependencies as well pretty much guarantees that it's always going to be super big. Those deps aren't ever built (and don't show up w/ cargo metadata), so it's not a huge concern.

I understand the desire to assume JS environment for convenience sake, but I don't think it's a good long-term solution. I guess as a compromise we could add the js feature, which would use the custom crate under the hood and would emit a deprecation notice with a link to docs explaining how to use custom crates.

I don't think we will be able to use the custom crate under the hood until the target-specific feature issue gets resolved (but it looks like there's a path to stabilization there). So I think we should have the impls in the main crate until that issue is fixed, then we can move to an external RNG crate.

Since the registration macro is quite simple, I don't see why it has to be included into getrandom.

That's a good point, I think users should be able to use a custom RNG crate without depending on getrandom directly. When we finalize the docs, I'll update the examples accordingly.

[josephlr]

I really dislike the fact that adding js feature results in 53 dependencies being added to Cargo.lock (47 if we exclude wasm-bindgen-test), which is not a small number.

Ya, I was initially concerned about this, but the fact that Cargo.lock includes dev dependencies as well pretty much guarantees that it's always going to be super big. Those deps aren't ever built (and don't show up w/ cargo metadata), so it's not a huge concern.

Another point is that all of these deps only show up in getrandom's Cargo.lock (which isn't even committed). They do not show up in the Cargo.lock of any crate that depends on getrandom, provided that they don't enable the "js" feature. For example, if I add a getrandom = 0.2 dep for my other project, I get the following Cargo.lock entry:

[[package]]
name = "getrandom"
version = "0.2.0"
dependencies = [
 "cfg-if",
 "libc",
 "wasi",
]

which is the same as getradnom v0.1, so we don't have to worry about polluting libstd's lockfile (for example).

[newpavlov]

They do not show up in the Cargo.lock of any crate that depends on getrandom

Good to know! I was afraid that it would be necessary to bring all those WASM dependencies in the vendoring scenario.

I don't think we will be able to use the custom crate under the hood until the target-specific feature issue gets resolved

Hm, I am not sure how absence of target-specific features blocks moving WASM implementation into a separate crate. But I realized that my proposal unfortunately has a problem: getrandom-js would not be able to enable the custom feature and be an optional dependency of getrandom at the same time, since it would result in a cyclic dependency.

I still don't quite like bringing WASM code back into getrandom and using the js feature, especially if it will take precedents over the custom feature (I would not be surprised if js will get unconditionally enabled for any sufficiently large dependency tree, thus making it impractical to define custom sources for non-JS non-WASI WASM applications), but I guess I can live with it.

[josephlr]

I still don't quite like bringing WASM code back into getrandom and using the js feature, especially if it will take precedents over the custom feature (I would not be surprised if js will get unconditionally enabled for any sufficiently large dependency tree, thus making it impractical to define custom sources for non-JS non-WASI WASM applications), but I guess I can live with it.

I agree, this is my biggest concern moving forward. Helping users to update and watching large projects and seeing if this gets enabled by default will be interesting. Using a feature seems like a good short-term solution, and it might be a good long-term solution (or it might not). It mostly depends on if non-JS wasm32 platforms use WASI/Custom targets or if they'll want to use wasm32-unknown-unknown. Only time will tell. Hopefully, demand for more features in libstd leads to wasm32-bindgen-web and wasm32-bindgen-node targets, but that's not in the current plans IIRC.

@newpavlov I've rebased this onto the latest 0.2, so this is ready for review. Similar, to #150, I'm leaving the doc updates for #135. Thanks for taking time to review this approach, it's helped a ton!

[Pauan]

@newpavlov Can you elaborate?

It won't work for traits, it won't work if different types are needed for different targets, it won't work for methods, and it won't work for non-C types.

The only reason it works for getrandom is because the type signature is so simple (fn(*mut u8, usize) -> u32), but that won't work for most crates.

I think it's easier to wrongly enable feature for convenience sake (e.g. if you test you library crate on WASM targets)

That's a good point, but the same can happen with custom crates as well (forgetting to put it into dev-dependencies).

Another problem is that feature can be enabled anywhere in your dependency tree and it could be quite hard to find the source of the problem, while finding the crate which has added a custom crate is much easier.

How is it easier? You still have to search through all of the crates and look at their Cargo.toml either way.

Also there is a problem of feature unification between dev and non-dev builds, granted it only affects crates in the same workspace (if I remember correctly), but still it could be quite annoying.

I've never heard of that, could you explain more?

Another problem is potential breaking changes in getrandom. You could have both 0.2 and 0.3 crates in your dependency tree and you would have to enable the feature for both of them, while with the custom crate approach it will work with the both versions simultaneously (assuming the extern function signature is not changed).

Why do you think that's a good thing? That sounds terrible to me! Having a crate being able to affect another crate like that is crazy spooky action at a distance.

It's even worse that it affects the same crate but with different versions. That will lead to bizarre bugs which are incredibly hard to debug. The whole point of major version bumps is to create a clean break from the previous version.

If a major version bump was done, the extern fn should be renamed so that it doesn't collide with previous versions. This adds a maintenance hazard which doesn't exist for Cargo features.

Second, about the error issue. We could add special features to getrandom (e.g. custom_js or custom_wasmbindgen/custom_stdweb, as well as custom_cpu), specific to some common custom crates, which would be used only for checking configuration correctness.

This sounds like it's adding even more ad-hoc complexity to workaround problems which don't exist with Cargo features.

Why not just use Cargo features? They are literally designed for this use case, they are simple, they are easy, they don't have spooky action at a distance, they can be used for everything (not just C functions), they don't cause issues with versioning, and they have better error messages.

The only benefit you have mentioned for separate crates is that they are potentially harder to accidentally enable (which I don't agree with, and you haven't provided much evidence for).