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

Open
thenewwazoo opened this issue Aug 4, 2017 · 15 comments
Open

Shared build+target dependency crates conflate features #4361

thenewwazoo opened this issue Aug 4, 2017 · 15 comments
Assignees
Labels

Comments

@thenewwazoo
Copy link

@thenewwazoo thenewwazoo commented Aug 4, 2017

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

This comment has been minimized.

Copy link
Author

@thenewwazoo thenewwazoo commented Aug 4, 2017

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

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Aug 9, 2017

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

This comment has been minimized.

Copy link

@dhardy 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

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Nov 20, 2017

@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

This comment has been minimized.

Copy link

@dhardy 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

This comment has been minimized.

Copy link

@cretz 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

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Nov 30, 2017

@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

This comment has been minimized.

Copy link

@cretz cretz commented Nov 30, 2017

@alexcrichton, yes, likely... sorry

@dhardy

This comment has been minimized.

Copy link

@dhardy dhardy commented Nov 30, 2017

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

@alexcrichton

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Nov 30, 2017

@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

This comment has been minimized.

Copy link

@dhardy 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

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Dec 1, 2017

@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

This comment has been minimized.

Copy link

@dhardy 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

This comment has been minimized.

Copy link

@archshift archshift commented Feb 5, 2018

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

This comment has been minimized.

Copy link
Contributor

@Eh2406 Eh2406 commented Mar 26, 2018

This appears to be related to #2589

@dhardy dhardy mentioned this issue May 16, 2018
5 of 8 tasks complete
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 added 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.