From fa54a79433f905c77c14984abef95ccb69192fe4 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 8 Aug 2018 17:38:04 -0700 Subject: [PATCH] Allow `cargo run` in workspaces. --- src/bin/cargo/commands/run.rs | 18 ++++++++--- src/cargo/ops/cargo_compile.rs | 39 +++++++++++----------- src/cargo/ops/cargo_run.rs | 58 +++++++++++++++++---------------- tests/testsuite/run.rs | 59 ++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 51 deletions(-) diff --git a/src/bin/cargo/commands/run.rs b/src/bin/cargo/commands/run.rs index 264c421e6e1..0cd4952a500 100644 --- a/src/bin/cargo/commands/run.rs +++ b/src/bin/cargo/commands/run.rs @@ -38,14 +38,18 @@ run. If you're passing arguments to both Cargo and the binary, the ones after 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::Build)?; + let mut compile_opts = args.compile_options(config, CompileMode::Build)?; if !args.is_present("example") && !args.is_present("bin") { - if let Some(default_run) = compile_opts.get_package(&ws)? - .and_then(|pkg| pkg.manifest().default_run()) - { + let default_runs: Vec<_> = compile_opts + .spec + .get_packages(&ws)? + .iter() + .filter_map(|pkg| pkg.manifest().default_run()) + .collect(); + if default_runs.len() == 1 { compile_opts.filter = CompileFilter::new( false, - vec![default_run.to_owned()], + vec![default_runs[0].to_owned()], false, vec![], false, @@ -56,7 +60,11 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { false, ); } else { + // ops::run will take care of errors if len pkgs != 1. compile_opts.filter = CompileFilter::Default { + // Force this to false because the code in ops::run is not + // able to pre-check features before compilation starts to + // enforce that only 1 binary is built. required_features_filterable: false, }; } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 33513381a7f..fe5623ce62a 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -83,24 +83,6 @@ impl<'a> CompileOptions<'a> { export_dir: None, }) } - - // Returns the unique specified package, or None - pub fn get_package<'b>(&self, ws: &'b Workspace) -> CargoResult> { - Ok(match self.spec { - Packages::All | Packages::Default | Packages::OptOut(_) => { - None - } - Packages::Packages(ref xs) => match xs.len() { - 0 => Some(ws.current()?), - 1 => Some(ws.members() - .find(|pkg| *pkg.name() == xs[0]) - .ok_or_else(|| { - format_err!("package `{}` is not a member of the workspace", xs[0]) - })?), - _ => None, - }, - }) - } } #[derive(Clone, PartialEq, Eq, Debug)] @@ -157,6 +139,27 @@ impl Packages { } Ok(specs) } + + pub fn get_packages<'ws>(&self, ws: &'ws Workspace) -> CargoResult> { + let packages: Vec<_> = match self { + Packages::Default => ws.default_members().collect(), + Packages::All => ws.members().collect(), + Packages::OptOut(ref opt_out) => ws + .members() + .filter(|pkg| !opt_out.iter().any(|name| pkg.name().as_str() == name)) + .collect(), + Packages::Packages(ref pkgs) => pkgs + .iter() + .map(|name| { + ws.members() + .find(|pkg| pkg.name().as_str() == name) + .ok_or_else(|| { + format_err!("package `{}` is not a member of the workspace", name) + }) + }).collect::>>()?, + }; + Ok(packages) + } } #[derive(Debug)] diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index 2095972e7ed..2428403b716 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -1,3 +1,4 @@ +use std::iter; use std::path::Path; use ops; @@ -11,23 +12,20 @@ pub fn run( ) -> CargoResult> { let config = ws.config(); - let pkg = options.get_package(ws)? - .unwrap_or_else(|| unreachable!("cargo run supports single package only")); - - // 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 bins: Vec<_> = pkg.manifest() - .targets() - .iter() - .filter(|a| { - !a.is_lib() && !a.is_custom_build() && if !options.filter.is_specific() { - a.is_bin() - } else { - options.filter.target_run(a) - } - }) - .map(|bin| (bin.name(), bin.kind())) - .collect(); + // 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 packages = options.spec.get_packages(ws)?; + let bins: Vec<_> = packages + .into_iter() + .flat_map(|pkg| { + iter::repeat(pkg).zip(pkg.manifest().targets().iter().filter(|target| { + !target.is_lib() && !target.is_custom_build() && if !options.filter.is_specific() { + target.is_bin() + } else { + options.filter.target_run(target) + } + })) + }).collect(); if bins.is_empty() { if !options.filter.is_specific() { @@ -38,31 +36,34 @@ pub fn run( } if bins.len() == 1 { - let &(name, kind) = bins.first().unwrap(); - if let TargetKind::ExampleLib(..) = kind { + let target = bins[0].1; + if let TargetKind::ExampleLib(..) = target.kind() { bail!( - "example target `{}` is a library and cannot be executed", - name - ) + "example target `{}` is a library and cannot be executed", + target.name() + ) } } if bins.len() > 1 { if !options.filter.is_specific() { - let names: Vec<&str> = bins.into_iter().map(|bin| bin.0).collect(); + let names: Vec<&str> = bins + .into_iter() + .map(|(_pkg, target)| target.name()) + .collect(); if nightly_features_allowed() { bail!( "`cargo run` could not determine which binary to run. \ - Use the `--bin` option to specify a binary, \ - or (on nightly) the `default-run` manifest key.\n\ - available binaries: {}", + Use the `--bin` option to specify a binary, \ + or (on nightly) the `default-run` manifest key.\n\ + available binaries: {}", names.join(", ") ) } else { bail!( "`cargo run` requires that a project only have one \ - executable; use the `--bin` option to specify which one \ - to run\navailable binaries: {}", + executable; use the `--bin` option to specify which one \ + to run\navailable binaries: {}", names.join(", ") ) } @@ -84,6 +85,7 @@ pub fn run( Some(path) => path.to_path_buf(), None => exe.to_path_buf(), }; + let pkg = bins[0].0; let mut process = compile.target_process(exe, pkg)?; process.args(args).cwd(config.cwd()); diff --git a/tests/testsuite/run.rs b/tests/testsuite/run.rs index 14d0869bdd9..16050ed4288 100644 --- a/tests/testsuite/run.rs +++ b/tests/testsuite/run.rs @@ -1098,3 +1098,62 @@ fn explicit_bin_with_args() { assert_that(p.cargo("run --bin foo hello world"), execs()); } + +#[test] +fn run_workspace() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b"] + "#, + ).file("a/Cargo.toml", &basic_bin_manifest("a")) + .file("a/src/main.rs", r#"fn main() {println!("run-a");}"#) + .file("b/Cargo.toml", &basic_bin_manifest("b")) + .file("b/src/main.rs", r#"fn main() {println!("run-b");}"#) + .build(); + + assert_that( + p.cargo("run"), + execs().with_status(101).with_stderr( + "\ +[ERROR] `cargo run` requires that a project only have one executable[..] +available binaries: a, b", + ), + ); + assert_that( + p.cargo("run --bin a"), + execs().with_status(0).with_stdout("run-a"), + ); +} + +#[test] +fn default_run_workspace() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b"] + "#, + ).file( + "a/Cargo.toml", + r#" + cargo-features = ["default-run"] + + [project] + name = "a" + version = "0.0.1" + default-run = "a" + "#, + ).file("a/src/main.rs", r#"fn main() {println!("run-a");}"#) + .file("b/Cargo.toml", &basic_bin_manifest("b")) + .file("b/src/main.rs", r#"fn main() {println!("run-b");}"#) + .build(); + + assert_that( + p.cargo("run").masquerade_as_nightly_cargo(), + execs().with_status(0).with_stdout("run-a"), + ); +}