Auto merge of #3198 - matklad:kill-command-type, r=alexcrichton

Remove CommandType struct

This removes `CommandType` struct as well as `cargo_rustc::process` function. So now all process creation goes thorough methods of `Compilation`.

This does change search path order from `util::dylib_path(), host_dylib_path()` to `host_dylib_path(), util::dylib_path()`, but I hope this is not a problem.

This also uncovers the fact that `rustdoc` is run sometimes with and sometimes without `host_dylib_path`. Is this intentional?

src/cargo/ops/cargo_rustc/compilation.rs

@@ -1,10 +1,10 @@
 use std::collections::{HashMap, HashSet};
-use std::ffi::{OsStr, OsString};
+use std::ffi::OsStr;
 use std::path::PathBuf;
 use semver::Version;
 
 use core::{PackageId, Package, Target};
-use util::{self, CargoResult, Config, ProcessBuilder, process};
+use util::{self, CargoResult, Config, ProcessBuilder, process, join_paths};
 
 /// A structure returning the result of a compilation.
 pub struct Compilation<'cfg> {
@@ -33,6 +33,10 @@ pub struct Compilation<'cfg> {
  /// Output directory for rust dependencies
  pub deps_output: PathBuf,
 
+ /// Library search path for compiler plugins and build scripts
+ /// which have dynamic dependencies.
+ pub plugins_dylib_path: PathBuf,
+
  /// Extra environment variables that were passed to compilations and should
  /// be passed to future invocations of programs.
  pub extra_env: HashMap<PackageId, Vec<(String, String)>>,
@@ -47,25 +51,14 @@ pub struct Compilation<'cfg> {
  config: &'cfg Config,
 }
 
