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 run' miscounts binaries due to features, can even panic #3867

Closed
BenWiederhake opened this issue Mar 24, 2017 · 5 comments · Fixed by #3879
Closed

'cargo run' miscounts binaries due to features, can even panic #3867

BenWiederhake opened this issue Mar 24, 2017 · 5 comments · Fixed by #3879

Comments

@BenWiederhake
Copy link
Contributor

Discovered while working indirectly on cargo_run.rs.

Example 1: One binary in the eyes of cargo_run (wrong), but cargo_compile produces no binaries (correct), making cargo_run panic:

// BEGIN Cargo.toml
[package]
name = "foo"
version = "0.1.0"
authors = []

[features]
default = []
phantom = []

[[bin]]
name = "foobar"
path = "src/main.rs"
required-features = ["phantom"]
// END Cargo.toml
// BEGIN src/lib.rs
// END src/lib.rs
// BEGIN src/main.rs
fn main() {
    println!("hello");
}
// END src/main.rs

Behavior:

$ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
thread 'main' panicked at 'assertion failed: `(left == right)` (left: `0`, right: `1`)', /checkout/src/cargo/ops/cargo_run.rs:56
note: Run with `RUST_BACKTRACE=1` for a backtrace.
$ echo $?
101
$

Example 2: Two binaries in the eyes of cargo_run (wrong), but cargo_compile produces only one binary (correct), making cargo_run bail unnecessarily:

// BEGIN Cargo.toml
[package]
name = "foo"
version = "0.1.0"
authors = []

[features]
default = [ "with-api-2" ]
# Names are chosen to suggest that there is some
# "default behavior" one could reasonably expect.
with-api-1 = []
with-api-2 = []

[[bin]]
name = "foobar1"
path = "src/main_v1.rs"
required-features = ["with-api-1"]

[[bin]]
name = "foobar2"
path = "src/main_v2.rs"
required-features = ["with-api-2"]
// END Cargo.toml
// BEGIN src/main_v1.rs
fn main() {
    println!("please pretend that this provides some API v1");
}
// END src/main_v1.rs
// BEGIN src/main_v2.rs
fn main() {
    println!("please pretend that this provides some API v2");
}
// END src/main_v2.rs

Behavior:

$ cargo run
error: `cargo run` requires that a project only have one executable; use the `--bin` option to specify which one to run
$

This might also be helpful for users having issue #2200.

@BenWiederhake BenWiederhake changed the title 'cargo run' miscounts binaries due to features, can even crash 'cargo run' miscounts binaries due to features, can even panic Mar 24, 2017
@BenWiederhake
Copy link
Contributor Author

Just compiling first and do any checks (e.g. "there is exactly one binary") afterwards doesn't work, as this would be a performance bug (unnecessarily slow) and many tests check for stdout, which would "suddenly" also contain compilation.

Refactoring cargo_compile.rs so that the outside modules can inspect the target list is probably the way to go.

Improving cargo_run's emulation of cargo_compile would probably handle this counter-example, but not others. Furthermore, I believe it's a bad idea to duplicate so much logic.

@alexcrichton
Copy link
Member

cc @jbendig

@jbendig
Copy link
Contributor

jbendig commented Mar 27, 2017

Thanks for reporting this issue! This is definitely wrong and I can reproduce it. I'll work on fixing it.

@BenWiederhake
Copy link
Contributor Author

Awesome, thank you! Please have a look at some thoughts I gave about this, and why it looks to me like that this will need a refactor of cargo_compile.rs.

@jbendig
Copy link
Contributor

jbendig commented Mar 28, 2017

@BenWiederhake Thanks! I'll take a look.

jbendig added a commit to jbendig/cargo that referenced this issue Mar 29, 2017
bors added a commit that referenced this issue Mar 30, 2017
Fix `cargo run` panic when required-features not satisfied

This PR fixes #3867 which is made up of two parts.

The first part involves `cargo run` triggering an assertion after compiling. This is triggered by the single binary selected for compilation being filtered out when required-features is specified and said features are not enabled. The cleanest approach to me involves just sticking a flag into `CompileFilter::Everything`. The flag then triggers the already existing error message when required-features is not satisfied. I think this works best because it localizes what is really a `cargo run` quirk without requiring any boilerplate or duplicate code.

The second part shows `cargo run` bailing when two binaries exist, both with required-features, but only one is resolved to be compiled due to default features. I feel like the current approach is correct because it's consistent with what normally happens when there are multiple binaries. I'm open to changing this, but for now, I've added a test to enforce this behavior.

cc @BenWiederhake: I took a quick peek at your branch to fix #3112 and I noticed that it probably won't merge cleanly with this PR. Just an FYI in case it makes sense to have this merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants