notifications: log directly on file deletions

Author: djc

src/dist/component/transaction.rs

@@ -36,7 +36,7 @@ use crate::utils;
 /// already exists.
 pub struct Transaction<'a> {
     prefix: InstallPrefix,
-    changes: Vec<ChangedItem<'a>>,
+    changes: Vec<ChangedItem>,
     tmp_cx: &'a temp::Context,
     committed: bool,
     process: &'a Process,
@@ -59,7 +59,7 @@ impl<'a> Transaction<'a> {
         self.committed = true;
     }
 
-    fn change(&mut self, item: ChangedItem<'a>) {
+    fn change(&mut self, item: ChangedItem) {
         self.changes.push(item);
     }
 
@@ -183,15 +183,15 @@ impl Drop for Transaction<'_> {
 /// package, or updating a component, distill down into a series of
 /// these primitives.
 #[derive(Debug)]
-enum ChangedItem<'a> {
+enum ChangedItem {
     AddedFile(PathBuf),
     AddedDir(PathBuf),
-    RemovedFile(PathBuf, temp::File<'a>),
+    RemovedFile(PathBuf, temp::File),
     RemovedDir(PathBuf, temp::Dir),
-    ModifiedFile(PathBuf, Option<temp::File<'a>>),
+    ModifiedFile(PathBuf, Option<temp::File>),
 }
 
-impl<'a> ChangedItem<'a> {
+impl ChangedItem {
     fn roll_back(&self, prefix: &InstallPrefix, process: &Process) -> Result<()> {
         use self::ChangedItem::*;
         match self {
@@ -259,7 +259,7 @@ impl<'a> ChangedItem<'a> {
         prefix: &InstallPrefix,
         component: &str,
         relpath: PathBuf,
-        tmp_cx: &'a temp::Context,
+        tmp_cx: &temp::Context,
         process: &Process,
     ) -> Result<Self> {
         let abs_path = prefix.abs_path(&relpath);
@@ -279,7 +279,7 @@ impl<'a> ChangedItem<'a> {
         prefix: &InstallPrefix,
         component: &str,
         relpath: PathBuf,
-        tmp_cx: &'a temp::Context,
+        tmp_cx: &temp::Context,
         process: &Process,
     ) -> Result<Self> {
         let abs_path = prefix.abs_path(&relpath);
@@ -298,7 +298,7 @@ impl<'a> ChangedItem<'a> {
     fn modify_file(
         prefix: &InstallPrefix,
         relpath: PathBuf,
-        tmp_cx: &'a temp::Context,
+        tmp_cx: &temp::Context,
     ) -> Result<Self> {
         let abs_path = prefix.abs_path(&relpath);
 

src/dist/download.rs

@@ -148,7 +148,7 @@ impl<'a> DownloadCfg<'a> {
         url_str: &str,
         update_hash: Option<&Path>,
         ext: &str,
-    ) -> Result<Option<(temp::File<'a>, String)>> {
+    ) -> Result<Option<(temp::File, String)>> {
         let hash = self.download_hash(url_str).await?;
         let partial_hash: String = hash.chars().take(UPDATE_HASH_LEN).collect();
 

src/dist/temp.rs

@@ -45,24 +45,27 @@ impl Drop for Dir {
 }
 
 #[derive(Debug)]
-pub struct File<'a> {
-    cfg: &'a Context,
+pub struct File {
     path: PathBuf,
 }
 
-impl ops::Deref for File<'_> {
+impl ops::Deref for File {
     type Target = Path;
 
     fn deref(&self) -> &Path {
         self.path.as_path()
     }
 }
 
-impl Drop for File<'_> {
+impl Drop for File {
     fn drop(&mut self) {
         if raw::is_file(&self.path) {
-            let n = Notification::FileDeletion(&self.path, fs::remove_file(&self.path));
-            (self.cfg.notify_handler)(n);
+            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")
+                }
+            }
         }
     }
 }
@@ -112,11 +115,11 @@ impl Context {
         }
     }
 
-    pub fn new_file(&self) -> Result<File<'_>> {
+    pub fn new_file(&self) -> Result<File> {
         self.new_file_with_ext("", "")
     }
 
-    pub(crate) fn new_file_with_ext(&self, prefix: &str, ext: &str) -> Result<File<'_>> {
+    pub(crate) fn new_file_with_ext(&self, prefix: &str, ext: &str) -> Result<File> {
         self.create_root()?;
 
         loop {
@@ -130,10 +133,7 @@ impl Context {
                 (self.notify_handler)(Notification::CreatingFile(&temp_file));
                 fs::File::create(&temp_file)
                     .with_context(|| CreatingError::File(PathBuf::from(&temp_file)))?;
-                return Ok(File {
-                    cfg: self,
-                    path: temp_file,
-                });
+                return Ok(File { path: temp_file });
             }
         }
     }

src/notifications.rs

@@ -1,5 +1,4 @@
 use std::fmt::{self, Display};
-use std::io;
 use std::path::{Path, PathBuf};
 
 use url::Url;
@@ -55,7 +54,6 @@ pub enum Notification<'a> {
     UsingReqwest,
     CreatingRoot(&'a Path),
     CreatingFile(&'a Path),
-    FileDeletion(&'a Path, io::Result<()>),
     SetAutoInstall(&'a str),
     SetDefaultToolchain(Option<&'a ToolchainName>),
     SetOverrideToolchain(&'a Path, &'a str),
@@ -118,10 +116,6 @@ impl Notification<'_> {
             | UsingReqwest => NotificationLevel::Debug,
             Error(_) => NotificationLevel::Error,
             CreatingRoot(_) | CreatingFile(_) => NotificationLevel::Debug,
-            FileDeletion(_, result) => match result {
-                Ok(_) => NotificationLevel::Debug,
-                Err(_) => NotificationLevel::Warn,
-            },
             ToolchainDirectory(_)
             | LookingForToolchain(_)
             | InstallingToolchain(_)
@@ -253,13 +247,6 @@ impl Display for Notification<'_> {
             UsingReqwest => write!(f, "downloading with reqwest"),
             CreatingRoot(path) => write!(f, "creating temp root: {}", path.display()),
             CreatingFile(path) => write!(f, "creating temp file: {}", path.display()),
-            FileDeletion(path, result) => {
-                if result.is_ok() {
-                    write!(f, "deleted temp file: {}", path.display())
-                } else {
-                    write!(f, "could not delete temp file: {}", 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}'"),