Remove more Notification variants (part 3)

src/cli/self_update.rs

@@ -1427,7 +1427,7 @@ mod tests {
             );
             assert_eq!(
                 for_host!(
-                    r"info: profile set to 'default'
+                    r"info: profile set to default
 info: default host triple is {0}
 "
                 ),

src/config.rs

@@ -8,7 +8,7 @@ use anyhow::{Context, Result, anyhow, bail};
 use serde::Deserialize;
 use thiserror::Error as ThisError;
 use tokio_stream::StreamExt;
-use tracing::{error, info, trace};
+use tracing::{debug, error, info, trace, warn};
 
 use crate::dist::AutoInstallMode;
 use crate::{
@@ -251,7 +251,7 @@ impl<'a> Cfg<'a> {
 
         let settings_file = SettingsFile::new(rustup_dir.join("settings.toml"));
         settings_file.with(|s| {
-            (notify_handler)(Notification::ReadMetadataVersion(s.version));
+            debug!("read metadata version: {}", s.version);
             if s.version == MetadataVersion::default() {
                 Ok(())
             } else {
@@ -359,7 +359,7 @@ impl<'a> Cfg<'a> {
             s.profile = Some(profile);
             Ok(())
         })?;
-        (self.notify_handler)(Notification::SetProfile(profile.as_str()));
+        info!("profile set to {}", profile.as_str());
         Ok(())
     }
 
@@ -368,7 +368,7 @@ impl<'a> Cfg<'a> {
             s.auto_self_update = Some(mode);
             Ok(())
         })?;
-        (self.notify_handler)(Notification::SetSelfUpdate(mode.as_str()));
+        info!("auto-self-update mode set to {}", mode.as_str());
         Ok(())
     }
 
@@ -444,19 +444,20 @@ impl<'a> Cfg<'a> {
     pub(crate) fn upgrade_data(&self) -> Result<()> {
         let current_version = self.settings_file.with(|s| Ok(s.version))?;
         if current_version == MetadataVersion::default() {
-            (self.notify_handler)(Notification::MetadataUpgradeNotNeeded(current_version));
+            info!("nothing to upgrade: metadata version is already '{current_version}'");
             return Ok(());
         }
 
-        (self.notify_handler)(Notification::UpgradingMetadata(
-            current_version,
-            MetadataVersion::default(),
-        ));
-
+        info!(
+            "upgrading metadata version from {current_version} to {}",
+            MetadataVersion::default()
+        );
         match current_version {
             MetadataVersion::V2 => {
                 // The toolchain installation format changed. Just delete them all.
-                (self.notify_handler)(Notification::UpgradeRemovesToolchains);
+                warn!(
+                    "this upgrade will remove all existing toolchains; you will need to reinstall them"
+                );
 
                 let dirs = utils::read_dir("toolchains", &self.toolchains_dir)?;
                 for dir in dirs {
@@ -570,7 +571,6 @@ impl<'a> Cfg<'a> {
         dir: &Path,
         settings: &Settings,
     ) -> Result<Option<(OverrideCfg, ActiveReason)>> {
-        let notify = self.notify_handler.as_ref();
         let mut dir = Some(dir);
 
         while let Some(d) = dir {
@@ -608,10 +608,17 @@ impl<'a> Cfg<'a> {
                 (Ok(contents), Ok(_)) => {
                     // both `rust-toolchain` and `rust-toolchain.toml` exist
 
-                    notify(Notification::DuplicateToolchainFile {
-                        rust_toolchain: &path_rust_toolchain,
-                        rust_toolchain_toml: &path_rust_toolchain_toml,
-                    });
+                    warn!(
+                        "both {} and {} exist; using contents of {0}",
+                        path_rust_toolchain
+                            .canonicalize()
+                            .unwrap_or_else(|_| PathBuf::from(&path_rust_toolchain))
+                            .display(),
+                        path_rust_toolchain_toml
+                            .canonicalize()
+                            .unwrap_or_else(|_| PathBuf::from(&path_rust_toolchain_toml))
+                            .display(),
+                    );
 
                     (path_rust_toolchain, Ok(contents), ParseMode::Both)
                 }
@@ -815,7 +822,7 @@ impl<'a> Cfg<'a> {
             force_non_host,
         )?;
         if verbose {
-            (self.notify_handler)(Notification::LookingForToolchain(toolchain));
+            debug!("looking for installed toolchain {toolchain}");
         }
         let components: Vec<_> = components.iter().map(AsRef::as_ref).collect();
         let targets: Vec<_> = targets.iter().map(AsRef::as_ref).collect();
@@ -837,7 +844,7 @@ impl<'a> Cfg<'a> {
             }
             Ok(mut distributable) => {
                 if verbose {
-                    (self.notify_handler)(Notification::UsingExistingToolchain(toolchain));
+                    info!("using existing install for {toolchain}");
                 }
                 let status = if !distributable.components_exist(&components, &targets)? {
                     distributable.update(&components, &targets, profile).await?
@@ -919,7 +926,7 @@ impl<'a> Cfg<'a> {
     /// Create an override for a toolchain
     pub(crate) fn make_override(&self, path: &Path, toolchain: &ToolchainName) -> Result<()> {
         self.settings_file.with_mut(|s| {
-            s.add_override(path, toolchain.to_string(), self.notify_handler.as_ref());
+            s.add_override(path, toolchain.to_string());
             Ok(())
         })
     }

src/dist/temp.rs

@@ -36,7 +36,10 @@ impl Drop for Dir {
             match remove_dir_all::remove_dir_all(&self.path) {
                 Ok(()) => debug!(path = %self.path.display(), "deleted temp directory"),
                 Err(e) => {
-                    warn!(path = %self.path.display(), error = %e, "could not delete temp directory")
+                    warn!(
+                        "could not delete temp directory {} ({e})",
+                        self.path.display()
+                    )
                 }
             }
         }
@@ -62,7 +65,7 @@ impl Drop for File {
             match fs::remove_file(&self.path) {
                 Ok(()) => debug!(path = %self.path.display(), "deleted temp file"),
                 Err(e) => {
-                    warn!(path = %self.path.display(), error = %e, "could not delete temp file")
+                    warn!("could not delete temp file {} ({e})", self.path.display())
                 }
             }
         }

src/install.rs

@@ -3,12 +3,12 @@
 use std::path::{Path, PathBuf};
 
 use anyhow::Result;
+use tracing::debug;
 
 use crate::{
     config::Cfg,
     dist::{self, DistOptions, prefix::InstallPrefix},
     errors::RustupError,
-    notifications::Notification,
     toolchain::{CustomToolchainName, LocalToolchainName, Toolchain},
     utils,
 };
@@ -44,27 +44,26 @@ impl InstallMethod<'_> {
         let _ = rayon::ThreadPoolBuilder::new()
             .num_threads(self.cfg().process.io_thread_count()?)
             .build_global();
-        let nh = &self.cfg().notify_handler;
         match self {
             InstallMethod::Copy { .. }
             | InstallMethod::Link { .. }
             | InstallMethod::Dist(DistOptions {
                 old_date_version: None,
                 ..
-            }) => nh(Notification::InstallingToolchain(&self.dest_basename())),
-            _ => nh(Notification::UpdatingToolchain(&self.dest_basename())),
+            }) => debug!("installing toolchain {}", self.dest_basename()),
+            _ => debug!("updating existing install for '{}'", self.dest_basename()),
         }
 
-        nh(Notification::ToolchainDirectory(&self.dest_path()));
+        debug!("toolchain directory: {}", self.dest_path().display());
         let updated = self.run(&self.dest_path()).await?;
 
         let status = match updated {
             false => {
-                nh(Notification::UpdateHashMatches);
+                debug!("toolchain is already up to date");
                 UpdateStatus::Unchanged
             }
             true => {
-                nh(Notification::InstalledToolchain(&self.dest_basename()));
+                debug!("toolchain {} installed", self.dest_basename());
                 match self {
                     InstallMethod::Dist(DistOptions {
                         old_date_version: Some((_, v)),

src/notifications.rs

@@ -1,10 +1,8 @@
 use std::fmt::{self, Display};
-use std::path::{Path, PathBuf};
 
 use crate::dist::TargetTriple;
-use crate::settings::MetadataVersion;
+use crate::utils::notify::NotificationLevel;
 use crate::utils::units;
-use crate::{dist::ToolchainDesc, toolchain::ToolchainName, utils::notify::NotificationLevel};
 
 #[derive(Debug)]
 pub(crate) enum Notification<'a> {
@@ -26,27 +24,6 @@ pub(crate) enum Notification<'a> {
     /// member, but the notification callback is already narrowed to
     /// utils::notifications by the time tar unpacking is called.
     SetDefaultBufferSize(usize),
-    SetOverrideToolchain(&'a Path, &'a str),
-    SetProfile(&'a str),
-    SetSelfUpdate(&'a str),
-    LookingForToolchain(&'a ToolchainDesc),
-    ToolchainDirectory(&'a Path),
-    UpdatingToolchain(&'a str),
-    InstallingToolchain(&'a str),
-    InstalledToolchain(&'a str),
-    UsingExistingToolchain(&'a ToolchainDesc),
-    UninstallingToolchain(&'a ToolchainName),
-    UninstalledToolchain(&'a ToolchainName),
-    UpdateHashMatches,
-    UpgradingMetadata(MetadataVersion, MetadataVersion),
-    MetadataUpgradeNotNeeded(MetadataVersion),
-    ReadMetadataVersion(MetadataVersion),
-    UpgradeRemovesToolchains,
-    /// Both `rust-toolchain` and `rust-toolchain.toml` exist within a directory
-    DuplicateToolchainFile {
-        rust_toolchain: &'a Path,
-        rust_toolchain_toml: &'a Path,
-    },
 }
 
 impl Notification<'_> {
@@ -61,22 +38,6 @@ impl Notification<'_> {
             | DownloadDataReceived(_, _)
             | DownloadFinished(_)
             | DownloadFailed(_) => NotificationLevel::Debug,
-            ToolchainDirectory(_)
-            | LookingForToolchain(_)
-            | InstallingToolchain(_)
-            | UpdatingToolchain(_)
-            | ReadMetadataVersion(_)
-            | InstalledToolchain(_)
-            | UpdateHashMatches => NotificationLevel::Debug,
-            SetOverrideToolchain(_, _)
-            | SetProfile(_)
-            | SetSelfUpdate(_)
-            | UsingExistingToolchain(_)
-            | UninstallingToolchain(_)
-            | UninstalledToolchain(_)
-            | UpgradingMetadata(_, _)
-            | MetadataUpgradeNotNeeded(_) => NotificationLevel::Info,
-            UpgradeRemovesToolchains | DuplicateToolchainFile { .. } => NotificationLevel::Warn,
         }
     }
 }
@@ -104,50 +65,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"),
-            SetOverrideToolchain(path, name) => write!(
-                f,
-                "override toolchain for '{}' set to '{}'",
-                path.display(),
-                name
-            ),
-            SetProfile(name) => write!(f, "profile set to '{name}'"),
-            SetSelfUpdate(mode) => write!(f, "auto-self-update mode set to '{mode}'"),
-            LookingForToolchain(name) => write!(f, "looking for installed toolchain '{name}'"),
-            ToolchainDirectory(path) => write!(f, "toolchain directory: '{}'", path.display()),
-            UpdatingToolchain(name) => write!(f, "updating existing install for '{name}'"),
-            InstallingToolchain(name) => write!(f, "installing toolchain '{name}'"),
-            InstalledToolchain(name) => write!(f, "toolchain '{name}' installed"),
-            UsingExistingToolchain(name) => write!(f, "using existing install for '{name}'"),
-            UninstallingToolchain(name) => write!(f, "uninstalling toolchain '{name}'"),
-            UninstalledToolchain(name) => write!(f, "toolchain '{name}' uninstalled"),
-            UpdateHashMatches => write!(f, "toolchain is already up to date"),
-            UpgradingMetadata(from_ver, to_ver) => write!(
-                f,
-                "upgrading metadata version from '{from_ver}' to '{to_ver}'"
-            ),
-            MetadataUpgradeNotNeeded(ver) => {
-                write!(f, "nothing to upgrade: metadata version is already '{ver}'")
-            }
-            ReadMetadataVersion(ver) => write!(f, "read metadata version: '{ver}'"),
-            UpgradeRemovesToolchains => write!(
-                f,
-                "this upgrade will remove all existing toolchains. you will need to reinstall them"
-            ),
-            DuplicateToolchainFile {
-                rust_toolchain,
-                rust_toolchain_toml,
-            } => write!(
-                f,
-                "both `{0}` and `{1}` exist. Using `{0}`",
-                rust_toolchain
-                    .canonicalize()
-                    .unwrap_or_else(|_| PathBuf::from(rust_toolchain))
-                    .display(),
-                rust_toolchain_toml
-                    .canonicalize()
-                    .unwrap_or_else(|_| PathBuf::from(rust_toolchain_toml))
-                    .display(),
-            ),
         }
     }
 }

src/settings.rs

@@ -6,11 +6,11 @@ use std::str::FromStr;
 
 use anyhow::{Context, Result};
 use serde::{Deserialize, Serialize};
+use tracing::info;
 
 use crate::cli::self_update::SelfUpdateMode;
 use crate::dist::{AutoInstallMode, Profile};
 use crate::errors::*;
-use crate::notifications::*;
 use crate::utils;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -111,14 +111,12 @@ impl Settings {
         self.overrides.remove(&key).is_some()
     }
 
-    pub(crate) fn add_override(
-        &mut self,
-        path: &Path,
-        toolchain: String,
-        notify_handler: &dyn Fn(Notification<'_>),
-    ) {
+    pub(crate) fn add_override(&mut self, path: &Path, toolchain: String) {
         let key = Self::path_to_key(path);
-        notify_handler(Notification::SetOverrideToolchain(path, &toolchain));
+        info!(
+            "override toolchain for {} set to {toolchain}",
+            path.display(),
+        );
         self.overrides.insert(key, toolchain);
     }
 

src/toolchain.rs

@@ -28,7 +28,6 @@ use crate::{
         prefix::InstallPrefix,
     },
     env_var, install,
-    notifications::Notification,
     utils::{self, raw::open_dir_following_links},
 };
 
@@ -544,7 +543,7 @@ impl<'a> Toolchain<'a> {
         };
         let fs_modified = match Self::exists(cfg, &(&name).into())? {
             true => {
-                (cfg.notify_handler)(Notification::UninstallingToolchain(&name));
+                info!("uninstalling toolchain {name}");
                 let installed_paths = match &name {
                     ToolchainName::Custom(_) => Ok(vec![InstalledPath::Dir { path: &path }]),
                     ToolchainName::Official(desc) => cfg.installed_paths(desc, &path),
@@ -562,7 +561,7 @@ impl<'a> Toolchain<'a> {
             false => {
                 // Might be a dangling symlink
                 if path.is_symlink() {
-                    (cfg.notify_handler)(Notification::UninstallingToolchain(&name));
+                    info!("uninstalling toolchain {name}");
                     fs::remove_dir_all(&path)?;
                     true
                 } else {
@@ -579,7 +578,7 @@ impl<'a> Toolchain<'a> {
         };
 
         if !path.is_symlink() && !path.exists() && fs_modified {
-            (cfg.notify_handler)(Notification::UninstalledToolchain(&name));
+            info!("toolchain {name} uninstalled");
         }
         Ok(())
     }

src/utils/mod.rs

@@ -129,7 +129,7 @@ pub(crate) fn filter_file<F: FnMut(&str) -> bool>(
 
 pub(crate) fn canonicalize_path(path: &Path) -> PathBuf {
     fs::canonicalize(path).unwrap_or_else(|_| {
-        warn!(path = %path.display(), "could not canonicalize path");
+        warn!("could not canonicalize path {}", path.display());
         PathBuf::from(path)
     })
 }
@@ -407,7 +407,7 @@ pub fn rename(
                     // Permission denied, but as we work in users home dirs and
                     // running programs like virus scanner are known to cause this
                     // the heuristic is quite good.
-                    info!(source = %src.display(), destination = %dest.display(), "renaming file in use, retrying");
+                    info!("retrying renaming {} to {}", src.display(), dest.display());
                     OperationResult::Retry(e)
                 }
                 #[cfg(target_os = "linux")]