From 3e07a3e66020fe56a570e211e469045faff31bfa Mon Sep 17 00:00:00 2001 From: Ximin Luo Date: Wed, 14 Mar 2018 15:50:09 +0100 Subject: [PATCH 1/6] Fix a bug in #5152 that causes rustc/rustdoc to fail unnecessarily --- src/bin/commands/rustc.rs | 7 ++++++- src/bin/commands/rustdoc.rs | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/bin/commands/rustc.rs b/src/bin/commands/rustc.rs index 268b835a630..b53e5a8abd1 100644 --- a/src/bin/commands/rustc.rs +++ b/src/bin/commands/rustc.rs @@ -62,7 +62,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { } }; let mut compile_opts = args.compile_options_for_single_package(config, mode)?; - compile_opts.target_rustc_args = Some(values(args, "args")); + let target_args = values(args, "args"); + compile_opts.target_rustc_args = if target_args.is_empty() { + None + } else { + Some(target_args) + }; ops::compile(&ws, &compile_opts)?; Ok(()) } diff --git a/src/bin/commands/rustdoc.rs b/src/bin/commands/rustdoc.rs index f3744cebf16..1712b855992 100644 --- a/src/bin/commands/rustdoc.rs +++ b/src/bin/commands/rustdoc.rs @@ -49,7 +49,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let ws = args.workspace(config)?; let mut compile_opts = args.compile_options_for_single_package(config, CompileMode::Doc { deps: false })?; - compile_opts.target_rustdoc_args = Some(values(args, "args")); + let target_args = values(args, "args"); + compile_opts.target_rustdoc_args = if target_args.is_empty() { + None + } else { + Some(target_args) + }; let doc_opts = DocOptions { open_result: args.is_present("open"), compile_opts, From 89d274875fe79f37bc36f22d6afa63f5f854de03 Mon Sep 17 00:00:00 2001 From: Ximin Luo Date: Thu, 15 Mar 2018 16:17:00 +0100 Subject: [PATCH 2/6] Revert "Work around #5134 for now" This reverts commit d46db71b3ff17dfc0f4be6308c8b94613d65a572. --- tests/testsuite/build.rs | 4 +--- tests/testsuite/install.rs | 10 ++++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 7cf2e011640..86a1e58c94c 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -5426,11 +5426,9 @@ fn avoid_dev_deps() { .file("src/main.rs", "fn main() {}") .build(); - // --bins is needed because of #5134 - assert_that(p.cargo("build").arg("--bins"), execs().with_status(101)); + assert_that(p.cargo("build"), execs().with_status(101)); assert_that( p.cargo("build") - .arg("--bins") .masquerade_as_nightly_cargo() .arg("-Zavoid-dev-deps"), execs().with_status(0), diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index b3e5c5bf368..5acdb62929e 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1222,9 +1222,8 @@ fn dev_dependencies_no_check() { .file("src/main.rs", "fn main() {}") .build(); - // --bins is needed because of #5134 - assert_that(p.cargo("build").arg("--bins"), execs().with_status(101)); - assert_that(p.cargo("install").arg("--bins"), execs().with_status(0)); + assert_that(p.cargo("build"), execs().with_status(101)); + assert_that(p.cargo("install"), execs().with_status(0)); } #[test] @@ -1256,10 +1255,9 @@ fn dev_dependencies_lock_file_untouched() { .file("a/src/lib.rs", "") .build(); - // --bins is needed because of #5134 - assert_that(p.cargo("build").arg("--bins"), execs().with_status(0)); + assert_that(p.cargo("build"), execs().with_status(0)); let lock = p.read_lockfile(); - assert_that(p.cargo("install").arg("--bins"), execs().with_status(0)); + assert_that(p.cargo("install"), execs().with_status(0)); let lock2 = p.read_lockfile(); assert!(lock == lock2, "different lockfiles"); } From 89d5161d8e13a3b973cdb09b0f179a82c3c2749a Mon Sep 17 00:00:00 2001 From: Ximin Luo Date: Thu, 15 Mar 2018 16:17:55 +0100 Subject: [PATCH 3/6] Fix need_dev_deps to return false during a default `cargo build` run - Also add a mode param to need_dev_deps to make clear the intent --- src/cargo/ops/cargo_compile.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 4a63b1b2391..9d36436532c 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -246,7 +246,7 @@ pub fn compile_ws<'a>( let specs = spec.into_package_id_specs(ws)?; let features = Method::split_features(features); let method = Method::Required { - dev_deps: ws.require_optional_deps() || filter.need_dev_deps(), + dev_deps: ws.require_optional_deps() || filter.need_dev_deps(mode), features: &features, all_features, uses_default_features: !no_default_features, @@ -442,15 +442,19 @@ impl CompileFilter { } } - pub fn need_dev_deps(&self) -> bool { - match *self { - CompileFilter::Default { .. } => true, - CompileFilter::Only { - ref examples, - ref tests, - ref benches, - .. - } => examples.is_specific() || tests.is_specific() || benches.is_specific(), + pub fn need_dev_deps(&self, mode: CompileMode) -> bool { + match mode { + CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, + CompileMode::Build | CompileMode::Doc { .. } | CompileMode::Check { .. } => match *self + { + CompileFilter::Default { .. } => false, + CompileFilter::Only { + ref examples, + ref tests, + ref benches, + .. + } => examples.is_specific() || tests.is_specific() || benches.is_specific(), + }, } } From 0bf8e541dbd1fc1fe6b8c37234b627a134ad3920 Mon Sep 17 00:00:00 2001 From: Ximin Luo Date: Thu, 15 Mar 2018 16:20:15 +0100 Subject: [PATCH 4/6] Add tests for --all-targets --- tests/testsuite/build.rs | 49 ++++++++++++++++++++++++++++++++++++++++ tests/testsuite/check.rs | 13 ++++++++++- tests/testsuite/rustc.rs | 49 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 86a1e58c94c..9025c51d3d8 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -5336,6 +5336,55 @@ fn build_filter_infer_profile() { ); } +#[test] +fn all_targets_with_and_without() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + assert_that( + p.cargo("build").arg("-v").arg("--all-targets"), + execs().with_status(0) + // bin + .with_stderr_contains("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \ + --emit=dep-info,link[..]") + // bench + .with_stderr_contains("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ + -C opt-level=3 --test [..]") + // unit test + .with_stderr_contains("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ + -C debuginfo=2 --test [..]"), + ); + assert_that(p.cargo("clean"), execs().with_status(0)); + assert_that( + p.cargo("build").arg("-v"), + execs().with_status(0) + // bin + .with_stderr_contains("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \ + --emit=dep-info,link[..]") + // bench + .with_stderr_does_not_contain("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ + -C opt-level=3 --test [..]") + // unit test + .with_stderr_does_not_contain("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ + -C debuginfo=2 --test [..]"), + ); +} + #[test] fn all_targets_no_lib() { let p = project("foo") diff --git a/tests/testsuite/check.rs b/tests/testsuite/check.rs index 7499f136d77..7a6b1b33165 100644 --- a/tests/testsuite/check.rs +++ b/tests/testsuite/check.rs @@ -606,7 +606,7 @@ fn check_virtual_all_implied() { } #[test] -fn check_all_targets() { +fn all_targets_with_and_without() { let foo = project("foo") .file("Cargo.toml", SIMPLE_MANIFEST) .file("src/main.rs", "fn main() {}") @@ -626,6 +626,17 @@ fn check_all_targets() { .with_stderr_contains("[..] --crate-name test2 tests[/]test2.rs [..]") .with_stderr_contains("[..] --crate-name bench3 benches[/]bench3.rs [..]"), ); + assert_that(foo.cargo("clean"), execs().with_status(0)); + assert_that( + foo.cargo("check").arg("-v"), + execs() + .with_status(0) + .with_stderr_contains("[..] --crate-name foo src[/]lib.rs [..]") + .with_stderr_contains("[..] --crate-name foo src[/]main.rs [..]") + .with_stderr_does_not_contain("[..] --crate-name example1 examples[/]example1.rs [..]") + .with_stderr_does_not_contain("[..] --crate-name test2 tests[/]test2.rs [..]") + .with_stderr_does_not_contain("[..] --crate-name bench3 benches[/]bench3.rs [..]"), + ); } #[test] diff --git a/tests/testsuite/rustc.rs b/tests/testsuite/rustc.rs index dd5de43fa0f..582a9ee175c 100644 --- a/tests/testsuite/rustc.rs +++ b/tests/testsuite/rustc.rs @@ -437,6 +437,55 @@ fn build_only_bar_dependency() { ); } +#[test] +fn all_targets_with_and_without() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + assert_that( + p.cargo("rustc").arg("-v").arg("--all-targets"), + execs().with_status(0) + // bin + .with_stderr_contains("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \ + --emit=dep-info,link[..]") + // bench + .with_stderr_contains("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ + -C opt-level=3 --test [..]") + // unit test + .with_stderr_contains("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ + -C debuginfo=2 --test [..]"), + ); + assert_that(p.cargo("clean"), execs().with_status(0)); + assert_that( + p.cargo("rustc").arg("-v"), + execs().with_status(0) + // bin + .with_stderr_contains("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \ + --emit=dep-info,link[..]") + // bench + .with_stderr_does_not_contain("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ + -C opt-level=3 --test [..]") + // unit test + .with_stderr_does_not_contain("\ + [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ + -C debuginfo=2 --test [..]"), + ); +} + #[test] fn fail_with_multiple_packages() { let foo = project("foo") From ce26ddfd68105c6685c2b03334257e36d48f3545 Mon Sep 17 00:00:00 2001 From: Ximin Luo Date: Thu, 15 Mar 2018 16:57:58 +0100 Subject: [PATCH 5/6] Rename stuff for clarity - generate_{auto => default}_target since it matches on CompileFilter::Default - CompileFilter::{matches => target_run} to make it clear it only affects `cargo run` - Add a comment pointing to generate_target for other subcommands --- src/cargo/ops/cargo_compile.rs | 8 +++++--- src/cargo/ops/cargo_run.rs | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 9d36436532c..bec0be93e3f 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -458,7 +458,9 @@ impl CompileFilter { } } - pub fn matches(&self, target: &Target) -> bool { + // this selects targets for "cargo run". for logic to select targets for + // other subcommands, see generate_targets and generate_default_targets + pub fn target_run(&self, target: &Target) -> bool { match *self { CompileFilter::Default { .. } => true, CompileFilter::Only { @@ -497,7 +499,7 @@ struct BuildProposal<'a> { required: bool, } -fn generate_auto_targets<'a>( +fn generate_default_targets<'a>( mode: CompileMode, targets: &'a [Target], profile: &'a Profile, @@ -719,7 +721,7 @@ fn generate_targets<'a>( } else { &profiles.test_deps }; - generate_auto_targets( + generate_default_targets( mode, pkg.targets(), profile, diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index b79c2e1142e..4bd31180851 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -33,7 +33,7 @@ pub fn run( !a.is_lib() && !a.is_custom_build() && if !options.filter.is_specific() { a.is_bin() } else { - options.filter.matches(a) + options.filter.target_run(a) } }) .map(|bin| bin.name()) From 0bc115575336d20d99bc9d0b56baab1ce286043d Mon Sep 17 00:00:00 2001 From: Ximin Luo Date: Mon, 19 Mar 2018 18:20:54 +0100 Subject: [PATCH 6/6] Split tests, apparently `cargo clean` does not work well on windows --- tests/testsuite/build.rs | 31 +++++++++++++++++++++++-------- tests/testsuite/check.rs | 32 ++++++++++++++++++++++---------- tests/testsuite/rustc.rs | 31 +++++++++++++++++++++++-------- 3 files changed, 68 insertions(+), 26 deletions(-) diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 9025c51d3d8..b7c44347341 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -5337,7 +5337,7 @@ fn build_filter_infer_profile() { } #[test] -fn all_targets_with_and_without() { +fn targets_selected_default() { let p = project("foo") .file( "Cargo.toml", @@ -5351,35 +5351,50 @@ fn all_targets_with_and_without() { .file("src/main.rs", "fn main() {}") .build(); assert_that( - p.cargo("build").arg("-v").arg("--all-targets"), + p.cargo("build").arg("-v"), execs().with_status(0) // bin .with_stderr_contains("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \ --emit=dep-info,link[..]") // bench - .with_stderr_contains("\ + .with_stderr_does_not_contain("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ -C opt-level=3 --test [..]") // unit test - .with_stderr_contains("\ + .with_stderr_does_not_contain("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ -C debuginfo=2 --test [..]"), ); - assert_that(p.cargo("clean"), execs().with_status(0)); +} + +#[test] +fn targets_selected_all() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); assert_that( - p.cargo("build").arg("-v"), + p.cargo("build").arg("-v").arg("--all-targets"), execs().with_status(0) // bin .with_stderr_contains("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \ --emit=dep-info,link[..]") // bench - .with_stderr_does_not_contain("\ + .with_stderr_contains("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ -C opt-level=3 --test [..]") // unit test - .with_stderr_does_not_contain("\ + .with_stderr_contains("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ -C debuginfo=2 --test [..]"), ); diff --git a/tests/testsuite/check.rs b/tests/testsuite/check.rs index 7a6b1b33165..264abf898a1 100644 --- a/tests/testsuite/check.rs +++ b/tests/testsuite/check.rs @@ -606,7 +606,7 @@ fn check_virtual_all_implied() { } #[test] -fn all_targets_with_and_without() { +fn targets_selected_default() { let foo = project("foo") .file("Cargo.toml", SIMPLE_MANIFEST) .file("src/main.rs", "fn main() {}") @@ -617,25 +617,37 @@ fn all_targets_with_and_without() { .build(); assert_that( - foo.cargo("check").arg("--all-targets").arg("-v"), + foo.cargo("check").arg("-v"), execs() .with_status(0) .with_stderr_contains("[..] --crate-name foo src[/]lib.rs [..]") .with_stderr_contains("[..] --crate-name foo src[/]main.rs [..]") - .with_stderr_contains("[..] --crate-name example1 examples[/]example1.rs [..]") - .with_stderr_contains("[..] --crate-name test2 tests[/]test2.rs [..]") - .with_stderr_contains("[..] --crate-name bench3 benches[/]bench3.rs [..]"), + .with_stderr_does_not_contain("[..] --crate-name example1 examples[/]example1.rs [..]") + .with_stderr_does_not_contain("[..] --crate-name test2 tests[/]test2.rs [..]") + .with_stderr_does_not_contain("[..] --crate-name bench3 benches[/]bench3.rs [..]"), ); - assert_that(foo.cargo("clean"), execs().with_status(0)); +} + +#[test] +fn targets_selected_all() { + let foo = project("foo") + .file("Cargo.toml", SIMPLE_MANIFEST) + .file("src/main.rs", "fn main() {}") + .file("src/lib.rs", "pub fn smth() {}") + .file("examples/example1.rs", "fn main() {}") + .file("tests/test2.rs", "#[test] fn t() {}") + .file("benches/bench3.rs", "") + .build(); + assert_that( - foo.cargo("check").arg("-v"), + foo.cargo("check").arg("--all-targets").arg("-v"), execs() .with_status(0) .with_stderr_contains("[..] --crate-name foo src[/]lib.rs [..]") .with_stderr_contains("[..] --crate-name foo src[/]main.rs [..]") - .with_stderr_does_not_contain("[..] --crate-name example1 examples[/]example1.rs [..]") - .with_stderr_does_not_contain("[..] --crate-name test2 tests[/]test2.rs [..]") - .with_stderr_does_not_contain("[..] --crate-name bench3 benches[/]bench3.rs [..]"), + .with_stderr_contains("[..] --crate-name example1 examples[/]example1.rs [..]") + .with_stderr_contains("[..] --crate-name test2 tests[/]test2.rs [..]") + .with_stderr_contains("[..] --crate-name bench3 benches[/]bench3.rs [..]"), ); } diff --git a/tests/testsuite/rustc.rs b/tests/testsuite/rustc.rs index 582a9ee175c..297e1050c10 100644 --- a/tests/testsuite/rustc.rs +++ b/tests/testsuite/rustc.rs @@ -438,7 +438,7 @@ fn build_only_bar_dependency() { } #[test] -fn all_targets_with_and_without() { +fn targets_selected_default() { let p = project("foo") .file( "Cargo.toml", @@ -452,35 +452,50 @@ fn all_targets_with_and_without() { .file("src/main.rs", "fn main() {}") .build(); assert_that( - p.cargo("rustc").arg("-v").arg("--all-targets"), + p.cargo("rustc").arg("-v"), execs().with_status(0) // bin .with_stderr_contains("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \ --emit=dep-info,link[..]") // bench - .with_stderr_contains("\ + .with_stderr_does_not_contain("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ -C opt-level=3 --test [..]") // unit test - .with_stderr_contains("\ + .with_stderr_does_not_contain("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ -C debuginfo=2 --test [..]"), ); - assert_that(p.cargo("clean"), execs().with_status(0)); +} + +#[test] +fn targets_selected_all() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); assert_that( - p.cargo("rustc").arg("-v"), + p.cargo("rustc").arg("-v").arg("--all-targets"), execs().with_status(0) // bin .with_stderr_contains("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \ --emit=dep-info,link[..]") // bench - .with_stderr_does_not_contain("\ + .with_stderr_contains("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ -C opt-level=3 --test [..]") // unit test - .with_stderr_does_not_contain("\ + .with_stderr_contains("\ [RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \ -C debuginfo=2 --test [..]"), );