From 285e4bae03a6d57bdb9c9dc25bf7f6630622665c Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 27 Mar 2018 15:51:04 +0200 Subject: [PATCH 1/5] Fix default target/rls dir inference --- src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index c3021649726..ed6b168e480 100644 --- a/src/config.rs +++ b/src/config.rs @@ -234,7 +234,7 @@ impl Config { // own artifacts somewhere else (e.g. when analyzing only a single crate in a workspace) if self.target_dir.is_none() { let target_dir = ws.target_dir().clone().into_path_unlocked(); - self.target_dir = Some(target_dir); + let target_dir = target_dir.join("rls"); trace!( "For project path {:?} Cargo told us to use this target/ dir: {:?}", project_dir, From 98ca1f9bd06c76bb886b9bd16d6cfb2a85417562 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 27 Mar 2018 15:52:48 +0200 Subject: [PATCH 2/5] Allow rust.target_dir to be inferred again after being specified --- src/build/cargo.rs | 2 +- src/config.rs | 36 ++++++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/build/cargo.rs b/src/build/cargo.rs index bb61258f909..958e4f633f2 100644 --- a/src/build/cargo.rs +++ b/src/build/cargo.rs @@ -140,7 +140,7 @@ fn run_cargo( let config = { let rls_config = rls_config.lock().unwrap(); - let target_dir = rls_config.target_dir.as_ref().map(|p| p as &Path); + let target_dir = rls_config.target_dir.as_ref().as_ref().map(|p| p as &Path); make_cargo_config(manifest_dir, target_dir, restore_env.get_old_cwd(), shell) }; diff --git a/src/config.rs b/src/config.rs index ed6b168e480..a7b6a25a2d5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -67,6 +67,16 @@ impl<'de, T: Deserialize<'de>> Deserialize<'de> for Inferrable { } } +impl Inferrable { + pub fn is_none(&self) -> bool { + if let &Inferrable::None = self { + return false; + } else { + return true; + } + } +} + impl Inferrable { /// Combine these inferrable values, preferring our own specified values /// when possible, and falling back the given default value. @@ -104,6 +114,7 @@ impl AsRef for Inferrable { } } } + /// RLS configuration options. #[derive(Clone, Debug, Deserialize, Serialize)] #[allow(missing_docs)] @@ -126,8 +137,7 @@ pub struct Config { pub build_on_save: bool, pub use_crate_blacklist: bool, /// Cargo target dir. If set overrides the default one. - #[serde(skip_deserializing, skip_serializing)] - pub target_dir: Option, + pub target_dir: Inferrable>, pub features: Vec, pub all_features: bool, pub no_default_features: bool, @@ -156,7 +166,7 @@ impl Default for Config { clear_env_rust_log: true, build_on_save: false, use_crate_blacklist: true, - target_dir: None, + target_dir: Inferrable::Inferred(None), features: vec![], all_features: false, no_default_features: false, @@ -173,6 +183,7 @@ impl Default for Config { impl Config { /// Join this configuration with the new config. pub fn update(&mut self, mut new: Config) { + new.target_dir = self.target_dir.combine_with_default(&new.target_dir, None); new.build_lib = self.build_lib.combine_with_default(&new.build_lib, false); new.build_bin = self.build_bin.combine_with_default(&new.build_bin, None); @@ -203,10 +214,11 @@ impl Config { /// Is this config incomplete, and needs additional values to be inferred? pub fn needs_inference(&self) -> bool { - match (&self.build_lib, &self.build_bin) { - (&Inferrable::None, _) | (_, &Inferrable::None) => true, - _ => false, + if self.build_bin.is_none() || self.build_lib.is_none() || self.target_dir.is_none() { + return true; } + + return true; } /// Tries to auto-detect certain option values if they were unspecified. @@ -232,13 +244,21 @@ impl Config { // Constructing a `Workspace` also probes the filesystem and detects where to place the // build artifacts. We need to rely on Cargo's behaviour directly not to possibly place our // own artifacts somewhere else (e.g. when analyzing only a single crate in a workspace) - if self.target_dir.is_none() { + match self.target_dir { + // We require an absolute path, so adjust a relative one if it's passed. + Inferrable::Specified(Some(ref mut path)) if path.is_relative() => { + *path = project_dir.join(&path); + } + _ => {}, + } + if self.target_dir.as_ref().is_none() { let target_dir = ws.target_dir().clone().into_path_unlocked(); let target_dir = target_dir.join("rls"); + self.target_dir.infer(Some(target_dir)); trace!( "For project path {:?} Cargo told us to use this target/ dir: {:?}", project_dir, - self.target_dir.as_ref().unwrap(), + self.target_dir.as_ref().as_ref().unwrap(), ); } From d3308dde7af947f5b830bc8ef666dff291a1e03b Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 27 Mar 2018 15:54:26 +0200 Subject: [PATCH 3/5] Remove now-obsolete FIXME --- src/build/cargo.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/build/cargo.rs b/src/build/cargo.rs index 958e4f633f2..dfaefef2274 100644 --- a/src/build/cargo.rs +++ b/src/build/cargo.rs @@ -671,7 +671,6 @@ 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") From 2487377e6520e0119a42ac95fed5e18c71f00879 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 27 Mar 2018 16:13:36 +0200 Subject: [PATCH 4/5] Fix tests --- src/test/harness.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/harness.rs b/src/test/harness.rs index 6fd4f4488fc..c264213d831 100644 --- a/src/test/harness.rs +++ b/src/test/harness.rs @@ -16,7 +16,7 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use analysis; -use config::Config; +use config::{Config, Inferrable}; use env_logger; use ls_types; use serde_json; @@ -59,7 +59,7 @@ impl Environment { .join(format!("{}", COUNTER.fetch_add(1, Ordering::Relaxed))); let mut config = Config::default(); - config.target_dir = Some(target_path.clone()); + config.target_dir = Inferrable::Specified(Some(target_path.clone())); config.unstable_features = true; let cache = Cache::new(project_path); From 77612998a62a697858078cff2f59d8b48b83ce06 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Wed, 28 Mar 2018 22:51:51 +0200 Subject: [PATCH 5/5] Apply feedback --- src/config.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/config.rs b/src/config.rs index a7b6a25a2d5..003a858d3c6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -69,10 +69,9 @@ impl<'de, T: Deserialize<'de>> Deserialize<'de> for Inferrable { impl Inferrable { pub fn is_none(&self) -> bool { - if let &Inferrable::None = self { - return false; - } else { - return true; + match *self { + Inferrable::None => true, + _ => false, } } } @@ -214,11 +213,9 @@ impl Config { /// Is this config incomplete, and needs additional values to be inferred? pub fn needs_inference(&self) -> bool { - if self.build_bin.is_none() || self.build_lib.is_none() || self.target_dir.is_none() { - return true; - } - - return true; + self.build_bin.is_none() || + self.build_lib.is_none() || + self.target_dir.is_none() } /// Tries to auto-detect certain option values if they were unspecified.