cargo-run-deps: Refactor identifying eligible deps

Relates to #2267.

Author: tchernobog

src/bin/cargo/commands/run.rs

@@ -104,39 +104,22 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
 }
 
 fn get_default_runs(ws: &Workspace<'_>, compile_opts: &CompileOptions) -> CargoResult<Vec<String>> {
-    const ERROR: &'static str =
-        "`cargo run` cannot find pkgid either in the workspace or among the workspace dependencies";
+    const ERROR_NOTFOUND: &'static str = "`cargo run` cannot find pkgid either in the workspace or among direct build or development dependencies";
+    let matching_dependencies = cargo::ops::packages_eligible_to_run(ws, &compile_opts.spec);
+    match matching_dependencies {
+        Ok(packages) => {
+            if packages.is_empty() {
+                anyhow::bail!(ERROR_NOTFOUND);
+            }
 
-    let workspace_packages = compile_opts.spec.get_packages(ws);
-
-    let default_runs = if let Ok(packages) = workspace_packages {
-        // Package is workspace member
-        packages
-            .iter()
-            .filter_map(|pkg| pkg.manifest().default_run())
-            .map(str::to_owned)
-            .collect()
-    } else if let Packages::Packages(ref pkg_names) = compile_opts.spec {
-        // Search dependencies
-        let (package_set, resolver) = ops::resolve_ws(ws)?;
-        let deps: Vec<_> = pkg_names
-            .iter()
-            .flat_map(|name| resolver.query(name))
-            .collect();
-
-        if deps.is_empty() {
-            anyhow::bail!(ERROR);
+            let default_runs = packages
+                .into_iter()
+                .filter_map(|pkg| pkg.manifest().default_run().map(|s| s.to_owned()))
+                .collect();
+            Ok(default_runs)
         }
-
-        package_set
-            .get_many(deps)?
-            .iter()
-            .filter_map(|pkg| pkg.manifest().default_run())
-            .map(str::to_owned)
-            .collect()
-    } else {
-        anyhow::bail!(ERROR);
-    };
-
-    Ok(default_runs)
+        Err(_) => {
+            anyhow::bail!(ERROR_NOTFOUND);
+        }
+    }
 }

src/cargo/ops/cargo_run.rs

@@ -1,10 +1,13 @@
+use std::collections::HashSet;
 use std::ffi::OsString;
 use std::iter;
 use std::path::Path;
 
 use crate::core::compiler::UnitOutput;
-use crate::core::{TargetKind, Workspace};
-use crate::ops;
+use crate::core::dependency::DepKind;
+use crate::core::resolver::Resolve;
+use crate::core::{Package, PackageIdSpec, PackageSet, TargetKind, Workspace};
+use crate::ops::{self, Packages};
 use crate::util::CargoResult;
 
 pub fn run(
@@ -20,19 +23,10 @@ pub fn run(
 
     // We compute the `bins` here *just for diagnosis*. The actual set of
     // packages to be run is determined by the `ops::compile` call below.
-    let (package_set, resolver) = ops::resolve_ws(ws)?;
-    let packages = if let ops::Packages::Packages(ref pkg_names) = options.spec {
-        let pkgids: Vec<_> = pkg_names
-            .iter()
-            .flat_map(|name| resolver.query(name))
-            .collect();
-        package_set.get_many(pkgids)?
-    } else {
-        options.spec.get_packages(ws)?
-    };
+    let packages = packages_eligible_to_run(ws, &options.spec)?;
 
     let bins: Vec<_> = packages
-        .into_iter()
+        .iter()
         .flat_map(|pkg| {
             iter::repeat(pkg).zip(pkg.manifest().targets().iter().filter(|target| {
                 !target.is_lib()
@@ -101,11 +95,51 @@ pub fn run(
         Ok(path) => path.to_path_buf(),
         Err(_) => path.to_path_buf(),
     };
-    let pkg = bins[0].0;
+    let pkg = &bins[0].0;
     let mut process = compile.target_process(exe, unit.kind, pkg, *script_meta)?;
     process.args(args).cwd(config.cwd());
 
     config.shell().status("Running", process.to_string())?;
 
     process.exec_replace()
 }
+
+pub fn packages_eligible_to_run<'a>(
+    ws: &Workspace<'a>,
+    request: &Packages,
+) -> CargoResult<Vec<Package>> {
+    let matching_dependencies = if let ops::Packages::Packages(ref pkg_names) = request {
+        let specs: HashSet<_> = pkg_names
+            .into_iter()
+            .flat_map(|s| PackageIdSpec::parse(s))
+            .collect();
+
+        let (package_set, resolver): (PackageSet<'a>, Resolve) = ops::resolve_ws(ws)?;
+
+        // Restrict all direct dependencies only to build and development ones.
+        // Cargo wouldn't be able to run anything after installation, so
+        // normal dependencies are out.
+        let direct_dependencies: Vec<_> = ws
+            .members()
+            .flat_map(|pkg| resolver.deps(pkg.package_id()))
+            .filter(|(_, manifest_deps)| {
+                manifest_deps.into_iter().any(|dep| match dep.kind() {
+                    DepKind::Development | DepKind::Build => true,
+                    DepKind::Normal => false,
+                })
+            })
+            .collect();
+
+        specs.into_iter().filter_map(|pkgidspec|
+            // Either a workspace match…
+            ws.members().find(|pkg| pkgidspec.matches(pkg.package_id()))
+                .or_else(|| { // …or a direct dependency as fallback
+                    let maybe_dep = direct_dependencies.iter().find(|(dep_pkgid, _)| pkgidspec.matches(*dep_pkgid));
+                    maybe_dep.map(|(dep_pkgid, _)| package_set.get_one(*dep_pkgid).unwrap())
+                })).cloned().collect()
+    } else {
+        request.get_packages(ws)?.into_iter().cloned().collect()
+    };
+
+    Ok(matching_dependencies)
+}

src/cargo/ops/mod.rs

@@ -16,7 +16,7 @@ pub use self::cargo_output_metadata::{output_metadata, ExportInfo, OutputMetadat
 pub use self::cargo_package::{package, package_one, PackageOpts};
 pub use self::cargo_pkgid::pkgid;
 pub use self::cargo_read_manifest::{read_package, read_packages};
-pub use self::cargo_run::run;
+pub use self::cargo_run::{packages_eligible_to_run, run};
 pub use self::cargo_test::{run_benches, run_tests, TestOptions};
 pub use self::cargo_uninstall::uninstall;
 pub use self::fix::{fix, fix_maybe_exec_rustc, FixOptions};

tests/testsuite/run.rs

@@ -1142,8 +1142,10 @@ fn run_multiple_packages() {
 
                 [workspace]
 
-                [dependencies]
-                d1 = { path = "d1" }
+                [build-dependencies]
+                b1 = { path = "b1" }
+
+                [dev-dependencies]
                 d2 = { path = "d2" }
                 d3 = { path = "../d3" } # outside of the workspace
 
@@ -1152,9 +1154,9 @@ fn run_multiple_packages() {
             "#,
         )
         .file("foo/src/foo.rs", "fn main() { println!(\"foo\"); }")
-        .file("foo/d1/Cargo.toml", &basic_bin_manifest("d1"))
-        .file("foo/d1/src/lib.rs", "")
-        .file("foo/d1/src/main.rs", "fn main() { println!(\"d1\"); }")
+        .file("foo/b1/Cargo.toml", &basic_bin_manifest("b1"))
+        .file("foo/b1/src/lib.rs", "")
+        .file("foo/b1/src/main.rs", "fn main() { println!(\"b1\"); }")
         .file("foo/d2/Cargo.toml", &basic_bin_manifest("d2"))
         .file("foo/d2/src/main.rs", "fn main() { println!(\"d2\"); }")
         .file("d3/Cargo.toml", &basic_bin_manifest("d3"))
@@ -1167,7 +1169,7 @@ fn run_multiple_packages() {
         process_builder
     };
 
-    cargo().arg("-p").arg("d1").with_stdout("d1").run();
+    cargo().arg("-p").arg("b1").with_stdout("b1").run();
 
     cargo()
         .arg("-p")
@@ -1179,7 +1181,7 @@ fn run_multiple_packages() {
 
     cargo().with_stdout("foo").run();
 
-    cargo().arg("-p").arg("d1").arg("-p").arg("d2")
+    cargo().arg("-p").arg("b1").arg("-p").arg("d2")
                     .with_status(1)
                     .with_stderr_contains("error: The argument '--package <SPEC>' was provided more than once, but cannot be used multiple times").run();
 
@@ -1197,37 +1199,47 @@ fn run_multiple_packages() {
 
 #[cargo_test]
 fn run_dependency_package() {
-    Package::new("bdep", "0.1.0")
+    for i in 1..=3 {
+        Package::new(&format!("bdep{}", i), "0.1.1")
+            .file(
+                "Cargo.toml",
+                &format!(
+                    r#"
+                [package]
+                name = "bdep{}"
+                version = "0.1.1"
+                authors = ["[email protected]"]
+
+                [[bin]]
+                name = "bdep{}"
+            "#,
+                    i, i
+                ),
+            )
+            .file(
+                "src/main.rs",
+                &format!("fn main() {{ println!(\"bdep{} 0.1.1\"); }}", i),
+            )
+            .publish();
+    }
+
+    Package::new("bdep1", "0.1.0") // older version
         .file(
             "Cargo.toml",
             r#"
             [package]
-            name = "bdep"
+            name = "bdep1"
             version = "0.1.0"
             authors = ["[email protected]"]
 
             [[bin]]
-            name = "bdep"
+            name = "bdep1"
         "#,
         )
-        .file("src/main.rs", "fn main() { println!(\"bdep 0.1.0\"); }")
+        .file("src/main.rs", "fn main() { println!(\"bdep1 0.1.0\"); }")
         .publish();
-    Package::new("bdep", "0.1.1")
-        .file(
-            "Cargo.toml",
-            r#"
-            [package]
-            name = "bdep"
-            version = "0.1.1"
-            authors = ["[email protected]"]
 
-            [[bin]]
-            name = "bdep"
-        "#,
-        )
-        .file("src/main.rs", "fn main() { println!(\"bdep 0.1.1\"); }")
-        .publish();
-    Package::new("libdep", "0.1.0")
+    Package::new("libdep", "0.1.0") // library (not runnable)
         .file(
             "Cargo.toml",
             r#"
@@ -1252,20 +1264,27 @@ fn run_dependency_package() {
                 edition = "2018"
 
                 [build-dependencies]
-                bdep = "0.1"
+                bdep1 = "0.1"
                 libdep = "0.1"
+
+                [dev-dependencies]
+                bdep2 = "0.1.1"
+
+                [dependencies]
+                bdep3 = "0"
             "#,
         )
         .file("src/main.rs", "fn main() { println!(\"foo\"); }")
         .build();
 
     let testcases = [
-        ("bdep", "bdep 0.1.1"),
-        ("bdep:0.1.1", "bdep 0.1.1"),
+        ("bdep1", "bdep1 0.1.1"),
+        ("bdep1:0.1.1", "bdep1 0.1.1"),
         (
-            "https://github.com/rust-lang/crates.io-index#bdep",
-            "bdep 0.1.1",
+            "https://github.com/rust-lang/crates.io-index#bdep1",
+            "bdep1 0.1.1",
         ),
+        ("bdep2", "bdep2 0.1.1"),
     ];
     for (pkgid, expected_output) in testcases {
         p.cargo("run")
@@ -1283,18 +1302,19 @@ fn run_dependency_package() {
         .run();
 
     let invalid_pkgids = [
-        "bdep:0.1.0",
-        "https://github.com/rust-lang/crates.io-index#bdep:0.1.0",
-        "bdep:0.2.0",
-        "https://github.com/rust-lang/crates.io-index#bdep:0.2.0",
+        "bdep1:0.1.0",
+        "https://github.com/rust-lang/crates.io-index#bdep1:0.1.0",
+        "bdep1:0.2.0",
+        "https://github.com/rust-lang/crates.io-index#bdep1:0.2.0",
+        "bdep3",
     ];
     for pkgid in invalid_pkgids {
         p.cargo("run")
                 .arg("-p")
                 .arg(pkgid)
                 .with_status(101)
                 .with_stderr_contains(
-                    "[ERROR] `cargo run` cannot find pkgid either in the workspace or among the workspace dependencies",
+                    "[ERROR] `cargo run` cannot find pkgid either in the workspace or among direct build or development dependencies",
                 )
                 .run();
     }