Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #18441 from Veykril/lw-psyvmlotlvqn
internal: Do not cache the config directory path
Lukas Wirth 2024-12-09
parent 1c3043b · parent 10a07a4 · commit 94032e8
-rw-r--r--crates/load-cargo/src/lib.rs24
-rw-r--r--crates/rust-analyzer/src/config.rs35
-rw-r--r--crates/rust-analyzer/src/global_state.rs11
-rw-r--r--crates/rust-analyzer/src/reload.rs8
-rw-r--r--crates/rust-analyzer/tests/slow-tests/ratoml.rs19
-rw-r--r--crates/rust-analyzer/tests/slow-tests/support.rs64
6 files changed, 107 insertions, 54 deletions
diff --git a/crates/load-cargo/src/lib.rs b/crates/load-cargo/src/lib.rs
index 0e1606a699..cf26845b11 100644
--- a/crates/load-cargo/src/lib.rs
+++ b/crates/load-cargo/src/lib.rs
@@ -123,7 +123,7 @@ pub fn load_workspace(
.collect()
};
- let project_folders = ProjectFolders::new(std::slice::from_ref(&ws), &[]);
+ let project_folders = ProjectFolders::new(std::slice::from_ref(&ws), &[], None);
loader.set_config(vfs::loader::Config {
load: project_folders.load,
watch: vec![],
@@ -153,7 +153,11 @@ pub struct ProjectFolders {
}
impl ProjectFolders {
- pub fn new(workspaces: &[ProjectWorkspace], global_excludes: &[AbsPathBuf]) -> ProjectFolders {
+ pub fn new(
+ workspaces: &[ProjectWorkspace],
+ global_excludes: &[AbsPathBuf],
+ user_config_dir_path: Option<&AbsPath>,
+ ) -> ProjectFolders {
let mut res = ProjectFolders::default();
let mut fsc = FileSetConfig::builder();
let mut local_filesets = vec![];
@@ -291,6 +295,22 @@ impl ProjectFolders {
}
}
+ if let Some(user_config_path) = user_config_dir_path {
+ let ratoml_path = {
+ let mut p = user_config_path.to_path_buf();
+ p.push("rust-analyzer.toml");
+ p
+ };
+
+ let file_set_roots = vec![VfsPath::from(ratoml_path.to_owned())];
+ let entry = vfs::loader::Entry::Files(vec![ratoml_path.to_owned()]);
+
+ res.watch.push(res.load.len());
+ res.load.push(entry);
+ local_filesets.push(fsc.len() as u64);
+ fsc.add_file_set(file_set_roots)
+ }
+
let fsc = fsc.build();
res.source_root_config = SourceRootConfig { fsc, local_filesets };
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index dd7351bcf2..694748f82f 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -3,11 +3,7 @@
//! Of particular interest is the `feature_flags` hash map: while other fields
//! configure the server itself, feature flags are passed into analysis, and
//! tweak things like automatic insertion of `()` in completions.
-use std::{
- env, fmt, iter,
- ops::Not,
- sync::{LazyLock, OnceLock},
-};
+use std::{env, fmt, iter, ops::Not, sync::OnceLock};
use cfg::{CfgAtom, CfgDiff};
use hir::Symbol;
@@ -804,25 +800,14 @@ impl std::ops::Deref for Config {
}
impl Config {
- /// Path to the root configuration file. This can be seen as a generic way to define what would be `$XDG_CONFIG_HOME/rust-analyzer/rust-analyzer.toml` in Linux.
- /// This path is equal to:
- ///
- /// |Platform | Value | Example |
- /// | ------- | ------------------------------------- | ---------------------------------------- |
- /// | Linux | `$XDG_CONFIG_HOME` or `$HOME`/.config | /home/alice/.config |
- /// | macOS | `$HOME`/Library/Application Support | /Users/Alice/Library/Application Support |
- /// | Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming |
- pub fn user_config_path() -> Option<&'static AbsPath> {
- static USER_CONFIG_PATH: LazyLock<Option<AbsPathBuf>> = LazyLock::new(|| {
- let user_config_path = if let Some(path) = env::var_os("__TEST_RA_USER_CONFIG_DIR") {
- std::path::PathBuf::from(path)
- } else {
- dirs::config_dir()?.join("rust-analyzer")
- }
- .join("rust-analyzer.toml");
- Some(AbsPathBuf::assert_utf8(user_config_path))
- });
- USER_CONFIG_PATH.as_deref()
+ /// Path to the user configuration dir. This can be seen as a generic way to define what would be `$XDG_CONFIG_HOME/rust-analyzer` in Linux.
+ pub fn user_config_dir_path() -> Option<AbsPathBuf> {
+ let user_config_path = if let Some(path) = env::var_os("__TEST_RA_USER_CONFIG_DIR") {
+ std::path::PathBuf::from(path)
+ } else {
+ dirs::config_dir()?.join("rust-analyzer")
+ };
+ Some(AbsPathBuf::assert_utf8(user_config_path))
}
pub fn same_source_root_parent_map(
@@ -1262,7 +1247,7 @@ pub struct NotificationsConfig {
pub cargo_toml_not_found: bool,
}
-#[derive(Deserialize, Serialize, Debug, Clone)]
+#[derive(Debug, Clone)]
pub enum RustfmtConfig {
Rustfmt { extra_args: Vec<String>, enable_range_formatting: bool },
CustomCommand { command: String, args: Vec<String> },
diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs
index 7fbeaa4e3e..5f83570284 100644
--- a/crates/rust-analyzer/src/global_state.rs
+++ b/crates/rust-analyzer/src/global_state.rs
@@ -392,7 +392,14 @@ impl GlobalState {
|| !self.config.same_source_root_parent_map(&self.local_roots_parent_map)
{
let config_change = {
- let user_config_path = Config::user_config_path();
+ let user_config_path = (|| {
+ let mut p = Config::user_config_dir_path()?;
+ p.push("rust-analyzer.toml");
+ Some(p)
+ })();
+
+ let user_config_abs_path = user_config_path.as_deref();
+
let mut change = ConfigChange::default();
let db = self.analysis_host.raw_database();
@@ -411,7 +418,7 @@ impl GlobalState {
.collect_vec();
for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files {
- if vfs_path.as_path() == user_config_path {
+ if vfs_path.as_path() == user_config_abs_path {
change.change_user_config(Some(db.file_text(file_id)));
continue;
}
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs
index bc85afa0e4..4549735fef 100644
--- a/crates/rust-analyzer/src/reload.rs
+++ b/crates/rust-analyzer/src/reload.rs
@@ -590,7 +590,7 @@ impl GlobalState {
}
watchers.extend(
- iter::once(Config::user_config_path())
+ iter::once(Config::user_config_dir_path().as_deref())
.chain(self.workspaces.iter().map(|ws| ws.manifest().map(ManifestPath::as_ref)))
.flatten()
.map(|glob_pattern| lsp_types::FileSystemWatcher {
@@ -613,7 +613,11 @@ impl GlobalState {
}
let files_config = self.config.files();
- let project_folders = ProjectFolders::new(&self.workspaces, &files_config.exclude);
+ let project_folders = ProjectFolders::new(
+ &self.workspaces,
+ &files_config.exclude,
+ Config::user_config_dir_path().as_deref(),
+ );
if (self.proc_macro_clients.is_empty() || !same_workspaces)
&& self.config.expand_proc_macros()
diff --git a/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/crates/rust-analyzer/tests/slow-tests/ratoml.rs
index a857e0c296..5dfaf0d365 100644
--- a/crates/rust-analyzer/tests/slow-tests/ratoml.rs
+++ b/crates/rust-analyzer/tests/slow-tests/ratoml.rs
@@ -46,7 +46,7 @@ impl RatomlTest {
project = project.with_config(client_config);
}
- let server = project.server().wait_until_workspace_is_loaded();
+ let server = project.server_with_lock(true).wait_until_workspace_is_loaded();
let mut case = Self { urls: vec![], server, tmp_path };
let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::<Vec<_>>();
@@ -72,7 +72,7 @@ impl RatomlTest {
let mut spl = spl.into_iter();
if let Some(first) = spl.next() {
if first == "$$CONFIG_DIR$$" {
- path = Config::user_config_path().unwrap().to_path_buf().into();
+ path = Config::user_config_dir_path().unwrap().into();
} else {
path = path.join(first);
}
@@ -285,7 +285,6 @@ enum Value {
// }
#[test]
-#[ignore = "the user config is currently not being watched on startup, fix this"]
fn ratoml_user_config_detected() {
if skip_slow_tests() {
return;
@@ -294,7 +293,7 @@ fn ratoml_user_config_detected() {
let server = RatomlTest::new(
vec![
r#"
-//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml
+//- /$$CONFIG_DIR$$/rust-analyzer.toml
assist.emitMustUse = true
"#,
r#"
@@ -322,7 +321,6 @@ enum Value {
}
#[test]
-#[ignore = "the user config is currently not being watched on startup, fix this"]
fn ratoml_create_user_config() {
if skip_slow_tests() {
return;
@@ -353,10 +351,7 @@ enum Value {
1,
InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
);
- server.create(
- "//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml",
- RatomlTest::EMIT_MUST_USE.to_owned(),
- );
+ server.create("//- /$$CONFIG_DIR$$/rust-analyzer.toml", RatomlTest::EMIT_MUST_USE.to_owned());
server.query(
InternalTestingFetchConfigOption::AssistEmitMustUse,
1,
@@ -365,7 +360,6 @@ enum Value {
}
#[test]
-#[ignore = "the user config is currently not being watched on startup, fix this"]
fn ratoml_modify_user_config() {
if skip_slow_tests() {
return;
@@ -386,7 +380,7 @@ enum Value {
Text(String),
}"#,
r#"
-//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml
+//- /$$CONFIG_DIR$$/rust-analyzer.toml
assist.emitMustUse = true"#,
],
vec!["p1"],
@@ -407,7 +401,6 @@ assist.emitMustUse = true"#,
}
#[test]
-#[ignore = "the user config is currently not being watched on startup, fix this"]
fn ratoml_delete_user_config() {
if skip_slow_tests() {
return;
@@ -428,7 +421,7 @@ enum Value {
Text(String),
}"#,
r#"
-//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml
+//- /$$CONFIG_DIR$$/rust-analyzer.toml
assist.emitMustUse = true"#,
],
vec!["p1"],
diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs
index 78572e37a9..5a88a5515c 100644
--- a/crates/rust-analyzer/tests/slow-tests/support.rs
+++ b/crates/rust-analyzer/tests/slow-tests/support.rs
@@ -1,7 +1,7 @@
use std::{
cell::{Cell, RefCell},
- fs,
- sync::Once,
+ env, fs,
+ sync::{Once, OnceLock},
time::Duration,
};
@@ -127,7 +127,53 @@ impl Project<'_> {
}
pub(crate) fn server(self) -> Server {
- static CONFIG_DIR_LOCK: Mutex<()> = Mutex::new(());
+ Project::server_with_lock(self, false)
+ }
+
+ /// `prelock` : Forcefully acquire a lock that will maintain the path to the config dir throughout the whole test.
+ ///
+ /// When testing we set the user config dir by setting an envvar `__TEST_RA_USER_CONFIG_DIR`.
+ /// This value must be maintained until the end of a test case. When tests run in parallel
+ /// this value may change thus making the tests flaky. As such, we use a `MutexGuard` that locks
+ /// the process until `Server` is dropped. To optimize parallelization we use a lock only when it is
+ /// needed, that is when a test uses config directory to do stuff. Our naive approach is to use a lock
+ /// if there is a path to config dir in the test fixture. However, in certain cases we create a
+ /// file in the config dir after server is run, something where our naive approach comes short.
+ /// Using a `prelock` allows us to force a lock when we know we need it.
+ pub(crate) fn server_with_lock(self, config_lock: bool) -> Server {
+ static CONFIG_DIR_LOCK: OnceLock<(Utf8PathBuf, Mutex<()>)> = OnceLock::new();
+
+ let config_dir_guard = if config_lock {
+ Some({
+ let (path, mutex) = CONFIG_DIR_LOCK.get_or_init(|| {
+ let value = TestDir::new().keep().path().to_owned();
+ env::set_var("__TEST_RA_USER_CONFIG_DIR", &value);
+ (value, Mutex::new(()))
+ });
+ #[allow(dyn_drop)]
+ (mutex.lock(), {
+ Box::new({
+ struct Dropper(Utf8PathBuf);
+ impl Drop for Dropper {
+ fn drop(&mut self) {
+ for entry in fs::read_dir(&self.0).unwrap() {
+ let path = entry.unwrap().path();
+ if path.is_file() {
+ fs::remove_file(path).unwrap();
+ } else if path.is_dir() {
+ fs::remove_dir_all(path).unwrap();
+ }
+ }
+ }
+ }
+ Dropper(path.clone())
+ }) as Box<dyn Drop>
+ })
+ })
+ } else {
+ None
+ };
+
let tmp_dir = self.tmp_dir.unwrap_or_else(|| {
if self.root_dir_contains_symlink {
TestDir::new_symlink()
@@ -160,13 +206,9 @@ impl Project<'_> {
assert!(mini_core.is_none());
assert!(toolchain.is_none());
- let mut config_dir_guard = None;
for entry in fixture {
if let Some(pth) = entry.path.strip_prefix("/$$CONFIG_DIR$$") {
- if config_dir_guard.is_none() {
- config_dir_guard = Some(CONFIG_DIR_LOCK.lock());
- }
- let path = Config::user_config_path().unwrap().join(&pth['/'.len_utf8()..]);
+ let path = Config::user_config_dir_path().unwrap().join(&pth['/'.len_utf8()..]);
fs::create_dir_all(path.parent().unwrap()).unwrap();
fs::write(path.as_path(), entry.text.as_bytes()).unwrap();
} else {
@@ -269,12 +311,14 @@ pub(crate) struct Server {
client: Connection,
/// XXX: remove the tempdir last
dir: TestDir,
- _config_dir_guard: Option<MutexGuard<'static, ()>>,
+ #[allow(dyn_drop)]
+ _config_dir_guard: Option<(MutexGuard<'static, ()>, Box<dyn Drop>)>,
}
impl Server {
+ #[allow(dyn_drop)]
fn new(
- config_dir_guard: Option<MutexGuard<'static, ()>>,
+ config_dir_guard: Option<(MutexGuard<'static, ()>, Box<dyn Drop>)>,
dir: TestDir,
config: Config,
) -> Server {