On 21/09/2025 06:30, Paul Eggert wrote:
On 2025-09-20 14:34, Kye Hunter wrote:
I don't
think there's any need to look beyond the most intuitive implementation
of the --update=older option: If the timestamps on the target are newer
than the timestamps on the source, don't do anything.
We're talking only mtime, right? --update looks only at mtime, not at
atime or ctime or whatever.
I'm only seeing extra calls to chmod being applied to directories
That's as per the documentation: --update affects only non-directories.
/* Avoid calling chown if we know it's not necessary. */ , so it seems
reasonable that the behavior of preserving the mode should be the same.
Sure, but the problem here is that it appears cp doesn't know whether
calling chmod is necessary. If deciding whether it's necessary is likely
more expensive than just doing the chmod, cp might as well just do the
chmod.
Again, to make this more concrete let's consider this code:
touch foo
cp -l foo bar
mkdir qux
cp --preserve=links --update=older --target-directory="./qux" *
strace cp --preserve=links --update=older --target-directory="./qux" *
Here, with the --update=older option, it seems unexpected that ./qux/foo
and ./qux/bar are being relinked the second time they are copied from
".", when what the manual describes is "--update[=UPDATE] control
which existing files are updated" and "'older' [...] results in files
being replaced if they're older than the corresponding source file."
--update affects only how non-directories are processed. In this
particular case, non-directories are indeed skipped; only the directory
qux is updated. So 'cp' is conforming to its documentation.
You're right that 'cp' could avoid some relinking if it kept track of
st_dev and st_ino for all destinations it's already updated. But 'cp'
would need to spend more RAM to do that, no? That could be a significant
burden in a recursive copy.
Code to show the current behavior of cp for scenario 2:
Unfortunately I'm not following that scenario or its description (it's
fairly long....).
This line even
comes with the helpful comment /* Note we currently replace DST_NAME
unconditionally, even if it was a newer separate file. */ , so actually
this behavior is somehow intentional, but I don't see any justification
given for this exception to the expected behavior of --update=older or
--no-preserve=links.
Nor do I. It seems a clear bug, or at least a problem. However, there
clearly was intent here: see Pádraig's commit
2aea1828a1aab158f68cccf3eac408203889021e dated 2011-07-27 which puts
into tests/cp/preserve/link the comment "a separate newer file in dest
will be overwritten!" indicating that the behavior you subscribe is at
least surprising.
I would be inclined to change the behavior to be the way you suggest.
This would simplify cp and make it a bit faster, and would make its
behavior easier to explain. However we're close to a release right now
and we should wait for Pádraig's opinion as he wrote the code (and the
test case) in question.
Thanks for helping me think through all this.
Yes this seems a bit hairy to change just before (hopefully tomorrow's) release.
As for the code/test, it's a while ago, but the exclamation mark by me
suggests the code change was a refactor, the behavior was surprising,
but worth adding a test for documentation at least. I.e. the behavior
could be changed IMHO (I've not thought about this much TBH).
cheers,
Padraig
cheers,
Padraig
