Change target filters in workspaces.

This changes it so that filters like `--bin`, `--test`, `--example`,
`--bench`, `--lib` will work in a workspace.  Today, these filters
require that they match *every* package in a workspace which makes
them not very useful.  This change makes it so that they only must
match at least one package.

src/cargo/ops/cargo_compile.rs

@@ -22,7 +22,7 @@
 //!       previously compiled dependency
 //!
 
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
 use std::sync::Arc;
 
@@ -398,7 +398,7 @@ impl CompileFilter {
     }
 
     // this selects targets for "cargo run". for logic to select targets for
-    // other subcommands, see generate_targets and generate_default_targets
+    // other subcommands, see generate_targets and filter_default_targets
     pub fn target_run(&self, target: &Target) -> bool {
         match *self {
             CompileFilter::Default { .. } => true,
@@ -443,8 +443,6 @@ fn generate_targets<'a>(
     resolve: &Resolve,
     build_config: &BuildConfig,
 ) -> CargoResult<Vec<Unit<'a>>> {
-    let mut units = Vec::new();
-
     // Helper for creating a Unit struct.
     let new_unit = |pkg: &'a Package, target: &'a Target, target_mode: CompileMode| {
         let profile_for = if build_config.mode.is_any_test() {
@@ -520,30 +518,28 @@ fn generate_targets<'a>(
         }
     };
 
-    for pkg in packages {
-        let features = resolve_all_features(resolve, pkg.package_id());
     // Create a list of proposed targets.  The `bool` value indicates
     // whether or not all required features *must* be present. If false,
     // and the features are not available, then it will be silently
     // skipped.  Generally, targets specified by name (`--bin foo`) are
     // required, all others can be silently skipped if features are
     // missing.
-        let mut proposals: Vec<(Unit<'a>, bool)> = Vec::new();
+    let mut proposals: Vec<(&Package, &Target, bool, CompileMode)> = Vec::new();
 
     match *filter {
         CompileFilter::Default {
             required_features_filterable,
         } => {
-                let default_units = generate_default_targets(pkg.targets(), build_config.mode)
+            for pkg in packages {
-                    .iter()
+                let default = filter_default_targets(pkg.targets(), build_config.mode);
-                    .map(|t| {
+                proposals.extend(default.into_iter().map(|target| {
                     (
-                            new_unit(pkg, t, build_config.mode),
+                        *pkg,
+                        target,
                         !required_features_filterable,
+                        build_config.mode,
                     )
-                    })
+                }));
-                    .collect::<Vec<_>>();
-                proposals.extend(default_units);
                 if build_config.mode == CompileMode::Test {
                     // Include doctest for lib.
                     if let Some(t) = pkg
@@ -551,7 +547,8 @@ fn generate_targets<'a>(
                         .iter()
                         .find(|t| t.is_lib() && t.doctested() && t.doctestable())
                     {
-                        proposals.push((new_unit(pkg, t, CompileMode::Doctest), false));
+                        proposals.push((pkg, t, false, CompileMode::Doctest));
+                    }
                 }
             }
         }
@@ -564,19 +561,32 @@ fn generate_targets<'a>(
             ref benches,
         } => {
             if lib {
-                    if let Some(target) = pkg.targets().iter().find(|t| t.is_lib()) {
+                let mut libs = Vec::new();
+                for pkg in packages {
+                    for target in pkg.targets().iter().filter(|t| t.is_lib()) {
                         if build_config.mode == CompileMode::Doctest && !target.doctestable() {
-                            bail!(
+                            ws.config()
+                                .shell()
+                                .warn(format!(
                                 "doc tests are not supported for crate type(s) `{}` in package `{}`",
                                 target.rustc_crate_types().join(", "),
                                 pkg.name()
-                            );
+                            ))?;
+                        } else {
+                            libs.push((*pkg, target, false, build_config.mode));
                         }
-                        proposals.push((new_unit(pkg, target, build_config.mode), false));
-                    } else if !all_targets {
-                        bail!("no library targets found in package `{}`", pkg.name())
                     }
                 }
+                if !all_targets && libs.is_empty() {
+                    let names = packages.iter().map(|pkg| pkg.name()).collect::<Vec<_>>();
+                    if names.len() == 1 {
+                        bail!("no library targets found in package `{}`", names[0]);
+                    } else {
+                        bail!("no library targets found in packages: {}", names.join(", "));
+                    }
+                }
+                proposals.extend(libs);
+            }
             // If --tests was specified, add all targets that would be
             // generated by `cargo test`.
             let test_filter = match *tests {
@@ -600,58 +610,71 @@ fn generate_targets<'a>(
                 _ => build_config.mode,
             };
 
-                proposals.extend(
+            proposals.extend(list_rule_targets(
-                    list_rule_targets(pkg, bins, "bin", Target::is_bin)?
+                packages,
-                        .into_iter()
+                bins,
-                        .map(|(t, required)| (new_unit(pkg, t, build_config.mode), required))
+                "bin",
-                        .chain(
+                Target::is_bin,
-                            list_rule_targets(pkg, examples, "example", Target::is_example)?
+                build_config.mode,
-                                .into_iter()
+            )?);
-                                .map(|(t, required)| {
+            proposals.extend(list_rule_targets(
-                                    (new_unit(pkg, t, build_config.mode), required)
+                packages,
-                                }),
+                examples,
-                        )
+                "example",
-                        .chain(
+                Target::is_example,
-                            list_rule_targets(pkg, tests, "test", test_filter)?
+                build_config.mode,
-                                .into_iter()
+            )?);
-                                .map(|(t, required)| (new_unit(pkg, t, test_mode), required)),
+            proposals.extend(list_rule_targets(
-                        )
+                packages,
-                        .chain(
+                tests,
-                            list_rule_targets(pkg, benches, "bench", bench_filter)?
+                "test",
-                                .into_iter()
+                test_filter,
-                                .map(|(t, required)| (new_unit(pkg, t, bench_mode), required)),
+                test_mode,
-                        )
+            )?);
-                        .collect::<Vec<_>>(),
+            proposals.extend(list_rule_targets(
-                );
+                packages,
+                benches,
+                "bench",
+                bench_filter,
+                bench_mode,
+            )?);
         }
     }
 
     // Only include targets that are libraries or have all required
     // features available.
-        for (unit, required) in proposals {
+    let mut features_map = HashMap::new();
-            let unavailable_features = match unit.target.required_features() {
+    let mut units = Vec::new();
-                Some(rf) => rf.iter().filter(|f| !features.contains(*f)).collect(),
+    for (pkg, target, required, mode) in proposals {
+        let unavailable_features = match target.required_features() {
+            Some(rf) => {
+                let features = features_map
+                    .entry(pkg)
+                    .or_insert_with(|| resolve_all_features(resolve, pkg.package_id()));
+                rf.iter().filter(|f| !features.contains(*f)).collect()
+            }
             None => Vec::new(),
         };
-            if unit.target.is_lib() || unavailable_features.is_empty() {
+        if target.is_lib() || unavailable_features.is_empty() {
+            let unit = new_unit(pkg, target, mode);
             units.push(unit);
         } else if required {
-                let required_features = unit.target.required_features().unwrap();
+            let required_features = target.required_features().unwrap();
             let quoted_required_features: Vec<String> = required_features
                 .iter()
                 .map(|s| format!("`{}`", s))
                 .collect();
             bail!(
-                    "target `{}` requires the features: {}\n\
+                "target `{}` in package `{}` requires the features: {}\n\
                  Consider enabling them by passing e.g. `--features=\"{}\"`",
-                    unit.target.name(),
+                target.name(),
+                pkg.name(),
                 quoted_required_features.join(", "),
                 required_features.join(" ")
             );
         }
         // else, silently skip target.
     }
-    }
     Ok(units)
 }
 
@@ -674,7 +697,7 @@ fn resolve_all_features(
 
 /// Given a list of all targets for a package, filters out only the targets
 /// that are automatically included when the user doesn't specify any targets.
-fn generate_default_targets(targets: &[Target], mode: CompileMode) -> Vec<&Target> {
+fn filter_default_targets(targets: &[Target], mode: CompileMode) -> Vec<&Target> {
     match mode {
         CompileMode::Bench => targets.iter().filter(|t| t.benched()).collect(),
         CompileMode::Test => targets
@@ -701,46 +724,66 @@ fn generate_default_targets(targets: &[Target], mode: CompileMode) -> Vec<&Targe
 }
 
 /// Returns a list of targets based on command-line target selection flags.
-/// The return value is a list of `(Target, bool)` pairs.  The `bool` value
+/// The return value is a list of `(Package, Target, bool, CompileMode)`
-/// indicates whether or not all required features *must* be present.
+/// tuples.  The `bool` value indicates whether or not all required features
+/// *must* be present.
 fn list_rule_targets<'a>(
-    pkg: &'a Package,
+    packages: &[&'a Package],
     rule: &FilterRule,
     target_desc: &'static str,
     is_expected_kind: fn(&Target) -> bool,
-) -> CargoResult<Vec<(&'a Target, bool)>> {
+    mode: CompileMode,
+) -> CargoResult<Vec<(&'a Package, &'a Target, bool, CompileMode)>> {
+    let mut result = Vec::new();
     match *rule {
-        FilterRule::All => Ok(pkg.targets()
+        FilterRule::All => {
-            .iter()
+            for pkg in packages {
-            .filter(|t| is_expected_kind(t))
+                for target in pkg.targets() {
-            .map(|t| (t, false))
+                    if is_expected_kind(target) {
-            .collect()),
+                        result.push((*pkg, target, false, mode));
-        FilterRule::Just(ref names) => names
-            .iter()
-            .map(|name| find_target(pkg, name, target_desc, is_expected_kind))
-            .collect(),
                     }
+                }
+            }
+        }
+        FilterRule::Just(ref names) => {
+            for name in names {
+                result.extend(find_named_targets(
+                    packages,
+                    name,
+                    target_desc,
+                    is_expected_kind,
+                    mode,
+                )?);
+            }
+        }
+    }
+    Ok(result)
 }
 
-/// Find the target for a specifically named target.
+/// Find the targets for a specifically named target.
-fn find_target<'a>(
+fn find_named_targets<'a>(
-    pkg: &'a Package,
+    packages: &[&'a Package],
     target_name: &str,
     target_desc: &'static str,
     is_expected_kind: fn(&Target) -> bool,
-) -> CargoResult<(&'a Target, bool)> {
+    mode: CompileMode,
-    match pkg.targets()
+) -> CargoResult<Vec<(&'a Package, &'a Target, bool, CompileMode)>> {
+    let mut result = Vec::new();
+    for pkg in packages {
+        for target in pkg.targets() {
+            if target.name() == target_name && is_expected_kind(target) {
+                result.push((*pkg, target, true, mode));
+            }
+        }
+    }
+    if result.is_empty() {
+        let suggestion = packages
             .iter()
-        .find(|t| t.name() == target_name && is_expected_kind(t))
+            .flat_map(|pkg| {
-    {
+                pkg.targets()
-        // When a target is specified by name, required features *must* be
-        // available.
-        Some(t) => Ok((t, true)),
-        None => {
-            let suggestion = pkg.targets()
                     .iter()
-                .filter(|t| is_expected_kind(t))
+                    .filter(|target| is_expected_kind(target))
-                .map(|t| (lev_distance(target_name, t.name()), t))
+            }).map(|target| (lev_distance(target_name, target.name()), target))
             .filter(|&(d, _)| d < 4)
             .min_by_key(|t| t.0)
             .map(|t| t.1);
@@ -754,5 +797,5 @@ fn find_target<'a>(
             None => bail!("no {} target named `{}`", target_desc, target_name),
         }
     }
-    }
+    Ok(result)
 }

tests/testsuite/build.rs

@@ -4798,3 +4798,71 @@ fn invalid_jobs() {
             .with_stderr("error: Invalid value: could not parse `over9000` as a number"),
     );
 }
+
+#[test]
+fn target_filters_workspace() {
+    let ws = project()
+        .at("ws")
+        .file(
+            "Cargo.toml",
+            r#"
+        [workspace]
+        members = ["a", "b"]
+        "#,
+        ).file("a/Cargo.toml", &basic_lib_manifest("a"))
+        .file("a/src/lib.rs", "")
+        .file("a/examples/ex1.rs", "fn main() {}")
+        .file("b/Cargo.toml", &basic_bin_manifest("b"))
+        .file("b/src/main.rs", "fn main() {}")
+        .file("b/examples/ex1.rs", "fn main() {}")
+        .build();
+
+    assert_that(
+        ws.cargo("build -v --example ex"),
+        execs().with_status(101).with_stderr(
+            "\
+[ERROR] no example target named `ex`
+
+Did you mean `ex1`?",
+        ),
+    );
+
+    assert_that(
+        ws.cargo("build -v --lib"),
+        execs()
+            .with_status(0)
+            .with_stderr_contains("[RUNNING] `rustc [..]a/src/lib.rs[..]"),
+    );
+
+    assert_that(
+        ws.cargo("build -v --example ex1"),
+        execs()
+            .with_status(0)
+            .with_stderr_contains("[RUNNING] `rustc [..]a/examples/ex1.rs[..]")
+            .with_stderr_contains("[RUNNING] `rustc [..]b/examples/ex1.rs[..]"),
+    );
+}
+
+#[test]
+fn target_filters_workspace_not_found() {
+    let ws = project()
+        .at("ws")
+        .file(
+            "Cargo.toml",
+            r#"
+        [workspace]
+        members = ["a", "b"]
+        "#,
+        ).file("a/Cargo.toml", &basic_bin_manifest("a"))
+        .file("a/src/main.rs", "fn main() {}")
+        .file("b/Cargo.toml", &basic_bin_manifest("b"))
+        .file("b/src/main.rs", "fn main() {}")
+        .build();
+
+    assert_that(
+        ws.cargo("build -v --lib"),
+        execs().with_status(101).with_stderr(
+            "[ERROR] no library targets found in packages: a, b",
+        ),
+    );
+}

tests/testsuite/required_features.rs

@@ -56,7 +56,7 @@ fn build_bin_default_features() {
             .arg("--no-default-features"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo` requires the features: `a`
+error: target `foo` in package `foo` requires the features: `a`
 Consider enabling them by passing e.g. `--features=\"a\"`
 ",
         ),
@@ -178,7 +178,7 @@ fn build_example_default_features() {
             .arg("--no-default-features"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo` requires the features: `a`
+error: target `foo` in package `foo` requires the features: `a`
 Consider enabling them by passing e.g. `--features=\"a\"`
 ",
         ),
@@ -251,7 +251,7 @@ fn build_example_multiple_required_features() {
         p.cargo("build").arg("--example=foo_1"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo_1` requires the features: `b`, `c`
+error: target `foo_1` in package `foo` requires the features: `b`, `c`
 Consider enabling them by passing e.g. `--features=\"b c\"`
 ",
         ),
@@ -288,7 +288,7 @@ Consider enabling them by passing e.g. `--features=\"b c\"`
             .arg("--no-default-features"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo_1` requires the features: `b`, `c`
+error: target `foo_1` in package `foo` requires the features: `b`, `c`
 Consider enabling them by passing e.g. `--features=\"b c\"`
 ",
         ),
@@ -299,7 +299,7 @@ Consider enabling them by passing e.g. `--features=\"b c\"`
             .arg("--no-default-features"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo_2` requires the features: `a`
+error: target `foo_2` in package `foo` requires the features: `a`
 Consider enabling them by passing e.g. `--features=\"a\"`
 ",
         ),
@@ -368,7 +368,7 @@ fn test_default_features() {
             .arg("--no-default-features"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo` requires the features: `a`
+error: target `foo` in package `foo` requires the features: `a`
 Consider enabling them by passing e.g. `--features=\"a\"`
 ",
         ),
@@ -551,7 +551,7 @@ fn bench_default_features() {
             .arg("--no-default-features"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo` requires the features: `a`
+error: target `foo` in package `foo` requires the features: `a`
 Consider enabling them by passing e.g. `--features=\"a\"`
 ",
         ),
@@ -756,7 +756,7 @@ fn install_default_features() {
     `[..]target`
 
 Caused by:
-  target `foo` requires the features: `a`
+  target `foo` in package `foo` requires the features: `a`
 Consider enabling them by passing e.g. `--features=\"a\"`
 "
         )),
@@ -781,7 +781,7 @@ Consider enabling them by passing e.g. `--features=\"a\"`
     `[..]target`
 
 Caused by:
-  target `foo` requires the features: `a`
+  target `foo` in package `foo` requires the features: `a`
 Consider enabling them by passing e.g. `--features=\"a\"`
 "
         )),
@@ -1053,7 +1053,7 @@ fn dep_feature_in_cmd_line() {
         p.cargo("build").arg("--bin=foo"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo` requires the features: `bar/a`
+error: target `foo` in package `foo` requires the features: `bar/a`
 Consider enabling them by passing e.g. `--features=\"bar/a\"`
 ",
         ),
@@ -1073,7 +1073,7 @@ Consider enabling them by passing e.g. `--features=\"bar/a\"`
         p.cargo("build").arg("--example=foo"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo` requires the features: `bar/a`
+error: target `foo` in package `foo` requires the features: `bar/a`
 Consider enabling them by passing e.g. `--features=\"bar/a\"`
 ",
         ),
@@ -1272,7 +1272,7 @@ fn run_default() {
         p.cargo("run"),
         execs().with_status(101).with_stderr(
             "\
-error: target `foo` requires the features: `a`
+error: target `foo` in package `foo` requires the features: `a`
 Consider enabling them by passing e.g. `--features=\"a\"`
 ",
         ),

tests/testsuite/test.rs

@@ -3533,7 +3533,9 @@ fn doctest_skip_staticlib() {
     assert_that(
         p.cargo("test --doc"),
         execs().with_status(101).with_stderr(
-            "[ERROR] doc tests are not supported for crate type(s) `staticlib` in package `foo`",
+            "\
+[WARNING] doc tests are not supported for crate type(s) `staticlib` in package `foo`
+[ERROR] no library targets found in package `foo`",
         ),
     );