impl centralized RustcWrapper logic

yaahc · yaahc · commit 0389edc69bcb · 2019-03-29T17:40:34.000-07:00
diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs
@@ -48,7 +48,7 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"
     }
 
     if let Some(code) = args.value_of("explain") {
-        let mut procss = config.rustc(None)?.process();
+        let mut procss = config.load_global_rustc(None)?.process();
         procss.arg("--explain").arg(code).exec()?;
         return Ok(());
     }
@@ -236,4 +236,3 @@ See 'cargo help <command>' for more information on a specific command.\n",
         )
         .subcommands(commands::builtin())
 }
-
diff --git a/src/bin/cargo/commands/clippy.rs b/src/bin/cargo/commands/clippy.rs
@@ -56,11 +56,11 @@ You can use tool lints to allow or deny lints from your code, eg.:
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
-    config.set_clippy_override(true);
     let ws = args.workspace(config)?;
 
     let mode = CompileMode::Check { test: false };
-    let compile_opts = args.compile_options(config, mode, Some(&ws))?;
+    let mut compile_opts = args.compile_options(config, mode, Some(&ws))?;
+    compile_opts.build_config.set_clippy_override(true);
 
     if !config.cli_unstable().unstable_options {
         return Err(failure::format_err!(
diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs
@@ -30,6 +30,8 @@ pub struct BuildConfig {
     /// Extra args to inject into rustc commands.
     pub extra_rustc_args: Vec<String>,
     pub rustfix_diagnostic_server: RefCell<Option<RustfixDiagnosticServer>>,
+    /// Use `clippy-driver` instead of `rustc`
+    pub clippy_override: bool,
 }
 
 impl BuildConfig {
@@ -102,6 +104,7 @@ impl BuildConfig {
             extra_rustc_env: Vec::new(),
             extra_rustc_args: Vec::new(),
             rustfix_diagnostic_server: RefCell::new(None),
+            clippy_override: false,
         })
     }
 
@@ -112,6 +115,11 @@ impl BuildConfig {
     pub fn test(&self) -> bool {
         self.mode == CompileMode::Test || self.mode == CompileMode::Bench
     }
+
+    /// Sets the clippy override. If this is true, clippy-driver is invoked instead of rustc.
+    pub fn set_clippy_override(&mut self, val: bool) {
+        self.clippy_override = val;
+    }
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs
@@ -9,6 +9,8 @@ use crate::core::profiles::Profiles;
 use crate::core::{Dependency, Workspace};
 use crate::core::{PackageId, PackageSet, Resolve};
 use crate::util::errors::CargoResult;
+use crate::util::paths;
+use crate::util::rustc::RustcWrapper;
 use crate::util::{profile, Cfg, CfgExpr, Config, Rustc};
 
 use super::{BuildConfig, BuildOutput, Kind, Unit};
@@ -50,7 +52,30 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
         profiles: &'a Profiles,
         extra_compiler_args: HashMap<Unit<'a>, Vec<String>>,
     ) -> CargoResult<BuildContext<'a, 'cfg>> {
-        let rustc = config.rustc(Some(ws))?;
+        let mut rustc = config.load_global_rustc(Some(ws))?;
+
+        if build_config.clippy_override {
+            let tool = config.get_tool("clippy-driver")?;
+            let tool = paths::resolve_executable(&tool).map_err(|e| {
+                let rustup_in_path = config
+                    .get_tool("rustup")
+                    .and_then(|tool| paths::resolve_executable(&tool))
+                    .is_ok();
+                let has_rustup_env = std::env::var("RUSTUP_TOOLCHAIN").is_ok();
+                if rustup_in_path || has_rustup_env {
+                    failure::format_err!("{}: please run `rustup component add clippy`", e)
+                } else {
+                    failure::format_err!("{}: please install clippy", e)
+                }
+            })?;
+            rustc.push_wrapper(RustcWrapper::new(tool));
+        } else if build_config.cargo_as_rustc_wrapper {
+            let mut wrapper = RustcWrapper::new(env::current_exe()?);
+            let prog = rustc.path.as_os_str().to_owned();
+            wrapper.env("RUSTC", prog);
+            rustc.push_wrapper(wrapper);
+        }
+
         let host_config = TargetConfig::new(config, &rustc.host)?;
         let target_config = match build_config.requested_target.as_ref() {
             Some(triple) => TargetConfig::new(config, triple)?,
diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs
@@ -75,20 +75,8 @@ pub struct Compilation<'cfg> {
 
 impl<'cfg> Compilation<'cfg> {
     pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> CargoResult<Compilation<'cfg>> {
-        // If we're using cargo as a rustc wrapper then we're in a situation
-        // like `cargo fix`. For now just disregard the `RUSTC_WRAPPER` env var
-        // (which is typically set to `sccache` for now). Eventually we'll
-        // probably want to implement `RUSTC_WRAPPER` for `cargo fix`, but we'll
-        // leave that open as a bug for now.
-        let mut rustc = if bcx.build_config.cargo_as_rustc_wrapper {
-            let mut rustc = bcx.rustc.process_no_wrapper();
-            let prog = rustc.get_program().to_owned();
-            rustc.env("RUSTC", prog);
-            rustc.program(env::current_exe()?);
-            rustc
-        } else {
-            bcx.rustc.process()
-        };
+        let mut rustc = bcx.rustc.process();
+
         if bcx.config.extra_verbose() {
             rustc.display_env_vars();
         }
diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs
@@ -22,13 +22,6 @@ use log::debug;
 use same_file::is_same_file;
 use serde::Serialize;
 
-use crate::core::manifest::TargetSourcePath;
-use crate::core::profiles::{Lto, PanicStrategy, Profile};
-use crate::core::{PackageId, Target};
-use crate::util::errors::{CargoResult, CargoResultExt, Internal, ProcessError};
-use crate::util::paths;
-use crate::util::{self, machine_message, process, Freshness, ProcessBuilder};
-use crate::util::{internal, join_paths, profile};
 pub use self::build_config::{BuildConfig, CompileMode, MessageFormat};
 pub use self::build_context::{BuildContext, FileFlavor, TargetConfig, TargetInfo};
 use self::build_plan::BuildPlan;
@@ -39,6 +32,13 @@ use self::job::{Job, Work};
 use self::job_queue::JobQueue;
 pub use self::layout::is_bad_artifact_name;
 use self::output_depinfo::output_depinfo;
+use crate::core::manifest::TargetSourcePath;
+use crate::core::profiles::{Lto, PanicStrategy, Profile};
+use crate::core::{PackageId, Target};
+use crate::util::errors::{CargoResult, CargoResultExt, Internal, ProcessError};
+use crate::util::paths;
+use crate::util::{self, machine_message, process, Freshness, ProcessBuilder};
+use crate::util::{internal, join_paths, profile};
 
 /// Indicates whether an object is for the host architcture or the target architecture.
 ///
diff --git a/src/cargo/ops/cargo_fetch.rs b/src/cargo/ops/cargo_fetch.rs
@@ -21,7 +21,7 @@ pub fn fetch<'a>(
     let jobs = Some(1);
     let config = ws.config();
     let build_config = BuildConfig::new(config, jobs, &options.target, CompileMode::Build)?;
-    let rustc = config.rustc(Some(ws))?;
+    let rustc = config.load_global_rustc(Some(ws))?;
     let target_info =
         TargetInfo::new(config, &build_config.requested_target, &rustc, Kind::Target)?;
     {
diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs
@@ -74,8 +74,6 @@ pub struct Config {
     env: HashMap<String, String>,
     /// Profiles loaded from config.
     profiles: LazyCell<ConfigProfiles>,
-    /// Use `clippy-driver` instead of `rustc`
-    clippy_override: bool,
 }
 
 impl Config {
@@ -131,7 +129,6 @@ impl Config {
             target_dir: None,
             env,
             profiles: LazyCell::new(),
-            clippy_override: false,
         }
     }
 
@@ -193,36 +190,14 @@ impl Config {
             .map(AsRef::as_ref)
     }
 
-    /// Sets the clippy override. If this is true, clippy-driver is invoked instead of rustc.
-    pub fn set_clippy_override(&mut self, val: bool) {
-        self.clippy_override = val;
-    }
-
     /// Gets the path to the `rustc` executable.
-    pub fn rustc(&self, ws: Option<&Workspace<'_>>) -> CargoResult<Rustc> {
+    pub fn load_global_rustc(&self, ws: Option<&Workspace<'_>>) -> CargoResult<Rustc> {
         let cache_location = ws.map(|ws| {
             ws.target_dir()
                 .join(".rustc_info.json")
                 .into_path_unlocked()
         });
-        let wrapper = if self.clippy_override {
-            let tool = self.get_tool("clippy-driver")?;
-            let tool = paths::resolve_executable(&tool).map_err(|e| {
-                let rustup_in_path = self
-                    .get_tool("rustup")
-                    .and_then(|tool| paths::resolve_executable(&tool))
-                    .is_ok();
-                let has_rustup_env = std::env::var("RUSTUP_TOOLCHAIN").is_ok();
-                if rustup_in_path || has_rustup_env {
-                    failure::format_err!("{}: please run `rustup component add clippy`", e)
-                } else {
-                    failure::format_err!("{}: please install clippy", e)
-                }
-            })?;
-            Some(tool)
-        } else {
-            self.maybe_get_tool("rustc-wrapper")?
-        };
+        let wrapper = self.maybe_get_tool("rustc-wrapper")?;
         Rustc::new(
             self.get_tool("rustc")?,
             wrapper,
@@ -782,7 +757,7 @@ impl Config {
 
     /// Looks for a path for `tool` in an environment variable or config path, defaulting to `tool`
     /// as a path.
-    fn get_tool(&self, tool: &str) -> CargoResult<PathBuf> {
+    pub fn get_tool(&self, tool: &str) -> CargoResult<PathBuf> {
         self.maybe_get_tool(tool)
             .map(|t| t.unwrap_or_else(|| PathBuf::from(tool)))
     }
diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs
@@ -47,7 +47,7 @@ pub mod process_builder;
 pub mod profile;
 mod progress;
 mod read2;
-mod rustc;
+pub mod rustc;
 mod sha256;
 pub mod to_semver;
 pub mod to_url;
diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs
@@ -80,10 +80,7 @@ impl ProcessBuilder {
 
     /// (chainable) Replaces the args list with the given `args`.
     pub fn args_replace<T: AsRef<OsStr>>(&mut self, args: &[T]) -> &mut ProcessBuilder {
-        self.args = args
-            .iter()
-            .map(|t| t.as_ref().to_os_string())
-            .collect();
+        self.args = args.iter().map(|t| t.as_ref().to_os_string()).collect();
         self
     }
 
diff --git a/src/cargo/util/rustc.rs b/src/cargo/util/rustc.rs
@@ -2,6 +2,7 @@
 
 use std::collections::hash_map::{Entry, HashMap};
 use std::env;
+use std::ffi::{OsStr, OsString};
 use std::hash::{Hash, Hasher, SipHasher};
 use std::path::{Path, PathBuf};
 use std::process::Stdio;
@@ -20,14 +21,64 @@ pub struct Rustc {
     pub path: PathBuf,
     /// An optional program that will be passed the path of the rust exe as its first argument, and
     /// rustc args following this.
-    pub wrapper: Option<PathBuf>,
+    pub wrapper: Option<RustcWrapper>,
     /// Verbose version information (the output of `rustc -vV`)
     pub verbose_version: String,
     /// The host triple (arch-platform-OS), this comes from verbose_version.
     pub host: String,
     cache: Mutex<Cache>,
 }
 
+/// Information on the `rustc` wrapper
+#[derive(Debug)]
+pub struct RustcWrapper {
+    path: PathBuf,
+    env: HashMap<String, Option<OsString>>,
+}
+
+impl RustcWrapper {
+    pub fn new<T: Into<PathBuf>>(path: T) -> RustcWrapper {
+        RustcWrapper {
+            path: path.into(),
+            env: HashMap::new(),
+        }
+    }
+
+    /// (chainable) Sets an environment variable for the process.
+    pub fn env<T: AsRef<OsStr>>(&mut self, key: &str, val: T) -> &mut RustcWrapper {
+        self.env
+            .insert(key.to_string(), Some(val.as_ref().to_os_string()));
+        self
+    }
+
+    /// (chainable) Unsets an environment variable for the process.
+    pub fn env_remove(&mut self, key: &str) -> &mut RustcWrapper {
+        self.env.insert(key.to_string(), None);
+        self
+    }
+
+    pub fn process(&self) -> ProcessBuilder {
+        let mut cmd = util::process(&self.path);
+
+        for (k, v) in &self.env {
+            match *v {
+                Some(ref v) => {
+                    cmd.env(k, v);
+                }
+                None => {
+                    cmd.env_remove(k);
+                }
+            }
+        }
+
+        cmd
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.path.as_os_str().is_empty()
+    }
+}
+
 impl Rustc {
     /// Runs the compiler at `path` to learn various pieces of information about
     /// it, with an optional wrapper.
@@ -59,7 +110,7 @@ impl Rustc {
 
         Ok(Rustc {
             path,
-            wrapper,
+            wrapper: wrapper.map(|w| RustcWrapper::new(w)),
             verbose_version,
             host,
             cache: Mutex::new(cache),
@@ -69,8 +120,8 @@ impl Rustc {
     /// Gets a process builder set up to use the found rustc version, with a wrapper if `Some`.
     pub fn process(&self) -> ProcessBuilder {
         match self.wrapper {
-            Some(ref wrapper) if !wrapper.as_os_str().is_empty() => {
-                let mut cmd = util::process(wrapper);
+            Some(ref wrapper) if !wrapper.is_empty() => {
+                let mut cmd = wrapper.process();
                 cmd.arg(&self.path);
                 cmd
             }
@@ -89,6 +140,10 @@ impl Rustc {
     pub fn cached_success(&self, cmd: &ProcessBuilder) -> CargoResult<bool> {
         self.cache.lock().unwrap().cached_success(cmd)
     }
+
+    pub fn push_wrapper(&mut self, wrapper: RustcWrapper) {
+        self.wrapper = Some(wrapper);
+    }
 }
 
 /// It is a well known that `rustc` is not the fastest compiler in the world.

Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`if let Some(code) = args.value_of("explain") {`
`51`		`- let mut procss = config.rustc(None)?.process();`
	`51`	`+ let mut procss = config.load_global_rustc(None)?.process();`
`52`	`52`	`procss.arg("--explain").arg(code).exec()?;`
`53`	`53`	`return Ok(());`
`54`	`54`	`}`
`@@ -236,4 +236,3 @@ See 'cargo help <command>' for more information on a specific command.\n",`
`236`	`236`	`)`
`237`	`237`	`.subcommands(commands::builtin())`
`238`	`238`	`}`
`239`		`-`
Original file line number	Diff line number	Diff line change
`@@ -30,6 +30,8 @@ pub struct BuildConfig {`
`30`	`30`	`/// Extra args to inject into rustc commands.`
`31`	`31`	`pub extra_rustc_args: Vec<String>,`
`32`	`32`	`pub rustfix_diagnostic_server: RefCell<Option<RustfixDiagnosticServer>>,`
	`33`	+ /// Use `clippy-driver` instead of `rustc`
	`34`	`+ pub clippy_override: bool,`
`33`	`35`	`}`
`34`	`36`
`35`	`37`	`impl BuildConfig {`
`@@ -102,6 +104,7 @@ impl BuildConfig {`
`102`	`104`	`extra_rustc_env: Vec::new(),`
`103`	`105`	`extra_rustc_args: Vec::new(),`
`104`	`106`	`rustfix_diagnostic_server: RefCell::new(None),`
	`107`	`+ clippy_override: false,`
`105`	`108`	`})`
`106`	`109`	`}`
`107`	`110`
`@@ -112,6 +115,11 @@ impl BuildConfig {`
`112`	`115`	`pub fn test(&self) -> bool {`
`113`	`116`	`self.mode == CompileMode::Test \|\| self.mode == CompileMode::Bench`
`114`	`117`	`}`
	`118`	`+`
	`119`	`+ /// Sets the clippy override. If this is true, clippy-driver is invoked instead of rustc.`
	`120`	`+ pub fn set_clippy_override(&mut self, val: bool) {`
	`121`	`+ self.clippy_override = val;`
	`122`	`+ }`
`115`	`123`	`}`
`116`	`124`
`117`	`125`	`#[derive(Clone, Copy, Debug, PartialEq, Eq)]`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ pub fn fetch<'a>(`
`21`	`21`	`let jobs = Some(1);`
`22`	`22`	`let config = ws.config();`
`23`	`23`	`let build_config = BuildConfig::new(config, jobs, &options.target, CompileMode::Build)?;`
`24`		`- let rustc = config.rustc(Some(ws))?;`
	`24`	`+ let rustc = config.load_global_rustc(Some(ws))?;`
`25`	`25`	`let target_info =`
`26`	`26`	`TargetInfo::new(config, &build_config.requested_target, &rustc, Kind::Target)?;`
`27`	`27`	`{`