Merge pull request #18441 from Veykril/lw-psyvmlotlvqn

internal: Do not cache the config directory path

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 };
 

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::{
+use std::{env, fmt, iter, ops::Not, sync::OnceLock};
-    env, fmt, iter,
-    ops::Not,
-    sync::{LazyLock, 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.
+    /// 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.
-    /// This path is equal to:
+    pub fn user_config_dir_path() -> Option<AbsPathBuf> {
-    ///
+        let user_config_path = if let Some(path) = env::var_os("__TEST_RA_USER_CONFIG_DIR") {
-    /// |Platform | Value                                 | Example                                  |
+            std::path::PathBuf::from(path)
-    /// | ------- | ------------------------------------- | ---------------------------------------- |
+        } else {
-    /// | Linux   | `$XDG_CONFIG_HOME` or `$HOME`/.config | /home/alice/.config                      |
+            dirs::config_dir()?.join("rust-analyzer")
-    /// | macOS   | `$HOME`/Library/Application Support   | /Users/Alice/Library/Application Support |
+        };
-    /// | Windows | `{FOLDERID_RoamingAppData}`           | C:\Users\Alice\AppData\Roaming           |
+        Some(AbsPathBuf::assert_utf8(user_config_path))
-    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()
     }
 
     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> },

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;
                     }

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()

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(
+    server.create("//- /$$CONFIG_DIR$$/rust-analyzer.toml", RatomlTest::EMIT_MUST_USE.to_owned());
-        "//- /$$CONFIG_DIR$$/rust-analyzer/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"],

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

@@ -1,7 +1,7 @@
 use std::{
     cell::{Cell, RefCell},
-    fs,
+    env, fs,
-    sync::Once,
+    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() {
+                let path = Config::user_config_dir_path().unwrap().join(&pth['/'.len_utf8()..]);
-                    config_dir_guard = Some(CONFIG_DIR_LOCK.lock());
-                }
-                let path = Config::user_config_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 {