Canonicalize config dir (#12136)

It turns out that my previous PR,
https://github.com/nushell/nushell/pull/11999, didn't properly
canonicalize `$nu.default-config-dir` in a scenario where
`XDG_CONFIG_HOME` (or the equivalent on each platform) was a symlink. To
remedy that, this PR makes `nu_path::config_dir()` return a
canonicalized path. This probably shouldn't break anything (except maybe
tests relying on the old behavior), since the canonical path will be
equivalent to non-canonical paths.

# User-Facing Changes

A user may get a path with symlinks resolved and `..`s replaced where
they previously didn't. I'm not sure where this would happen, though,
and anyway, the canonical path is probably the "correct" thing to
present to the user. We're using `omnipath` to make the path presentable
to the user on Windows, so there's no danger of someone getting an path
with `\\?` there.

# Tests + Formatting

The tests for config files have been updated to run the binary using the
`Director` so that it has access to the `XDG_CONFIG_HOME`/`HOME`
environment variables to be able to change the config directory.

crates/nu-cli/src/config_files.rs

@@ -71,8 +71,7 @@ pub fn add_plugin_file(
             let e = ParseError::FileNotFound(plugin_file.item, plugin_file.span);
             report_error(&working_set, &e);
         }
-    } else if let Some(plugin_path) = nu_path::config_dir() {
+    } else if let Some(mut plugin_path) = nu_path::config_dir() {
-        let mut plugin_path = canonicalize_with(&plugin_path, cwd).unwrap_or(plugin_path);
         // Path to store plugins signatures
         plugin_path.push(storage_path);
         plugin_path.push(PLUGIN_FILE);

crates/nu-path/src/helpers.rs

@@ -7,7 +7,7 @@ pub fn home_dir() -> Option<PathBuf> {
 }
 
 pub fn config_dir() -> Option<PathBuf> {
-    dirs_next::config_dir()
+    dirs_next::config_dir().map(|path| canonicalize(&path).unwrap_or(path))
 }
 
 #[cfg(windows)]

src/tests/test_config_path.rs

@@ -1,12 +1,8 @@
 use nu_test_support::nu;
+use nu_test_support::playground::{Executable, Playground};
 use pretty_assertions::assert_eq;
 use std::fs;
-use std::path::Path;
+use std::path::{Path, PathBuf};
-
-#[cfg(any(target_os = "linux", target_os = "macos"))]
-use nu_test_support::playground::Playground;
-#[cfg(any(target_os = "linux", target_os = "macos"))]
-use std::path::PathBuf;
 
 #[cfg(not(target_os = "windows"))]
 fn adjust_canonicalization<P: AsRef<Path>>(p: P) -> String {
@@ -40,76 +36,97 @@ fn setup_fake_config(playground: &mut Playground) -> PathBuf {
             "XDG_CONFIG_HOME",
             &playground.cwd().join(config_link).display().to_string(),
         );
-        Path::new(config_link).join("nushell")
+        playground.cwd().join(config_link).join("nushell")
     }
 
     #[cfg(target_os = "macos")]
     {
         let fake_home = "fake_home";
         let home_link = "home_link";
-        let dir_end = "fake-home/Library/Application\\ Support/nushell";
+        let dir_end = "Library/Application Support/nushell";
         playground.mkdir(&format!("{fake_home}/{dir_end}"));
         playground.symlink(fake_home, home_link);
         playground.with_env(
             "HOME",
             &playground.cwd().join(home_link).display().to_string(),
         );
-        PathBuf::from(home_link).join(dir_end)
+        playground.cwd().join(home_link).join(dir_end)
     }
 }
 
