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.

4 files changed, 53 insertions, 97 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))
 }
diff --git a/src/cmd/list_files.rs b/src/cmd/list_files.rs
index ec3e52e..71b0d68 100644
--- a/src/cmd/list_files.rs
+++ b/src/cmd/list_files.rs
@@ -6,27 +6,18 @@ use crate::fsentry::{FilesystemEntry, FilesystemKind};
 use tempfile::NamedTempFile;
 
 pub fn list_files(config: &ClientConfig, gen_ref: &str) -> Result<(), ObnamError> {
-    // Create a named temporary file. We don't meed the open file
-    // handle, so we discard that.
-    let dbname = {
-        let temp = NamedTempFile::new()?;
-        let (_, dbname) = temp.keep()?;
-        dbname
-    };
+    let temp = NamedTempFile::new()?;
 
     let client = BackupClient::new(config)?;
 
     let genlist = client.list_generations()?;
     let gen_id: String = genlist.resolve(gen_ref)?;
 
-    let gen = client.fetch_generation(&gen_id, &dbname)?;
+    let gen = client.fetch_generation(&gen_id, temp.path())?;
     for file in gen.files()? {
         println!("{}", format_entry(&file.entry(), file.reason()));
     }
 
-    // Delete the temporary file.
-    std::fs::remove_file(&dbname)?;
-
     Ok(())
 }
 
diff --git a/src/cmd/restore.rs b/src/cmd/restore.rs
index b394d7d..cb42114 100644
--- a/src/cmd/restore.rs
+++ b/src/cmd/restore.rs
@@ -19,13 +19,7 @@ use structopt::StructOpt;
 use tempfile::NamedTempFile;
 
 pub fn restore(config: &ClientConfig, gen_ref: &str, to: &Path) -> Result<(), ObnamError> {
-    // Create a named temporary file. We don't meed the open file
-    // handle, so we discard that.
-    let dbname = {
-        let temp = NamedTempFile::new()?;
-        let (_, dbname) = temp.keep()?;
-        dbname
-    };
+    let temp = NamedTempFile::new()?;
 
     let client = BackupClient::new(config)?;
 
@@ -33,7 +27,7 @@ pub fn restore(config: &ClientConfig, gen_ref: &str, to: &Path) -> Result<(), Ob
     let gen_id: String = genlist.resolve(gen_ref)?;
     info!("generation id is {}", gen_id);
 
-    let gen = client.fetch_generation(&gen_id, &dbname)?;
+    let gen = client.fetch_generation(&gen_id, temp.path())?;
     info!("restoring {} files", gen.file_count()?);
     let progress = create_progress_bar(gen.file_count()?, true);
     for file in gen.files()? {
@@ -49,9 +43,6 @@ pub fn restore(config: &ClientConfig, gen_ref: &str, to: &Path) -> Result<(), Ob
     }
     progress.finish();
 
-    // Delete the temporary file.
-    std::fs::remove_file(&dbname)?;
-
     Ok(())
 }
 
diff --git a/src/cmd/show_gen.rs b/src/cmd/show_gen.rs
index c7a4bdd..143aed6 100644
--- a/src/cmd/show_gen.rs
+++ b/src/cmd/show_gen.rs
@@ -6,19 +6,13 @@ use indicatif::HumanBytes;
 use tempfile::NamedTempFile;
 
 pub fn show_generation(config: &ClientConfig, gen_ref: &str) -> Result<(), ObnamError> {
-    // Create a named temporary file. We don't meed the open file
-    // handle, so we discard that.
-    let dbname = {
-        let temp = NamedTempFile::new()?;
-        let (_, dbname) = temp.keep()?;
-        dbname
-    };
+    let temp = NamedTempFile::new()?;
 
     let client = BackupClient::new(config)?;
 
     let genlist = client.list_generations()?;
     let gen_id: String = genlist.resolve(gen_ref)?;
-    let gen = client.fetch_generation(&gen_id, &dbname)?;
+    let gen = client.fetch_generation(&gen_id, temp.path())?;
     let files = gen.files()?;
 
     let total_bytes = files.iter().fold(0, |acc, file| {
@@ -35,8 +29,5 @@ pub fn show_generation(config: &ClientConfig, gen_ref: &str) -> Result<(), Obnam
     println!("file-bytes: {}", HumanBytes(total_bytes));
     println!("file-bytes-raw: {}", total_bytes);
 
-    // Delete the temporary file.
-    std::fs::remove_file(&dbname)?;
-
     Ok(())
 }