notifications: log directly on directory deletions

Author: djc

src/dist/component/package.rs

@@ -138,10 +138,10 @@ impl Package for DirectoryPackage {
 
 #[derive(Debug)]
 #[allow(dead_code)] // temp::Dir is held for drop.
-pub(crate) struct TarPackage<'a>(DirectoryPackage, temp::Dir<'a>);
+pub(crate) struct TarPackage(DirectoryPackage, temp::Dir);
 
-impl<'a> TarPackage<'a> {
-    pub(crate) fn new<R: Read>(stream: R, cx: &PackageContext<'a>) -> Result<Self> {
+impl TarPackage {
+    pub(crate) fn new<R: Read>(stream: R, cx: &PackageContext<'_>) -> Result<Self> {
         let temp_dir = cx.tmp_cx.new_directory()?;
         let mut archive = tar::Archive::new(stream);
         // The rust-installer packages unpack to a directory called
@@ -528,7 +528,7 @@ fn unpack_without_first_dir<R: Read>(
     Ok(())
 }
 
-impl Package for TarPackage<'_> {
+impl Package for TarPackage {
     fn contains(&self, component: &str, short_name: Option<&str>) -> bool {
         self.0.contains(component, short_name)
     }
@@ -547,16 +547,16 @@ impl Package for TarPackage<'_> {
 }
 
 #[derive(Debug)]
-pub(crate) struct TarGzPackage<'a>(TarPackage<'a>);
+pub(crate) struct TarGzPackage(TarPackage);
 
-impl<'a> TarGzPackage<'a> {
-    pub(crate) fn new<R: Read>(stream: R, cx: &PackageContext<'a>) -> Result<Self> {
+impl TarGzPackage {
+    pub(crate) fn new<R: Read>(stream: R, cx: &PackageContext<'_>) -> Result<Self> {
         let stream = flate2::read::GzDecoder::new(stream);
         Ok(TarGzPackage(TarPackage::new(stream, cx)?))
     }
 }
 
-impl Package for TarGzPackage<'_> {
+impl Package for TarGzPackage {
     fn contains(&self, component: &str, short_name: Option<&str>) -> bool {
         self.0.contains(component, short_name)
     }
@@ -575,16 +575,16 @@ impl Package for TarGzPackage<'_> {
 }
 
 #[derive(Debug)]
-pub(crate) struct TarXzPackage<'a>(TarPackage<'a>);
+pub(crate) struct TarXzPackage(TarPackage);
 
-impl<'a> TarXzPackage<'a> {
-    pub(crate) fn new<R: Read>(stream: R, cx: &PackageContext<'a>) -> Result<Self> {
+impl TarXzPackage {
+    pub(crate) fn new<R: Read>(stream: R, cx: &PackageContext<'_>) -> Result<Self> {
         let stream = xz2::read::XzDecoder::new(stream);
         Ok(TarXzPackage(TarPackage::new(stream, cx)?))
     }
 }
 
-impl Package for TarXzPackage<'_> {
+impl Package for TarXzPackage {
     fn contains(&self, component: &str, short_name: Option<&str>) -> bool {
         self.0.contains(component, short_name)
     }
@@ -603,16 +603,16 @@ impl Package for TarXzPackage<'_> {
 }
 
 #[derive(Debug)]
-pub(crate) struct TarZStdPackage<'a>(TarPackage<'a>);
+pub(crate) struct TarZStdPackage(TarPackage);
 
-impl<'a> TarZStdPackage<'a> {
-    pub(crate) fn new<R: Read>(stream: R, cx: &PackageContext<'a>) -> Result<Self> {
+impl TarZStdPackage {
+    pub(crate) fn new<R: Read>(stream: R, cx: &PackageContext<'_>) -> Result<Self> {
         let stream = zstd::stream::read::Decoder::new(stream)?;
         Ok(TarZStdPackage(TarPackage::new(stream, cx)?))
     }
 }
 
-impl Package for TarZStdPackage<'_> {
+impl Package for TarZStdPackage {
     fn contains(&self, component: &str, short_name: Option<&str>) -> bool {
         self.0.contains(component, short_name)
     }

src/dist/component/transaction.rs

@@ -187,7 +187,7 @@ enum ChangedItem<'a> {
     AddedFile(PathBuf),
     AddedDir(PathBuf),
     RemovedFile(PathBuf, temp::File<'a>),
-    RemovedDir(PathBuf, temp::Dir<'a>),
+    RemovedDir(PathBuf, temp::Dir),
     ModifiedFile(PathBuf, Option<temp::File<'a>>),
 }
 

src/dist/temp.rs

@@ -3,7 +3,7 @@ use std::{fmt, fs, ops};
 
 pub(crate) use anyhow::{Context as _, Result};
 use thiserror::Error as ThisError;
-use tracing::debug;
+use tracing::{debug, warn};
 
 use crate::notifications::Notification;
 use crate::utils::{self, raw};
@@ -19,27 +19,27 @@ pub(crate) enum CreatingError {
 }
 
 #[derive(Debug)]
-pub(crate) struct Dir<'a> {
-    cfg: &'a Context,
+pub(crate) struct Dir {
     path: PathBuf,
 }
 
-impl ops::Deref for Dir<'_> {
+impl ops::Deref for Dir {
     type Target = Path;
 
     fn deref(&self) -> &Path {
         self.path.as_path()
     }
 }
 
-impl Drop for Dir<'_> {
+impl Drop for Dir {
     fn drop(&mut self) {
         if raw::is_directory(&self.path) {
-            let n = Notification::DirectoryDeletion(
-                &self.path,
-                remove_dir_all::remove_dir_all(&self.path),
-            );
-            (self.cfg.notify_handler)(n);
+            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")
+                }
+            }
         }
     }
 }
@@ -93,7 +93,7 @@ impl Context {
         .with_context(|| CreatingError::Root(PathBuf::from(&self.root_directory)))
     }
 
-    pub(crate) fn new_directory(&self) -> Result<Dir<'_>> {
+    pub(crate) fn new_directory(&self) -> Result<Dir> {
         self.create_root()?;
 
         loop {
@@ -107,10 +107,7 @@ impl Context {
                 debug!(name = "temp", path = %temp_dir.display(), "creating directory");
                 fs::create_dir(&temp_dir)
                     .with_context(|| CreatingError::Directory(PathBuf::from(&temp_dir)))?;
-                return Ok(Dir {
-                    cfg: self,
-                    path: temp_dir,
-                });
+                return Ok(Dir { path: temp_dir });
             }
         }
     }

src/notifications.rs

@@ -56,7 +56,6 @@ pub enum Notification<'a> {
     CreatingRoot(&'a Path),
     CreatingFile(&'a Path),
     FileDeletion(&'a Path, io::Result<()>),
-    DirectoryDeletion(&'a Path, io::Result<()>),
     SetAutoInstall(&'a str),
     SetDefaultToolchain(Option<&'a ToolchainName>),
     SetOverrideToolchain(&'a Path, &'a str),
@@ -119,7 +118,7 @@ impl Notification<'_> {
             | UsingReqwest => NotificationLevel::Debug,
             Error(_) => NotificationLevel::Error,
             CreatingRoot(_) | CreatingFile(_) => NotificationLevel::Debug,
-            FileDeletion(_, result) | DirectoryDeletion(_, result) => match result {
+            FileDeletion(_, result) => match result {
                 Ok(_) => NotificationLevel::Debug,
                 Err(_) => NotificationLevel::Warn,
             },
@@ -261,13 +260,6 @@ impl Display for Notification<'_> {
                     write!(f, "could not delete temp file: {}", path.display())
                 }
             }
-            DirectoryDeletion(path, result) => {
-                if result.is_ok() {
-                    write!(f, "deleted temp directory: {}", path.display())
-                } else {
-                    write!(f, "could not delete temp directory: {}", path.display())
-                }
-            }
             SetAutoInstall(auto) => write!(f, "auto install set to '{auto}'"),
             SetDefaultToolchain(None) => write!(f, "default toolchain unset"),
             SetDefaultToolchain(Some(name)) => write!(f, "default toolchain set to '{name}'"),