Fix config guard lock for ratoml tests

This commit is contained in:
Lukas Wirth 2024-10-30 12:03:08 +01:00 committed by Lukas Wirth
parent 4fcecbb55e
commit 10a07a443d
6 changed files with 58 additions and 62 deletions

View File

@ -156,7 +156,7 @@ impl ProjectFolders {
pub fn new( pub fn new(
workspaces: &[ProjectWorkspace], workspaces: &[ProjectWorkspace],
global_excludes: &[AbsPathBuf], global_excludes: &[AbsPathBuf],
user_config_dir_path: Option<&'static AbsPath>, user_config_dir_path: Option<&AbsPath>,
) -> ProjectFolders { ) -> ProjectFolders {
let mut res = ProjectFolders::default(); let mut res = ProjectFolders::default();
let mut fsc = FileSetConfig::builder(); let mut fsc = FileSetConfig::builder();
@ -302,7 +302,7 @@ impl ProjectFolders {
p p
}; };
let file_set_roots: Vec<VfsPath> = vec![VfsPath::from(ratoml_path.to_owned())]; let file_set_roots = vec![VfsPath::from(ratoml_path.to_owned())];
let entry = vfs::loader::Entry::Files(vec![ratoml_path.to_owned()]); let entry = vfs::loader::Entry::Files(vec![ratoml_path.to_owned()]);
res.watch.push(res.load.len()); res.watch.push(res.load.len());

View File

@ -3,11 +3,7 @@
//! Of particular interest is the `feature_flags` hash map: while other fields //! Of particular interest is the `feature_flags` hash map: while other fields
//! configure the server itself, feature flags are passed into analysis, and //! configure the server itself, feature flags are passed into analysis, and
//! tweak things like automatic insertion of `()` in completions. //! tweak things like automatic insertion of `()` in completions.
use std::{ use std::{env, fmt, iter, ops::Not, sync::OnceLock};
env, fmt, iter,
ops::Not,
sync::{LazyLock, OnceLock},
};
use cfg::{CfgAtom, CfgDiff}; use cfg::{CfgAtom, CfgDiff};
use hir::Symbol; use hir::Symbol;
@ -805,16 +801,13 @@ impl std::ops::Deref for Config {
impl Config { impl Config {
/// 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. /// 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<&'static AbsPath> { pub fn user_config_dir_path() -> Option<AbsPathBuf> {
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") {
let user_config_path = if let Some(path) = env::var_os("__TEST_RA_USER_CONFIG_DIR") { std::path::PathBuf::from(path)
std::path::PathBuf::from(path) } else {
} else { dirs::config_dir()?.join("rust-analyzer")
dirs::config_dir()?.join("rust-analyzer") };
}; Some(AbsPathBuf::assert_utf8(user_config_path))
Some(AbsPathBuf::assert_utf8(user_config_path))
});
USER_CONFIG_PATH.as_deref()
} }
pub fn same_source_root_parent_map( pub fn same_source_root_parent_map(
@ -1254,7 +1247,7 @@ pub struct NotificationsConfig {
pub cargo_toml_not_found: bool, pub cargo_toml_not_found: bool,
} }
#[derive(Deserialize, Serialize, Debug, Clone)] #[derive(Debug, Clone)]
pub enum RustfmtConfig { pub enum RustfmtConfig {
Rustfmt { extra_args: Vec<String>, enable_range_formatting: bool }, Rustfmt { extra_args: Vec<String>, enable_range_formatting: bool },
CustomCommand { command: String, args: Vec<String> }, CustomCommand { command: String, args: Vec<String> },

View File

@ -392,13 +392,13 @@ impl GlobalState {
|| !self.config.same_source_root_parent_map(&self.local_roots_parent_map) || !self.config.same_source_root_parent_map(&self.local_roots_parent_map)
{ {
let config_change = { let config_change = {
let user_config_path = { let user_config_path = (|| {
let mut p = Config::user_config_dir_path().unwrap().to_path_buf(); let mut p = Config::user_config_dir_path()?;
p.push("rust-analyzer.toml"); p.push("rust-analyzer.toml");
p Some(p)
}; })();
let user_config_abs_path = Some(user_config_path.as_path()); let user_config_abs_path = user_config_path.as_deref();
let mut change = ConfigChange::default(); let mut change = ConfigChange::default();
let db = self.analysis_host.raw_database(); let db = self.analysis_host.raw_database();

View File

@ -590,7 +590,7 @@ impl GlobalState {
} }
watchers.extend( watchers.extend(
iter::once(Config::user_config_dir_path()) iter::once(Config::user_config_dir_path().as_deref())
.chain(self.workspaces.iter().map(|ws| ws.manifest().map(ManifestPath::as_ref))) .chain(self.workspaces.iter().map(|ws| ws.manifest().map(ManifestPath::as_ref)))
.flatten() .flatten()
.map(|glob_pattern| lsp_types::FileSystemWatcher { .map(|glob_pattern| lsp_types::FileSystemWatcher {
@ -616,7 +616,7 @@ impl GlobalState {
let project_folders = ProjectFolders::new( let project_folders = ProjectFolders::new(
&self.workspaces, &self.workspaces,
&files_config.exclude, &files_config.exclude,
Config::user_config_dir_path().to_owned(), Config::user_config_dir_path().as_deref(),
); );
if (self.proc_macro_clients.is_empty() || !same_workspaces) if (self.proc_macro_clients.is_empty() || !same_workspaces)

View File

@ -30,23 +30,6 @@ impl RatomlTest {
fixtures: Vec<&str>, fixtures: Vec<&str>,
roots: Vec<&str>, roots: Vec<&str>,
client_config: Option<serde_json::Value>, client_config: Option<serde_json::Value>,
) -> Self {
RatomlTest::new_with_lock(fixtures, roots, client_config, false)
}
fn new_locked(
fixtures: Vec<&str>,
roots: Vec<&str>,
client_config: Option<serde_json::Value>,
) -> Self {
RatomlTest::new_with_lock(fixtures, roots, client_config, true)
}
fn new_with_lock(
fixtures: Vec<&str>,
roots: Vec<&str>,
client_config: Option<serde_json::Value>,
prelock: bool,
) -> Self { ) -> Self {
let tmp_dir = TestDir::new(); let tmp_dir = TestDir::new();
let tmp_path = tmp_dir.path().to_owned(); let tmp_path = tmp_dir.path().to_owned();
@ -63,7 +46,7 @@ impl RatomlTest {
project = project.with_config(client_config); project = project.with_config(client_config);
} }
let server = project.server_with_lock(prelock).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 mut case = Self { urls: vec![], server, tmp_path };
let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::<Vec<_>>(); let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::<Vec<_>>();
@ -89,7 +72,7 @@ impl RatomlTest {
let mut spl = spl.into_iter(); let mut spl = spl.into_iter();
if let Some(first) = spl.next() { if let Some(first) = spl.next() {
if first == "$$CONFIG_DIR$$" { if first == "$$CONFIG_DIR$$" {
path = Config::user_config_dir_path().unwrap().to_path_buf().into(); path = Config::user_config_dir_path().unwrap().into();
} else { } else {
path = path.join(first); path = path.join(first);
} }
@ -307,7 +290,7 @@ fn ratoml_user_config_detected() {
return; return;
} }
let server = RatomlTest::new_locked( let server = RatomlTest::new(
vec![ vec![
r#" r#"
//- /$$CONFIG_DIR$$/rust-analyzer.toml //- /$$CONFIG_DIR$$/rust-analyzer.toml
@ -343,7 +326,7 @@ fn ratoml_create_user_config() {
return; return;
} }
let mut server = RatomlTest::new_locked( let mut server = RatomlTest::new(
vec![ vec![
r#" r#"
//- /p1/Cargo.toml //- /p1/Cargo.toml
@ -382,7 +365,7 @@ fn ratoml_modify_user_config() {
return; return;
} }
let mut server = RatomlTest::new_locked( let mut server = RatomlTest::new(
vec![ vec![
r#" r#"
//- /p1/Cargo.toml //- /p1/Cargo.toml
@ -423,7 +406,7 @@ fn ratoml_delete_user_config() {
return; return;
} }
let mut server = RatomlTest::new_locked( let mut server = RatomlTest::new(
vec![ vec![
r#" r#"
//- /p1/Cargo.toml //- /p1/Cargo.toml

View File

@ -1,7 +1,7 @@
use std::{ use std::{
cell::{Cell, RefCell}, cell::{Cell, RefCell},
env, fs, env, fs,
sync::Once, sync::{Once, OnceLock},
time::Duration, time::Duration,
}; };
@ -140,13 +140,36 @@ impl Project<'_> {
/// if there is a path to config dir in the test fixture. However, in certain cases we create a /// 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. /// 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. /// Using a `prelock` allows us to force a lock when we know we need it.
pub(crate) fn server_with_lock(self, prelock: bool) -> Server { pub(crate) fn server_with_lock(self, config_lock: bool) -> Server {
static CONFIG_DIR_LOCK: Mutex<()> = Mutex::new(()); static CONFIG_DIR_LOCK: OnceLock<(Utf8PathBuf, Mutex<()>)> = OnceLock::new();
let mut config_dir_guard = if prelock { let config_dir_guard = if config_lock {
let v = Some(CONFIG_DIR_LOCK.lock()); Some({
env::set_var("__TEST_RA_USER_CONFIG_DIR", TestDir::new().path()); let (path, mutex) = CONFIG_DIR_LOCK.get_or_init(|| {
v 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 { } else {
None None
}; };
@ -185,11 +208,6 @@ impl Project<'_> {
for entry in fixture { for entry in fixture {
if let Some(pth) = entry.path.strip_prefix("/$$CONFIG_DIR$$") { if let Some(pth) = entry.path.strip_prefix("/$$CONFIG_DIR$$") {
if config_dir_guard.is_none() {
config_dir_guard = Some(CONFIG_DIR_LOCK.lock());
env::set_var("__TEST_RA_USER_CONFIG_DIR", TestDir::new().path());
}
let path = Config::user_config_dir_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::create_dir_all(path.parent().unwrap()).unwrap();
fs::write(path.as_path(), entry.text.as_bytes()).unwrap(); fs::write(path.as_path(), entry.text.as_bytes()).unwrap();
@ -293,12 +311,14 @@ pub(crate) struct Server {
client: Connection, client: Connection,
/// XXX: remove the tempdir last /// XXX: remove the tempdir last
dir: TestDir, dir: TestDir,
_config_dir_guard: Option<MutexGuard<'static, ()>>, #[allow(dyn_drop)]
_config_dir_guard: Option<(MutexGuard<'static, ()>, Box<dyn Drop>)>,
} }
impl Server { impl Server {
#[allow(dyn_drop)]
fn new( fn new(
config_dir_guard: Option<MutexGuard<'static, ()>>, config_dir_guard: Option<(MutexGuard<'static, ()>, Box<dyn Drop>)>,
dir: TestDir, dir: TestDir,
config: Config, config: Config,
) -> Server { ) -> Server {