fix: allow generation temporary files to be automatically deleted

By not calling NamedTempFile::persist, the files get deleted
automatically when the file is closed or the struct is dropped.
Previously we were deleting the temporary files manually, which meant
that sometimes they weren't deleted if the program crashed at an
unfortunate time.

1 files changed, 47 insertions, 64 deletions

diff --git a/src/cmd/backup.rs b/src/cmd/backup.rs
index 94b8761..4dc9370 100644
--- a/src/cmd/backup.rs
+++ b/src/cmd/backup.rs
@@ -7,7 +7,6 @@ use crate::fsiter::FsIterator;
 use crate::generation::NascentGeneration;
 use bytesize::MIB;
 use log::info;
-use std::path::{Path, PathBuf};
 use std::time::SystemTime;
 use tempfile::NamedTempFile;
 
@@ -16,47 +15,13 @@ const SQLITE_CHUNK_SIZE: usize = MIB as usize;
 pub fn backup(config: &ClientConfig) -> Result<(), ObnamError> {
     let runtime = SystemTime::now();
 
-    // Create a named temporary file. We don't meed the open file
-    // handle, so we discard that.
-    let oldname = {
-        let temp = NamedTempFile::new()?;
-        let (_, dbname) = temp.keep()?;
-        dbname
-    };
-
-    // Create a named temporary file. We don't meed the open file
-    // handle, so we discard that.
-    let newname = {
-        let temp = NamedTempFile::new()?;
-        let (_, dbname) = temp.keep()?;
-        dbname
-    };
-
     let client = BackupClient::new(config)?;
     let genlist = client.list_generations()?;
-    let file_count = match genlist.resolve("latest") {
-        Err(_) => {
-            let run = InitialBackup::new(config, &client)?;
-            let count = initial_backup(&config.roots, &newname, &run)?;
-            count
-        }
-        Ok(old) => {
-            let mut run = IncrementalBackup::new(config, &client)?;
-            let count = incremental_backup(&old, &config.roots, &newname, &oldname, &mut run)?;
-            count
-        }
+    let (gen_id, file_count) = match genlist.resolve("latest") {
+        Err(_) => initial_backup(&config, &client)?,
+        Ok(old_ref) => incremental_backup(&old_ref, &config, &client)?,
     };
 
-    // Upload the SQLite file, i.e., the named temporary file, which
-    // still exists, since we persisted it above.
-    let progress = BackupProgress::upload_generation();
-    let gen_id = client.upload_generation(&newname, SQLITE_CHUNK_SIZE)?;
-    progress.finish();
-
-    // Delete the temporary file.q
-    std::fs::remove_file(&newname)?;
-    std::fs::remove_file(&oldname)?;
-
     report_stats(&runtime, file_count, &gen_id)?;
 
     Ok(())
@@ -71,35 +36,53 @@ fn report_stats(runtime: &SystemTime, file_count: i64, gen_id: &ChunkId) -> Resu
 }
 
 fn initial_backup(
-    roots: &[PathBuf],
-    newname: &Path,
-    run: &InitialBackup,
-) -> Result<i64, ObnamError> {
-    info!("fresh backup without a previous generation");
+    config: &ClientConfig,
+    client: &BackupClient,
+) -> Result<(ChunkId, i64), ObnamError> {
+    let run = InitialBackup::new(config, &client)?;
+    let newtemp = NamedTempFile::new()?;
+    let count = {
+        info!("fresh backup without a previous generation");
+
+        let mut new = NascentGeneration::create(newtemp.path())?;
+        for root in &config.roots {
+            let iter = FsIterator::new(root);
+            new.insert_iter(iter.map(|entry| run.backup(entry)))?;
+        }
+        new.file_count()
+    };
+
+    let progress = BackupProgress::upload_generation();
+    let gen_id = client.upload_generation(newtemp.path(), SQLITE_CHUNK_SIZE)?;
+    progress.finish();
 
-    let mut new = NascentGeneration::create(&newname)?;
-    for root in roots {
-        let iter = FsIterator::new(root);
-        new.insert_iter(iter.map(|entry| run.backup(entry)))?;
-    }
-    Ok(new.file_count())
+    Ok((gen_id, count))
 }
 
 fn incremental_backup(
-    old: &str,
-    roots: &[PathBuf],
-    newname: &Path,
-    oldname: &Path,
-    run: &mut IncrementalBackup,
-) -> Result<i64, ObnamError> {
-    info!("incremental backup based on {}", old);
+    old_ref: &str,
+    config: &ClientConfig,
+    client: &BackupClient,
+) -> Result<(ChunkId, i64), ObnamError> {
+    let mut run = IncrementalBackup::new(config, &client)?;
+    let newtemp = NamedTempFile::new()?;
+    let count = {
+        info!("incremental backup based on {}", old_ref);
+        let oldtemp = NamedTempFile::new()?;
+
+        let old = run.fetch_previous_generation(old_ref, oldtemp.path())?;
+        run.start_backup(&old)?;
+        let mut new = NascentGeneration::create(newtemp.path())?;
+        for root in &config.roots {
+            let iter = FsIterator::new(root);
+            new.insert_iter(iter.map(|entry| run.backup(entry, &old)))?;
+        }
+        new.file_count()
+    };
+
+    let progress = BackupProgress::upload_generation();
+    let gen_id = client.upload_generation(newtemp.path(), SQLITE_CHUNK_SIZE)?;
+    progress.finish();
 
-    let old = run.fetch_previous_generation(old, oldname)?;
-    run.start_backup(&old)?;
-    let mut new = NascentGeneration::create(&newname)?;
-    for root in roots {
-        let iter = FsIterator::new(root);
-        new.insert_iter(iter.map(|entry| run.backup(entry, &old)))?;
-    }
-    Ok(new.file_count())
+    Ok((gen_id, count))
 }