Merge branch 'badlive' into 'main'

handle files, dirs with problematic permissions

Closes #82

See merge request larswirzenius/obnam!115

8 files changed, 100 insertions, 24 deletions

diff --git a/obnam.md b/obnam.md
index bfaff5d..15cddb2 100644
--- a/obnam.md
+++ b/obnam.md
@@ -1430,6 +1430,44 @@ then file live/data.dat is restored to rest
 then file live/bad.dat is not restored to rest
 ~~~
 
+## Unreadable directory
+
+This scenario verifies that Obnam will skip a file in a directory it
+can't read. Obnam should warn about that, but not give an error.
+
+~~~scenario
+given an installed obnam
+and a running chunk server
+and a client config based on smoke.yaml
+and a file live/unreadable/data.dat containing some random data
+and file live/unreadable has mode 000
+when I run obnam --config smoke.yaml backup
+then stdout contains "live/unreadable"
+then backup generation is GEN
+when I invoke obnam --config smoke.yaml restore <GEN> rest
+then file live/unreadable is restored to rest
+then file live/unreadable/data.dat is not restored to rest
+~~~
+
+## Unexecutable directory
+
+This scenario verifies that Obnam will skip a file in a directory it
+can't read. Obnam should warn about that, but not give an error.
+
+~~~scenario
+given an installed obnam
+and a running chunk server
+and a client config based on smoke.yaml
+and a file live/dir/data.dat containing some random data
+and file live/dir has mode 600
+when I run obnam --config smoke.yaml backup
+then stdout contains "live/dir"
+then backup generation is GEN
+when I invoke obnam --config smoke.yaml restore <GEN> rest
+then file live/dir is restored to rest
+then file live/dir/data.dat is not restored to rest
+~~~
+
 ## Restore latest generation
 
 This scenario verifies that the latest backup generation can be
diff --git a/src/cmd/backup.rs b/src/cmd/backup.rs
index 4dc9370..547775c 100644
--- a/src/cmd/backup.rs
+++ b/src/cmd/backup.rs
@@ -1,5 +1,5 @@
 use crate::backup_progress::BackupProgress;
-use crate::backup_run::{IncrementalBackup, InitialBackup};
+use crate::backup_run::{BackupError, IncrementalBackup, InitialBackup};
 use crate::chunkid::ChunkId;
 use crate::client::{BackupClient, ClientConfig};
 use crate::error::ObnamError;
@@ -17,18 +17,28 @@ pub fn backup(config: &ClientConfig) -> Result<(), ObnamError> {
 
     let client = BackupClient::new(config)?;
     let genlist = client.list_generations()?;
-    let (gen_id, file_count) = match genlist.resolve("latest") {
+    let (gen_id, file_count, warnings) = match genlist.resolve("latest") {
         Err(_) => initial_backup(&config, &client)?,
         Ok(old_ref) => incremental_backup(&old_ref, &config, &client)?,
     };
 
-    report_stats(&runtime, file_count, &gen_id)?;
+    for w in warnings.iter() {
+        println!("warning: {}", w);
+    }
+
+    report_stats(&runtime, file_count, &gen_id, warnings.len())?;
 
     Ok(())
 }
 
-fn report_stats(runtime: &SystemTime, file_count: i64, gen_id: &ChunkId) -> Result<(), ObnamError> {
+fn report_stats(
+    runtime: &SystemTime,
+    file_count: i64,
+    gen_id: &ChunkId,
+    num_warnings: usize,
+) -> Result<(), ObnamError> {
     println!("status: OK");
+    println!("warnings: {}", num_warnings);
     println!("duration: {}", runtime.elapsed()?.as_secs());
     println!("file-count: {}", file_count);
     println!("generation-id: {}", gen_id);
@@ -38,34 +48,41 @@ fn report_stats(runtime: &SystemTime, file_count: i64, gen_id: &ChunkId) -> Resu
 fn initial_backup(
     config: &ClientConfig,
     client: &BackupClient,
-) -> Result<(ChunkId, i64), ObnamError> {
+) -> Result<(ChunkId, i64, Vec<BackupError>), ObnamError> {
     let run = InitialBackup::new(config, &client)?;
     let newtemp = NamedTempFile::new()?;
+    let mut all_warnings = vec![];
     let count = {
+        println!("create nascent");
         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)))?;
+            let warnings = new.insert_iter(iter.map(|entry| run.backup(entry)))?;
+            for w in warnings {
+                all_warnings.push(w);
+            }
         }
         new.file_count()
     };
+    run.drop();
 
     let progress = BackupProgress::upload_generation();
     let gen_id = client.upload_generation(newtemp.path(), SQLITE_CHUNK_SIZE)?;
     progress.finish();
 
-    Ok((gen_id, count))
+    Ok((gen_id, count, all_warnings))
 }
 
 fn incremental_backup(
     old_ref: &str,
     config: &ClientConfig,
     client: &BackupClient,
-) -> Result<(ChunkId, i64), ObnamError> {
+) -> Result<(ChunkId, i64, Vec<BackupError>), ObnamError> {
     let mut run = IncrementalBackup::new(config, &client)?;
     let newtemp = NamedTempFile::new()?;
+    let mut all_warnings = vec![];
     let count = {
         info!("incremental backup based on {}", old_ref);
         let oldtemp = NamedTempFile::new()?;
@@ -75,7 +92,10 @@ fn incremental_backup(
         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)))?;
+            let warnings = new.insert_iter(iter.map(|entry| run.backup(entry, &old)))?;
+            for w in warnings {
+                all_warnings.push(w);
+            }
         }
         new.file_count()
     };
