cp: do not inherit the source's mtime from clonefile on macOS - #14157
cp: do not inherit the source's mtime from clonefile on macOS#14157henrikottesorensen wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a macOS-specific cp behavior where a copy performed via clonefile(2) inherits the source file’s timestamps (notably mtime). After a successful clone, it explicitly stamps the destination with the current time so default cp matches expected cross-platform behavior (while -p continues to preserve timestamps via the existing attribute-copying path).
Changes:
- On macOS, after a successful
clonefile(2)call, set the destination atime/mtime toFileTime::now()to avoid inheriting the source’s timestamps by default. - Add macOS-only regression tests ensuring (1) default
cpdoes not inherit mtime from a clone, and (2)-pstill preserves timestamps across a clone.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/by-util/test_cp.rs | Adds macOS regression tests for mtime behavior with default clonefile copies and with -p. |
| src/uu/cp/src/platform/macos.rs | After successful clonefile(2), stamps destination timestamps to “now” to avoid inheriting source mtime by default. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
comment #0 is way too long, please make it shorter |
| // The clone succeeded. clonefile(2) copies the source's metadata, timestamps included, so | ||
| // the destination arrives looking as old as the source - where every other platform gives a | ||
| // fresh copy its own mtime, because there the file is simply written and nothing sets the | ||
| // times at all. Linux reflinks (FICLONE on btrfs/XFS/ZFS) clone data only, so this is a | ||
| // macOS-only divergence rather than a property of cloning. | ||
| // | ||
| // Stamping unconditionally is safe: when the caller asked to preserve timestamps, | ||
| // copy_attributes runs after this and sets the source's back, so the cost is one redundant | ||
| // utimensat on the -p path and correctness on every other. |
There was a problem hiding this comment.
make this comment shorter
There was a problem hiding this comment.
Shortened from 12 lines to 3 — kept only why the stamp is needed and why doing it unconditionally is safe:
// 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.|
GNU testsuite comparison: |
| use std::os::unix::fs::OpenOptionsExt; | ||
| use std::path::Path; | ||
|
|
||
| use filetime::FileTime; |
There was a problem hiding this comment.
Done — macos.rs now uses File::set_times with std::fs::FileTimes, and both new tests use SystemTime + FileTimes following 1d03876. Rebased onto that commit, so this branch no longer references filetime at all.
I left the // todo: replace with std in test_cp_reflink_never_does_not_clonefile alone, since it predates this PR — happy to convert it here too if you'd prefer.
clonefile(2) copies the source's metadata, timestamps included, so a default copy on macOS gives the destination the source's mtime. Every other platform gives a fresh copy its own - including GNU cp on macOS, which clones the same file on the same filesystem (--debug reports reflink: yes, and the copy costs no disk) and still stamps the destination itself. This is not inherent to cloning. FICLONE clones data only and leaves the caller to set metadata, so a reflinked copy on Linux keeps its own mtime; measured with --reflink=always on btrfs, XFS and ZFS, where uutils itself already behaves correctly. macOS is the only platform where it does not. copy_attributes sets timestamps only inside handle_preserve, i.e. under -p. With default options nothing sets them at all, which is right everywhere the destination is freshly written and gets its own times implicitly - but after clonefile it arrives already carrying the source's, and nothing corrects it. The practical cost is that timestamp-driven build systems treat the copy as unmodified: a file restored from a backup copy is older than artefacts built from it, so make, ninja and MSBuild skip the rebuild silently. That is what uutils#14052 was reported for; uutils#14076 fixed the --reflink=never half of it, and the default (--reflink=auto) still clones. Stamp the destination after a successful clone. Guarded on success, because the fallback path writes the file normally and already has its own times. Unconditional rather than gated on the preserve flags, since copy_on_write does not receive them and does not need to: under -p, copy_attributes runs afterwards and puts the source's times back, so the cost is one redundant utimensat on that path. The clone itself is kept, so none of clonefile's speed or space benefit is given up. Two tests, both macOS-only. The first fails without this change, with the destination wearing the source's timestamp. The second passes either way by design: it pins that -p still preserves across a clone, which is the failure mode an unconditional stamp could otherwise introduce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
82c66f9 to
8cdc73b
Compare
|
Updated for both review comments, and rebased onto current
The rebase should also clear the one red check. The two GNU regressions it reported — Re-verified after the rewrite that the regression test still fails without the fix, since a rewritten test is exactly where one can quietly stop testing. With the fix removed it fails with both sides reading |
The divergence
On macOS, a plain
cpgives the destination the source's mtime. Everywhere else — including GNUcpon macOS, cloning the same file on the same filesystem — a copy gets its own. That difference survives #14076, which guarded--reflink=neveronly, while the default is--reflink=auto.macOS 15.7.9, APFS, 400 MB file, uutils at
main(1a6fb19):--reflink=never-p--debug→reflink: yes)/bin/cp(macOS built-in)GNU is the important row. It is not avoiding the problem by declining to clone — it clones on APFS exactly as uutils does, reports
reflink: yes, costs no disk, and still stamps the destination with its own time. Same platform, same filesystem, same operation, opposite result.Nor is this "what cloning does" generally. Linux's equivalent — the
FICLONEioctl (ioctl_ficlonerange(2)) — clones data only and leaves the caller to set metadata, so a reflinked copy on Linux still gets a fresh mtime. Ubuntu 26.04 LTS, 200 MB file,--reflink=alwaysthroughout so a silent fallback to a byte copy would have failed rather than passed quietly — GNU 9.7 (/usr/bin/cp) against uutils 0.8.0 from therust-coreutilspackage (/usr/bin/coreutils cp; the package installs alongside GNU rather than taking over/usr/bin/cp):reflink=1)bcloneused0 → 200M--reflink=alwaysfails,Operation not supportedThe ext4 row is the control for the other three:
--reflink=alwaysgenuinely refuses on a filesystem without cloning rather than quietly falling back to a byte copy, so its success above means a clone really did happen. A plain copy there gets its own mtime as usual, and-ppreserves.So uutils already gets this right on Linux, cloning and stamping the destination itself. macOS is the only platform where it does not.
The cause is that macOS's
clonefile(2)system call (Darwin, since OS X 10.12) copies the source's metadata, timestamps included, whereas Linux'sFICLONEioctl does not. The two are not quite the same operation:clonefileis specified as producing a copy of the file, attributes and all, whileFICLONEshares a range of extents and says nothing about the inode's attributes — so a caller that treats them as interchangeable inherits metadata on one platform and not the other.copy_attributesonly sets timestamps insidehandle_preserve(attributes.timestamps, …), i.e. only under-p; with default options nothing sets them at all. That is correct everywhere else, because the destination is freshly written and gets its own times implicitly — but afterclonefileit is created already carrying the source's, and nothing corrects it.Why this matters
An mtime is not cosmetic metadata — it is the input every incremental build system makes its decisions from.
make,ninja, MSBuild,rsyncand most backup tooling ask one question about a file: is it newer than the thing derived from it? A copy that arrives wearing the source's timestamp answers that question wrongly.The concrete shape, and the one that produced #14052: restore a file from a backup copy —
cp file.bak file— and it lands older than the binary built from the previous version of it. The build system concludes the target is up to date, skips the rebuild, and the next run tests the stale artefact.Three properties make this expensive out of proportion to its size:
cpreports success, because it did succeed — the bytes are correct. Only the timestamp lies.cp. It is the most inert-seeming command in the toolbox. Every other candidate — the compiler, the build system, the cache, the test runner — gets investigated first, precisely because copying a file is assumed to be the one step that cannot change semantics.The platform difference sharpens all of this. The same script, the same repository, the same commands behave one way on Linux and another on macOS — so it presents as "works on CI, fails on my laptop" (or the reverse), which is the category of bug that gets attributed to anything except the tool that caused it.
--reflink=nevernow works and is a genuine escape hatch, but it only helps somebody who has already worked out what is happening. The default is what everything in the wild uses.The change
After a successful clone, stamp the destination with the current time:
copy_on_writedoes not receive them and does not need to: under-p,copy_attributesruns afterwards and puts the source's times back. The cost is one redundantutimensaton the-ppath and correctness on every other. Gating it would mean threadingAttributesinto the platform layer for no behavioural gain.filetimeis already a dependency ofuu_cp.clonefile's speed or space benefit is given up — which is what GNU does too.Testing
cargo test --no-default-features --features cp --test tests test_cp, macOS 15.7.9 (arm64, APFS):main, no change, no new testsThat third row is the point of the second one: with the fix reverted,
test_cp_default_does_not_inherit_mtime_from_clonefails with the destination wearing the source's stamp —Two tests are added, both macOS-only, alongside the existing
test_cp_reflink_never_does_not_clonefile:test_cp_default_does_not_inherit_mtime_from_clone— its sibling case, where the clone does happen and the mtime must still be fresh. This is the one that fails without the change.test_cp_preserve_timestamps_survives_clone— passes either way by design. It is not there to catch this bug, but to pin that-pstill preserves across a clone, which is the failure mode an unconditional stamp could otherwise introduce.Also checked by hand against GNU 9.11 and BSD
cpon the same files, all matching:cp -aandcp -a -rstill preserve timestamps;cp -rtrees, symlinks andcp -Pare unchanged; overwriting an existing destination (the remove-and-clone-again path) works and yields a fresh mtime; mode and xattr handling are unchanged (a0777source still lands0755underumask 022, as with GNU and BSD); an unreadable source fails identically before and after, and identically to GNU and BSD.Not included
copy_debug.reflinkis initialised toUnsupportedand only set toYesin the fallback branch taken when cloning fails, so--debugreportsreflink: unsupportedon a copy that did reflink — where GNU reportsreflink: yesfor the same copy on the same file. Correcting it changes the expected output of four existing tests, which is a separate argument from this one; happy to open it separately.Developed with Claude Code. Every figure quoted above was produced by running the command on the stated system rather than inferred — the Linux rows on a VM with the filesystems built for the purpose, the macOS rows on this machine against a build of
main— and the test results are real runs, including the one confirming the new regression test fails without the change.🤖 Generated with Claude Code