Feature-dependent dependencies are documented to not work

[jsgf]

Bug Report

Version

Current git master

Crates

• tracing
• tracing-core

Description

According to the Cargo documentation:

Unlike in your Rust source code, you cannot use [target.'cfg(feature = "my_crate")'.dependencies] to add dependencies based on optional crate features. Use the [features] section instead.

However, the tracing crates are apparently doing this successfully:

[target.'cfg(not(feature = "std"))'.dependencies]
spin = "0.5"
lazy_static = { version = "1", features = ["spin_no_std"] }

It seems like something which could break in future versions of Cargo.

Corresponding Cargo issue: rust-lang/cargo#7442

[jsgf]

According to rust-lang/cargo#7442 (comment), cfg(not(feature = "std")) always evaluates to true, so this is a no-op.

[hawkw]

Thanks for bringing this up!

The intention was to follow the convention commonly used by other crates which can conditionally support no_std, where rather than having a feature flag that enables no_std support, instead, there is a feature flag for the std dependency that can be disabled. However, in order to support no_std, we have to enable other dependencies/dependency feature flags which are not required when the standard library is present. I was hoping that this use of target.'cfg(...)'.dependencies would allow us to do that, but I guess that's not the case.

I think the only viable solution here is to change from having a std feature to having a no_std feature. Then, it would be easy to only add the spin dependency (and lazy_static/spin_no_std feature) when the no_std feature is enabled. Unfortunately, this is technically a breaking change, if the feature flags are considered part of the crate's API surface. I don't know if there are currently any downstream users who depend on default-features = false enabling no_std support, though.

[jsgf]

Well, right now everything is getting the no_std behaviour whether they want it or not, but I'm not sure what effect that has (other than any dep graph containing this crate will be using lazy_static with spin).

[hawkw]

Upon further consideration, I think the best approach is probably to do the following:

1. Vendor any code from dependencies that are only needed for no_std support in-tree.
2. Mark the vendored code as #[cfg(not(feature = "std"))]
3. Carefully watch the upstreams the vendored code comes from for any major bugfixes
4. Hope Cargo eventually adds better mechanisms for conditionally supporting no_std than feature flags.

This has some potential downsides for no_std users, primarily: potential binary bloat, and the inability to pick up fixes in tracing's dependencies until tracing updates the vendored copies of those dependencies' code. However, these only affect potential future no_std users, and wouldn't be a problem for a majority of the userbase; and we wouldn't have to issue a breaking change release for a change that isn't breaking for any current users.

[hawkw]

PR #424 fixes this. Once we publish new tracing and tracing-core releases, the incorrect dependencies should be gone! :) Thanks @jsgf for the report and @thombles for taking the time to fix this!