Merge pull request #49 from cgwalters/preserve-mode

dirext: Preserve mode on existing files when doing atomic writes

src/dirext.rs

@@ -57,6 +57,16 @@ pub trait CapStdExtDirExt {
     /// This uses [`cap_tempfile::TempFile`], which is wrapped in a [`std::io::BufWriter`]
     /// and passed to the closure.
     ///
+    /// # Existing files and metadata
+    ///
+    /// If the target path already exists and is a regular file (not a symbolic link or directory),
+    /// then its access permissions (Unix mode) will be preserved.  However, other metadata
+    /// such as extended attributes will *not* be preserved automatically.  To do this will
+    /// require a higher level wrapper which queries the existing file and gathers such metadata
+    /// before replacement.
+    ///
+    /// # Example, including setting permissions
+    ///
     /// The closure may also perform other file operations beyond writing, such as changing
     /// file permissions:
     ///
@@ -251,9 +261,22 @@ impl CapStdExtDirExt for Dir {
     {
         let destname = destname.as_ref();
         let (d, name) = subdir_of(self, destname)?;
-        let t = cap_tempfile::TempFile::new(&d)?;
+        let existing_metadata = d.symlink_metadata_optional(destname)?;
+        // If the target is already a file, then acquire its mode, which we will preserve by default.
+        // We don't follow symlinks here for replacement, and so we definitely don't want to pick up its mode.
+        let existing_perms = existing_metadata
+            .filter(|m| m.is_file())
+            .map(|m| m.permissions());
+        let mut t = cap_tempfile::TempFile::new(&d)?;
+        // Apply the permissions, if we have them
+        if let Some(existing_perms) = existing_perms {
+            t.as_file_mut().set_permissions(existing_perms)?;
+        }
+        // We always operate in terms of buffered writes
         let mut bufw = std::io::BufWriter::new(t);
+        // Call the provided closure to generate the file content
         let r = f(&mut bufw)?;
+        // Flush the buffer, and rename the temporary file into place
         bufw.into_inner()
             .map_err(From::from)
             .and_then(|t| t.replace(name))?;

tests/it/main.rs

@@ -202,6 +202,38 @@ fn link_tempfile_with() -> Result<()> {
     );
     assert_eq!(td.metadata(p)?.permissions().mode() & 0o777, 0o700);
 
+    // Ensure we preserve the executable bit on an existing file
+    assert_eq!(td.metadata(p).unwrap().permissions().mode() & 0o700, 0o700);
+    td.atomic_write(p, "atomic replacement 4\n").unwrap();
+    assert_eq!(
+        td.read_to_string(p).unwrap().as_str(),
+        "atomic replacement 4\n"
+    );
+    assert_eq!(td.metadata(p)?.permissions().mode() & 0o777, 0o700);
+
+    // But we should ignore permissions on a symlink (both existing and broken)
+    td.remove_file(p)?;
+    let p2 = Path::new("bar");
+    td.atomic_write_with_perms(p2, "link target", Permissions::from_mode(0o755))
+        .unwrap();
+    td.symlink(p2, p)?;
+    td.atomic_write(p, "atomic replacement symlink\n").unwrap();
+    assert_eq!(td.metadata(p)?.permissions(), default_perms);
+    // And break the link
+    td.remove_file(p2)?;
+    td.atomic_write(p, "atomic replacement symlink\n").unwrap();
+    assert_eq!(td.metadata(p)?.permissions(), default_perms);
+
+    // Also test with mode 0600
+    td.atomic_write_with_perms(p, "self-only file", Permissions::from_mode(0o600))
+        .unwrap();
+    assert_eq!(td.metadata(p).unwrap().permissions().mode() & 0o777, 0o600);
+    td.atomic_write(p, "self-only file v2").unwrap();
+    assert_eq!(td.metadata(p).unwrap().permissions().mode() & 0o777, 0o600);
+    // But we can override
+    td.atomic_write_with_perms(p, "self-only file v3", Permissions::from_mode(0o640))
+        .unwrap();
+    assert_eq!(td.metadata(p).unwrap().permissions().mode() & 0o777, 0o640);
     Ok(())
 }