diff --git a/src/uu/cp/src/platform/macos.rs b/src/uu/cp/src/platform/macos.rs index 20b38edfd6..0c7be8ea61 100644 --- a/src/uu/cp/src/platform/macos.rs +++ b/src/uu/cp/src/platform/macos.rs @@ -8,6 +8,7 @@ use std::fs::{self, File, OpenOptions}; use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::OpenOptionsExt; use std::path::Path; +use std::time::SystemTime; use uucore::buf_copy; use uucore::display::Quotable; @@ -99,6 +100,18 @@ pub(crate) fn copy_on_write( } } + if attempt_clone && error == 0 { + // clonefile(2) copies the source's metadata, mtime included, where a plain copy leaves the + // destination with its own. Unconditional is safe: -p restores the source's afterwards in + // copy_attributes. + let now = SystemTime::now(); + let times = fs::FileTimes::new().set_accessed(now).set_modified(now); + + if let Err(e) = File::open(dest).and_then(|f| f.set_times(times)) { + return Err(CpError::IoErrContext(e, context.to_owned())); + } + } + if !attempt_clone || error != 0 { // clonefile(2) is either not supported or it errored out (possibly because the FS does not // support COW). diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 2342d4c667..781760cb91 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2818,6 +2818,70 @@ fn test_cp_reflink_never_does_not_clonefile() { ); } +// The sibling of the test above, for the default path. `--reflink=auto` still calls clonefile(2), +// which copies the source's metadata including mtime — where a copy gets its own mtime on every +// other platform, and where GNU cp on macOS clones the very same file (`--debug` reports +// `reflink: yes`) and still stamps the destination. So a default copy must not inherit the +// source's old mtime either. +#[test] +#[cfg(target_os = "macos")] +fn test_cp_default_does_not_inherit_mtime_from_clone() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("src", "default clone contents"); + // Stamp the source well into the past; a clonefile copies it verbatim. + let past = std::time::SystemTime::UNIX_EPOCH + Duration::from_secs(1_000_000_000); // 2001-09-09 + let file = std::fs::OpenOptions::new() + .write(true) + .open(at.plus("src")) + .unwrap(); + file.set_times( + std::fs::FileTimes::new() + .set_accessed(past) + .set_modified(past), + ) + .unwrap(); + + ucmd.arg("src").arg("dst").succeeds(); + + assert_eq!(at.read("dst"), "default clone contents"); + let src_mtime = at.metadata("src").modified().unwrap(); + let dst_mtime = at.metadata("dst").modified().unwrap(); + assert_ne!( + dst_mtime, src_mtime, + "a default copy must not inherit the source's mtime, even when clonefile(2) is used" + ); +} + +// The other direction, and the reason the fix above can stamp unconditionally: `-p` asks for the +// source's timestamps, and copy_attributes restores them after the clone. Without this, a fix that +// freshened every clone would silently break preservation. +#[test] +#[cfg(target_os = "macos")] +fn test_cp_preserve_timestamps_survives_clone() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("src", "preserved clone contents"); + let past = std::time::SystemTime::UNIX_EPOCH + Duration::from_secs(1_000_000_000); // 2001-09-09 + let file = std::fs::OpenOptions::new() + .write(true) + .open(at.plus("src")) + .unwrap(); + file.set_times( + std::fs::FileTimes::new() + .set_accessed(past) + .set_modified(past), + ) + .unwrap(); + + ucmd.arg("-p").arg("src").arg("dst").succeeds(); + + let src_mtime = at.metadata("src").modified().unwrap(); + let dst_mtime = at.metadata("dst").modified().unwrap(); + assert_eq!( + dst_mtime, src_mtime, + "-p must still preserve the source's mtime across a clonefile(2) copy" + ); +} + #[test] #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] fn test_cp_reflink_bad() {