Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared build+target dependency crates conflate features #4361

Closed
thenewwazoo opened this issue Aug 4, 2017 · 16 comments
Closed

Shared build+target dependency crates conflate features #4361

thenewwazoo opened this issue Aug 4, 2017 · 16 comments
Assignees
Labels
A-features Area: features — conditional compilation

Comments

@thenewwazoo
Copy link

Sorry about the title, this one was hard for me to nail down.

I'm working on a project (embedded, no_std) that relies on both bindgen as a build-dependency and libc as a dependency. When I set default-features = false in Cargo.toml for libc, the use of bindgen (which depends upon libc) influences the features enabled in libc when building for the target. To wit:

[package]
...

[build-dependencies]
bindgen = "0.26.3"

[dependencies.libc]
version = "0.2"
default-features = false

When bindgen is specified as a build-dependency, the libc crate is built with the command line:

rustc --crate-name libc /Users/bmatt/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.29/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg feature="default" --cfg feature="use_std" -C metadata= ...`

this fails for want of std, namely error[E0463]: can't find crate for 'std'.

When bindgen is removed, the libc crate is build with the command line:

rustc --crate-name libc /Users/bmatt/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.29/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=...

this fails for reasons related to missing types (which appears to be a bug in libc?), e.g. error[E0412]: cannot find type c_char in this scope.

@thenewwazoo
Copy link
Author

Okay, it looks like the underlying reason that libc fails to compile in the latter case is a misunderstanding on my part about compiling libc at all on os-less targets, but I still think there's something weird. :)

@alexcrichton alexcrichton added the A-features Area: features — conditional compilation label Aug 9, 2017
@alexcrichton
Copy link
Member

IIRC there's another issue or two related to this in Cargo's issue tracker. It's related to how feature resolution works today which happens on a per-crate basis instead of just per-crate-within-a-target basis.

@dhardy
Copy link

dhardy commented Nov 17, 2017

This may be the same problem I'm hitting?

rand has:

[features]
default = ["std"]
std = ["rand_core/std", "libc"]

[dependencies]
libc = { version = "0.2", optional = true }
rand_core = { path = 'rand_core', default-features = false }

rand_core (sub-workspace in same repo) has:

[features]
default = ["std"]
std = []

The build command incorrectly uses default features for rand_core:

$ cargo build --all --no-default-features --verbose
   Compiling rand_core v0.0.1 (file:///home/dhardy/rust/rand/rand_core)
     Running `rustc --crate-name rand_core rand_core/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=5fd26c510ee70ad9 -C extra-filename=-5fd26c510ee70ad9 --out-dir /home/dhardy/rust/rand/target/debug/deps -L dependency=/home/dhardy/rust/rand/target/debug/deps`
   Compiling rand v0.3.17 (file:///home/dhardy/rust/rand)
     Running `rustc --crate-name rand src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=2229235c293652d2 -C extra-filename=-2229235c293652d2 --out-dir /home/dhardy/rust/rand/target/debug/deps -L dependency=/home/dhardy/rust/rand/target/debug/deps --extern rand_core=/home/dhardy/rust/rand/target/debug/deps/librand_core-5fd26c510ee70ad9.rlib`
error[E0277]: the trait bound `jitter_rng::TimerError: std::error::Error` is not satisfied

(error is because rand_core uses std feature while rand does not).

The only workaround I see is to remove default = ["std"] from rand_core. Any chance this can get fixed?

Repo and version: https://github.com/dhardy/rand/tree/53ab77e320184b0950dfbdd44439fd44bdb57700

@alexcrichton
Copy link
Member

@dhardy I think that's the same issue, yes, but I believe it's because --all and --no-default-features doesn't work quite as you think it does. I forget exactly what happens but I don't think that builds everything with default features disabled everywhere. I'd have to dig more to understand precisely what's going on.

@dhardy
Copy link

dhardy commented Nov 20, 2017

I don't really understand why --all can have anything to do with it though? If rand is built without default features and without "std" then it should depend on rand_core with default-features = false, right?

That --all will cause Cargo to also build rand_core should have nothing to do with it, as far as I understand. But it's strange that without --all the build succeeds:

$ cargo build --no-default-features --verbose
   Compiling rand_core v0.0.1 (file:///home/dhardy/rust/rand/rand_core)
     Running `rustc --crate-name rand_core rand_core/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=80ab4ec61aa4e283 -C extra-filename=-80ab4ec61aa4e283 --out-dir /home/dhardy/rust/rand/target/debug/deps -L dependency=/home/dhardy/rust/rand/target/debug/deps`
   Compiling rand v0.3.17 (file:///home/dhardy/rust/rand)
     Running `rustc --crate-name rand src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=2229235c293652d2 -C extra-filename=-2229235c293652d2 --out-dir /home/dhardy/rust/rand/target/debug/deps -L dependency=/home/dhardy/rust/rand/target/debug/deps --extern rand_core=/home/dhardy/rust/rand/target/debug/deps/librand_core-80ab4ec61aa4e283.rlib`
    Finished dev [unoptimized + debuginfo] target(s) in 0.84 secs

@cretz
Copy link

cretz commented Nov 29, 2017

Note, this is failing with error[E0412]: cannot find type `c_char` in this scope when compiling a lib via Cargo relying on libc for the wasm32-unknown-unknown target too.

@alexcrichton
Copy link
Member

@dhardy sorry for the delayed response but Cargo has the concept of a "current package", and flags like --no-default-features only apply to the current package. That means if the workspace contains any crates that depends on rand, for example, with default features then default features will still be enabled.

@cretz I think that may be unrelated?

@cretz
Copy link

cretz commented Nov 30, 2017

@alexcrichton, yes, likely... sorry

@dhardy
Copy link

dhardy commented Nov 30, 2017

@alexcrichton I wasn't building anything which depended on rand.

@alexcrichton
Copy link
Member

@dhardy try doing:

 cargo build --no-default-features --features rand_core/std

and you should see the same error. The --no-default-features applies only to the "current crate" which is rand itself. The std feature on rand_core is listed as a default feature, so it's still activated.

@dhardy
Copy link

dhardy commented Dec 1, 2017

@alexcrichton that's besides the point. As I understand it, cargo build --all --no-default-features should build each crate in the workspace correctly. This might entail the following:

  • building rand_core with default features (whether or not this is correct is debatable but besides the point here)
  • building rand without default features
  • building rand's dependencies correctly: that means rand_core without default features

Yes, this may mean rand_core gets compiled twice with different feature-sets all to do the same "build". If Cargo can't separate compiled code by feature flags that's a related issue, and one required to solve this.

I guess the problem would be more apparent if there was a way to depend on another crate explicitly without certain features; that way there would be a feature resolution error if Cargo tried to build rand_core only once (instead of the linker error seen here). Actually, such a thing would be quite useful; it would also give a better error message if a user depended on both rand and rand_core without default features for only one of them (in which case obviously Cargo can't build rand_core twice).

@alexcrichton
Copy link
Member

@dhardy heh sorry so this is when it quickly balloons into a larger design question! What I meant previously is just to explain what Cargo currently does, and then also what Cargo currently does is that it only ever compiles a crate once, never twice with different feature sets. Cargo by default unions all features for a crates so they're required to be additive, and this has been Cargo's behavior for quite some time as well!

In that sense I believe right now cargo build --all --no-default-features is working as-is right now. It doesn't work for the rand crate which is arguably a bug in Cargo's design, but there's no easy fix here as it'd be some sort of change in the design of Cargo itself, not a quick bugfix.

@dhardy
Copy link

dhardy commented Dec 1, 2017

Heh, yes it is a deeper Cargo design problem. Feature union (half) makes sense when crate X has multiple dependencies within the same build target (single lib or bin), but when the output targets are separate (libs or bins) it doesn't make sense IMO, but as you say Cargo can't do anything else for now.

@archshift
Copy link

Given Rust's 2018 mission of being friendlier to use for embedded systems, I feel that this should be a higher priority. It basically renders it impossible to use any build-time crates which depend on libc (bindgen!) while trying to use libc in your no_std library.

@Eh2406
Copy link
Contributor

Eh2406 commented Mar 26, 2018

This appears to be related to #2589

veeg pushed a commit to veeg/shiplift that referenced this issue Feb 22, 2019
This is in line with best practices recommended
by serde[1]. This will also resolve downstream
crates depending on shiplift who enable the
serde derive feature, due to Cargos unification
of features for each crate[2].

[1]: serde-rs/serde#1441
[2]: rust-lang/cargo#4361 (comment)
softprops pushed a commit to softprops/shiplift that referenced this issue Feb 22, 2019
This is in line with best practices recommended
by serde[1]. This will also resolve downstream
crates depending on shiplift who enable the
serde derive feature, due to Cargos unification
of features for each crate[2].

[1]: serde-rs/serde#1441
[2]: rust-lang/cargo#4361 (comment)
@ehuss ehuss self-assigned this Dec 6, 2019
@ehuss
Copy link
Contributor

ehuss commented Feb 24, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-features Area: features — conditional compilation
Projects
None yet
Development

No branches or pull requests

7 participants