Skip to content

Commit 085aac3

Browse files
committed
Auto merge of rust-lang#17930 - Veykril:config-user-config, r=alibektas
Remove the ability to configure the user config path Being able to do this makes little sense as this is effectively a cyclic dependency (and we do not want to fixpoint this really).
2 parents a9e3555 + 879fa66 commit 085aac3

File tree

9 files changed

+75
-65
lines changed

9 files changed

+75
-65
lines changed

src/tools/rust-analyzer/crates/rust-analyzer/src/bin/main.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ fn run_server() -> anyhow::Result<()> {
227227
.filter(|workspaces| !workspaces.is_empty())
228228
.unwrap_or_else(|| vec![root_path.clone()]);
229229
let mut config =
230-
Config::new(root_path, capabilities, workspace_roots, visual_studio_code_version, None);
230+
Config::new(root_path, capabilities, workspace_roots, visual_studio_code_version);
231231
if let Some(json) = initialization_options {
232232
let mut change = ConfigChange::default();
233233
change.change_client_config(json);

src/tools/rust-analyzer/crates/rust-analyzer/src/cli/scip.rs

-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ impl flags::Scip {
3737
lsp_types::ClientCapabilities::default(),
3838
vec![],
3939
None,
40-
None,
4140
);
4241

4342
if let Some(p) = self.config_path {

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

+31-39
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
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::{fmt, iter, ops::Not, sync::OnceLock};
6+
use std::{
7+
env, fmt, iter,
8+
ops::Not,
9+
sync::{LazyLock, OnceLock},
10+
};
711

812
use cfg::{CfgAtom, CfgDiff};
9-
use dirs::config_dir;
1013
use hir::Symbol;
1114
use ide::{
1215
AssistConfig, CallableSnippets, CompletionConfig, DiagnosticsConfig, ExprFillDefaultMode,
@@ -735,7 +738,6 @@ pub enum RatomlFileKind {
735738
}
736739

737740
#[derive(Debug, Clone)]
738-
// FIXME @alibektas : Seems like a clippy warning of this sort should tell that combining different ConfigInputs into one enum was not a good idea.
739741
#[allow(clippy::large_enum_variant)]
740742
enum RatomlFile {
741743
Workspace(GlobalLocalConfigInput),
@@ -757,16 +759,6 @@ pub struct Config {
757759
/// by receiving a `lsp_types::notification::DidChangeConfiguration`.
758760
client_config: (FullConfigInput, ConfigErrors),
759761

760-
/// 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.
761-
/// If not specified by init of a `Config` object this value defaults to :
762-
///
763-
/// |Platform | Value | Example |
764-
/// | ------- | ------------------------------------- | ---------------------------------------- |
765-
/// | Linux | `$XDG_CONFIG_HOME` or `$HOME`/.config | /home/alice/.config |
766-
/// | macOS | `$HOME`/Library/Application Support | /Users/Alice/Library/Application Support |
767-
/// | Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming |
768-
user_config_path: VfsPath,
769-
770762
/// Config node whose values apply to **every** Rust project.
771763
user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>,
772764

@@ -794,8 +786,25 @@ impl std::ops::Deref for Config {
794786
}
795787

796788
impl Config {
797-
pub fn user_config_path(&self) -> &VfsPath {
798-
&self.user_config_path
789+
/// 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.
790+
/// This path is equal to:
791+
///
792+
/// |Platform | Value | Example |
793+
/// | ------- | ------------------------------------- | ---------------------------------------- |
794+
/// | Linux | `$XDG_CONFIG_HOME` or `$HOME`/.config | /home/alice/.config |
795+
/// | macOS | `$HOME`/Library/Application Support | /Users/Alice/Library/Application Support |
796+
/// | Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming |
797+
pub fn user_config_path() -> Option<&'static AbsPath> {
798+
static USER_CONFIG_PATH: LazyLock<Option<AbsPathBuf>> = LazyLock::new(|| {
799+
let user_config_path = if let Some(path) = env::var_os("__TEST_RA_USER_CONFIG_DIR") {
800+
std::path::PathBuf::from(path)
801+
} else {
802+
dirs::config_dir()?.join("rust-analyzer")
803+
}
804+
.join("rust-analyzer.toml");
805+
Some(AbsPathBuf::assert_utf8(user_config_path))
806+
});
807+
USER_CONFIG_PATH.as_deref()
799808
}
800809

801810
pub fn same_source_root_parent_map(
@@ -1315,24 +1324,8 @@ impl Config {
13151324
caps: lsp_types::ClientCapabilities,
13161325
workspace_roots: Vec<AbsPathBuf>,
13171326
visual_studio_code_version: Option<Version>,
1318-
user_config_path: Option<Utf8PathBuf>,
13191327
) -> Self {
13201328
static DEFAULT_CONFIG_DATA: OnceLock<&'static DefaultConfigData> = OnceLock::new();
1321-
let user_config_path = if let Some(user_config_path) = user_config_path {
1322-
user_config_path.join("rust-analyzer").join("rust-analyzer.toml")
1323-
} else {
1324-
let p = config_dir()
1325-
.expect("A config dir is expected to existed on all platforms ra supports.")
1326-
.join("rust-analyzer")
1327-
.join("rust-analyzer.toml");
1328-
Utf8PathBuf::from_path_buf(p).expect("Config dir expected to be abs.")
1329-
};
1330-
1331-
// A user config cannot be a virtual path as rust-analyzer cannot support watching changes in virtual paths.
1332-
// See `GlobalState::process_changes` to get more info.
1333-
// FIXME @alibektas : Temporary solution. I don't think this is right as at some point we may allow users to specify
1334-
// custom USER_CONFIG_PATHs which may also be relative.
1335-
let user_config_path = VfsPath::from(AbsPathBuf::assert(user_config_path));
13361329

13371330
Config {
13381331
caps: ClientCapabilities::new(caps),
@@ -1345,7 +1338,6 @@ impl Config {
13451338
default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())),
13461339
source_root_parent_map: Arc::new(FxHashMap::default()),
13471340
user_config: None,
1348-
user_config_path,
13491341
detached_files: Default::default(),
13501342
validation_errors: Default::default(),
13511343
ratoml_file: Default::default(),
@@ -3417,7 +3409,7 @@ mod tests {
34173409
#[test]
34183410
fn proc_macro_srv_null() {
34193411
let mut config =
3420-
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
3412+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
34213413

34223414
let mut change = ConfigChange::default();
34233415
change.change_client_config(serde_json::json!({
@@ -3432,7 +3424,7 @@ mod tests {
34323424
#[test]
34333425
fn proc_macro_srv_abs() {
34343426
let mut config =
3435-
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
3427+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
34363428
let mut change = ConfigChange::default();
34373429
change.change_client_config(serde_json::json!({
34383430
"procMacro" : {
@@ -3446,7 +3438,7 @@ mod tests {
34463438
#[test]
34473439
fn proc_macro_srv_rel() {
34483440
let mut config =
3449-
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
3441+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
34503442

34513443
let mut change = ConfigChange::default();
34523444

@@ -3466,7 +3458,7 @@ mod tests {
34663458
#[test]
34673459
fn cargo_target_dir_unset() {
34683460
let mut config =
3469-
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
3461+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
34703462

34713463
let mut change = ConfigChange::default();
34723464

@@ -3484,7 +3476,7 @@ mod tests {
34843476
#[test]
34853477
fn cargo_target_dir_subdir() {
34863478
let mut config =
3487-
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
3479+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
34883480

34893481
let mut change = ConfigChange::default();
34903482
change.change_client_config(serde_json::json!({
@@ -3502,7 +3494,7 @@ mod tests {
35023494
#[test]
35033495
fn cargo_target_dir_relative_dir() {
35043496
let mut config =
3505-
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
3497+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
35063498

35073499
let mut change = ConfigChange::default();
35083500
change.change_client_config(serde_json::json!({
@@ -3523,7 +3515,7 @@ mod tests {
35233515
#[test]
35243516
fn toml_unknown_key() {
35253517
let config =
3526-
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
3518+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
35273519

35283520
let mut change = ConfigChange::default();
35293521

src/tools/rust-analyzer/crates/rust-analyzer/src/diagnostics/to_proto.rs

-1
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,6 @@ mod tests {
548548
ClientCapabilities::default(),
549549
Vec::new(),
550550
None,
551-
None,
552551
),
553552
);
554553
let snap = state.snapshot();

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ impl GlobalState {
380380
|| !self.config.same_source_root_parent_map(&self.local_roots_parent_map)
381381
{
382382
let config_change = {
383-
let user_config_path = self.config.user_config_path();
383+
let user_config_path = Config::user_config_path();
384384
let mut change = ConfigChange::default();
385385
let db = self.analysis_host.raw_database();
386386

@@ -399,7 +399,7 @@ impl GlobalState {
399399
.collect_vec();
400400

401401
for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files {
402-
if vfs_path == *user_config_path {
402+
if vfs_path.as_path() == user_config_path {
403403
change.change_user_config(Some(db.file_text(file_id)));
404404
continue;
405405
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ impl GlobalState {
565565
};
566566

567567
watchers.extend(
568-
iter::once(self.config.user_config_path().as_path())
568+
iter::once(Config::user_config_path())
569569
.chain(self.workspaces.iter().map(|ws| ws.manifest().map(ManifestPath::as_ref)))
570570
.flatten()
571571
.map(|glob_pattern| lsp_types::FileSystemWatcher {

src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs

+4-8
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use lsp_types::{
88
};
99
use paths::Utf8PathBuf;
1010

11+
use rust_analyzer::config::Config;
1112
use rust_analyzer::lsp::ext::{InternalTestingFetchConfig, InternalTestingFetchConfigParams};
1213
use serde_json::json;
1314
use test_utils::skip_slow_tests;
@@ -24,7 +25,6 @@ struct RatomlTest {
2425
urls: Vec<Url>,
2526
server: Server,
2627
tmp_path: Utf8PathBuf,
27-
user_config_dir: Utf8PathBuf,
2828
}
2929

3030
impl RatomlTest {
@@ -41,11 +41,7 @@ impl RatomlTest {
4141

4242
let full_fixture = fixtures.join("\n");
4343

44-
let user_cnf_dir = TestDir::new();
45-
let user_config_dir = user_cnf_dir.path().to_owned();
46-
47-
let mut project =
48-
Project::with_fixture(&full_fixture).tmp_dir(tmp_dir).user_config_dir(user_cnf_dir);
44+
let mut project = Project::with_fixture(&full_fixture).tmp_dir(tmp_dir);
4945

5046
for root in roots {
5147
project = project.root(root);
@@ -57,7 +53,7 @@ impl RatomlTest {
5753

5854
let server = project.server().wait_until_workspace_is_loaded();
5955

60-
let mut case = Self { urls: vec![], server, tmp_path, user_config_dir };
56+
let mut case = Self { urls: vec![], server, tmp_path };
6157
let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::<Vec<_>>();
6258
case.urls = urls;
6359
case
@@ -81,7 +77,7 @@ impl RatomlTest {
8177
let mut spl = spl.into_iter();
8278
if let Some(first) = spl.next() {
8379
if first == "$$CONFIG_DIR$$" {
84-
path = self.user_config_dir.clone();
80+
path = Config::user_config_path().unwrap().to_path_buf().into();
8581
} else {
8682
path = path.join(first);
8783
}

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

+22-12
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use std::{
88
use crossbeam_channel::{after, select, Receiver};
99
use lsp_server::{Connection, Message, Notification, Request};
1010
use lsp_types::{notification::Exit, request::Shutdown, TextDocumentIdentifier, Url};
11+
use parking_lot::{Mutex, MutexGuard};
1112
use paths::{Utf8Path, Utf8PathBuf};
1213
use rust_analyzer::{
1314
config::{Config, ConfigChange, ConfigErrors},
@@ -27,7 +28,6 @@ pub(crate) struct Project<'a> {
2728
roots: Vec<Utf8PathBuf>,
2829
config: serde_json::Value,
2930
root_dir_contains_symlink: bool,
30-
user_config_path: Option<Utf8PathBuf>,
3131
}
3232

3333
impl Project<'_> {
@@ -51,15 +51,9 @@ impl Project<'_> {
5151
}
5252
}),
5353
root_dir_contains_symlink: false,
54-
user_config_path: None,
5554
}
5655
}
5756

58-
pub(crate) fn user_config_dir(mut self, config_path_dir: TestDir) -> Self {
59-
self.user_config_path = Some(config_path_dir.path().to_owned());
60-
self
61-
}
62-
6357
pub(crate) fn tmp_dir(mut self, tmp_dir: TestDir) -> Self {
6458
self.tmp_dir = Some(tmp_dir);
6559
self
@@ -91,6 +85,7 @@ impl Project<'_> {
9185
}
9286

9387
pub(crate) fn server(self) -> Server {
88+
static CONFIG_DIR_LOCK: Mutex<()> = Mutex::new(());
9489
let tmp_dir = self.tmp_dir.unwrap_or_else(|| {
9590
if self.root_dir_contains_symlink {
9691
TestDir::new_symlink()
@@ -122,9 +117,13 @@ impl Project<'_> {
122117
assert!(mini_core.is_none());
123118
assert!(toolchain.is_none());
124119

120+
let mut config_dir_guard = None;
125121
for entry in fixture {
126122
if let Some(pth) = entry.path.strip_prefix("/$$CONFIG_DIR$$") {
127-
let path = self.user_config_path.clone().unwrap().join(&pth['/'.len_utf8()..]);
123+
if config_dir_guard.is_none() {
124+
config_dir_guard = Some(CONFIG_DIR_LOCK.lock());
125+
}
126+
let path = Config::user_config_path().unwrap().join(&pth['/'.len_utf8()..]);
128127
fs::create_dir_all(path.parent().unwrap()).unwrap();
129128
fs::write(path.as_path(), entry.text.as_bytes()).unwrap();
130129
} else {
@@ -201,7 +200,6 @@ impl Project<'_> {
201200
},
202201
roots,
203202
None,
204-
self.user_config_path,
205203
);
206204
let mut change = ConfigChange::default();
207205

@@ -213,7 +211,7 @@ impl Project<'_> {
213211

214212
config.rediscover_workspaces();
215213

216-
Server::new(tmp_dir.keep(), config)
214+
Server::new(config_dir_guard, tmp_dir.keep(), config)
217215
}
218216
}
219217

@@ -228,18 +226,30 @@ pub(crate) struct Server {
228226
client: Connection,
229227
/// XXX: remove the tempdir last
230228
dir: TestDir,
229+
_config_dir_guard: Option<MutexGuard<'static, ()>>,
231230
}
232231

233232
impl Server {
234-
fn new(dir: TestDir, config: Config) -> Server {
233+
fn new(
234+
config_dir_guard: Option<MutexGuard<'static, ()>>,
235+
dir: TestDir,
236+
config: Config,
237+
) -> Server {
235238
let (connection, client) = Connection::memory();
236239

237240
let _thread = stdx::thread::Builder::new(stdx::thread::ThreadIntent::Worker)
238241
.name("test server".to_owned())
239242
.spawn(move || main_loop(config, connection).unwrap())
240243
.expect("failed to spawn a thread");
241244

242-
Server { req_id: Cell::new(1), dir, messages: Default::default(), client, _thread }
245+
Server {
246+
req_id: Cell::new(1),
247+
dir,
248+
messages: Default::default(),
249+
client,
250+
_thread,
251+
_config_dir_guard: config_dir_guard,
252+
}
243253
}
244254

245255
pub(crate) fn doc_id(&self, rel_path: &str) -> TextDocumentIdentifier {

src/tools/rust-analyzer/crates/vfs/src/vfs_path.rs

+14
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,20 @@ impl fmt::Debug for VfsPathRepr {
313313
}
314314
}
315315

316+
impl PartialEq<AbsPath> for VfsPath {
317+
fn eq(&self, other: &AbsPath) -> bool {
318+
match &self.0 {
319+
VfsPathRepr::PathBuf(lhs) => lhs == other,
320+
VfsPathRepr::VirtualPath(_) => false,
321+
}
322+
}
323+
}
324+
impl PartialEq<VfsPath> for AbsPath {
325+
fn eq(&self, other: &VfsPath) -> bool {
326+
other == self
327+
}
328+
}
329+
316330
/// `/`-separated virtual path.
317331
///
318332
/// This is used to describe files that do not reside on the file system.

0 commit comments

Comments
 (0)