Skip to content

Commit 94032e8

Browse files
VeykrilVeykril
authored
Merge pull request #18441 from Veykril/lw-psyvmlotlvqn
internal: Do not cache the config directory path
2 parents 1c3043b + 10a07a4 commit 94032e8

File tree

6 files changed

+107
-54
lines changed

6 files changed

+107
-54
lines changed

‎crates/load-cargo/src/lib.rs‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ pub fn load_workspace(
123123
.collect()
124124
};
125125

126-
let project_folders = ProjectFolders::new(std::slice::from_ref(&ws), &[]);
126+
let project_folders = ProjectFolders::new(std::slice::from_ref(&ws), &[], None);
127127
loader.set_config(vfs::loader::Config {
128128
load: project_folders.load,
129129
watch: vec![],
@@ -153,7 +153,11 @@ pub struct ProjectFolders {
153153
}
154154

155155
impl ProjectFolders {
156-
pub fn new(workspaces: &[ProjectWorkspace], global_excludes: &[AbsPathBuf]) -> ProjectFolders {
156+
pub fn new(
157+
workspaces: &[ProjectWorkspace],
158+
global_excludes: &[AbsPathBuf],
159+
user_config_dir_path: Option<&AbsPath>,
160+
) -> ProjectFolders {
157161
let mut res = ProjectFolders::default();
158162
let mut fsc = FileSetConfig::builder();
159163
let mut local_filesets = vec![];
@@ -291,6 +295,22 @@ impl ProjectFolders {
291295
}
292296
}
293297

298+
if let Some(user_config_path) = user_config_dir_path {
299+
let ratoml_path = {
300+
let mut p = user_config_path.to_path_buf();
301+
p.push("rust-analyzer.toml");
302+
p
303+
};
304+
305+
let file_set_roots = vec![VfsPath::from(ratoml_path.to_owned())];
306+
let entry = vfs::loader::Entry::Files(vec![ratoml_path.to_owned()]);
307+
308+
res.watch.push(res.load.len());
309+
res.load.push(entry);
310+
local_filesets.push(fsc.len() as u64);
311+
fsc.add_file_set(file_set_roots)
312+
}
313+
294314
let fsc = fsc.build();
295315
res.source_root_config = SourceRootConfig { fsc, local_filesets };
296316

‎crates/rust-analyzer/src/config.rs‎

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,7 @@
33
//! Of particular interest is the `feature_flags` hash map: while other fields
44
//! configure the server itself, feature flags are passed into analysis, and
55
//! tweak things like automatic insertion of `()` in completions.
6-
use std::{
7-
env, fmt, iter,
8-
ops::Not,
9-
sync::{LazyLock, OnceLock},
10-
};
6+
use std::{env, fmt, iter, ops::Not, sync::OnceLock};
117

128
use cfg::{CfgAtom, CfgDiff};
139
use hir::Symbol;
@@ -804,25 +800,14 @@ impl std::ops::Deref for Config {
804800
}
805801

806802
impl Config {
807-
/// 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.
808-
/// This path is equal to:
809-
///
810-
/// |Platform | Value | Example |
811-
/// | ------- | ------------------------------------- | ---------------------------------------- |
812-
/// | Linux | `$XDG_CONFIG_HOME` or `$HOME`/.config | /home/alice/.config |
813-
/// | macOS | `$HOME`/Library/Application Support | /Users/Alice/Library/Application Support |
814-
/// | Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming |
815-
pub fn user_config_path() -> Option<&'static AbsPath> {
816-
static USER_CONFIG_PATH: LazyLock<Option<AbsPathBuf>> = LazyLock::new(|| {
817-
let user_config_path = if let Some(path) = env::var_os("__TEST_RA_USER_CONFIG_DIR") {
818-
std::path::PathBuf::from(path)
819-
} else {
820-
dirs::config_dir()?.join("rust-analyzer")
821-
}
822-
.join("rust-analyzer.toml");
823-
Some(AbsPathBuf::assert_utf8(user_config_path))
824-
});
825-
USER_CONFIG_PATH.as_deref()
803+
/// 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.
804+
pub fn user_config_dir_path() -> Option<AbsPathBuf> {
805+
let user_config_path = if let Some(path) = env::var_os("__TEST_RA_USER_CONFIG_DIR") {
806+
std::path::PathBuf::from(path)
807+
} else {
808+
dirs::config_dir()?.join("rust-analyzer")
809+
};
810+
Some(AbsPathBuf::assert_utf8(user_config_path))
826811
}
827812

828813
pub fn same_source_root_parent_map(
@@ -1262,7 +1247,7 @@ pub struct NotificationsConfig {
12621247
pub cargo_toml_not_found: bool,
12631248
}
12641249

1265-
#[derive(Deserialize, Serialize, Debug, Clone)]
1250+
#[derive(Debug, Clone)]
12661251
pub enum RustfmtConfig {
12671252
Rustfmt { extra_args: Vec<String>, enable_range_formatting: bool },
12681253
CustomCommand { command: String, args: Vec<String> },

‎crates/rust-analyzer/src/global_state.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,14 @@ impl GlobalState {
392392
|| !self.config.same_source_root_parent_map(&self.local_roots_parent_map)
393393
{
394394
let config_change = {
395-
let user_config_path = Config::user_config_path();
395+
let user_config_path = (|| {
396+
let mut p = Config::user_config_dir_path()?;
397+
p.push("rust-analyzer.toml");
398+
Some(p)
399+
})();
400+
401+
let user_config_abs_path = user_config_path.as_deref();
402+
396403
let mut change = ConfigChange::default();
397404
let db = self.analysis_host.raw_database();
398405

@@ -411,7 +418,7 @@ impl GlobalState {
411418
.collect_vec();
412419

413420
for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files {
414-
if vfs_path.as_path() == user_config_path {
421+
if vfs_path.as_path() == user_config_abs_path {
415422
change.change_user_config(Some(db.file_text(file_id)));
416423
continue;
417424
}

‎crates/rust-analyzer/src/reload.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ impl GlobalState {
590590
}
591591

592592
watchers.extend(
593-
iter::once(Config::user_config_path())
593+
iter::once(Config::user_config_dir_path().as_deref())
594594
.chain(self.workspaces.iter().map(|ws| ws.manifest().map(ManifestPath::as_ref)))
595595
.flatten()
596596
.map(|glob_pattern| lsp_types::FileSystemWatcher {
@@ -613,7 +613,11 @@ impl GlobalState {
613613
}
614614

615615
let files_config = self.config.files();
616-
let project_folders = ProjectFolders::new(&self.workspaces, &files_config.exclude);
616+
let project_folders = ProjectFolders::new(
617+
&self.workspaces,
618+
&files_config.exclude,
619+
Config::user_config_dir_path().as_deref(),
620+
);
617621

618622
if (self.proc_macro_clients.is_empty() || !same_workspaces)
619623
&& self.config.expand_proc_macros()

‎crates/rust-analyzer/tests/slow-tests/ratoml.rs‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ impl RatomlTest {
4646
project = project.with_config(client_config);
4747
}
4848

49-
let server = project.server().wait_until_workspace_is_loaded();
49+
let server = project.server_with_lock(true).wait_until_workspace_is_loaded();
5050

5151
let mut case = Self { urls: vec![], server, tmp_path };
5252
let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::<Vec<_>>();
@@ -72,7 +72,7 @@ impl RatomlTest {
7272
let mut spl = spl.into_iter();
7373
if let Some(first) = spl.next() {
7474
if first == "$$CONFIG_DIR$$" {
75-
path = Config::user_config_path().unwrap().to_path_buf().into();
75+
path = Config::user_config_dir_path().unwrap().into();
7676
} else {
7777
path = path.join(first);
7878
}
@@ -285,7 +285,6 @@ enum Value {
285285
// }
286286

287287
#[test]
288-
#[ignore = "the user config is currently not being watched on startup, fix this"]
289288
fn ratoml_user_config_detected() {
290289
if skip_slow_tests() {
291290
return;
@@ -294,7 +293,7 @@ fn ratoml_user_config_detected() {
294293
let server = RatomlTest::new(
295294
vec![
296295
r#"
297-
//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml
296+
//- /$$CONFIG_DIR$$/rust-analyzer.toml
298297
assist.emitMustUse = true
299298
"#,
300299
r#"
@@ -322,7 +321,6 @@ enum Value {
322321
}
323322

324323
#[test]
325-
#[ignore = "the user config is currently not being watched on startup, fix this"]
326324
fn ratoml_create_user_config() {
327325
if skip_slow_tests() {
328326
return;
@@ -353,10 +351,7 @@ enum Value {
353351
1,
354352
InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
355353
);
356-
server.create(
357-
"//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml",
358-
RatomlTest::EMIT_MUST_USE.to_owned(),
359-
);
354+
server.create("//- /$$CONFIG_DIR$$/rust-analyzer.toml", RatomlTest::EMIT_MUST_USE.to_owned());
360355
server.query(
361356
InternalTestingFetchConfigOption::AssistEmitMustUse,
362357
1,
@@ -365,7 +360,6 @@ enum Value {
365360
}
366361

367362
#[test]
368-
#[ignore = "the user config is currently not being watched on startup, fix this"]
369363
fn ratoml_modify_user_config() {
370364
if skip_slow_tests() {
371365
return;
@@ -386,7 +380,7 @@ enum Value {
386380
Text(String),
387381
}"#,
388382
r#"
389-
//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml
383+
//- /$$CONFIG_DIR$$/rust-analyzer.toml
390384
assist.emitMustUse = true"#,
391385
],
392386
vec!["p1"],
@@ -407,7 +401,6 @@ assist.emitMustUse = true"#,
407401
}
408402

409403
#[test]
410-
#[ignore = "the user config is currently not being watched on startup, fix this"]
411404
fn ratoml_delete_user_config() {
412405
if skip_slow_tests() {
413406
return;
@@ -428,7 +421,7 @@ enum Value {
428421
Text(String),
429422
}"#,
430423
r#"
431-
//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml
424+
//- /$$CONFIG_DIR$$/rust-analyzer.toml
432425
assist.emitMustUse = true"#,
433426
],
434427
vec!["p1"],

‎crates/rust-analyzer/tests/slow-tests/support.rs‎

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::{
22
cell::{Cell, RefCell},
3-
fs,
4-
sync::Once,
3+
env, fs,
4+
sync::{Once, OnceLock},
55
time::Duration,
66
};
77

@@ -127,7 +127,53 @@ impl Project<'_> {
127127
}
128128

129129
pub(crate) fn server(self) -> Server {
130-
static CONFIG_DIR_LOCK: Mutex<()> = Mutex::new(());
130+
Project::server_with_lock(self, false)
131+
}
132+
133+
/// `prelock` : Forcefully acquire a lock that will maintain the path to the config dir throughout the whole test.
134+
///
135+
/// When testing we set the user config dir by setting an envvar `__TEST_RA_USER_CONFIG_DIR`.
136+
/// This value must be maintained until the end of a test case. When tests run in parallel
137+
/// this value may change thus making the tests flaky. As such, we use a `MutexGuard` that locks
138+
/// the process until `Server` is dropped. To optimize parallelization we use a lock only when it is
139+
/// needed, that is when a test uses config directory to do stuff. Our naive approach is to use a lock
140+
/// if there is a path to config dir in the test fixture. However, in certain cases we create a
141+
/// file in the config dir after server is run, something where our naive approach comes short.
142+
/// Using a `prelock` allows us to force a lock when we know we need it.
143+
pub(crate) fn server_with_lock(self, config_lock: bool) -> Server {
144+
static CONFIG_DIR_LOCK: OnceLock<(Utf8PathBuf, Mutex<()>)> = OnceLock::new();
145+
146+
let config_dir_guard = if config_lock {
147+
Some({
148+
let (path, mutex) = CONFIG_DIR_LOCK.get_or_init(|| {
149+
let value = TestDir::new().keep().path().to_owned();
150+
env::set_var("__TEST_RA_USER_CONFIG_DIR", &value);
151+
(value, Mutex::new(()))
152+
});
153+
#[allow(dyn_drop)]
154+
(mutex.lock(), {
155+
Box::new({
156+
struct Dropper(Utf8PathBuf);
157+
impl Drop for Dropper {
158+
fn drop(&mut self) {
159+
for entry in fs::read_dir(&self.0).unwrap() {
160+
let path = entry.unwrap().path();
161+
if path.is_file() {
162+
fs::remove_file(path).unwrap();
163+
} else if path.is_dir() {
164+
fs::remove_dir_all(path).unwrap();
165+
}
166+
}
167+
}
168+
}
169+
Dropper(path.clone())
170+
}) as Box<dyn Drop>
171+
})
172+
})
173+
} else {
174+
None
175+
};
176+
131177
let tmp_dir = self.tmp_dir.unwrap_or_else(|| {
132178
if self.root_dir_contains_symlink {
133179
TestDir::new_symlink()
@@ -160,13 +206,9 @@ impl Project<'_> {
160206
assert!(mini_core.is_none());
161207
assert!(toolchain.is_none());
162208

163-
let mut config_dir_guard = None;
164209
for entry in fixture {
165210
if let Some(pth) = entry.path.strip_prefix("/$$CONFIG_DIR$$") {
166-
if config_dir_guard.is_none() {
167-
config_dir_guard = Some(CONFIG_DIR_LOCK.lock());
168-
}
169-
let path = Config::user_config_path().unwrap().join(&pth['/'.len_utf8()..]);
211+
let path = Config::user_config_dir_path().unwrap().join(&pth['/'.len_utf8()..]);
170212
fs::create_dir_all(path.parent().unwrap()).unwrap();
171213
fs::write(path.as_path(), entry.text.as_bytes()).unwrap();
172214
} else {
@@ -269,12 +311,14 @@ pub(crate) struct Server {
269311
client: Connection,
270312
/// XXX: remove the tempdir last
271313
dir: TestDir,
272-
_config_dir_guard: Option<MutexGuard<'static, ()>>,
314+
#[allow(dyn_drop)]
315+
_config_dir_guard: Option<(MutexGuard<'static, ()>, Box<dyn Drop>)>,
273316
}
274317

275318
impl Server {
319+
#[allow(dyn_drop)]
276320
fn new(
277-
config_dir_guard: Option<MutexGuard<'static, ()>>,
321+
config_dir_guard: Option<(MutexGuard<'static, ()>, Box<dyn Drop>)>,
278322
dir: TestDir,
279323
config: Config,
280324
) -> Server {

0 commit comments

Comments
 (0)