fix(config)!: store override paths as given

`rustup override set` canonicalized the directory before storing it, so
`settings.toml` could end up with a path the user never typed: `--path`
on a symlink recorded its target, and a relative `--path` to a directory
that did not exist yet was recorded verbatim and could never match.

The current directory is now stored as rustup sees it, and `--path` is
only made absolute, relative to that directory. Since overrides are
matched by the directory a stored path leads to, entries written by
earlier versions keep working, and one set on a symlink applies to its
target as well.

Author: cachebag

src/cli/rustup_mode.rs

@@ -3,7 +3,7 @@ use std::{
     env::consts::EXE_SUFFIX,
     fmt,
     io::{self, Write},
-    path::{Path, PathBuf},
+    path::{Path, PathBuf, absolute},
     process::ExitStatus,
     str::FromStr,
     sync::Arc,
@@ -1750,7 +1750,12 @@ async fn override_add(
         Err(e) => Err(e)?,
     }
 
-    cfg.make_override(path.unwrap_or(&cfg.current_dir), &toolchain)?;
+    // Relative to the directory rustup runs in, which `absolute` alone would
+    // take from the OS rather than from `cfg`.
+    let path = path
+        .map(|path| absolute(cfg.current_dir.join(path)))
+        .transpose()?;
+    cfg.make_override(path.as_deref().unwrap_or(&cfg.current_dir), &toolchain)?;
     Ok(ExitCode::SUCCESS)
 }
 
@@ -1774,7 +1779,7 @@ fn override_remove(
         }
         list
     } else if let Some(path) = path {
-        vec![path.to_owned()]
+        vec![absolute(cfg.current_dir.join(path))?]
     } else {
         vec![cfg.current_dir.clone()]
     };

src/settings.rs

@@ -105,16 +105,8 @@ pub struct Settings {
 }
 
 impl Settings {
-    fn path_to_key(path: &Path) -> String {
-        if path.exists() {
-            utils::canonicalize_path(path).display().to_string()
-        } else {
-            path.display().to_string()
-        }
-    }
-
     pub(crate) fn remove_override(&mut self, path: &Path) -> bool {
-        let key = Self::path_to_key(path);
+        let key = path.display().to_string();
         if self.overrides.remove(&key).is_some() {
             return true;
         }
@@ -128,7 +120,7 @@ impl Settings {
     }
 
     pub(crate) fn add_override(&mut self, path: &Path, toolchain: String) {
-        let key = Self::path_to_key(path);
+        let key = path.display().to_string();
         // Keep a single override per directory, whichever path it was stored under.
         if let Ok(dir) = Handle::from_path(path) {
             self.overrides.retain(|stored, _| !leads_to(stored, &dir));

tests/suite/cli_exact.rs

@@ -513,21 +513,14 @@ info: override toolchain for '[PATH]' removed
 #[tokio::test]
 async fn list_overrides() {
     let cx = CliTestContext::new(Scenario::SimpleV2).await;
-    let cwd = fs::canonicalize(cx.config.current_dir()).unwrap();
-    let mut cwd_formatted = format!("{}", cwd.display());
-
-    if cfg!(windows) {
-        cwd_formatted.drain(..4);
-    }
-
     cx.config
         .expect(["rustup", "override", "add", "nightly"])
         .await
         .is_ok();
     cx.config
         .expect(["rustup", "override", "list"])
         .await
-        .extend_redactions([("[CWD]", cwd_formatted)])
+        .extend_redactions([("[CWD]", cx.config.current_dir())])
         .is_ok()
         .with_stdout(snapbox::str![[r#"
 [CWD]	nightly             
@@ -550,24 +543,19 @@ async fn list_overrides_with_nonexistent() {
             .expect(["rustup", "override", "add", "nightly"])
             .await
             .is_ok();
-        fs::canonicalize(dir.path()).unwrap()
+        dir.path().to_owned()
     };
     // FIXME TempDir seems to succumb to difficulties removing dirs on windows
     let _ = raw::remove_dir(&nonexistent_path);
     assert!(!nonexistent_path.exists());
-    let mut path_formatted = format!("{}", nonexistent_path.display());
-
-    if cfg!(windows) {
-        path_formatted.drain(..4);
-    }
 
     cx.config
         .expect(["rustup", "override", "list"])
         .await
-        .extend_redactions([("[PATH]", path_formatted + " (not a directory)")])
+        .extend_redactions([("[PATH]", nonexistent_path)])
         .is_ok()
         .with_stdout(snapbox::str![[r#"
-[PATH]	nightly             
+[PATH] (not a directory)	nightly             
 
 
 "#]])
@@ -765,14 +753,15 @@ async fn override_set_with_path_symlink() {
         .await
         .is_ok();
 
-    // The symlink is resolved before the override is stored.
+    // The override is stored under the path as given, and applies to the
+    // directory however it is reached.
     cx.config
         .expect(["rustup", "override", "list"])
         .await
         .extend_redactions([("[LINK]", link.clone()), ("[TARGET]", target.clone())])
         .is_ok()
         .with_stdout(snapbox::str![[r#"
-[TARGET]	nightly             
+[LINK]	nightly             
 
 "#]])
         .with_stderr(snapbox::str![[""]]);
@@ -801,13 +790,15 @@ async fn override_set_with_path_relative_nonexistent() {
         .await
         .is_ok();
 
-    // The relative path is stored as given, so it can never match a directory.
+    // The relative path is made absolute, so it matches the directory once
+    // that exists.
     cx.config
         .expect(["rustup", "override", "list"])
         .await
+        .extend_redactions([("[CWD]", cx.config.current_dir())])
         .is_ok()
         .with_stdout(snapbox::str![[r#"
-later (not a directory)                 	nightly             
+[CWD]/later (not a directory)	nightly             
 
 
 "#]])
@@ -824,7 +815,7 @@ info: you may remove overrides for non-existent directories with
         .await
         .is_ok()
         .with_stdout(snapbox::str![[r#"
-1.1.0 (hash-stable-1.1.0)
+1.3.0 (hash-nightly-2)
 
 "#]]);
 }