Merge pull request #456 from Xanewok/simplify-process-builder

Simplify `ProcessBuilder` modification

Author: nrc

src/build/cargo.rs

@@ -8,10 +8,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use cargo::core::{PackageId, Shell, Target, Workspace, Verbosity};
+use cargo::core::{PackageId, Shell, Target, TargetKind, Workspace, Verbosity};
 use cargo::ops::{compile_with_exec, Executor, Context, Packages, CompileOptions, CompileMode, CompileFilter, Unit};
-use cargo::util::{Config as CargoConfig, process_builder, ProcessBuilder, homedir, important_paths, ConfigValue, CargoResult};
-use cargo::util::errors::{CargoErrorKind, process_error};
+use cargo::util::{Config as CargoConfig, ProcessBuilder, homedir, important_paths, ConfigValue, CargoResult};
 use serde_json;
 
 use data::Analysis;
@@ -296,38 +295,42 @@ impl Executor for RlsExecutor {
             }
         }
 
-        let rustc_exe = env::var("RUSTC").unwrap_or(env::args().next().unwrap());
-        let mut cmd = Command::new(&rustc_exe);
+        // Prepare our own call to `rustc` as follows:
+        // 1. Use $RUSTC wrapper if specified, otherwise use RLS executable
+        //    as an rustc shim (needed to distribute via the stable channel)
+        // 2. For non-primary packages or build scripts, execute the call
+        // 3. Otherwise, we'll want to use the compilation to drive the analysis:
+        //    i.  Modify arguments to account for the RLS settings (e.g.
+        //        compiling under cfg(test) mode or passing a custom sysroot)
+        //    ii. Execute the call and store the final args/envs to be used for
+        //        later in-process execution of the compiler
+        let mut cmd = cargo_cmd.clone();
+        let rls_executable = env::args().next().unwrap();
+        cmd.program(env::var("RUSTC").unwrap_or(rls_executable));
         cmd.env(::RUSTC_SHIM_ENV_VAR_NAME, "1");
 
         // We only want to intercept rustc call targeting current crate to cache
         // args/envs generated by cargo so we can run only rustc later ourselves
         // Currently we don't cache nor modify build script args
-        let is_build_script = crate_name == "build_script_build";
+        let is_build_script = *target.kind() == TargetKind::CustomBuild;
         if !self.is_primary_crate(id) || is_build_script {
-            let mut cmd = Command::new(&env::var("RUSTC").unwrap_or("rustc".to_owned()));
             let build_script_notice = if is_build_script {" (build script)"} else {""};
             trace!("rustc not intercepted - {}{}", id.name(), build_script_notice);
 
+            // Recreate the original command, minus -Zsave-analysis. Since the
+            // shim sets it internally, be sure not to use it.
             if ::CRATE_BLACKLIST.contains(&&*crate_name) {
-                // Recreate the command, minus -Zsave-analysis.
-                for a in cargo_args {
-                    if a != "-Zsave-analysis" {
-                        cmd.arg(a);
-                    }
-                }
-                cmd.envs(cargo_cmd.get_envs().iter().filter_map(|(k, v)| v.as_ref().map(|v| (k, v))));
-                if let Some(cwd) = cargo_cmd.get_cwd() {
-                    cmd.current_dir(cwd);
-                }
-                return match cmd.status() {
-                    Ok(ref e) if e.success() => Ok(()),
-                    _ => Err(CargoErrorKind::ProcessErrorKind(process_error(
-                                "process didn't exit successfully", None, None)).into()),
-                };
-            } else {
+                let mut cargo_cmd = cargo_cmd.clone();
+                let args: Vec<_> = cargo_cmd.get_args().iter().cloned()
+                    .filter(|x| x != "Zsave-analysis").collect();
+                cargo_cmd.args_replace(&args);
+
                 return cargo_cmd.exec();
             }
+            // TODO: Make sure we don't pass any unstable options (incl. -Zsave-analysis)
+            // to the shim. For stable toolchains it won't accept those as arguments,
+            // but rather it sets them internally instead to work around that
+            return cmd.exec();
         }
 
         trace!("rustc intercepted - args: {:?} envs: {:?}", cargo_args, cargo_cmd.get_envs());
@@ -370,43 +373,28 @@ impl Executor for RlsExecutor {
             // so the dep-info is ready by the time we return from this callback.
             // NB: In `workspace_mode` regular compilation is performed here (and we don't
             // only calculate dep-info) so it should fix the problem mentioned above.
-            // Since ProcessBuilder doesn't allow to modify args, we need to create
-            // our own command here from scratch here.
-            for a in &args {
-                // Emitting only dep-info is possible only for final crate type, as
-                // as others may emit required metadata for dependent crate types
-                if a.starts_with("--emit") && is_final_crate_type && !self.workspace_mode {
-                    cmd.arg("--emit=dep-info");
-                } else if a != "-Zsave-analysis" {
-                    cmd.arg(a);
-                }
-            }
-            cmd.envs(cargo_cmd.get_envs().iter().filter_map(|(k, v)| v.as_ref().map(|v| (k, v))));
-            if let Some(cwd) = cargo_cmd.get_cwd() {
-                cmd.current_dir(cwd);
-            }
+            let modified = args.iter()
+                .filter(|a| *a != "-Zsave-analysis")
+                .map(|a| {
+                    // Emitting only dep-info is possible only for final crate type, as
+                    // as others may emit required metadata for dependent crate types
+                    if a.starts_with("--emit") && is_final_crate_type && !self.workspace_mode {
+                        "--emit=dep-info"
+                    } else { a }
+                })
+                .collect::<Vec<_>>();
+            cmd.args_replace(&modified);
         }
 
         // Cache executed command for the build plan
         {
             let mut cx = self.compilation_cx.lock().unwrap();
-
-            let mut store_cmd = process_builder::process(&rustc_exe);
-            store_cmd.args(&args.iter().map(OsString::from).collect::<Vec<_>>());
-            for (k, v) in cargo_cmd.get_envs().clone() {
-                if let Some(v) = v {
-                    store_cmd.env(&k, v);
-                }
-            }
-            if let Some(cwd) = cargo_cmd.get_cwd() {
-                cmd.current_dir(cwd);
-            }
-
-            cx.build_plan.cache_compiler_job(id, target, &store_cmd);
+            cx.build_plan.cache_compiler_job(id, target, &cmd);
         }
 
         // Prepare modified cargo-generated args/envs for future rustc calls
-        args.insert(0, rustc_exe);
+        let rustc = cargo_cmd.get_program().to_owned().into_string().unwrap();
+        args.insert(0, rustc);
         let envs = cargo_cmd.get_envs().clone();
 
         if self.workspace_mode {
@@ -426,7 +414,7 @@ impl Executor for RlsExecutor {
                 _ => {}
             }
         } else {
-            cmd.status().expect("Couldn't execute rustc");
+            cmd.exec()?;
         }
 
         // Finally, store the modified cargo-generated args/envs for future rustc calls