Re: bug in cp -R -L ?

[Jim Meyering]

Christophe LYON <address@hidden> wrote:
> I am using coreutils-5.93 and I have the following problem:
> Consider the following hierarchy:
> mkdir:
>       file
> src:
>       dir1:
>               cp1 -> ../../mydir
>       dir2:
>               cp2 -> ../../mydir
>
> Now, I want to use
> cp -R -L src dest
> which fails with the following message:
> cp: will not create hard link `dest/src/dir2/cp2' to directory
> `dest/src/dir1/cp1'

Thank you for reporting that bug!
Here's how I've fixed it on the coreutils trunk.
The same patch will soon appear on the stable (b5) branch.

2006-02-04  Jim Meyering  <address@hidden>

        * src/copy.c (copy_internal): cp -RL no longer fails when encountering
        the same directory more than once in the hierarchy beneath a single
        command-line argument.  That is legitimate, e.g. when there are
        two or more symbolic links, each pointing to some directory that
        would not otherwise be copied.  Reported by Christophe LYON.
        * tests/cp/cp-deref: New file.  Test for today's fix.
        * tests/cp/Makefile.am (TESTS): Add cp-deref.

FYI, I've included two versions of the copy.c patch below.
The first is a more readable one that ignores white-space changes,
but you should not apply it with patch.

W A R N I N G: do not apply this first patch.  If you want to patch
your 5.93 sources, use the longer, complete patch below.

diff -u -p -B -b -w -r1.195 -r1.196
--- src/DO_NOT_APPLY-copy.c     2 Jan 2006 21:34:53 -0000       1.195
+++ src/DO_NOT_APPLY-copy.c     4 Feb 2006 10:49:21 -0000       1.196
@@ -1302,16 +1302,25 @@ copy_internal (char const *src_name, cha
                     quote_n (0, top_level_src_name),
                     quote_n (1, top_level_dst_name));
              *copy_into_self = true;
+             goto un_backup;
+           }
+         else if (x->dereference == DEREF_ALWAYS)
+           {
+             /* This happens when e.g., encountering a directory for the
+                second or subsequent time via symlinks when cp is invoked
+                with -R and -L.  E.g.,
+                rm -rf a b c d; mkdir a b c d; ln -s ../c a; ln -s ../c b;
+                cp -RL a b d
+             */
            }
          else
            {
              error (0, 0, _("will not create hard link %s to directory %s"),
                     quote_n (0, dst_name), quote_n (1, earlier_file));
-           }
-
          goto un_backup;
        }
-
+       }
+      else
       {
        bool link_failed = (link (earlier_file, dst_name) != 0);
 


Index: src/copy.c
===================================================================
RCS file: /fetish/cu/src/copy.c,v
retrieving revision 1.195
retrieving revision 1.196
diff -u -p -u -r1.195 -r1.196
--- src/copy.c  2 Jan 2006 21:34:53 -0000       1.195
+++ src/copy.c  4 Feb 2006 10:49:21 -0000       1.196
@@ -1302,40 +1302,49 @@ copy_internal (char const *src_name, cha
                     quote_n (0, top_level_src_name),
                     quote_n (1, top_level_dst_name));
              *copy_into_self = true;
+             goto un_backup;
+           }
+         else if (x->dereference == DEREF_ALWAYS)
+           {
+             /* This happens when e.g., encountering a directory for the
+                second or subsequent time via symlinks when cp is invoked
+                with -R and -L.  E.g.,
+                rm -rf a b c d; mkdir a b c d; ln -s ../c a; ln -s ../c b;
+                cp -RL a b d
+             */
            }
          else
            {
              error (0, 0, _("will not create hard link %s to directory %s"),
                     quote_n (0, dst_name), quote_n (1, earlier_file));
+             goto un_backup;
            }
-
-         goto un_backup;
        }
+      else
+       {
+         bool link_failed = (link (earlier_file, dst_name) != 0);
 
-      {
-       bool link_failed = (link (earlier_file, dst_name) != 0);
+         /* If the link failed because of an existing destination,
+            remove that file and then call link again.  */
+         if (link_failed && errno == EEXIST)
+           {
+             if (unlink (dst_name) != 0)
+               {
+                 error (0, errno, _("cannot remove %s"), quote (dst_name));
+                 goto un_backup;
+               }
+             link_failed = (link (earlier_file, dst_name) != 0);
+           }
 
-       /* If the link failed because of an existing destination,
-          remove that file and then call link again.  */
-       if (link_failed && errno == EEXIST)
-         {
-           if (unlink (dst_name) != 0)
-             {
-               error (0, errno, _("cannot remove %s"), quote (dst_name));
-               goto un_backup;
-             }
-           link_failed = (link (earlier_file, dst_name) != 0);
-         }
-
-       if (link_failed)
-         {
-           error (0, errno, _("cannot create hard link %s to %s"),
-                  quote_n (0, dst_name), quote_n (1, earlier_file));
-           goto un_backup;
-         }
+         if (link_failed)
+           {
+             error (0, errno, _("cannot create hard link %s to %s"),
+                    quote_n (0, dst_name), quote_n (1, earlier_file));
+             goto un_backup;
+           }
 
-       return true;
-      }
+         return true;
+       }
     }
 
   if (x->move_mode)

Index: tests/cp/Makefile.am
===================================================================
RCS file: /fetish/cu/tests/cp/Makefile.am,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -p -u -r1.29 -r1.30
--- tests/cp/Makefile.am        17 Nov 2005 19:06:55 -0000      1.29
+++ tests/cp/Makefile.am        4 Feb 2006 09:37:41 -0000       1.30
@@ -3,6 +3,7 @@ AUTOMAKE_OPTIONS = 1.1 gnits
 
 XFAIL_TESTS = acl
 TESTS = \
+  cp-deref \
   acl \
   preserve-2 r-vs-symlink link-preserve \
   backup-1 no-deref-link1 no-deref-link2 no-deref-link3 backup-is-src \

Index: tests/cp/cp-deref
===================================================================
RCS file: tests/cp/cp-deref
diff -N tests/cp/cp-deref
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/cp/cp-deref   4 Feb 2006 10:44:19 -0000       1.3
@@ -0,0 +1,35 @@
+#!/bin/sh
+# cp -RL dir1 dir2' must handle the case in which each of dir1 and dir2
+# contain a symlink pointing to some third directory.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  cp --version
+fi
+
+pwd=`pwd`
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
+trap 'status=$?; cd $pwd; chmod -R u+rwx $t0; rm -rf $t0 && exit $status' 0
+trap '(exit $?); exit $?' 1 2 13 15
+
+framework_failure=0
+mkdir -p $tmp || framework_failure=1
+cd $tmp || framework_failure=1
+mkdir a b c d || framework_failure=1
+ln -s ../c a || framework_failure=1
+ln -s ../c b || framework_failure=1
+
+if test $framework_failure = 1; then
+  echo "$0: failure in testing framework" 1>&2
+  (exit 1); exit 1
+fi
+
+fail=0
+
+# Before coreutils-5.94, the following would fail with this message:
+# cp: will not create hard link `d/b/c' to directory `d/a/c'
+cp -RL a b d || fail=1
+test -d a/c || fail=1
+test -d b/c || fail=1
+
+(exit $fail); exit $fail