Merge pull request #731 from rust-lang-nursery/packages

 Autodetect the package currently being edited

Author: Xanewok

Cargo.lock

@@ -12,6 +12,7 @@ build = "build.rs"
 [dependencies]
 # cargo hash delivered with nightly-2018-02-01
 cargo = { git = "https://github.com/rust-lang/cargo", rev = "1d6dfea44f97199d5d5c177c7dadcde393eaff9a" }
+cargo_metadata = "0.4"
 env_logger = "0.5"
 failure = "0.1.1"
 jsonrpc-core = "8.0.1"

Cargo.toml

@@ -13,11 +13,11 @@ use cargo::ops::{compile_with_exec, CompileFilter, CompileMode, CompileOptions,
                  Packages, Unit};
 use cargo::util::{homedir, important_paths, CargoResult, Config as CargoConfig, ConfigValue,
                   ProcessBuilder};
-use serde_json;
 use failure;
+use serde_json;
 
 use data::Analysis;
-use build::{BufWriter, BuildResult, CompilationContext, Internals};
+use build::{BufWriter, BuildResult, CompilationContext, Internals, PackageArg};
 use build::environment::{self, Environment, EnvironmentLock};
 use config::Config;
 use vfs::Vfs;
@@ -32,7 +32,7 @@ use std::sync::{Arc, Mutex};
 use std::thread;
 
 // Runs an in-process instance of Cargo.
