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

--bin B resolves features differently than -p B in a workspace #8157

Open
fabianvdW opened this issue Apr 25, 2020 · 8 comments
Open

--bin B resolves features differently than -p B in a workspace #8157

fabianvdW opened this issue Apr 25, 2020 · 8 comments
Labels
A-features Area: features — conditional compilation A-workspaces Area: workspaces C-bug Category: bug S-needs-team-input Status: Needs input from team on whether/how to proceed.

Comments

@fabianvdW
Copy link

Problem

Suppose I have a workspace, containing crate A,B and C. A has the feature F, but it is not a default-feature. Both B and C depend on A, however B does not use F, while C does.

The file ./A/src/lib.rs looks like this:

pub fn A(){
    if cfg!(feature = "F"){
        println!("Using library A with Feature F")
    }else {
        println!("Using library A")
    }
}

, while B and C just call A::A() in their main function.
Behaviour:

cargo run --bin B
Using library A with Feature F

cargo build
"./target/debug/B.exe"
Using library A with Feature F

cargo run -p B
Using library A

This behaviour just seems inconsistent. I don't understand how B is ever built with F, as in my real use case this imposes a performance penalty for B, so I will have to fall back to using cargo run -p B. The behaviour I expect is:

cargo run --bin B
Using library A

cargo build
"./target/debug/B.exe"
Using library A

cargo run -p B
Using library A

Steps

  1. Make a workspace containing three crates A , B, C.

A/src/lib.rs as above
B/src/main.rs:

fn main() {
    A::A();
}

A/Cargo.toml:

[features]
default = []
F = []

B/Cargo.toml

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

C/Cargo.toml

[dependencies.A]
path = "../A"
default-features = false
features = ["F"]
  1. Run cargo commands as above

Possible Solution(s)

Notes

Output of cargo version:
cargo 1.43.0 (3532cf738 2020-03-17)
Using latest stable Rust, rustc --version:
rustc 1.43.0 (4fb7144ed 2020-04-20)

@fabianvdW fabianvdW added the C-bug Category: bug label Apr 25, 2020
@fabianvdW fabianvdW changed the title Features in environment Features in workspace Apr 25, 2020
@ehuss
Copy link
Contributor

ehuss commented Apr 25, 2020

I can see how that can be confusing. cargo run --bin B has an implicit --workspace flag which means it is building all workspace members, and then filtering out the binary B. Workspace features are unified for all selected packages. Feature resolution looks at which packages are being built, but doesn't know about the filtering operation (which is done after resolution).

It may be very difficult to work around that. Generating the target proposals requires having resolution already being performed.

It may be possible to special-case this scenario and constrain the initial specs (so it would infer if you did --bin B that you also implied -p B), but I can imagine a lot of edge cases that would make that tricky to get right.

@ehuss ehuss added A-features Area: features — conditional compilation A-workspaces Area: workspaces labels Apr 25, 2020
@fabianvdW
Copy link
Author

But is it actually wanted that even when all packages are built, A isn't build twice, once with F and without F? Atleast in my use-case this is what I would expect from cargo build.

@ehuss
Copy link
Contributor

ehuss commented Apr 25, 2020

It's questionable, some projects will want the unification (because it cuts down on compile time), and some do not (because it can alter behavior or prevent compilation). This is tracked in #4463 among others.

@fabianvdW
Copy link
Author

I see, in that this case fixing this will just be a minor enhancement, as I can just use the working command. It would be nice if one could just configure such things for each workspace.

@ehuss ehuss changed the title Features in workspace --bin B resolves features differently than -p B in a workspace Jul 6, 2020
@Veetaha
Copy link

Veetaha commented Jul 6, 2020

@ehuss

cargo run --bin B has an implicit --workspace flag which means it is building all workspace

I don't think so, apparently cargo run --bin compiles only what is needed to build the binary (this is very noticable in my project which is a huge cargo workspace) which is what --bin flags' purpose is, isn't it?

@ehuss
Copy link
Contributor

ehuss commented Jul 6, 2020

As explained above, it selects the entire workspace, and then filters it down to whichever package contains the named binary. The sequence is roughly:

  1. Select entire workspace (implied --workspace).
  2. Resolve features across entire workspace.
  3. Filter selected packages to find one with the named binary.
  4. Build that binary.

It may be possible to move step 3 above step 2 (so it resolves features only for the selected package), but I think it will be tricky.

@UkoeHB
Copy link

UkoeHB commented Nov 28, 2023

This issue is very confusing. I have a WASM binary in the same workspace as a cross-platform binary. The WASM binary was failing to compile because of dep features leaking in from the other binary when using cargo run --bin. I had to basically guess what the problem was, because cargo tree is completely useless for debugging this.

@epage
Copy link
Contributor

epage commented Sep 11, 2024

FYI I've posted rust-lang/rfcs#3692

iliana added a commit to oxidecomputer/omicron that referenced this issue Dec 10, 2024
#5799 modified the `cargo build` command line omicron-package runs.
Previously it built up a list of packages to be built using the `-p`
flag; that PR changed it to use `--bin`. The goal was to build only the
binaries that are necessary for shipping; this avoids building
sled-agent-sim during releng, for instance.

We did not realize it at the time, but this invited the specter of
rust-lang/cargo#8157 to wreak havoc; namely:

- Without `--package`, Cargo uses the `default-members` key of the
workspace Cargo.toml to determine which packages to build. `--bin` does
not cause the same thing to happen; saying `--bin` does _not_ imply
`--package [the package that the bin belongs to]`.
- `omicron-dev` belongs to `default-members` and has a normal dependency
on `nexus-test-utils`, which enables the `"testing"` feature of
`nexus-db-queries`.

#7208 is a known result
of this problem, but there might be more.

Fortunately the solution seems fairly easy, without reverting the
relevant changes from #5799: use _both_ `--package` and `--bin`. With
this change, the `"testing"` feature is no longer shown in the `cargo
build --unit-graph` and `nm target/release/nexus | demangle | grep
validate_volume_invar` no longer shows any matching testing-only
symbols.
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 C-bug Category: bug S-needs-team-input Status: Needs input from team on whether/how to proceed.
Projects
None yet
Development

No branches or pull requests

5 participants