-fn test_config_path_helper() {
+fn run(playground: &mut Playground, command: &str) -> String {
-    let config_dir = nu_path::config_dir().expect("Could not get config directory");
+    let result = playground.pipeline(command).execute().map_err(|e| {
-    let config_dir_nushell = config_dir.join("nushell");
+        let outcome = e.output.map(|outcome| {
+            format!(
+                "out: '{}', err: '{}'",
+                String::from_utf8_lossy(&outcome.out),
+                String::from_utf8_lossy(&outcome.err)
+            )
+        });
+        format!(
+            "desc: {}, exit: {:?}, outcome: {}",
+            e.desc,
+            e.exit,
+            outcome.unwrap_or("empty".to_owned())
+        )
+    });
+    String::from_utf8_lossy(&result.unwrap().out)
+        .trim()
+        .to_string()
+}
+
+fn test_config_path_helper(playground: &mut Playground, config_dir_nushell: PathBuf) {
     // Create the config dir folder structure if it does not already exist
     if !config_dir_nushell.exists() {
         let _ = fs::create_dir_all(&config_dir_nushell);
     }
-    let cwd = std::env::current_dir().expect("Could not get current working directory");
 
     let config_dir_nushell =
         std::fs::canonicalize(&config_dir_nushell).expect("canonicalize config dir failed");
-    let actual = nu!(cwd: &cwd, "$nu.default-config-dir");
+    let actual = run(playground, "$nu.default-config-dir");
-    assert_eq!(actual.out, adjust_canonicalization(&config_dir_nushell));
+    assert_eq!(actual, adjust_canonicalization(&config_dir_nushell));
 
     let config_path = config_dir_nushell.join("config.nu");
     // We use canonicalize here in case the config or env is symlinked since $nu.config-path is returning the canonicalized path in #8653
     let canon_config_path =
         adjust_canonicalization(std::fs::canonicalize(&config_path).unwrap_or(config_path));
-    let actual = nu!(cwd: &cwd, "$nu.config-path");
+    let actual = run(playground, "$nu.config-path");
-    assert_eq!(actual.out, canon_config_path);
+    assert_eq!(actual, canon_config_path);
 
     let env_path = config_dir_nushell.join("env.nu");
     let canon_env_path =
         adjust_canonicalization(std::fs::canonicalize(&env_path).unwrap_or(env_path));
-    let actual = nu!(cwd: &cwd, "$nu.env-path");
+    let actual = run(playground, "$nu.env-path");
-    assert_eq!(actual.out, canon_env_path);
+    assert_eq!(actual, canon_env_path);
 
     let history_path = config_dir_nushell.join("history.txt");
     let canon_history_path =
         adjust_canonicalization(std::fs::canonicalize(&history_path).unwrap_or(history_path));
-    let actual = nu!(cwd: &cwd, "$nu.history-path");
+    let actual = run(playground, "$nu.history-path");
-    assert_eq!(actual.out, canon_history_path);
+    assert_eq!(actual, canon_history_path);
 
     let login_path = config_dir_nushell.join("login.nu");
     let canon_login_path =
         adjust_canonicalization(std::fs::canonicalize(&login_path).unwrap_or(login_path));
-    let actual = nu!(cwd: &cwd, "$nu.loginshell-path");
+    let actual = run(playground, "$nu.loginshell-path");
-    assert_eq!(actual.out, canon_login_path);
+    assert_eq!(actual, canon_login_path);
 
     #[cfg(feature = "plugin")]
     {
         let plugin_path = config_dir_nushell.join("plugin.nu");
         let canon_plugin_path =
             adjust_canonicalization(std::fs::canonicalize(&plugin_path).unwrap_or(plugin_path));
-        let actual = nu!(cwd: &cwd, "$nu.plugin-path");
+        let actual = run(playground, "$nu.plugin-path");
-        assert_eq!(actual.out, canon_plugin_path);
+        assert_eq!(actual, canon_plugin_path);
     }
 }
 
 #[test]
 fn test_default_config_path() {
-    test_config_path_helper();
+    Playground::setup("default_config_path", |_, playground| {
+        let config_dir = nu_path::config_dir().expect("Could not get config directory");
+        test_config_path_helper(playground, config_dir.join("nushell"));
+    });
 }
 
 /// Make the config folder a symlink to a temporary folder without any config files
@@ -118,9 +135,8 @@ fn test_default_config_path() {
 #[test]
 fn test_default_symlinked_config_path_empty() {
     Playground::setup("symlinked_empty_config_dir", |_, playground| {
-        let _ = setup_fake_config(playground);
+        let config_dir_nushell = setup_fake_config(playground);
-
+        test_config_path_helper(playground, config_dir_nushell);
-        test_config_path_helper();
     });
 }
 
@@ -148,7 +164,7 @@ fn test_default_symlink_config_path_broken_symlink_config_files() {
                 );
             }
 
-            test_config_path_helper();
+            test_config_path_helper(playground, fake_config_dir_nushell);
         },
     );
 }
@@ -178,7 +194,7 @@ fn test_default_config_path_symlinked_config_files() {
                 playground.symlink(empty_file, fake_config_dir_nushell.join(config_file));
             }
 
-            test_config_path_helper();
+            test_config_path_helper(playground, fake_config_dir_nushell);
         },
     );
 }