Please keep me in Cc: as I'm not subscribed.
I failed to find references to discussions about this (intended?)
behaviour, so I'm filing this report. Please forgive me if I've missed
something elementary.
$ echo testfile > file; ln -s file link; opts="-l -Ll -Hl";
for o in $opts; do cp $o link cp${o}; done;
ls -li
total 4
3358742 lrwxrwxrwx 4 gpiero gpiero 4 Aug 23 22:27 cp-Hl -> file
3358742 lrwxrwxrwx 4 gpiero gpiero 4 Aug 23 22:27 cp-Ll -> file
3358742 lrwxrwxrwx 4 gpiero gpiero 4 Aug 23 22:27 cp-l -> file
3358741 -rw-r--r-- 1 gpiero gpiero 9 Aug 23 22:27 file
3358742 lrwxrwxrwx 4 gpiero gpiero 4 Aug 23 22:27 link -> file
I would have expected both 'cp-Hl' and 'cp-Ll' to be hardlinked to
'file', not to 'link'.
$ cp --version | head -n1
cp (GNU coreutils) 8.21
$ uname -a
Linux caimano 3.10-2-amd64 #1 SMP Debian 3.10.5-1 (2013-08-07) x86_64 GNU/Linux
$ readlink -f /lib/x86_64-linux-gnu/libc.so.6
/lib/x86_64-linux-gnu/libc-2.17.so
A tentative draft patch is attached. It still lacks unit tests and a way
to determine if the implementation of linkat() supports flags.
But most of all, please note that I'm not fully aware of the
implications (including portability issues) of such a change.
AFAICS, at least in one case the patch changes the behaviour of cp:
$ echo testfile > file; ln -s file link;
cp -l link cp-l.old; ../src/cp -l link cp-l.new;
ls -li
total 8
8912975 -rw-r--r-- 2 gpiero gpiero 9 Aug 23 22:58 cp-l.new
8913010 lrwxrwxrwx 2 gpiero gpiero 4 Aug 23 22:58 cp-l.old -> file
8912975 -rw-r--r-- 2 gpiero gpiero 9 Aug 23 22:58 file
8913010 lrwxrwxrwx 2 gpiero gpiero 4 Aug 23 22:58 link -> file
This is caused by the following code:
$ tail -n+1135 src/cp.c | head -n8
if (x.dereference == DEREF_UNDEFINED)
{
if (x.recursive)
/* This is compatible with FreeBSD. */
x.dereference = DEREF_NEVER;
else
x.dereference = DEREF_ALWAYS;
}
Not sure why this snippet is there[0], but it seems to me that it leads
to inconsistent behaviour:
$ echo testfile > file; ln -s file link;
cp link cp; cp -r link cp-r;
ls -li
total 8
3358743 -rw-r--r-- 1 gpiero gpiero 9 Aug 23 23:06 cp
3358744 lrwxrwxrwx 1 gpiero gpiero 4 Aug 23 23:06 cp-r -> file
3358741 -rw-r--r-- 1 gpiero gpiero 9 Aug 23 23:06 file
3358742 lrwxrwxrwx 1 gpiero gpiero 4 Aug 23 23:06 link -> file
I think it is counterintuitive that '--recursive' had effects on the
referentiation of the link.[1]
Anyway, given that x.dereference is set to DEREF_ALWAYS, I think the
modified behaviour of `cp -l` could be considered correct.
Thanks,
Gian Piero.
[0] I suspect this is for backward compatibility. In principle, I think
DEREF_UNDEF should always set DEREF_NEVER, but this change would modify
a common, and well spread, behaviour.
[1] The same stands for '--link', as demonstrated in the first example,
when 'cp-l' has been created as a hardlink to the symlink (even if for
different reasons: in that case DEREF_ALWAYS was in place, but linkat()
didn't follow the symlink).
Fri Aug 23 20:29:12 CEST 2013 Gian Piero Carrubba <[email protected]>
* DRAFT
diff -rN -u old-coreutils/src/copy.c new-coreutils/src/copy.c
--- old-coreutils/src/copy.c 2013-08-23 22:16:15.938940800 +0200
+++ new-coreutils/src/copy.c 2013-08-23 22:16:15.942940823 +0200
@@ -1566,10 +1566,15 @@
be created as a symbolic link to SRC_NAME. */
static bool
create_hard_link (char const *src_name, char const *dst_name,
- bool replace, bool verbose)
+ bool replace, bool verbose, bool dereference)
{
- /* We want to guarantee that symlinks are not followed. */
- bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0);
+ int flags;
+ if (dereference)
+ flags = AT_SYMLINK_FOLLOW;
+ else
+ flags = 0; /* We want to guarantee that symlinks are not followed. */
+
+ bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags) != 0);
/* If the link failed because of an existing destination,
remove that file and then call link again. */
@@ -1582,7 +1587,7 @@
}
if (verbose)
printf (_("removed %s\n"), quote (dst_name));
- link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0);
+ link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags) != 0);
}
if (link_failed)
@@ -1748,7 +1753,10 @@
/* Note we currently replace DST_NAME unconditionally,
even if it was a newer separate file. */
if (! create_hard_link (earlier_file, dst_name, true,
- x->verbose))
+ x->verbose,
+ x->dereference == DEREF_ALWAYS
+ || (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS
+ && command_line_arg)))
{
goto un_backup;
}
@@ -2078,7 +2086,10 @@
}
else
{
- if (! create_hard_link (earlier_file, dst_name, true, x->verbose))
+ if (! create_hard_link (earlier_file, dst_name, true, x->verbose,
+ x->dereference == DEREF_ALWAYS
+ || (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS
+ && command_line_arg)))
goto un_backup;
return true;
@@ -2389,7 +2400,10 @@
&& !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
&& x->dereference == DEREF_NEVER))
{
- if (! create_hard_link (src_name, dst_name, false, false))
+ if (! create_hard_link (src_name, dst_name, false, false,
+ x->dereference == DEREF_ALWAYS
+ || (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS
+ && command_line_arg)))
goto un_backup;
}
else if (S_ISREG (src_mode)