@@ -84,5 +104,5 @@ fn incremental_backup(
     let gen_id = client.upload_generation(newtemp.path(), SQLITE_CHUNK_SIZE)?;
     progress.finish();
 
-    Ok((gen_id, count))
+    Ok((gen_id, count, all_warnings))
 }
diff --git a/src/error.rs b/src/error.rs
index 73d4a66..a905458 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -5,11 +5,10 @@ use crate::generation::{LocalGenerationError, NascentError};
 use crate::genlist::GenerationListError;
 use std::time::SystemTimeError;
 use tempfile::PersistError;
-use thiserror::Error;
 
 /// Define all the kinds of errors that functions corresponding to
 /// subcommands of the main program can return.
-#[derive(Debug, Error)]
+#[derive(Debug, thiserror::Error)]
 pub enum ObnamError {
     #[error(transparent)]
     GenerationListError(#[from] GenerationListError),
diff --git a/src/fsiter.rs b/src/fsiter.rs
index 57b6fd5..b778cf3 100644
--- a/src/fsiter.rs
+++ b/src/fsiter.rs
@@ -1,6 +1,6 @@
 use crate::fsentry::{FilesystemEntry, FsEntryError};
-use log::{debug, error};
-use std::path::Path;
+use log::{debug, warn};
+use std::path::{Path, PathBuf};
 use walkdir::{DirEntry, IntoIter, WalkDir};
 
 /// Iterator over file system entries in a directory tree.
@@ -13,8 +13,8 @@ pub enum FsIterError {
     #[error(transparent)]
     WalkError(#[from] walkdir::Error),
 
-    #[error(transparent)]
-    IoError(#[from] std::io::Error),
+    #[error("I/O error on {0}: {1}")]
+    IoError(PathBuf, #[source] std::io::Error),
 
     #[error(transparent)]
     FsEntryError(#[from] FsEntryError),
@@ -50,8 +50,8 @@ fn new_entry(e: &DirEntry) -> FsIterResult<FilesystemEntry> {
     let meta = match meta {
         Ok(meta) => meta,
         Err(err) => {
-            error!("failed to get metadata: {}", err);
-            return Err(err.into());
+            warn!("failed to get metadata for {}: {}", path.display(), err);
+            return Err(FsIterError::IoError(path.to_path_buf(), err));
         }
     };
     let entry = FilesystemEntry::from_metadata(path, &meta)?;
diff --git a/src/generation.rs b/src/generation.rs
index 4655c17..240f46a 100644
--- a/src/generation.rs
+++ b/src/generation.rs
@@ -2,6 +2,7 @@ use crate::backup_reason::Reason;
 use crate::backup_run::{BackupError, BackupResult};
 use crate::chunkid::ChunkId;
 use crate::fsentry::FilesystemEntry;
+use log::debug;
 use rusqlite::Connection;
 use std::path::{Path, PathBuf};
 
@@ -64,15 +65,23 @@ impl NascentGeneration {
     pub fn insert_iter<'a>(
         &mut self,
         entries: impl Iterator<Item = BackupResult<(FilesystemEntry, Vec<ChunkId>, Reason)>>,
-    ) -> NascentResult<()> {
+    ) -> NascentResult<Vec<BackupError>> {
         let t = self.conn.transaction()?;
+        let mut warnings = vec![];
         for r in entries {
-            let (e, ids, reason) = r?;
-            self.fileno += 1;
-            sql::insert_one(&t, e, self.fileno, &ids[..], reason)?;
+            match r {
+                Err(err) => {
+                    debug!("ignoring backup error {}", err);
+                    warnings.push(err);
+                }
+                Ok((e, ids, reason)) => {
+                    self.fileno += 1;
+                    sql::insert_one(&t, e, self.fileno, &ids[..], reason)?;
+                }
+            }
         }
         t.commit()?;
-        Ok(())
+        Ok(warnings)
     }
 }
 
diff --git a/subplot/client.py b/subplot/client.py
index 0e724b8..966324e 100644
--- a/subplot/client.py
+++ b/subplot/client.py
@@ -11,6 +11,11 @@ def install_obnam(ctx):
     runcmd_prepend_to_path(ctx, dirname=os.path.join(srcdir, "target", "debug"))
 
 
+def uninstall_obnam(ctx):
+    runcmd_run = globals()["runcmd_run"]
+    runcmd_run(ctx, ["chmod", "-R", "u+rwX", "."])
+
+
 def configure_client(ctx, filename=None):
     get_file = globals()["get_file"]
 
diff --git a/subplot/client.yaml b/subplot/client.yaml
index eef4714..b76b96a 100644
--- a/subplot/client.yaml
+++ b/subplot/client.yaml
@@ -1,5 +1,6 @@
 - given: "an installed obnam"
   function: install_obnam
+  cleanup: uninstall_obnam
 
 - given: "a client config based on {filename}"
   function: configure_client
diff --git a/subplot/data.py b/subplot/data.py
index 9396215..1455bf4 100644
--- a/subplot/data.py
+++ b/subplot/data.py
@@ -73,7 +73,11 @@ def file_is_restored(ctx, filename=None, restored=None):
 
 def file_is_not_restored(ctx, filename=None, restored=None):
     filename = os.path.join(restored, "./" + filename)
-    exists = os.path.exists(filename)
+    logging.info(r"verifying that {filename} does not exist")
+    try:
+        exists = os.path.exists(filename)
+    except PermissionError:
+        exists = False
     logging.debug(f"restored? {filename} {exists}")
     assert not exists