notifications: log directly when path canonicalization fails

Author: djc

src/cli/rustup_mode.rs

@@ -1557,10 +1557,7 @@ fn override_remove(
     };
 
     for p in &paths {
-        if cfg
-            .settings_file
-            .with_mut(|s| Ok(s.remove_override(p, cfg.notify_handler.as_ref())))?
-        {
+        if cfg.settings_file.with_mut(|s| Ok(s.remove_override(p)))? {
             info!("override toolchain for '{}' removed", p.display());
         } else {
             info!("no override toolchain for '{}'", p.display());

src/config.rs

@@ -576,7 +576,7 @@ impl<'a> Cfg<'a> {
 
         while let Some(d) = dir {
             // First check the override database
-            if let Some(name) = settings.dir_override(d, notify) {
+            if let Some(name) = settings.dir_override(d) {
                 let reason = ActiveReason::OverrideDB(d.to_owned());
                 // Note that `rustup override set` fully resolves it's input
                 // before writing to settings.toml, so resolving here may not

src/notifications.rs

@@ -50,7 +50,6 @@ pub enum Notification<'a> {
     DownloadFinished(Option<&'a str>),
     /// Download has failed.
     DownloadFailed(&'a str),
-    NoCanonicalPath(&'a Path),
     ResumingPartialDownload,
     /// This would make more sense as a crate::notifications::Notification
     /// member, but the notification callback is already narrowed to
@@ -135,7 +134,6 @@ impl Notification<'_> {
             | UsingCurl
             | UsingReqwest => NotificationLevel::Debug,
             RenameInUse(_, _) => NotificationLevel::Info,
-            NoCanonicalPath(_) => NotificationLevel::Warn,
             Error(_) => NotificationLevel::Error,
             CreatingRoot(_) | CreatingFile(_) => NotificationLevel::Debug,
             FileDeletion(_, result) | DirectoryDeletion(_, result) => match result {
@@ -281,7 +279,6 @@ impl Display for Notification<'_> {
             DownloadDataReceived(data, _) => write!(f, "received some data of size {}", data.len()),
             DownloadFinished(_) => write!(f, "download finished"),
             DownloadFailed(_) => write!(f, "download failed"),
-            NoCanonicalPath(path) => write!(f, "could not canonicalize path: '{}'", path.display()),
             ResumingPartialDownload => write!(f, "resuming partial download"),
             UsingCurl => write!(f, "downloading with curl"),
             UsingReqwest => write!(f, "downloading with reqwest"),

src/settings.rs

@@ -98,22 +98,16 @@ pub struct Settings {
 }
 
 impl Settings {
-    fn path_to_key(path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> String {
+    fn path_to_key(path: &Path) -> String {
         if path.exists() {
-            utils::canonicalize_path(path, notify_handler)
-                .display()
-                .to_string()
+            utils::canonicalize_path(path).display().to_string()
         } else {
             path.display().to_string()
         }
     }
 
-    pub(crate) fn remove_override(
-        &mut self,
-        path: &Path,
-        notify_handler: &dyn Fn(Notification<'_>),
-    ) -> bool {
-        let key = Self::path_to_key(path, notify_handler);
+    pub(crate) fn remove_override(&mut self, path: &Path) -> bool {
+        let key = Self::path_to_key(path);
         self.overrides.remove(&key).is_some()
     }
 
@@ -123,17 +117,13 @@ impl Settings {
         toolchain: String,
         notify_handler: &dyn Fn(Notification<'_>),
     ) {
-        let key = Self::path_to_key(path, notify_handler);
+        let key = Self::path_to_key(path);
         notify_handler(Notification::SetOverrideToolchain(path, &toolchain));
         self.overrides.insert(key, toolchain);
     }
 
-    pub(crate) fn dir_override(
-        &self,
-        dir: &Path,
-        notify_handler: &dyn Fn(Notification<'_>),
-    ) -> Option<String> {
-        let key = Self::path_to_key(dir, notify_handler);
+    pub(crate) fn dir_override(&self, dir: &Path) -> Option<String> {
+        let key = Self::path_to_key(dir);
         self.overrides.get(&key).cloned()
     }
 

src/toolchain.rs

@@ -97,11 +97,11 @@ impl<'a> Toolchain<'a> {
             }
             ActiveReason::OverrideDB(path) => format!(
                 "the directory override for '{}' specifies an uninstalled toolchain",
-                utils::canonicalize_path(path, cfg.notify_handler.as_ref()).display(),
+                utils::canonicalize_path(path).display(),
             ),
             ActiveReason::ToolchainFile(path) => format!(
                 "the toolchain file at '{}' specifies an uninstalled toolchain",
-                utils::canonicalize_path(path, cfg.notify_handler.as_ref()).display(),
+                utils::canonicalize_path(path).display(),
             ),
             ActiveReason::Default => {
                 "the default toolchain does not describe an installed toolchain".to_string()

src/utils/mod.rs

@@ -11,7 +11,7 @@ use std::process::ExitStatus;
 use anyhow::{Context, Result, anyhow, bail};
 use retry::delay::{Fibonacci, jitter};
 use retry::{OperationResult, retry};
-use tracing::debug;
+use tracing::{debug, warn};
 use url::Url;
 
 use crate::errors::*;
@@ -127,12 +127,9 @@ pub(crate) fn filter_file<F: FnMut(&str) -> bool>(
     })
 }
 
-pub(crate) fn canonicalize_path<'a, N>(path: &'a Path, notify_handler: &dyn Fn(N)) -> PathBuf
-where
-    N: From<Notification<'a>>,
-{
+pub(crate) fn canonicalize_path(path: &Path) -> PathBuf {
     fs::canonicalize(path).unwrap_or_else(|_| {
-        notify_handler(Notification::NoCanonicalPath(path).into());
+        warn!(path = %path.display(), "could not canonicalize path");
         PathBuf::from(path)
     })
 }