Use cached build plan under workspace_mode if available to perform build

Because of how cached build plan works, under `workspace_mode` every member
package must be forcefully rebuilt to store the arguments Cargo would generate
for the target. This is required when only a certain member package will be
rebuilt initially, but changes made to another one will pull a rebuild of a
package whose rustc call wasn't cached.
Furthermore, sometimes user can quickly input changes and save, before any build
can be made. When this happens and there are no modified files, build plan will
return an empty job queue, since no files are dirty at that time. When that
happens, it delegates to Cargo, just when build scripts are modified. In the
future, detecting dirty files and guessing which files relate to which rebuilt
package is a concrete area for improvement.

Author: Xanewok

src/build/cargo.rs

@@ -227,6 +227,8 @@ impl RlsExecutor {
         }
     }
 
+    /// Returns wheter a given package is a primary one (every member of the
+    /// workspace is considered as such).
     fn is_primary_crate(&self, id: &PackageId) -> bool {
         if self.workspace_mode {
             self.member_packages.lock().unwrap().contains(id)
@@ -253,13 +255,9 @@ impl Executor for RlsExecutor {
     }
 
     fn force_rebuild(&self, unit: &Unit) -> bool {
-        // TODO: Currently workspace_mode doesn't use rustc, so it doesn't
-        // need args. When we start using rustc, we might consider doing
-        // force_rebuild to retrieve args for given package if they're stale/missing
-        if self.workspace_mode {
-            return false;
-        }
-
+        // In workspace_mode we need to force rebuild every package in the
+        // workspace, even if it's not dirty at a time, to cache compiler
+        // invocations in the build plan.
         // We only do a cargo build if we want to force rebuild the last
         // crate (e.g., because some args changed). Therefore we should
         // always force rebuild the primary crate.

src/build/mod.rs

@@ -30,9 +30,9 @@ use std::time::Duration;
 mod environment;
 mod cargo;
 mod rustc;
-pub mod plan;
+mod plan;
 
-use self::plan::Plan as BuildPlan;
+use self::plan::{Plan as BuildPlan, WorkStatus};
 
 /// Manages builds.
 ///
@@ -401,14 +401,27 @@ impl Internals {
         let needs_to_run_cargo = self.compilation_cx.lock().unwrap().args.is_empty();
         let workspace_mode = self.config.lock().unwrap().workspace_mode;
 
-        if workspace_mode || needs_to_run_cargo {
-            let result = cargo::cargo(self);
+        if workspace_mode {
+            // If the build plan has already been cached, use it, unless Cargo
+            // has to be specifically rerun (e.g. when build scripts changed)
+            let work = {
+                let cx = self.compilation_cx.lock().unwrap();
+                let modified: Vec<_> = self.vfs.get_changes().keys().cloned().collect();
 
-            match result {
-                BuildResult::Err => return BuildResult::Err,
-                _ if workspace_mode => return result,
-                _ => {},
+                cx.build_plan.prepare_work(&modified)
             };
+            return match work {
+                // In workspace_mode, cargo performs the full build and returns
+                // appropriate diagnostics/analysis data
+                WorkStatus::NeedsCargo => cargo::cargo(self),
+                WorkStatus::Execute(job_queue) => job_queue.execute(self),
+            };
+        // In single package mode Cargo needs to be run to cache args/envs for
+        // future rustc calls
+        } else if needs_to_run_cargo {
+            if let BuildResult::Err = cargo::cargo(self) {
+                return BuildResult::Err;
+            }
         }
 
         let compile_cx = self.compilation_cx.lock().unwrap();

src/build/plan.rs

@@ -32,6 +32,8 @@ use cargo::core::{PackageId, Profile, Target, TargetKind};
 use cargo::ops::{Kind, Unit, Context};
 use cargo::util::{CargoResult, ProcessBuilder};
 
+use super::{BuildResult, Internals};
+
 /// Main key type by which `Unit`s will be distinguished in the build plan.
 pub type UnitKey = (PackageId, TargetKind);
 /// Holds the information how exactly the build will be performed for a given
@@ -61,6 +63,8 @@ impl Plan {
         *self = Plan::new();
     }
 
+    /// Returns whether a build plan has cached compiler invocations and dep
+    /// graph so it's at all able to return a job queue via `prepare_work`.
     pub fn is_ready(&self) -> bool {
         self.compiler_jobs.is_empty() == false
     }
@@ -229,6 +233,8 @@ impl Plan {
     }
 
     pub fn prepare_work<T: AsRef<Path> + fmt::Debug>(&self, modified: &[T]) -> WorkStatus {
+        if self.is_ready() == false { return WorkStatus::NeedsCargo; }
+
         let dirties = self.fetch_dirty_units(modified);
         trace!("fetch_dirty_units: for files {:?}, these units are dirty: {:?}", modified, dirties);
 
@@ -240,11 +246,15 @@ impl Plan {
 
             let queue = self.topological_sort(&graph);
             trace!("Topologically sorted dirty graph: {:?}", queue);
-            let jobs = queue.iter()
+            let jobs: Vec<_> = queue.iter()
                 .map(|x| self.compiler_jobs.get(x).unwrap().clone())
                 .collect();
 
-            WorkStatus::Execute(JobQueue(jobs))
+            if jobs.is_empty() {
+                WorkStatus::NeedsCargo
+            } else {
+                WorkStatus::Execute(JobQueue(jobs))
+            }
         }
     }
 }
@@ -260,6 +270,45 @@ impl JobQueue {
     pub fn dequeue(&mut self) -> Option<ProcessBuilder> {
         self.0.pop()
     }
+
+    /// Performs a rustc build using cached compiler invocations.
+    pub(super) fn execute(mut self, internals: &Internals) -> BuildResult {
+        // TODO: In case of an empty job queue we shouldn't be here, since the
+        // returned results will replace currently held diagnostics/analyses.
+        // Either allow to return a BuildResult::Squashed here or just delegate
+        // to Cargo (which we do currently) in `prepare_work`
+        assert!(self.0.is_empty() == false);
+
+        let build_dir = internals.compilation_cx.lock().unwrap().build_dir.clone().unwrap();
+
+        let mut compiler_messages = vec![];
+        let mut analyses = vec![];
+        // Go through cached compiler invocations sequentially, collecting each
+        // invocation's compiler messages for diagnostics and analysis data
+        while let Some(job) = self.dequeue() {
+            trace!("Executing: {:?}", job);
+            let mut args: Vec<_> = job.get_args().iter().cloned()
+                .map(|x| x.into_string().unwrap()).collect();
+            // TODO: is it crucial to pass *actual* executed program (e.g. rustc
+            // shim) or does the compiler API just care about the order and not
+            // which exact binary was run (1st argument)?
+            args.insert(0, "rustc".to_owned());
+
+            match super::rustc::rustc(&internals.vfs, &args, job.get_envs(),
+                                      &build_dir, internals.config.clone(),
+                                      internals.env_lock.as_facade()) {
+                BuildResult::Success(mut messages, mut analysis) |
+                BuildResult::Failure(mut messages, mut analysis) => {
+                    compiler_messages.append(&mut messages);
+                    analyses.append(&mut analysis);
+                },
+                BuildResult::Err => { return BuildResult:: Err },
+                _ => {}
+            }
+        }
+        // TODO: Should we combine with ::Failure? What's the difference between those two?
+        return BuildResult::Success(compiler_messages, analyses);
+    }
 }
 
 fn key_from_unit(unit: &Unit) -> UnitKey {