bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au

[Jim Meyering]

Pádraig Brady wrote:
...
>>> The link-creation code there cannot be removed.
>>> Consider this scenario:
>>>
>>>   src/{a,b}  # a and b are linked
>>>   dst/src/a
>>>
>>> If cp -au encounters src/a first, then the new "remember_copied"
>>> call records the required info so when it encounters src/b it can
>>> make the desired link from the preexisting dst/src/a to dst/src/b.
>>> In that case, the link-creation code is indeed unnecessary.
>>>
>>> However, what if it encounters src/b first?
>>> In that case, it must copy src/b to dst/src/b (new inode!)
>>> Then, when it encounters src/a, it must *remove* the preexisting dst/src/a
>>
>> This "must" is my interpretation that --preserve=link (implied
>> by -a and -p) has a higher priority than the --update (-u) option.
>
> That's a bit surprising, thought understandable I think.
> If the separate file in the dest is newer it will be replaced
> by an older link from the source.  That could legitimately happen
> I suppose if the original copy was with -rp, then the timestamps
> would be newer than those in dest.
> I'll update the test to enforce the replacement of newer files in dest
>
>>> and replace it with a hard link to dst/src/b.
>>> As an alternative, cp could conceivably remove the just-copied dst/src/b,
>>> replacing it with a hard-link to dst/src/a.
>>>
>>> The trouble I have with all of this is that we can see two different
>>> outcomes, depending on the order in which "a" and "b" (in the src
>>> directory) are processed by cp.  In one case, the preexisting and
>>> up-to-date dst/src/a is not modified.  In the other, it is removed,
>>> and replaced by a hard-link to potentially-differing-content "dst/src/b".
>
> TBH I don't understand all the implications.
> Note the `touch` commands in the test operate on the separate inodes in dest.

Oh!  Sorry I missed that.

> Note also your original test didn't fail for me on ext4 on F15.

When I apply the change below, recompile "cp" and run
  make check -C tests TESTS=cp/preserve-link VERBOSE=yes
with the test tweaked to run cp via strace,
  strace -e stat,lstat -o /tmp/cp-strace cp -au s t || fail=1
I get this, which shows it's processing s/link before s/f.

  stat("t", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
  lstat("s", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
  lstat("t/s", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
  lstat("s/link", {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
  stat("t/s/link", 0x7fffa85ca0a0)        = -1 ENOENT (No such file or 
directory)
  lstat("s/f", {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
  stat("t/s/f", {st_mode=S_IFREG|0600, st_size=0, ...}) = 0

I suspect you'll see that it's processing those two files in the reverse
order on your system.  In case it's kernel-related, I'm using this:
  2.6.38.8-32.fc15.x86_64
and the disk is an SSD.

diff --git a/src/copy.c b/src/copy.c
index aaf7e79..f4b6587 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1636,10 +1636,7 @@ copy_internal (char const *src_name, char const 
*dst_name,
                      hard-linked to another one.  In that case, we'll use
                      the mapping information to link the corresponding
                      destination names.  */
-                  earlier_file = remember_copied (dst_name, src_sb.st_ino,
-                                                  src_sb.st_dev);
-                  if (earlier_file)
-                    goto create_hard_link;
+                  remember_copied (dst_name, src_sb.st_ino, src_sb.st_dev);

                   return true;
                 }
@@ -1961,7 +1958,6 @@ copy_internal (char const *src_name, char const *dst_name,
         }
       else
         {
-        create_hard_link:;
           /* We want to guarantee that symlinks are not followed.  */
           bool link_failed = (linkat (AT_FDCWD, earlier_file, AT_FDCWD,
                                       dst_name, 0) != 0);