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

cargo test --all ignores different feature sets for different members in a workspace #3620

Closed
alekseysidorov opened this issue Jan 31, 2017 · 10 comments
Assignees
Labels
A-features Area: features — conditional compilation A-workspaces Area: workspaces S-propose-close Status: A team member has nominated this for closing, pending further input from the team

Comments

@alekseysidorov
Copy link

Hello, here is a minimal example for this issue.

https://github.com/alekseysidorov/rust_bugs/tree/master/cargo_test_all

Core crate toml:

[package]
name = "core"
version = "0.1.0"
authors = ["Aleksey Sidorov <gorthauer87@yandex.ru>"]

[dependencies]


[features]
default = []
crazy = []

First dep toml

[package]
name = "dep_first"
version = "0.1.0"
authors = ["Aleksey Sidorov <gorthauer87@yandex.ru>"]

[dependencies]
core = { path="../core", features=["crazy"] }

Second dep toml with different features

[package]
name = "dep_second"
version = "0.1.0"
authors = ["Aleksey Sidorov <gorthauer87@yandex.ru>"]

[dependencies]
core = { path="../core"}

And try to run cargo test --all in workspace root.

cargo test --all --verbose 
   Compiling core v0.1.0 (file:///Users/aleksey/develop/rust_bugs/cargo_test_all/core)
     Running `rustc --crate-name core core/src/lib.rs --crate-type lib --emit=dep-info,link -g --cfg 'feature="default"' --cfg 'feature="crazy"' -C metadata=39d9f125bda27c50 -C extra-filename=-39d9f125bda27c50 --out-dir /Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps -L dependency=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps`
     Running `rustc --crate-name core core/src/lib.rs --emit=dep-info,link -g --test --cfg 'feature="default"' --cfg 'feature="crazy"' -C metadata=e54347fe71e91065 -C extra-filename=-e54347fe71e91065 --out-dir /Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps -L dependency=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps`
   Compiling dep_second v0.1.0 (file:///Users/aleksey/develop/rust_bugs/cargo_test_all/dep_second)
   Compiling dep_first v0.1.0 (file:///Users/aleksey/develop/rust_bugs/cargo_test_all/dep_first)
     Running `rustc --crate-name dep_second dep_second/src/lib.rs --emit=dep-info,link -g --test -C metadata=da6a2289e18e3cde -C extra-filename=-da6a2289e18e3cde --out-dir /Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps -L dependency=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps --extern core=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps/libcore-39d9f125bda27c50.rlib`
     Running `rustc --crate-name dep_first dep_first/src/lib.rs --emit=dep-info,link -g --test -C metadata=a681b278103164c3 -C extra-filename=-a681b278103164c3 --out-dir /Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps -L dependency=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps --extern core=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps/libcore-39d9f125bda27c50.rlib`
     Running `rustc --crate-name dep_first dep_first/src/lib.rs --crate-type lib --emit=dep-info,link -g -C metadata=f059f03ca4bbbcf6 -C extra-filename=-f059f03ca4bbbcf6 --out-dir /Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps -L dependency=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps --extern core=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps/libcore-39d9f125bda27c50.rlib`
error[E0425]: unresolved function `core::i_am_normal`
 --> dep_second/src/lib.rs:9:9
  |
9 |         core::i_am_normal();
  |         ^^^^^^^^^^^^^^^^^ no resolution found

error: aborting due to previous error

Build failed, waiting for other jobs to finish...
error: Could not compile `dep_second`.

Caused by:
  process didn't exit successfully: `rustc --crate-name dep_second dep_second/src/lib.rs --emit=dep-info,link -g --test -C metadata=da6a2289e18e3cde -C extra-filename=-da6a2289e18e3cde --out-dir /Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps -L dependency=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps --extern core=/Users/aleksey/develop/rust_bugs/cargo_test_all/target/debug/deps/libcore-39d9f125bda27c50.rlib` (exit code: 101)

It seems that the core has not been rebuilt with the feature set for second dependency

@alekseysidorov alekseysidorov changed the title cargo test --all ignores different feature sets for dependencies. cargo test --all ignores different feature sets for dependencies Jan 31, 2017
@alexcrichton
Copy link
Member

Due to the way Cargo's architected right now, this is going to be extremely difficult to fix, if at all. In general though this is where Cargo requires crates to have features be additive for them to be composable.

@retep998
Copy link
Member

retep998 commented Mar 4, 2017

There's two problems here:

  1. Not enough documentation telling the user that features are additive and trying to use them in any other way will only result in pain and suffering. There are many crates out there that don't respect this, including -sys crates that use features to choose how a given native library is compiled.
  2. There isn't any system in cargo that allows for non-additive configuration of a dependency, thus causing people to incorrectly use the closest thing they can, which is features.

@Dushistov
Copy link

Dushistov commented Jul 26, 2017

@alexcrichton

Due to the way Cargo's architected right now, this is going to be extremely difficult to fix, if at all. In general though this is where Cargo requires crates to have features be additive for them to be composable.

May be then introduce virtual crates for that?

For example if crateX has Cargo.toml like this:

[breaking-features]
change_everything = []

And if somebody enable change_everything feature for crateX,
then cargo add to dependency tree not "crateX", but "crateX\0change_everything",
and the rest of cargo code works as before?

@nipunn1313
Copy link
Contributor

We ran into this as well, but totally with open source deps
Here is a concrete minimal example

One crate in workspace

[package]
name = "eitherwrap"
version = "0.1.0"
authors = ["Nipunn Koorapati <nipunn@dropbox.com>"]

[dependencies]
either = "1.1"
itertoolswrap = {path = "../itertoolswrap"}

Second crate in workspace

[package]
name = "itertoolswrap"
version = "0.1.0"
authors = ["Nipunn Koorapati <nipunn@dropbox.com>"]

[dependencies]
itertools = "0.6.0"

If you alternate between running cargo test in each of the crates separately, it forces a rebuild each time because of the difference in features (itertools requests no-default-features from either).

nipunn-mbp:featuresworkspace nipunn$ cd eitherwrap/ ; cargo build ; cd ..
   Compiling either v1.1.0
   Compiling itertools v0.6.2
   Compiling itertoolswrap v0.1.0 (file:///Users/nipunn/src/rust_play/featuresworkspace/itertoolswrap)
   Compiling eitherwrap v0.1.0 (file:///Users/nipunn/src/rust_play/featuresworkspace/eitherwrap)
    Finished dev [unoptimized + debuginfo] target(s) in 1.67 secs
nipunn-mbp:featuresworkspace nipunn$ cd itertoolswrap/ ; cargo build ; cd ..
   Compiling either v1.1.0
   Compiling itertools v0.6.2
   Compiling itertoolswrap v0.1.0 (file:///Users/nipunn/src/rust_play/featuresworkspace/itertoolswrap)
    Finished dev [unoptimized + debuginfo] target(s) in 1.49 secs
nipunn-mbp:featuresworkspace nipunn$ cd eitherwrap/ ; cargo build ; cd ..
   Compiling itertools v0.6.2
   Compiling itertoolswrap v0.1.0 (file:///Users/nipunn/src/rust_play/featuresworkspace/itertoolswrap)
   Compiling eitherwrap v0.1.0 (file:///Users/nipunn/src/rust_play/featuresworkspace/eitherwrap)
    Finished dev [unoptimized + debuginfo] target(s) in 1.42 secs
nipunn-mbp:featuresworkspace nipunn$ cd itertoolswrap/ ; cargo build ; cd ..
   Compiling itertools v0.6.2
   Compiling itertoolswrap v0.1.0 (file:///Users/nipunn/src/rust_play/featuresworkspace/itertoolswrap)
    Finished dev [unoptimized + debuginfo] target(s) in 1.30 secs

We have a workspace with 70 crates in it.
The dependency crates we are using do have additive features, but it causes the code to recompile.

Our CI effectively has a loop which effectively runs

for crate in workspace:
    cd crate ; cargo test ; cd -

Since some random subset of our crates ask for either with features and some don't, running tests for all crates forces us to recompile deps repeatedly, which hurts. Especially since itertools is such a low level dep, it's tough. Our CI feels a lot of pain recompiling the same libraries many times.

Looking at our log, for a workspace with 70 crates, we have to recompile itertools 10 different times (which kicks off a chain of recompiles)

Any advice on how to improve the situation?

@nipunn1313
Copy link
Contributor

I think if we included the hashes of all the deps into the metadata for the compiled library, that would improve our problem. At least then the deps would get compiled a max of 2 times in our case, rather than back-n-forth

To illustrate, here are the commands for itertools in the two cases (added newlines for readability and redacted some absolute paths)

Running `rustc --crate-name itertools itertools-0.6.2/src/lib.rs
--crate-type lib --emit=dep-info,link -C debuginfo=2
-C metadata=4ed3e3cf3bc8df3d -C extra-filename=-4ed3e3cf3bc8df3d
--out-dir target/debug/deps
-L dependency=target/debug/deps
--extern either=target/debug/deps/libeither-f93178e8a5af0b1d.rlib
--cap-lints allow`

vs

     Running `rustc --crate-name itertools itertools-0.6.2/src/lib.rs
--crate-type lib --emit=dep-info,link -C debuginfo=2
-C metadata=4ed3e3cf3bc8df3d -C extra-filename=-4ed3e3cf3bc8df3d
--out-dir 
target/debug/deps
-L dependency=
target/debug/deps 
--extern either=target/debug/deps/libeither-4dcab0f19fb09534.rlib
--cap-lints allow`

I don't believe the metadata for itertools should be identical in these two cases because they are clearly linking against different versions of libeither (compiled with different features)

This won't help the cargo test --all issue, but it does seem closely related.

@nipunn1313
Copy link
Contributor

Here is the repro I was working with https://github.com/nipunn1313/cargoissue3620

bors added a commit that referenced this issue Sep 9, 2017
Hashed dependencies of metadata into the metadata of a lib

This fixes one part of #3620. To my understanding, the more fundamental fix is more challenging
bors added a commit that referenced this issue Oct 15, 2019
Reject feature flags in a virtual workspace.

This generates an error if feature flags are used in the root of a virtual workspace. Previously these flags were completely ignored.  In the interest of avoiding confusion, I think it would be good to be explicit that these don't currently work.  This could alternatively be a warning, but I think it is better to reject it outright.

cc #4753, #3620, #5015, #6195, etc.
@ehuss ehuss self-assigned this Jan 15, 2020
@ehuss ehuss changed the title cargo test --all ignores different feature sets for dependencies cargo test --all ignores different feature sets for different members in a workspace Feb 21, 2020
@ehuss ehuss added the A-workspaces Area: workspaces label Mar 26, 2020
@epage
Copy link
Contributor

epage commented Oct 6, 2023

If this is about mutually exclusive features, should we close this in favor of #2980

Marking as "propose to close" just so we don't lose track of this in case no one comes back with clarification on that.

@epage epage added the S-propose-close Status: A team member has nominated this for closing, pending further input from the team label Oct 6, 2023
@weihanglo
Copy link
Member

I think this is more like a general usability issue of feature unification, such as #4463.

I believe the documentation is way better than it was in 2017, which now it even talks about mutual-exclusive features.

#5210 is an issue looking into this problem from the other way round. Should we close all of them and point to #4463 or #2980 instead?

@epage
Copy link
Contributor

epage commented Oct 13, 2023

I think we should merge all of the feature unification issues. Its something I was already planning on doing as part of my sweep through the issues but you are welcome to it.

@weihanglo
Copy link
Member

close as per discussion above

@weihanglo weihanglo closed this as not planned Won't fix, can't repro, duplicate, stale Oct 13, 2023
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 A-workspaces Area: workspaces S-propose-close Status: A team member has nominated this for closing, pending further input from the team
Projects
None yet
Development

No branches or pull requests

8 participants