Restore dir/symlink ownership if possible

Fixes #228

Author: sourcefrog

NEWS.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Fixed: Restore now sets Unix user/group ownership on symlinks and directories. Previously, only file ownership was restored. (Setting file ownership typically requires restoring as root.)
+
 - Performance: Also keep a cache of the existence of blocks that have not yet been read.
 
 - Changed: The format and keys written by `--metrics-json` has changed.

src/owner/unix.rs

@@ -15,17 +15,15 @@
 //! Unix implementation of file ownership.
 
 use std::io;
-use std::os::unix::fs::MetadataExt;
+use std::os::unix::fs::{lchown, MetadataExt};
 use std::sync::Mutex;
 use std::{fs, path::Path};
 
 use lazy_static::lazy_static;
-use nix::errno::Errno;
-use nix::unistd;
 use uzers::{Groups, Users, UsersCache};
 
 use super::Owner;
-use crate::{Error, Result};
+use crate::Result;
 
 lazy_static! {
     static ref USERS_CACHE: Mutex<UsersCache> = Mutex::new(UsersCache::new());
@@ -44,34 +42,29 @@ impl From<&fs::Metadata> for Owner {
     }
 }
 
-#[mutants::skip] // TODO: Test that at least groups are restored!
+#[mutants::skip] // TODO: Difficult to test as non-root but we could at least test that at least groups are restored!
 pub(crate) fn set_owner(owner: &Owner, path: &Path) -> Result<()> {
     let users_cache = USERS_CACHE.lock().unwrap();
     let uid_opt = owner
         .user
         .as_ref()
         .and_then(|user| users_cache.get_user_by_name(&user))
-        .map(|user| user.uid())
-        .map(unistd::Uid::from_raw);
+        .map(|user| user.uid());
     let gid_opt = owner
         .group
         .as_ref()
         .and_then(|group| users_cache.get_group_by_name(&group))
-        .map(|group| group.gid())
-        .map(unistd::Gid::from_raw);
+        .map(|group| group.gid());
     drop(users_cache);
     // TODO: use `std::os::unix::fs::chown(path, uid, gid)?;` once stable
-    match unistd::chown(path, uid_opt, gid_opt) {
+    match lchown(path, uid_opt, gid_opt) {
         Ok(()) => Ok(()),
-        Err(Errno::EPERM) => {
+        Err(err) if err.kind() == io::ErrorKind::PermissionDenied => {
             // If the restore is not run as root (or with special capabilities)
             // then we probably can't set ownership, and there's no point
             // complaining
             Ok(())
         }
-        Err(errno) => Err(Error::SetOwner {
-            path: path.to_path_buf(),
-            source: io::Error::from_raw_os_error(errno as i32),
-        }),
+        Err(err) => Err(err.into()),
     }
 }

src/restore.rs

@@ -111,6 +111,7 @@ pub fn restore(
                     path,
                     unix_mode: entry.unix_mode(),
                     mtime: entry.mtime(),
+                    owner: entry.owner().clone(),
                 })
             }
             Kind::File => {
@@ -163,6 +164,7 @@ struct DirDeferral {
     path: PathBuf,
     unix_mode: UnixMode,
     mtime: OffsetDateTime,
+    owner: Owner,
 }
 
 fn apply_deferrals(deferrals: &[DirDeferral]) -> Result<RestoreStats> {
@@ -171,8 +173,13 @@ fn apply_deferrals(deferrals: &[DirDeferral]) -> Result<RestoreStats> {
         path,
         unix_mode,
         mtime,
+        owner,
     } in deferrals
     {
+        if let Err(err) = owner.set_owner(path) {
+            error!(?path, ?err, "Error restoring ownership");
+            stats.errors += 1;
+        }
         if let Err(err) = unix_mode.set_permissions(path) {
             error!(?path, ?err, "Failed to set directory permissions");
             stats.errors += 1;
@@ -254,7 +261,8 @@ fn restore_file(
 }
 
 #[cfg(unix)]
-fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result<()> {
+fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result<RestoreStats> {
+    let mut stats = RestoreStats::default();
     use std::os::unix::fs as unix_fs;
     if let Some(ref target) = entry.symlink_target() {
         if let Err(source) = unix_fs::symlink(target, path) {
@@ -263,6 +271,10 @@ fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result<()> {
                 source,
             });
         }
+        if let Err(err) = &entry.owner().set_owner(path) {
+            error!(?path, ?err, "Error restoring ownership");
+            stats.errors += 1;
+        }
         let mtime = entry.mtime().to_file_time();
         if let Err(source) = set_symlink_file_times(path, mtime, mtime) {
             return Err(Error::RestoreModificationTime {
@@ -272,15 +284,16 @@ fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result<()> {
         }
     } else {
         error!(apath = ?entry.apath(), "No target in symlink entry");
+        stats.errors += 1;
     }
-    Ok(())
+    Ok(stats)
 }
 
 #[cfg(not(unix))]
 #[mutants::skip]
-fn restore_symlink(_restore_path: &Path, entry: &IndexEntry) -> Result<()> {
+fn restore_symlink(_restore_path: &Path, entry: &IndexEntry) -> Result<RestoreStats> {
     // TODO: Add a test with a canned index containing a symlink, and expect
     // it cannot be restored on Windows and can be on Unix.
     warn!("Can't restore symlinks on non-Unix: {}", entry.apath());
-    Ok(())
+    Ok(RestoreStats::default())
 }