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

[Jim Meyering]

Pádraig Brady wrote:
> On 25/07/11 17:26, Pádraig Brady wrote:
>> Actually I'm wondering now whether the new code
>> should be unconditionally replacing the dest?
>> What if the dest is a separate newer file?
>> I.E. I think the following amended test should pass?
>> Also what about backups if the separate file is older?
>
> Well backups take a different path, as do
> older or non existing destination paths.
> So how about this change to just remove
> the new create_hard_link: call and beef up the test?

Hi Pádraig,

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
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".

Test comments below.

> diff --git a/src/copy.c b/src/copy.c
> index df8b1db..d6a0d1a 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -1633,11 +1633,11 @@ copy_internal (char const *src_name, char const 
> *dst_name,
>                       this src/dest pair, in case this source file is
>                       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;
> +                     destination names.  Note we don't hard link DST_NAME
> +                     here, because it may be a separate file with newer
> +                     or same timestamp. If it's older than SRC_NAME,
> +                     then this path is not taken.  */
> +                  remember_copied (dst_name, src_sb.st_ino, src_sb.st_dev);
...

> diff --git a/tests/cp/preserve-link b/tests/cp/preserve-link
> index d0da873..6beae02 100755
> --- a/tests/cp/preserve-link
> +++ b/tests/cp/preserve-link
> @@ -28,13 +28,31 @@ same_inode()
>
>  mkdir -p s t/s || framework_failure_
>  touch s/f t/s/f || framework_failure_
> +
> +# a non existing link must be linked in the dest tree
>  ln s/f s/link || framework_failure_
>
> +# an existing link must be updated
> +ln s/f s/linke || framework_failure_
> +ln t/s/f t/s/linke || framework_failure_
> +
> +# an updated older file must be overwritten
> +ln s/f s/fileo || framework_failure_
> +touch -d "-1 hour" t/s/fileo || framework_failure_

Since this "fileo" is hard-linked to f (and f to "linke"),
changing its mtime with touch changes those of the other files, too.

> +# an updated non linked file must not be overwritten
> +ln s/f s/fileu || framework_failure_
> +touch -d "+1 hour" t/s/fileu || framework_failure_

Doesn't this undo the effects of the preceding touch?

You might want to use a separate "cp" command,
with separate inputs -- or even a separate test script,
moving "same_inode" into a init.cfg.

>  # This must create a hard link, t/s/link, to the existing file, t/s/f.
>  # With cp from coreutils-8.12 and prior, it would mistakenly copy
>  # the file rather than creating the link.
> +touch -d "+1 hour" t/s/f || framework_failure_
>  cp -au s t || fail=1
>
>  same_inode t/s/f t/s/link || fail=1
> +same_inode t/s/f t/s/linke || fail=1
> +same_inode t/s/f t/s/fileo || fail=1
> +same_inode t/s/f t/s/fileu && fail=1
>
>  Exit $fail