From 8c775840262acad08b8e2400360ef325e9c63d97 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Fri, 23 Feb 2018 12:08:03 +1300 Subject: [PATCH] Autodetect the package currently being edited --- Cargo.lock | 1 + Cargo.toml | 1 + src/build/cargo.rs | 25 ++++++++--- src/build/mod.rs | 43 ++++++++++++++---- src/build/plan.rs | 107 +++++++++++++++++++++++++++++++++++++++++---- src/lsp_data.rs | 6 +-- src/main.rs | 1 + 7 files changed, 159 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index da20a768775..9a885a132f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1054,6 +1054,7 @@ name = "rls" version = "0.126.0" dependencies = [ "cargo 0.26.0 (git+https://github.com/rust-lang/cargo?rev=1d6dfea44f97199d5d5c177c7dadcde393eaff9a)", + "cargo_metadata 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "clippy_lints 0.0.186 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index b502351b0d8..d3fb1c33fce 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/src/build/cargo.rs b/src/build/cargo.rs index 70d236d31e7..83afb51b7e0 100644 --- a/src/build/cargo.rs +++ b/src/build/cargo.rs @@ -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>, + package_arg: PackageArg, rls_config: Arc>, vfs: Arc, env_lock: Arc, @@ -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[..]), diff --git a/src/build/mod.rs b/src/build/mod.rs index 2c0c79ab27a..ef8ea59f301 100644 --- a/src/build/mod.rs +++ b/src/build/mod.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; } } diff --git a/src/build/plan.rs b/src/build/plan.rs index e56fe2b5461..7db2aae466f 100644 --- a/src/build/plan.rs +++ b/src/build/plan.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>, /// Cached compiler calls used when creating a compiler call queue. pub compiler_jobs: HashMap, + // An object for finding the package which a file belongs to and this inferring + // a package argument. + package_map: Option, + // The package argument used in the last Cargo build. + prev_package_arg: Option, } 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 + fmt::Debug>(&self, modified: &[T]) -> WorkStatus { - if !self.is_ready() { - return WorkStatus::NeedsCargo; + pub fn prepare_work + 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, + // A map from a file's path, to the package it belongs to. + map_cache: Mutex>, +} + +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 { + 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 + fmt::Debug>(&self, modified_files: &[T]) -> PackageArg { + let mut packages: Option> = 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 { + 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) -> Option { + match package_paths.get(path) { + Some(package) => Some(package.clone()), + None => { + Self::map_uncached(path.parent()?, package_paths) + } + } + } +} + pub struct JobQueue(Vec); impl JobQueue { diff --git a/src/lsp_data.rs b/src/lsp_data.rs index ab37f4b01ce..c93c6bb5174 100644 --- a/src/lsp_data.rs +++ b/src/lsp_data.rs @@ -54,11 +54,11 @@ where /// Parse the given URI into a `PathBuf`. pub fn parse_file_path(uri: &Url) -> Result { - 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) } } diff --git a/src/main.rs b/src/main.rs index 9468b415c3c..4ff7e97e4da 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,6 +23,7 @@ #![allow(needless_pass_by_value)] extern crate cargo; +extern crate cargo_metadata; extern crate env_logger; #[macro_use] extern crate failure;