Support --test/--bin/--lib in cargo-miri #1525
Conversation
| match arg { | ||
| arg if arg == "--" => break, | ||
| arg if arg == "--lib" => lib_present = true, | ||
| arg if arg == "--bin" => { |
There was a problem hiding this comment.
This looks like it supports --bin name but not --bin=name.
cargo-miri/bin.rs
Outdated
| @@ -430,10 +496,7 @@ fn in_cargo_miri() { | |||
| _ => continue, | |||
| } | |||
| // Forward user-defined `cargo` args until first `--`. | |||
There was a problem hiding this comment.
| // Forward user-defined `cargo` args until first `--`. | |
| // Forward further `cargo` args. |
cargo-miri/bin.rs
Outdated
| let mut additional_args = Vec::new(); | ||
| while let Some(arg) = args.next() { | ||
| match arg { | ||
| arg if arg == "--" => break, |
There was a problem hiding this comment.
Please add a comment saying that this is where the Miri arguments start.
|
Thanks a lot! This looks much cleaner than I thought was possible. ;) However, this now applies to both On the other hand, I hope to get around to resolving #739 within the next two weeks. So it does not seem worth the effort to spend a lot of time re-implementing cargo's logic here, if we are able to just reuse the existing logic. So in that sense I am fine landing this with the existing logic, but please add a FIXME. |
| // Now we run `cargo check $FLAGS $ARGS`, giving the user the | ||
| // change to add additional arguments. `FLAGS` is set to identify | ||
| // this target. The user gets to control what gets actually passed to Miri. | ||
| let mut cmd = cargo(); | ||
| cmd.arg("check"); | ||
| match (subcommand, kind.as_str()) { | ||
| (MiriCommand::Run, "bin") => { | ||
| // FIXME: we just run all the binaries here. |
There was a problem hiding this comment.
The first line here is still true -- cargo miri run runs all binaries when there are several, where cargo run picks default-run or errors of that key is not specified in Cargo.toml. So please don't fully remove this FIXME.
|
OK, made those changes. Reusing cargo's logic sounds great. |
cargo-miri/bin.rs
Outdated
| show_error(format!("\"--bin\" takes one argument.")); | ||
| } | ||
| } | ||
| arg if arg.starts_with("--bin=") => bin_targets.push((&arg[6..]).to_string()), |
There was a problem hiding this comment.
| arg if arg.starts_with("--bin=") => bin_targets.push((&arg[6..]).to_string()), | |
| arg if arg.starts_with("--bin=") => bin_targets.push((&arg["--bin".len()..]).to_string()), |
Let's avoid magic constants. Same goes for --test.
|
Thanks, this is great! Let's land this. @bors r+ It would also be good to have some testcases (cargo-miri is being tested by |
|
📌 Commit 64e2d3e has been approved by |
|
☀️ Test successful - checks-travis, status-appveyor |
Test cargo miri target selection This is a followup to #1525, adding a few test invocations with targets specified in the cargo arguments.
This PR addresses a FIXME in cargo-miri, and filters the targets to be checked when any of the
--bin, '--test, or--lib` flags are passed.