Auto merge of #3177 - matklad:kill-exec-engine, r=alexcrichton

Remove ExecEngine abstraction

Hi! Not sure what was the idea behind exec engine (perhaps to allow swapping it out during tests?), but looks like it does absolutely nothing at the moment, and can be removed.

src/bin/bench.rs

@@ -89,7 +89,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
             all_features: options.flag_all_features,
             no_default_features: options.flag_no_default_features,
             spec: &options.flag_package,
-            exec_engine: None,
             release: true,
             mode: ops::CompileMode::Bench,
             filter: ops::CompileFilter::new(options.flag_lib,

src/bin/build.rs

@@ -85,7 +85,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
         all_features: options.flag_all_features,
         no_default_features: options.flag_no_default_features,
         spec: &options.flag_package,
-        exec_engine: None,
         mode: ops::CompileMode::Build,
         release: options.flag_release,
         filter: ops::CompileFilter::new(options.flag_lib,

src/bin/doc.rs

@@ -81,7 +81,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
             all_features: options.flag_all_features,
             no_default_features: options.flag_no_default_features,
             spec: &options.flag_package,
-            exec_engine: None,
             filter: ops::CompileFilter::new(options.flag_lib,
                                             &options.flag_bin,
                                             &empty,

src/bin/install.rs

@@ -109,7 +109,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
         all_features: options.flag_all_features,
         no_default_features: options.flag_no_default_features,
         spec: &[],
-        exec_engine: None,
         mode: ops::CompileMode::Build,
         release: !options.flag_debug,
         filter: ops::CompileFilter::new(false, &options.flag_bin, &[],

src/bin/run.rs

@@ -82,7 +82,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
         all_features: options.flag_all_features,
         no_default_features: options.flag_no_default_features,
         spec: &[],
-        exec_engine: None,
         release: options.flag_release,
         mode: ops::CompileMode::Build,
         filter: if examples.is_empty() && bins.is_empty() {

src/bin/rustc.rs

@@ -103,7 +103,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
         all_features: options.flag_all_features,
         no_default_features: options.flag_no_default_features,
         spec: &options.flag_package.map_or(Vec::new(), |s| vec![s]),
-        exec_engine: None,
         mode: mode,
         release: options.flag_release,
         filter: ops::CompileFilter::new(options.flag_lib,

src/bin/rustdoc.rs

@@ -90,7 +90,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
             all_features: options.flag_all_features,
             no_default_features: options.flag_no_default_features,
             spec: &options.flag_package.map_or(Vec::new(), |s| vec![s]),
-            exec_engine: None,
             release: options.flag_release,
             filter: ops::CompileFilter::new(options.flag_lib,
                                             &options.flag_bin,

src/bin/test.rs

@@ -123,7 +123,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
             all_features: options.flag_all_features,
             no_default_features: options.flag_no_default_features,
             spec: &options.flag_package,
-            exec_engine: None,
             release: options.flag_release,
             mode: mode,
             filter: filter,

src/cargo/ops/cargo_compile.rs

@@ -24,13 +24,12 @@
 
 use std::collections::HashMap;
 use std::path::PathBuf;
-use std::sync::Arc;
 
 use core::registry::PackageRegistry;
 use core::{Source, SourceId, PackageSet, Package, Target};
 use core::{Profile, TargetKind, Profiles, Workspace, PackageIdSpec};
 use core::resolver::{Method, Resolve};
-use ops::{self, BuildOutput, ExecEngine};
+use ops::{self, BuildOutput};
 use sources::PathSource;
 use util::config::Config;
 use util::{CargoResult, profile, human, ChainError};
@@ -53,8 +52,6 @@ pub struct CompileOptions<'a> {
     /// Filter to apply to the root package to select which targets will be
     /// built.
     pub filter: CompileFilter<'a>,
-    /// Engine which drives compilation
-    pub exec_engine: Option<Arc<Box<ExecEngine>>>,
     /// Whether this is a release build or not
     pub release: bool,
     /// Mode for this compile.
@@ -159,7 +156,7 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>,
     let CompileOptions { config, jobs, target, spec, features,
                          all_features, no_default_features,
                          release, mode, message_format,
-                         ref filter, ref exec_engine,
+                         ref filter,
                          ref target_rustdoc_args,
                          ref target_rustc_args } = *options;
 
@@ -247,7 +244,6 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>,
     let mut ret = {
         let _p = profile::start("compiling");
         let mut build_config = try!(scrape_build_config(config, jobs, target));
-        build_config.exec_engine = exec_engine.clone();
         build_config.release = release;
         build_config.test = mode == CompileMode::Test;
         build_config.json_errors = message_format == MessageFormat::Json;

src/cargo/ops/cargo_package.rs

@@ -293,7 +293,6 @@ fn run_verify(ws: &Workspace, tar: &File, opts: &PackageOpts) -> CargoResult<()>
         all_features: false,
         spec: &[],
         filter: ops::CompileFilter::Everything,
-        exec_engine: None,
         release: false,
         message_format: ops::MessageFormat::Human,
         mode: ops::CompileMode::Build,

src/cargo/ops/cargo_rustc/engine.rs → src/cargo/ops/cargo_rustc/command.rs

@@ -2,32 +2,10 @@ use std::collections::HashMap;
 use std::ffi::{OsStr, OsString};
 use std::fmt;
 use std::path::Path;
-use std::process::Output;
 
-use util::{CargoResult, ProcessError, ProcessBuilder, process};
+use util::{CargoResult, ProcessBuilder, process};
 use util::Config;
 
-/// Trait for objects that can execute commands.
-pub trait ExecEngine: Send + Sync {
-    fn exec(&self, CommandPrototype) -> Result<(), ProcessError>;
-    fn exec_with_output(&self, CommandPrototype) -> Result<Output, ProcessError>;
-}
-
-/// Default implementation of `ExecEngine`.
-#[derive(Clone, Copy)]
-pub struct ProcessEngine;
-
-impl ExecEngine for ProcessEngine {
-    fn exec(&self, command: CommandPrototype) -> Result<(), ProcessError> {
-        command.into_process_builder().exec()
-    }
-
-    fn exec_with_output(&self, command: CommandPrototype)
-                        -> Result<Output, ProcessError> {
-        command.into_process_builder().exec_with_output()
-    }
-}
-
 /// Prototype for a command that must be executed.
 #[derive(Clone)]
 pub struct CommandPrototype {

src/cargo/ops/cargo_rustc/context.rs

@@ -16,7 +16,6 @@ use super::fingerprint::Fingerprint;
 use super::layout::{Layout, LayoutProxy};
 use super::links::Links;
 use super::{Kind, Compilation, BuildConfig};
-use super::{ProcessEngine, ExecEngine};
 
 #[derive(Clone, Copy, Eq, PartialEq, Hash)]
 pub struct Unit<'a> {
@@ -34,7 +33,6 @@ pub struct Context<'a, 'cfg: 'a> {
     pub packages: &'a PackageSet<'cfg>,
     pub build_state: Arc<BuildState>,
     pub build_explicit_deps: HashMap<Unit<'a>, (PathBuf, Vec<String>)>,
-    pub exec_engine: Arc<Box<ExecEngine>>,
     pub fingerprints: HashMap<Unit<'a>, Arc<Fingerprint>>,
     pub compiled: HashSet<Unit<'a>>,
     pub build_config: BuildConfig,
@@ -72,9 +70,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             None => None,
         };
 
-        let engine = build_config.exec_engine.as_ref().cloned().unwrap_or({
-            Arc::new(Box::new(ProcessEngine))
-        });
         let current_package = try!(ws.current()).package_id().clone();
         Ok(Context {
             host: host_layout,
@@ -88,7 +83,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             compilation: Compilation::new(config),
             build_state: Arc::new(BuildState::new(&build_config)),
             build_config: build_config,
-            exec_engine: engine,
             fingerprints: HashMap::new(),
             profiles: profiles,
             compiled: HashSet::new(),

src/cargo/ops/cargo_rustc/job_queue.rs

@@ -13,7 +13,7 @@ use util::{CargoResult, profile, internal};
 
 use super::{Context, Kind, Unit};
 use super::job::Job;
-use super::engine::CommandPrototype;
+use super::command::CommandPrototype;
 
 /// A management structure of the entire dependency graph to compile.
 ///

src/cargo/ops/cargo_rustc/mod.rs

@@ -18,14 +18,14 @@ use self::job_queue::JobQueue;
 
 pub use self::compilation::Compilation;
 pub use self::context::{Context, Unit};
-pub use self::engine::{CommandPrototype, CommandType, ExecEngine, ProcessEngine};
+pub use self::command::{CommandPrototype, CommandType};
 pub use self::layout::{Layout, LayoutProxy};
 pub use self::custom_build::{BuildOutput, BuildMap, BuildScripts};
 
-mod context;
+mod command;
 mod compilation;
+mod context;
 mod custom_build;
-mod engine;
 mod fingerprint;
 mod job;
 mod job_queue;
@@ -42,7 +42,6 @@ pub struct BuildConfig {
     pub requested_target: Option<String>,
     pub target: TargetConfig,
     pub jobs: u32,
-    pub exec_engine: Option<Arc<Box<ExecEngine>>>,
     pub release: bool,
     pub test: bool,
     pub doc_all: bool,
@@ -467,7 +466,6 @@ fn rustdoc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
     let name = unit.pkg.name().to_string();
     let build_state = cx.build_state.clone();
     let key = (unit.pkg.package_id().clone(), unit.kind);
-    let exec_engine = cx.exec_engine.clone();
 
     Ok(Work::new(move |state| {
         if let Some(output) = build_state.outputs.lock().unwrap().get(&key) {
@@ -476,7 +474,7 @@ fn rustdoc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
             }
         }
         state.running(&rustdoc);
-        exec_engine.exec(rustdoc).chain_error(|| {
+        rustdoc.into_process_builder().exec().chain_error(|| {
             human(format!("Could not document `{}`.", name))
         })
     }))

src/cargo/ops/cargo_test.rs

@@ -1,6 +1,6 @@
 use std::ffi::{OsString, OsStr};
 
-use ops::{self, ExecEngine, ProcessEngine, Compilation};
+use ops::{self, Compilation};
 use util::{self, CargoResult, CargoTestError, ProcessError};
 use core::Workspace;
 
@@ -98,7 +98,7 @@ fn run_unit_tests(options: &TestOptions,
             shell.status("Running", cmd.to_string())
         }));
 
-        if let Err(e) = ExecEngine::exec(&ProcessEngine, cmd) {
+        if let Err(e) = cmd.into_process_builder().exec() {
             errors.push(e);
             if !options.no_fail_fast {
                 break
@@ -175,7 +175,7 @@ fn run_doc_tests(options: &TestOptions,
             try!(config.shell().verbose(|shell| {
                 shell.status("Running", p.to_string())
             }));
-            if let Err(e) = ExecEngine::exec(&ProcessEngine, p) {
+            if let Err(e) = p.into_process_builder().exec() {
                 errors.push(e);
                 if !options.no_fail_fast {
                     return Ok(errors);

src/cargo/ops/mod.rs

@@ -5,7 +5,7 @@ pub use self::cargo_read_manifest::{read_manifest,read_package,read_packages};
 pub use self::cargo_rustc::{compile_targets, Compilation, Layout, Kind, Unit};
 pub use self::cargo_rustc::{Context, LayoutProxy};
 pub use self::cargo_rustc::{BuildOutput, BuildConfig, TargetConfig};
-pub use self::cargo_rustc::{CommandType, CommandPrototype, ExecEngine, ProcessEngine};
+pub use self::cargo_rustc::{CommandType, CommandPrototype};
 pub use self::cargo_run::run;
 pub use self::cargo_install::{install, install_list, uninstall};
 pub use self::cargo_new::{new, init, NewOptions, VersionControl};