From 0ff36dfe42325d51d1da786e4b2bb8fc86868ef2 Mon Sep 17 00:00:00 2001 From: Yash Thakur <45539777+ysthakur@users.noreply.github.com> Date: Wed, 13 Mar 2024 07:26:06 -0400 Subject: [PATCH] Canonicalize each component of config files (#12167) # Description Because `std::fs::canonicalize` requires the path to exist, this PR makes it so that when canonicalizing any config file, the `$nu.default-config-dir/nushell` part is canonicalized first, then `$nu.default-config-dir/nushell/foo.nu` is canonicalized. This should also fix the issue @devyn pointed out [here](https://github.com/nushell/nushell/pull/12118#issuecomment-1989546708) where a couple of the tests failed if one's `~/.config/nushell` folder was actually a symlink to a different folder. The tests previously didn't canonicalize the expected paths. I was going to make a PR that caches the config directory on startup (as suggested by fdncred and Ian in Discord), but I can make that part of this PR if we want to avoid creating unnecessary PRs. I think it probably makes more sense to separate them though. # User-Facing Changes # Tests + Formatting # After Submitting --- crates/nu-cli/src/config_files.rs | 4 +++- crates/nu-protocol/src/eval_const.rs | 6 ++++-- src/config_files.rs | 3 ++- src/tests/test_config_path.rs | 29 ++++++++++++++-------------- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 49620ff5fa..794d5dcc03 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -74,8 +74,10 @@ pub fn add_plugin_file( } else if let Some(mut plugin_path) = nu_path::config_dir() { // Path to store plugins signatures plugin_path.push(storage_path); + let mut plugin_path = canonicalize_with(&plugin_path, &cwd).unwrap_or(plugin_path); plugin_path.push(PLUGIN_FILE); - engine_state.plugin_signatures = Some(plugin_path.clone()); + let plugin_path = canonicalize_with(&plugin_path, &cwd).unwrap_or(plugin_path); + engine_state.plugin_signatures = Some(plugin_path); } } diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index bb5652cc6a..63ec61973c 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -54,7 +54,8 @@ pub fn create_nu_constant(engine_state: &EngineState, span: Span) -> Result Result canonicalize_with(&s.item, cwd).ok(), None => nu_path::config_dir().map(|mut p| { p.push(NUSHELL_FOLDER); + let mut p = canonicalize_with(&p, cwd).unwrap_or(p); p.push(default_config_name); - p + canonicalize_with(&p, cwd).unwrap_or(p) }), }; diff --git a/src/tests/test_config_path.rs b/src/tests/test_config_path.rs index 0b73df999e..86a2ccf79e 100644 --- a/src/tests/test_config_path.rs +++ b/src/tests/test_config_path.rs @@ -1,3 +1,4 @@ +use nu_path::canonicalize_with; use nu_test_support::nu; use nu_test_support::playground::{Executable, Playground}; use pretty_assertions::assert_eq; @@ -20,18 +21,24 @@ fn adjust_canonicalization>(p: P) -> String { } } -/// Make the config directory a symlink that points to a temporary folder. +/// Make the config directory a symlink that points to a temporary folder, and also makes +/// the nushell directory inside a symlink. /// Returns the path to the `nushell` config folder inside, via the symlink. fn setup_fake_config(playground: &mut Playground) -> PathBuf { - let config_dir = "config"; + let config_dir = "config_real"; let config_link = "config_link"; - playground.mkdir(&format!("{config_dir}/nushell")); + let nushell_real = "nushell_real"; + let nushell_config_dir = Path::new(config_dir).join("nushell").display().to_string(); + playground.mkdir(nushell_real); + playground.mkdir(config_dir); + playground.symlink(nushell_real, &nushell_config_dir); playground.symlink(config_dir, config_link); playground.with_env( "XDG_CONFIG_HOME", &playground.cwd().join(config_link).display().to_string(), ); - playground.cwd().join(config_link).join("nushell") + let path = Path::new(config_link).join("nushell"); + canonicalize_with(&path, playground.cwd()).unwrap_or(path) } fn run(playground: &mut Playground, command: &str) -> String { @@ -211,13 +218,10 @@ fn test_xdg_config_empty() { playground.with_env("XDG_CONFIG_HOME", ""); let actual = nu!("$nu.default-config-dir"); + let expected = dirs_next::config_dir().unwrap().join("nushell"); assert_eq!( actual.out, - dirs_next::config_dir() - .unwrap() - .join("nushell") - .display() - .to_string() + adjust_canonicalization(expected.canonicalize().unwrap_or(expected)) ); }); } @@ -228,13 +232,10 @@ fn test_xdg_config_bad() { playground.with_env("XDG_CONFIG_HOME", r#"mn2''6t\/k*((*&^//k//: "#); let actual = nu!("$nu.default-config-dir"); + let expected = dirs_next::config_dir().unwrap().join("nushell"); assert_eq!( actual.out, - dirs_next::config_dir() - .unwrap() - .join("nushell") - .display() - .to_string() + adjust_canonicalization(expected.canonicalize().unwrap_or(expected)) ); }); }