-#[derive(Clone, Debug)]
-pub enum CommandType {
- Rustc,
- Rustdoc,
-
- /// The command is to be executed for the target architecture.
- Target(OsString),
-
- /// The command is to be executed for the host architecture.
- Host(OsString),
-}
-
 impl<'cfg> Compilation<'cfg> {
  pub fn new(config: &'cfg Config) -> Compilation<'cfg> {
  Compilation {
  libraries: HashMap::new(),
  native_dirs: HashSet::new(), // TODO: deprecated, remove
  root_output: PathBuf::from("/"),
  deps_output: PathBuf::from("/"),
+ plugins_dylib_path: PathBuf::from("/"),
  tests: Vec::new(),
  binaries: Vec::new(),
  extra_env: HashMap::new(),
@@ -78,65 +71,65 @@ impl<'cfg> Compilation<'cfg> {
 
  /// See `process`.
  pub fn rustc_process(&self, pkg: &Package) -> CargoResult<ProcessBuilder> {
- self.process(CommandType::Rustc, pkg)
+ self.fill_env(try!(self.config.rustc()).process(), pkg, true)
  }
 
  /// See `process`.
- pub fn rustdoc_process(&self, pkg: &Package)
- -> CargoResult<ProcessBuilder> {
- self.process(CommandType::Rustdoc, pkg)
+ pub fn rustdoc_process(&self, pkg: &Package) -> CargoResult<ProcessBuilder> {
+ self.fill_env(process(&*try!(self.config.rustdoc())), pkg, true)
  }
 
  /// See `process`.
- pub fn target_process<T: AsRef<OsStr>>(&self, cmd: T, pkg: &Package)
-  -> CargoResult<ProcessBuilder> {
- self.process(CommandType::Target(cmd.as_ref().to_os_string()), pkg)
+ pub fn host_process<T: AsRef<OsStr>>(&self, cmd: T, pkg: &Package)
+ -> CargoResult<ProcessBuilder> {
+ self.fill_env(process(cmd), pkg, true)
  }
 
  /// See `process`.
- pub fn host_process<T: AsRef<OsStr>>(&self, cmd: T, pkg: &Package)
- -> CargoResult<ProcessBuilder> {
- self.process(CommandType::Host(cmd.as_ref().to_os_string()), pkg)
+ pub fn target_process<T: AsRef<OsStr>>(&self, cmd: T, pkg: &Package)
+  -> CargoResult<ProcessBuilder> {
+ self.fill_env(process(cmd), pkg, false)
  }
 
  /// Prepares a new process with an appropriate environment to run against
  /// the artifacts produced by the build process.
  ///
  /// The package argument is also used to configure environment variables as
  /// well as the working directory of the child process.
- pub fn process(&self, cmd: CommandType, pkg: &Package)
- -> CargoResult<ProcessBuilder> {
- let mut search_path = vec![];
-
- // Add -L arguments, after stripping off prefixes like "native=" or "framework=".
- for dir in self.native_dirs.iter() {
- let dir = match dir.to_str() {
- Some(s) => {
- let mut parts = s.splitn(2, '=');
- match (parts.next(), parts.next()) {
- (Some("native"), Some(path)) |
- (Some("crate"), Some(path)) |
- (Some("dependency"), Some(path)) |
- (Some("framework"), Some(path)) |
- (Some("all"), Some(path)) => path.into(),
- _ => dir.clone(),
+ fn fill_env(&self, mut cmd: ProcessBuilder, pkg: &Package, is_host: bool)
+ -> CargoResult<ProcessBuilder> {
+
+ let mut search_path = if is_host {
+ vec![self.plugins_dylib_path.clone()]
+ } else {
+ let mut search_path = vec![];
+
+ // Add -L arguments, after stripping off prefixes like "native=" or "framework=".
+ for dir in self.native_dirs.iter() {
+ let dir = match dir.to_str() {
+ Some(s) => {
+ let mut parts = s.splitn(2, '=');
+ match (parts.next(), parts.next()) {
+ (Some("native"), Some(path)) |
+ (Some("crate"), Some(path)) |
+ (Some("dependency"), Some(path)) |
+ (Some("framework"), Some(path)) |
+ (Some("all"), Some(path)) => path.into(),
+ _ => dir.clone(),
+ }
  }
- }
- None => dir.clone(),
- };
- search_path.push(dir);
- }
- search_path.push(self.root_output.clone());
- search_path.push(self.deps_output.clone());
- search_path.extend(util::dylib_path().into_iter());
- let search_path = try!(util::join_paths(&search_path,
- util::dylib_path_envvar()));
- let mut cmd = match cmd {
- CommandType::Rustc => try!(self.config.rustc()).process(),
- CommandType::Rustdoc => process(&*try!(self.config.rustdoc())),
- CommandType::Target(ref s) |
- CommandType::Host(ref s) => process(s),
+ None => dir.clone(),
+ };
+ search_path.push(dir);
+ }
+ search_path.push(self.root_output.clone());
+ search_path.push(self.deps_output.clone());
+ search_path
  };
+
+ search_path.extend(util::dylib_path().into_iter());
+ let search_path = try!(join_paths(&search_path, util::dylib_path_envvar()));
+
  cmd.env(util::dylib_path_envvar(), &search_path);
  if let Some(env) = self.extra_env.get(pkg.package_id()) {
  for &(ref k, ref v) in env {

src/cargo/ops/cargo_rustc/context.rs

@@ -110,6 +110,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
  None => {}
  }
 
+ self.compilation.plugins_dylib_path = self.host.deps().to_path_buf();
+
  let layout = self.target.as_ref().unwrap_or(&self.host);
  self.compilation.root_output = layout.dest().to_path_buf();
  self.compilation.deps_output = layout.deps().to_path_buf();
@@ -280,11 +282,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
  }
  }
 
- /// Returns the path for plugin/dylib dependencies
- pub fn host_dylib_path(&self) -> &Path {
- self.host.deps()
- }
-
  /// Returns the appropriate output directory for the specified package and
  /// target.
  pub fn out_dir(&self, unit: &Unit) -> PathBuf {

src/cargo/ops/cargo_rustc/custom_build.rs

@@ -10,7 +10,6 @@ use util::{internal, ChainError, profile, paths};
 
 use super::job::Work;
 use super::{fingerprint, Kind, Context, Unit};
-use super::compilation::CommandType;
 
 /// Contains the parsed output of a custom build script.
 #[derive(Clone, Debug, Hash)]
@@ -98,7 +97,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
  // package's library profile.
  let profile = cx.lib_profile(unit.pkg.package_id());
  let to_exec = to_exec.into_os_string();
- let mut cmd = try!(super::process(CommandType::Host(to_exec), unit.pkg, cx));
+ let mut cmd = try!(cx.compilation.host_process(to_exec, unit.pkg));
  cmd.env("OUT_DIR", &build_output)
  .env("CARGO_MANIFEST_DIR", unit.pkg.root())
  .env("NUM_JOBS", &cx.jobs().to_string())

src/cargo/ops/cargo_rustc/mod.rs

@@ -16,7 +16,7 @@ use util::{Config, internal, ChainError, profile, join_paths, short_hash};
 use self::job::{Job, Work};
 use self::job_queue::JobQueue;
 
-pub use self::compilation::{Compilation, CommandType};
+pub use self::compilation::Compilation;
 pub use self::context::{Context, Unit};
 pub use self::layout::{Layout, LayoutProxy};
 pub use self::custom_build::{BuildOutput, BuildMap, BuildScripts};
@@ -415,7 +415,7 @@ fn add_plugin_deps(rustc: &mut ProcessBuilder,
 fn prepare_rustc(cx: &Context,
  crate_types: Vec<&str>,
  unit: &Unit) -> CargoResult<ProcessBuilder> {
- let mut base = try!(process(CommandType::Rustc, unit.pkg, cx));
+ let mut base = try!(cx.compilation.rustc_process(unit.pkg));
  build_base_args(cx, &mut base, unit, &crate_types);
  build_plugin_args(&mut base, cx, unit);
  try!(build_deps_args(&mut base, cx, unit));
@@ -424,7 +424,7 @@ fn prepare_rustc(cx: &Context,
 
 
 fn rustdoc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
- let mut rustdoc = try!(process(CommandType::Rustdoc, unit.pkg, cx));
+ let mut rustdoc = try!(cx.compilation.rustdoc_process(unit.pkg));
  rustdoc.arg(&root_path(cx, unit))
  .cwd(cx.config.cwd())
  .arg("--crate-name").arg(&unit.target.crate_name());
@@ -673,21 +673,6 @@ fn build_deps_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit)
  }
 }
 
-pub fn process(cmd: CommandType, pkg: &Package,
- cx: &Context) -> CargoResult<ProcessBuilder> {
- // When invoking a tool, we need the *host* deps directory in the dynamic
- // library search path for plugins and such which have dynamic dependencies.
- let mut search_path = util::dylib_path();
- search_path.push(cx.host_dylib_path().to_path_buf());
-
- // We want to use the same environment and such as normal processes, but we
- // want to override the dylib search path with the one we just calculated.
- let search_path = try!(join_paths(&search_path, util::dylib_path_envvar()));
- let mut cmd = try!(cx.compilation.process(cmd, pkg));
- cmd.env(util::dylib_path_envvar(), &search_path);
- Ok(cmd)
-}
-
 fn envify(s: &str) -> String {
  s.chars()
  .flat_map(|c| c.to_uppercase())