Features of dependencies are enabled if they're enabled in build-dependencies; breaks no_std libs

[phil-opp]

If a crate is used both as a normal dependency and a build-dependency, features enabled for the build-dependency are also enabled for the normal dependency. This leads to build failures in the no_std world, because an use_std feature that is turned on for the build-dependency is also turned on for the normal dependency.

Minimal example

cargo new bar --lib
cd bar
cargo new foo --lib

In bar/foo/Cargo.toml:

[features]
default = ["use_std"]
use_std = []

In bar/foo/src/lib.rs:

#[cfg_attr(not(feature = "use_std"), no_std)]

pub fn return_42() -> u32 { 42 }

#[cfg(feature = "use_std")]
pub fn print_42() {
    println!("{}", return_42());
}

In bar/Cargo.toml:

[dependencies.foo]
path = "foo"
default-features = false

[build-dependencies.foo]
path = "foo"

In bar/src/lib.rs:

#![no_std]

extern crate foo;

pub fn return_42() -> u32 {
    foo::return_42()
}

Compiling with cargo build --verbose:

   Compiling foo v0.1.0 (file:///…/bar/foo)
     Running `rustc --crate-name foo foo/src/lib.rs --crate-type lib --emit=dep-info,link 
              -C debuginfo=2
              --cfg 'feature="default"' --cfg 'feature="use_std"'
              -C metadata=5cd2c90dafa7c435 -C extra-filename=-5cd2c90dafa7c435
              --out-dir /…/bar/target/debug/deps
              -C incremental=/…/bar/target/debug/incremental
              -L dependency=/…/bar/target/debug/deps`
   Compiling bar v0.1.0 (file:///…/bar)                 
     Running `rustc --crate-name bar src/lib.rs --crate-type lib --emit=dep-info,link
              -C debuginfo=2 -C metadata=0a475a34b1347c78
              -C extra-filename=-0a475a34b1347c78 --out-dir /…/bar/target/debug/deps
              -C incremental=/…/bar/target/debug/incremental
              -L dependency=/…/bar/target/debug/deps
              --extern foo=/…/bar/target/debug/deps/libfoo-5cd2c90dafa7c435.rlib`

We see that --cfg 'feature="default"' --cfg 'feature="use_std"' is passed, i.e. the use_std feature is activated even though default-features = false is used. When we comment out the [build-dependencies.foo] section, foo is built without the use_std feature.

Breaking no_std libs

The problem is that compilation for custom targets using xargo/cargo-xbuild fails when the std feature is activated for the custom target:

cargo xbuild --target some-custom-target.json --verbose
+ "rustc" "--print" "sysroot"
+ "rustc" "--print" "target-list"
+ RUSTFLAGS="--sysroot /…/bar/target/sysroot"
+ "cargo" "build" "--target" "some-custom-target" "--verbose"
   Compiling foo v0.1.0 (file:///…/bar/foo)
     Running `rustc --crate-name foo foo/src/lib.rs --crate-type lib --emit=dep-info,link
              -C debuginfo=2
              --cfg 'feature="default"' --cfg 'feature="use_std"'
              -C metadata=beb376bcf23896f4
              -C extra-filename=-beb376bcf23896f4
              --out-dir /…/bar/target/some-custom-target/debug/deps
              --target /…/bar/some-custom-target.json
              -C incremental=/…/bar/target/some-custom-target/debug/incremental
              -L dependency=/…/bar/target/some-custom-target/debug/deps
              -L dependency=/…/bar/target/debug/deps
              --sysroot /…/bar/target/sysroot`
error[E0463]: can't find crate for `std`                                 ] 0/2: foo
  |
  = note: the `some-custom-target-6722811146760112169` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: Could not compile `foo`.

We see that the use_std feature is turned on even though we are compiling for the custom target. So this is definitely not the build dependency getting built because build scripts are compiled for the host target.

Workarounds

There are two ways to work around this, but both have sigificant downsides:

• Use the host dependency in no_std mode and don't use any std-dependent features and build scripts. This makes writing build scripts harder.
• Use a forked and renamed dependency so that two different libraries are used. This comes with a high maintainance burden and is thus not practical in most cases.

Both workarounds don't really work for build dependencies of dependencies.

Versions

cargo 1.29.0-nightly (af9e40c26 2018-07-05)
release: 1.29.0
commit-hash: af9e40c26b4ea2ebd6f31ee86ee61d5ac1c74eb0
commit-date: 2018-07-05

rustc 1.29.0-nightly (254f8796b 2018-07-13)
binary: rustc
commit-hash: 254f8796b729810846e2b97620032ecaf103db33
commit-date: 2018-07-13
host: x86_64-unknown-linux-gnu
release: 1.29.0-nightly
LLVM version: 7.0

[phil-opp]

Oh wow, it seems like this issue was already reported multiple times:

#4866 #4664 #2589 #4361

The oldest report is from April 2016, over two years ago. Given that embedded is a target domain of Rust's 2018 roadmap and build scripts are a common stable feature of cargo, I think we should tackle this issue. Especially because it leads to very confusing errors and there are no good workarounds.

[alexcrichton]

Indeed the intention is to solve this issue!

@Eh2406 I'm curious, would you be interested in tackling this issue? I think it has implications on resolution but it's not exclusively concerned with crate graph resolution. There's a lot of interaction with the backend as well where Cargo actually invokes rustc

[Eh2406]

I am happy to help any way I can. However, this has been open for a long time because it is very hard, and tangled with many of the other things that need to be rethought.

The goal is that build script and the real build have the same dependency with different features.

• How does this interact with other profiles, why is this flexibility limited to build/real what about test and release? This is made more complicated as the idea of a "profile" is already stretched to the breaking point and @ehuss has been thinking of redesigning parts of it (I think).
• Is this eavan correct, for normal additive features? If my build script generates extra code if the feature serde is on, and my main uses that code with cfg(feature("serde")) then they head better aggry about if serde is on.
• How does this interact with private dependencies? When do we combine the features of two otherwize private dependencies? How are we going to implement private dependencies? (it triggers a lot of the same questions.)
• Cargo.lock currently uses name-ver as a unique id, how do we add support for some new id. How does this not brake older cargo. If it does brake older cargo then what else should we do at the same time? Queue the stalled rfc on versioning of Cargo.toml/Cargo.lock

...

[alexcrichton]

Oh in general I see this as having a pretty simple (sort of) model of what should happen. Right now in cargo's backend we have a graph of "units" where each unit is basically a tuple of (package, target, profile, info). Here we have a precise dependency graph of what to do and each "unit" corresponds to one process, be it rustc, rustdoc, a build script, etc. Occasionally a unit needs to ask "what features are active for me?" but this question doesn't have an answer right now. We can at best coarsely answer "this package has all these features enabled", but that's not correct when you have cross-compiling in effect.

The "solution" here is to basically propagate features not through the crate graph resolution, but rather through the unit dependencies. When one unit depends on another it depends on it with a set of features, and those features get unioned over time into a unit, not a package. We can probably make this distinction a bit more coarse and group it by (unit, kind) where kind is in the "info" above and basically says "compile for the target" or "compile for the host".

If that all makes sense then the profile bits I don't think interact with features at all. Each unit already has a profile selected as well, so if it did the information is in theory there! I also don't think that public/private dependencies would affect much here too, I think it's mostly just feature propagation? (as we do today, only a little less aggressively). And finally for Cargo.lock features don't come into play either, that's just a resolution of crate graph versions and we pessimistically assume that all features are activated to generate it, so there's no need to record features in Cargo.lock

All that leaves build scripts (I think?) and that's indeed true! In theory this doesn't matter as build scripts are compiled with environment variables indicating enabled features, but they're also today compiled with --cfg directives which can indeed cause problems I think. I haven't thought this one through too much :)

[Eh2406]

That sounds a lot more doable than I was expecting. Conveniently, Cargo.lock does not record features so that is off the list.

Let me try and explain what I was thinking with relation to public/private dependencies.
let's say we are compiling a witch depends on b and c each of which depends on d.
Currently no matter the nature of that "depends"; b and c will both see the same version of d and, the d that b and c see have the same features, and d will only bilt once per target.
After a minimal fix to this, if b is only a build dependency, then we wave the second invariant, and consequently the third as well. #5237 asked us to wave the first invariant for similar reasons.
After impl private dependencies then, if b only has a private dependency on d then we will wave some or all the invariants, design work tbd. So there is a similar problem shape. Although as I type, I am not seeing a deeper connection.

On another concern, do all bild unit dependencies on d see the same features, or do each get only the features it asked for?

[alexcrichton]

Hm yeah I'm not sure if there's a connection there either, but it's something I suspect that fleshing out the implementation would show pretty readily!

Currently whether or not you're a build or normal dependency all feature calculations are done the same way, you're always compiled with the same set of features in both contexts (which is the bug here)

[Eh2406]

Currently whether or not you're a build or normal dependency all feature calculations are done the same way, you're always compiled with the same set of features in both contexts (which is the bug here)

Agreed, so after this bug is fixed if I depend on a and d while a has a build dependency on d with feature foo then:

• my main.rs sees d as not having foo
• but a's build.rs sees d as having foo.
  (that is the definition of fixing this bug.)

My current question is, I depend on a, b and d while a has a build dependency on d with feature foo and b has a build dependency on d with feature bar then:

• my main.rs sees d as not having neither foo nor bar.
• but a's build.rs sees d as having foo but does it see bar?
• but d's build.rs sees d as having bar but does it see foo?

[alexcrichton]

@Eh2406 for your first scenario, indeed! That's what I imagine for this issue as well.

In your second scenario then a and b will see d with both the foo and bar features because we can only compile the library once and features are defined to get unioned. I think the main thrust of this issue is what then cross compiling (more than one target involved) we're already compiling two artifacts so we should no longer be unioning across targets.

[ehuss]

because we can only compile the library once

Just a random idea, I was thinking that maybe the Unit could include the features to lift this restriction. At least for build-deps, it would be easy to do utilizing the ProfileFor flag.

[Eh2406]

@alexcrichton That would make the change smaller. I am concerned how do we explain the rule for when do "features get unioned". For example:

we're already compiling two artifacts so we should no longer be unioning across targets

implies that this only works if cross compiling!? That my main.rs sees d as having foo and bar if it is being built locally, but not if cross compiling.

@ehuss Lol. Two people respond, two contradictory answers. :-) I am glad to hear that the other alternative is conceivable doable.

[alexcrichton]

@Eh2406 another excellent question! This is where you get to a sort of obscure part of Cargo today where let's say you've got a compiler for x86_64-unknown-linux-gnu installed. It turns out these two invocations are actually pretty different:

• cargo build
• cargo build --target x86_64-unknown-linux-gnu

In the latter invocation Cargo thinks it's cross compiling so it'll build more than the first invocation. This means that if you have a big dependency which is both a target and a build dependency the first command will build it once while the second command will build it twice.

The behavior here has been in place since the early days of Cargo but has definitely been brought up as suspect over time. The benefit is that cargo build can be faster, but the downside is that you'd naively think the two commands above are equivalent but they're not.

So to answer your question, Cargo already gives an "uncomfortable answer" to when you're not cross compiling you get different results than if you're cross compiling. In that sense I think it's ok to not worry about it with respect to this issue. If we ever fix this issue it'll naturally fall out that features will be separated as well.

@ehuss due to the unioning nature of features we have to iteratively walk the dependency graph to discover features enabled, so I think this probably won't be embedded in a Unit directly but rather be stored as a Unit => Vec<String> map or something like that which is built after we calculate all Units and their dependencies.

[Eh2406]

So currently cargo build and cargo build --target x86_64-unknown-linux-gnu are observable different, one compiles deps once and the other twice.
After this issue is fixed they will additionality have different output code; a build.rs will see all features with cargo build and only dev features with cargo build --target x86_64-unknown-linux-gnu.

edit: that is backwards. build.rs will always see all features. main.rs will only see real deps features in cargo build --target x86_64-unknown-linux-gnu but will see all features in cargo build

That is a significantly bigger change, but we can chalk that up to just another part of that long standing oddity.

[alexcrichton]

@Eh2406 I think you're right yeah! Unfortunately no matter how this is sliced it's a sort of big change :(

[Eh2406]

I'm convinced despite the braking of things this needs to happen. Where should I get started?

Relevant: #5656 if we make them more different we should document when to add it to CI.

[alexcrichton]

@Eh2406 I think the basic steps here are to delete the Resolve::features method and then "get everything compiling again". It basically makes no sense to query what features are activated for a package any more unless you also have the context of "for what build target?" in mind

[Eh2406]

Ok so I have a branch with a new build_target: Option<()> peram on features, and the 14 places where it is called are passing None. What Is next? What type goes in that arg, and what do we do with it? Next time I am awake I can start figuring it out. Presumably some Enom (dev | target) or None for all, unless we have one already, and then do a graph traversal for features needed by that type for that id.

[alexcrichton]

@Eh2406 hm from this point I'm not really sure how this would be best done! I haven't thought through this too much myself but I'm fine reviewing any implementation! I suspect the first thing that sticks on the wall will be good enough :)

[Eh2406]

@ehuss I know you have been working on different build args for different target like things. (sorry if I have the terminology wrong.) Do you have advice for me? Specificky is there existing systems/types I should be using/aware of for consistency with your work?

[ehuss]

IIUC, the intent is to only fix this for the cross-compile case? If that's correct, I don't think it would interact with the profile changes at all. As Alex mentioned, it would probably need to build a map so that given a Unit it can return the correct set of features to use (looking at kind to determine if it is a build-dep and pkg to figure out which features). Then in build_base_args() and rustdoc() it should use that map to determine the correct --cfg flags.

At least that's my best guess of how I understand it. I'm not terribly familiar with how features or Resolve work, so I don't really know how to compute the feature set.

[Eh2406]

@ehuss That was very helpful! I am not very familiar with how anything but Resolve work, so that gives me somewhere to start. I can definitely do the inner Resolve side, I may bug you again to see if I got the outer part correct.

[ehuss]

Hm, but thinking about it more, kind by itself isn't enough to distinguish between a build-dep and other deps. That's the part that I was thinking about before, it doesn't seem like Unit has enough information to differentiate them. It could say build_config.requested_target.is_some() && unit.kind==Host to say it is a cross-compile build-dep, but that would be wrong for proc-macro and plugins. Maybe that doesn't matter, or has another way to mitigate or differentiate.

I'm also a little uneasy about only addressing the cross-compile build-dep case. Although that probably covers that vast majority of use cases, it seems like it could be confusing. There have been quite a few feature requests for more granular feature and dependency control. But fixing this one case would be a big win, so it's probably a good first step!

[Eh2406]

So I think for the inner Resolve side I need to do a BFS over the dependency tree to build a sub tree for things needed each Kind. In a quick scan I did not fined where we determined what Kind a dep is; I'd prefer to reuse the existing code if possible.

Edit: looks like the code you pointed me to is already doing a BFS over the dependency tree, exactly to figure out how all the pieces fit together. So recalculating features should "just" be worked in there, and removed from Resolve as @alexcrichton originally suggested.

[Eh2406]

@ehuss you commented you had free cycles, want to give this a try?

[ehuss]

I'll take a look. One question I had, when a feature is enabled with slash syntax, should it enable it for both the regular dependency and the build dependency? For example, --features somedep/feat1 or inside a [features] block? Or should it only affect certain types of dependencies? Or does there need to be more control for selecting the type of dependency?

[Nemo157]

I just checked whether I had any crates in my cache that optionally enabled features for a build dependency, looks like out of the 145 versions of crates with build-dependencies (of 1298 versions of crates in my cache) none of them do.

This script needs ripgrep, yq (for tq) and jq installed:

→ rg -0 --no-ignore -l -g 'Cargo.toml' 'build-dependencies' | xargs -0 tq -r '
    select(has("features") and has("build-dependencies"))
  | ([.features[][] | select(contains("/")) | (. / "/")[0]] | unique) as $optionaled
  | (."build-dependencies" | keys) as $deps
  | select(any($deps | .[]; . as $dep | any($optionaled | .[]; . == $dep)))
  | .package.name
'

I think eventually we would need to have more control over selecting the type of dependency. Maybe not so much for build dependencies, but for test dependencies I can definitely see situations where testing an optional feature requires activating extra optional features of a dependency just for the tests.

It seems like there should be an RFC detailing the full intended design of features and optional dependencies, there's likely to be interaction between fixing this, #5364, #5565, #1596, #1796, #2589 (dup of this), and #3494. Having an overall roadmap of how these would all work together could help answer questions like that.

[ehuss]

Non-unification of build-dependency features has been implemented and is available as a nightly-only feature on the latest nightly 2020-02-23. See the tracking issue at #7915.

If people following this issue could try it out, and leave your feedback on the tracking issue (#7915), I would appreciate it. Particularly we'd like to know if it helps your project, does it cause any breakage, and does it significantly increase initial compile time.