-pub(super) fn cargo(internals: &Internals) -> BuildResult {
+pub(super) fn cargo(internals: &Internals, package_arg: PackageArg) -> BuildResult {
     let workspace_mode = internals.config.lock().unwrap().workspace_mode;
 
     let compilation_cx = internals.compilation_cx.clone();
@@ -54,6 +54,7 @@ pub(super) fn cargo(internals: &Internals) -> BuildResult {
     let handle = thread::spawn(move || {
         run_cargo(
             compilation_cx,
+            package_arg,
             config,
             vfs,
             env_lock,
@@ -90,6 +91,7 @@ pub(super) fn cargo(internals: &Internals) -> BuildResult {
 
 fn run_cargo(
     compilation_cx: Arc<Mutex<CompilationContext>>,
+    package_arg: PackageArg,
     rls_config: Arc<Mutex<Config>>,
     vfs: Arc<Vfs>,
     env_lock: Arc<EnvironmentLock>,
@@ -113,6 +115,7 @@ fn run_cargo(
 
         compilation_cx.build_dir.as_ref().unwrap().clone()
     };
+
     // Note that this may not be equal build_dir when inside a workspace member
     let manifest_path = important_paths::find_root_manifest_for_wd(None, &build_dir)?;
     trace!("root manifest_path: {:?}", &manifest_path);
@@ -131,6 +134,11 @@ fn run_cargo(
 
     let ws = Workspace::new(&manifest_path, &config)?;
 
+    let packages = match package_arg {
+        PackageArg::Unknown | PackageArg::All => vec![],
+        PackageArg::Package(s) => vec![s]
+    };
+
     // TODO: It might be feasible to keep this CargoOptions structure cached and regenerate
     // it on every relevant configuration change
     let (opts, rustflags, clear_env_rust_log) =
@@ -142,7 +150,14 @@ fn run_cargo(
             trace!("Cargo compilation options:\n{:?}", opts);
             let rustflags = prepare_cargo_rustflags(&rls_config);
 
-            if !rls_config.workspace_mode {
+
+            if rls_config.workspace_mode {
+                for package in &packages {
+                    if let None = ws.members().find(|x| x.name() == package) {
+                        warn!("cargo - couldn't find member package `{}` specified in `analyze_package` configuration", package);
+                    }
+                }
+            } else {
                 // Warn about invalid specified bin target or package depending on current mode
                 // TODO: Return client notifications along with diagnostics to inform the user
                 let cur_pkg_targets = ws.current()?.targets();
@@ -158,7 +173,7 @@ fn run_cargo(
             (opts, rustflags, rls_config.clear_env_rust_log)
         };
 
-    let spec = Packages::from_flags(false, &[], &[])?;
+    let spec = Packages::from_flags(false, &[], &packages)?;
 
     let compile_opts = CompileOptions {
         target: opts.target.as_ref().map(|t| &t[..]),
@@ -630,6 +645,7 @@ pub fn make_cargo_config(build_dir: &Path,
         let target_dir = target_dir
             .map(|d| d.to_str().unwrap().to_owned())
             .unwrap_or_else(|| {
+                // FIXME(#730) should be using the workspace root here, not build_dir
                 build_dir
                     .join("target")
                     .join("rls")

src/build/cargo.rs

@@ -13,9 +13,10 @@
 pub use self::cargo::make_cargo_config;
 
 use actions::post_build::PostBuildHandler;
+use cargo::util::important_paths;
+use config::Config;
 use data::Analysis;
 use vfs::Vfs;
-use config::Config;
 
 use self::environment::EnvironmentLock;
 
@@ -119,6 +120,15 @@ pub enum BuildPriority {
     Normal,
 }
 
+impl BuildPriority {
+    fn is_cargo(&self) -> bool {
+        match *self {
+            BuildPriority::Cargo => true,
+            _ => false,
+        }
+    }
+}
+
 /// Information passed to Cargo/rustc to build.
 #[derive(Debug)]
 struct CompilationContext {
@@ -138,13 +148,22 @@ impl CompilationContext {
         CompilationContext {
             args: vec![],
             envs: HashMap::new(),
+            cwd: None,
             build_dir: None,
             build_plan: BuildPlan::new(),
-            cwd: None,
         }
     }
 }
 
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum PackageArg {
+    Unknown,
+    // --all
+    All,
+    // -p ...
+    Package(String),
+}
+
 /// Status of the build queue.
 ///
 /// Pending should only be replaced if it is built or squashed. `InProgress` can be
@@ -429,11 +448,11 @@ impl Internals {
                 .map_or(true, |dir| dir != new_build_dir)
             {
                 // We'll need to re-run cargo in this case.
-                assert!(priority == BuildPriority::Cargo);
+                assert!(priority.is_cargo());
                 (*compilation_cx).build_dir = Some(new_build_dir.to_owned());
             }
 
-            if priority == BuildPriority::Cargo {
+            if priority.is_cargo() {
                 // Killing these args indicates we'll do a full Cargo build.
                 compilation_cx.args = vec![];
                 compilation_cx.envs = HashMap::new();
@@ -485,19 +504,27 @@ impl Internals {
             // has to be specifically rerun (e.g. when build scripts changed)
             let work = {
                 let modified: Vec<_> = self.dirty_files.lock().unwrap().keys().cloned().collect();
-                let cx = self.compilation_cx.lock().unwrap();
-                cx.build_plan.prepare_work(&modified)
+                let mut cx = self.compilation_cx.lock().unwrap();
+                let manifest_path = important_paths::find_root_manifest_for_wd(None, cx.build_dir.as_ref().unwrap());
+                let manifest_path = match manifest_path {
+                    Ok(mp) => mp,
+                    Err(e) => {
+                        let msg = format!("Error reading manifest path: {:?}", e);
+                        return BuildResult::Err(msg, None);
+                    }
+                };
+                cx.build_plan.prepare_work(&manifest_path, &modified, needs_to_run_cargo)
             };
             return match work {
                 // In workspace_mode, cargo performs the full build and returns
                 // appropriate diagnostics/analysis data
-                WorkStatus::NeedsCargo => cargo::cargo(self),
+                WorkStatus::NeedsCargo(package_arg) => cargo::cargo(self, package_arg),
                 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 e @ BuildResult::Err(..) = cargo::cargo(self) {
+            if let e @ BuildResult::Err(..) = cargo::cargo(self, PackageArg::Unknown) {
                 return e;
             }
         }

src/build/mod.rs

@@ -27,11 +27,15 @@
 use std::collections::{HashMap, HashSet};
 use std::fmt;
 use std::path::{Path, PathBuf};
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 
+use build::PackageArg;
 use cargo::core::{PackageId, Profile, Target, TargetKind};
 use cargo::ops::{Context, Kind, Unit};
 use cargo::util::{CargoResult, ProcessBuilder};
+use cargo_metadata;
+use lsp_data::parse_file_path;
+use url::Url;
 
 use super::{BuildResult, Internals};
 
@@ -49,6 +53,11 @@ pub struct Plan {
     pub rev_dep_graph: HashMap<UnitKey, HashSet<UnitKey>>,
     /// Cached compiler calls used when creating a compiler call queue.
     pub compiler_jobs: HashMap<UnitKey, ProcessBuilder>,
+    // An object for finding the package which a file belongs to and this inferring
+    // a package argument.
+    package_map: Option<PackageMap>,
+    // The package argument used in the last Cargo build.
+    prev_package_arg: Option<PackageArg>,
 }
 
 impl Plan {
@@ -58,11 +67,16 @@ impl Plan {
             dep_graph: HashMap::new(),
             rev_dep_graph: HashMap::new(),
             compiler_jobs: HashMap::new(),
+            package_map: None,
+            prev_package_arg: None,
         }
     }
 
     pub fn clear(&mut self) {
-        *self = Plan::new();
+        self.units = HashMap::new();
+        self.dep_graph = HashMap::new();
+        self.rev_dep_graph = HashMap::new();
+        self.compiler_jobs = HashMap::new();
     }
 
     /// Returns whether a build plan has cached compiler invocations and dep
@@ -273,23 +287,34 @@ impl Plan {
         }
     }
 
-    pub fn prepare_work<T: AsRef<Path> + fmt::Debug>(&self, modified: &[T]) -> WorkStatus {
-        if !self.is_ready() {
-            return WorkStatus::NeedsCargo;
+    pub fn prepare_work<T: AsRef<Path> + fmt::Debug>(&mut self, manifest_path: &Path, modified: &[T], requested_cargo: bool) -> WorkStatus {
+        if self.package_map.is_none() || requested_cargo {
+            self.package_map = Some(PackageMap::new(manifest_path));
+        }
+
+        let package_arg = self.package_map.as_ref().unwrap().compute_package_arg(modified);
+        let package_arg_changed = match self.prev_package_arg {
+            Some(ref ppa) => ppa != &package_arg,
+            None => true,
+        };
+        self.prev_package_arg = Some(package_arg.clone());
+
+        if !self.is_ready() || requested_cargo || package_arg_changed {
+            return WorkStatus::NeedsCargo(package_arg);
         }
 
         let dirties = self.fetch_dirty_units(modified);
         trace!(
             "fetch_dirty_units: for files {:?}, these units are dirty: {:?}",
             modified,
-            dirties
+            dirties,
         );
 
         if dirties
             .iter()
             .any(|&(_, ref kind)| *kind == TargetKind::CustomBuild)
         {
-            WorkStatus::NeedsCargo
+            WorkStatus::NeedsCargo(package_arg)
         } else {
             let graph = self.dirty_rev_dep_graph(&dirties);
             trace!("Constructed dirty rev dep graph: {:?}", graph);
@@ -302,7 +327,7 @@ impl Plan {
                 .collect();
 
             if jobs.is_empty() {
-                WorkStatus::NeedsCargo
+                WorkStatus::NeedsCargo(package_arg)
             } else {
                 WorkStatus::Execute(JobQueue(jobs))
             }
@@ -311,10 +336,74 @@ impl Plan {
 }
 
 pub enum WorkStatus {
-    NeedsCargo,
+    NeedsCargo(PackageArg),
     Execute(JobQueue),
 }
 
+// Maps paths to packages.
+#[derive(Debug)]
+struct PackageMap {
+    // A map from a manifest directory to the package name.
+    package_paths: HashMap<PathBuf, String>,
+    // A map from a file's path, to the package it belongs to.
+    map_cache: Mutex<HashMap<PathBuf, String>>,
+}
+
+impl PackageMap {
+    fn new(manifest_path: &Path) -> PackageMap {
+        PackageMap {
+            package_paths: Self::discover_package_paths(manifest_path),
+            map_cache: Mutex::new(HashMap::new()),
+        }
+    }
+
+    fn discover_package_paths(manifest_path: &Path) -> HashMap<PathBuf, String> {
+        trace!("read metadata {:?}", manifest_path);
+        let metadata = cargo_metadata::metadata(Some(manifest_path)).expect("Can't read crate metadata");
+        metadata
+            .workspace_members
+            .into_iter()
+            .map(|wm| {
+                assert!(wm.url.starts_with("path+"));
+                let url = Url::parse(&wm.url[5..]).expect("Bad URL");
+                let path = parse_file_path(&url).expect("URL not a path");
+                (path, wm.name)
+            })
+            .collect()
+    }
+
+    fn compute_package_arg<T: AsRef<Path> + fmt::Debug>(&self, modified_files: &[T]) -> PackageArg {
+        let mut packages: Option<HashSet<String>> = modified_files.iter().map(|p| self.map(p.as_ref())).collect();
+        match packages {
+            Some(ref mut packages) if packages.len() == 1 => {
+                PackageArg::Package(packages.drain().next().unwrap())
+            }
+            _ => PackageArg::All,
+        }
+    }
+
+    fn map(&self, path: &Path) -> Option<String> {
+        let mut map_cache = self.map_cache.lock().unwrap();
+        if map_cache.contains_key(path) {
+            return Some(map_cache[path].clone());
+        }
+
+        let result = Self::map_uncached(path, &self.package_paths)?;
+
+        map_cache.insert(path.to_owned(), result.clone());
+        Some(result)
+    }
+
+    fn map_uncached(path: &Path, package_paths: &HashMap<PathBuf, String>) -> Option<String> {
+        match package_paths.get(path) {
+            Some(package) => Some(package.clone()),
+            None => {
+                Self::map_uncached(path.parent()?, package_paths)
+            }
+        }
+    }
+}
+
 pub struct JobQueue(Vec<ProcessBuilder>);
 
 impl JobQueue {

src/build/plan.rs

@@ -54,11 +54,11 @@ where
 
 /// Parse the given URI into a `PathBuf`.
 pub fn parse_file_path(uri: &Url) -> Result<PathBuf, UrlFileParseError> {
-    if uri.scheme() != "file" {
-        Err(UrlFileParseError::InvalidScheme)
-    } else {
+    if uri.scheme() == "file" {
         uri.to_file_path()
             .map_err(|_err| UrlFileParseError::InvalidFilePath)
+    } else {
+        Err(UrlFileParseError::InvalidScheme)
     }
 }