GNU bug report logs -
#15173
[cp] --link overrides dereference settings
Previous Next
Reported by: Gian Piero Carrubba <gpiero <at> rm-rf.it>
Date: Fri, 23 Aug 2013 21:55:02 UTC
Severity: normal
Tags: fixed
Merged with 23120
Done: Bernhard Voelker <mail <at> bernhard-voelker.de>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15173 in the body.
You can then email your comments to 15173 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Fri, 23 Aug 2013 21:55:03 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Gian Piero Carrubba <gpiero <at> rm-rf.it>:
New bug report received and forwarded. Copy sent to
bug-coreutils <at> gnu.org.
(Fri, 23 Aug 2013 21:55:03 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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).
[draft.patch (text/x-diff, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Sun, 20 Oct 2013 01:12:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 15173 <at> debbugs.gnu.org (full text, mbox):
Hello Gian,
On 08/23/2013 11:40 PM, Gian Piero Carrubba wrote:
> 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.
thanks for your bug report, the thorough thoughts about it and the
patch proposal.
> $ 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'.
I'd also say that the above is an error ... or at least contradicts
to what is documented in the code (copy.c, line 2377):
Yet cp, invoked with '--link --no-dereference',
should not follow the link.
> --- 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. */
Instead of passing 'flags' to linkat (of which I don't know if it works
on all platforms), I'd rather dereference the symlink manually:
char *src_name_new = canonicalize_filename_mode (src_name, CAN_EXISTING);
> 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
Not ideal ...
> 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;
> }
Good analysis!
My guess is that this because POSIX [1] allows either behavior:
If source_file is a file of type symbolic link:
[...]
If the -R option was specified:
If none of the options -H, -L, nor -P were specified, it is
unspecified which of -H, -L, or -P will be used as a default.
[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html
This means that coreutils has chosen in between what makes most sense
and compatibility to other implementations.
> 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]
I think that is correct as POSIX requires this (see also in [1] above).
Instead, I'd change the above initialization of x.dereference in the
case the --link option is specified.
I have wrapped it for you into a Git patch (below), yet without a test
case.
This is the result:
$ : > file; ln -s file link
$ for opt in "" -l -lH -lL -lP -r ; do cp $opt link cp$opt ; done
$ ls -ldogi link file cp*
15335993 -rw-r--r-- 1 0 Oct 20 03:09 cp
15335992 lrwxrwxrwx 3 4 Oct 20 03:08 cp-l -> file
15335991 -rw-r--r-- 3 0 Oct 20 03:08 cp-lH
15335991 -rw-r--r-- 3 0 Oct 20 03:08 cp-lL
15335992 lrwxrwxrwx 3 4 Oct 20 03:08 cp-lP -> file
15335994 lrwxrwxrwx 1 4 Oct 20 03:09 cp-r -> file
15335991 -rw-r--r-- 3 0 Oct 20 03:08 file
15335992 lrwxrwxrwx 3 4 Oct 20 03:08 link -> file
WDYT?
BTW: tests/cp/same-file.sh now fails with the patch.
Have a nice day,
Berny
From fca06077e11d6338b35029bc61fbee5dbfb5ab66 Mon Sep 17 00:00:00 2001
From: Gian Piero Carrubba <gpiero <at> rm-rf.it>
Date: Sun, 20 Oct 2013 03:04:41 +0200
Subject: [PATCH] cp: fix --link --dereference [DRAFT]
* src/copy.c (create_hard_link): Add a bool 'dereference' parameter,
and canonicalize the given src_name when dereference is true.
(copy_internal): Add a value for the above new parameter in all
three calls to create_hard_link: true if the src_name should be
dereferenced before creating the hard link, false otherwise.
* src/cp.c (main): after parsing the options, if x.dereference is
still DEFEF_UNDEFINED and the --links option was specified, initialize
x.dereference to DEREF_NEVER.
* NEWS (Changes in behavior): Mention the change.
This fixes http://bugs.gnu.org/15173
---
NEWS | 4 ++++
src/copy.c | 42 ++++++++++++++++++++++++++++++------------
src/cp.c | 2 +-
3 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/NEWS b/NEWS
index 39dd3d6..c1d637c 100644
--- a/NEWS
+++ b/NEWS
@@ -70,6 +70,10 @@ GNU coreutils NEWS -*- outline -*-
** Changes in behavior
+ cp --link --dereference now dereferences a symbolic link as source files
+ before creating the hard link in the destination.
+ Previously, it would create a hard link of the symbolic link.
+
dd status=none now suppresses all non fatal diagnostic messages,
not just the transfer counts.
diff --git a/src/copy.c b/src/copy.c
index f66ab46..fa4d734 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1558,18 +1558,23 @@ restore_default_fscreatecon_or_die (void)
_("failed to restore the default file creation context"));
}
-/* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE and
- VERBOSE settings. Return true upon success. Otherwise, diagnose
- the failure and return false.
- If SRC_NAME is a symbolic link it will not be followed. If the system
- doesn't support hard links to symbolic links, then DST_NAME will
- be created as a symbolic link to SRC_NAME. */
+/* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE,
+ VERBOSE and DEREFERENCE settings. Return true upon success.
+ Otherwise, diagnose the failure and return false.
+ If SRC_NAME is a symbolic link it will not be followed unless DEREFERENCE
+ is true. If the system doesn't support hard links to symbolic links,
+ then DST_NAME will 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)
{
+ char *src_deref = NULL;
+ if (dereference)
+ src_deref = canonicalize_filename_mode (src_name, CAN_EXISTING);
+
/* We want to guarantee that symlinks are not followed. */
- bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0);
+ bool link_failed = (linkat (AT_FDCWD, dereference ? src_deref : src_name,
+ AT_FDCWD, dst_name, 0) != 0);
/* If the link failed because of an existing destination,
remove that file and then call link again. */
@@ -1578,13 +1583,17 @@ create_hard_link (char const *src_name, char const *dst_name,
if (unlink (dst_name) != 0)
{
error (0, errno, _("cannot remove %s"), quote (dst_name));
+ free (src_deref);
return false;
}
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, dereference ? src_deref : src_name,
+ AT_FDCWD, dst_name, 0) != 0);
}
+ free (src_deref);
+
if (link_failed)
{
error (0, errno, _("cannot create hard link %s to %s"),
@@ -1748,7 +1757,10 @@ copy_internal (char const *src_name, char const *dst_name,
/* 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 +2090,10 @@ copy_internal (char const *src_name, char const *dst_name,
}
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 +2404,10 @@ copy_internal (char const *src_name, char const *dst_name,
&& !(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)
diff --git a/src/cp.c b/src/cp.c
index 7bc8630..6efb152 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -1135,7 +1135,7 @@ main (int argc, char **argv)
if (x.dereference == DEREF_UNDEFINED)
{
- if (x.recursive)
+ if (x.recursive || x.hard_link)
/* This is compatible with FreeBSD. */
x.dereference = DEREF_NEVER;
else
--
1.8.3.1
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Wed, 23 Oct 2013 21:13:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 15173 <at> debbugs.gnu.org (full text, mbox):
* [Sun, Oct 20, 2013 at 03:11:04AM +0200] Bernhard Voelker:
>Hello Gian,
Hi Bernhard,
thank you for your answer and the fixed patch.
[...]
>Instead of passing 'flags' to linkat (of which I don't know if it works
>on all platforms), I'd rather dereference the symlink manually:
>
> char *src_name_new = canonicalize_filename_mode (src_name, CAN_EXISTING);
Good idea. It sounds sensible and should avoid portability issues (I
have to admit I didn't investigate further about those).
>> 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
>
>Not ideal ...
Surely not, but I think the 'new' behaviour is the "right" one.
Being my turn for quoting the SUS :)
If source_file is a file of type symbolic link:
If the -R option was not specified, cp shall take actions based on
the type and contents of the file referenced by the symbolic link,
and not by the symbolic link itself
This is also consistent with:
$ echo testfile > file; ln -s file link; cp link cp ; ls -li
total 8
5678474 -rw-r--r-- 1 gpiero gpiero 9 Oct 23 22:45 cp
5678472 -rw-r--r-- 1 gpiero gpiero 9 Oct 23 22:45 file
5678473 lrwxrwxrwx 1 gpiero gpiero 4 Oct 23 22:45 link -> file
where 'cp' is a copy of 'file', not of 'link'.
>My guess is that this because POSIX [1] allows either behavior:
>
> If source_file is a file of type symbolic link:
> [...]
> If the -R option was specified:
> If none of the options -H, -L, nor -P were specified, it is
> unspecified which of -H, -L, or -P will be used as a default.
>
>[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html
>
>This means that coreutils has chosen in between what makes most sense
>and compatibility to other implementations.
>
>> 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]
>
>I think that is correct as POSIX requires this (see also in [1] above).
Not sure. Probably I failed to clearly explain what I think the problem
is, but I miss to see how POSIX mandates it (from the quote you posted,
being '-r' the only option, the behaviour is unspecified, so I don't
think we could contradict POSIX modifying how it behaves).
What I mean is that currently, if the source is a symlink:
- `cp` copies the file referenced by the symlink
- `cp -r` copies the symlink itself
This is inconsistent, as '--recursive' should not have effect on the
dereferencing setting, being them two orthogonal features. It seems like
a side-(not-expected/desired)-effect. If I wanted to copy the symlink, I
should have used `cp -P`.
What I propose is:
- if the symlink points to a regular file, `cp -r` should copy the file
referenced by the symlink (acts like a plain `cp`, that in turn acts
the same as `cp -H` in this case)
- if the symlink points to a directory, ditto but taking care of not
dereferencing the symlinks inside the dir (acts like if '-H' was used,
and again like a plain `cp`, with the only obvious difference that a
plain `cp` would fail in this case)
I don't think this would be "prohibited" by POSIX (and as a bonus it's a
singe line patch :) ).
Anyway, all this probably belongs to another bug report (and though I
think it's the "right thing", I'm also afraid of the possible impact of
such a change).
>Instead, I'd change the above initialization of x.dereference in the
>case the --link option is specified.
>
>I have wrapped it for you into a Git patch (below), yet without a test
>case.
>
>This is the result:
>
> $ : > file; ln -s file link
> $ for opt in "" -l -lH -lL -lP -r ; do cp $opt link cp$opt ; done
> $ ls -ldogi link file cp*
> 15335993 -rw-r--r-- 1 0 Oct 20 03:09 cp
> 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 cp-l -> file
> 15335991 -rw-r--r-- 3 0 Oct 20 03:08 cp-lH
> 15335991 -rw-r--r-- 3 0 Oct 20 03:08 cp-lL
> 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 cp-lP -> file
> 15335994 lrwxrwxrwx 1 4 Oct 20 03:09 cp-r -> file
> 15335991 -rw-r--r-- 3 0 Oct 20 03:08 file
> 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 link -> file
As said above, I think `cp -l` should dereference the symlink, so I
would omit the change in the initialization of x.dereference.
>BTW: tests/cp/same-file.sh now fails with the patch.
I hope I can double check and submit a patch for that test during the
next days. As for the test case, what do you suggest ? The test you
posted above (for opt in "" -l -lH -lL -lP -r ...) would be sufficient ?
Ciao,
Gian Piero.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Tue, 29 Oct 2013 18:29:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 10/20/2013 02:11 AM, Bernhard Voelker wrote:
> Hello Gian,
>
> On 08/23/2013 11:40 PM, Gian Piero Carrubba wrote:
>> 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.
>
> thanks for your bug report, the thorough thoughts about it and the
> patch proposal.
This looks like a valid issue...
>> $ 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'.
>
> I'd also say that the above is an error ... or at least contradicts
> to what is documented in the code (copy.c, line 2377):
>
> Yet cp, invoked with '--link --no-dereference',
> should not follow the link.
Yes I would think that `ln -L` and `cp --link -L` should
be equivalent in this regard.
>> --- 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. */
>
> Instead of passing 'flags' to linkat (of which I don't know if it works
> on all platforms), I'd rather dereference the symlink manually:
>
> char *src_name_new = canonicalize_filename_mode (src_name, CAN_EXISTING);
Actually since linkat() has defined semantics,
_and thanks to gnulib, guaranteed semantics_
we should just leverage the gnulib magic here,
and use linkat with flags.
>> 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
>
> Not ideal ...
>
>> 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;
>> }
Interesting. You'd nearly think it should be the
other way around, given linking to symlinks when
copying to other directories, may give invalid
symlinks if they link outside the copied hierarchy.
> Good analysis!
> My guess is that this because POSIX [1] allows either behavior:
>
> If source_file is a file of type symbolic link:
> [...]
> If the -R option was specified:
> If none of the options -H, -L, nor -P were specified, it is
> unspecified which of -H, -L, or -P will be used as a default.
>
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html
>
> This means that coreutils has chosen in between what makes most sense
> and compatibility to other implementations.
>
>> 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]
>
> I think that is correct as POSIX requires this (see also in [1] above).
>
> Instead, I'd change the above initialization of x.dereference in the
> case the --link option is specified.
>
> I have wrapped it for you into a Git patch (below), yet without a test
> case.
>
> This is the result:
>
> $ : > file; ln -s file link
> $ for opt in "" -l -lH -lL -lP -r ; do cp $opt link cp$opt ; done
> $ ls -ldogi link file cp*
Nice basis for a test.
> 15335993 -rw-r--r-- 1 0 Oct 20 03:09 cp
> 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 cp-l -> file
> 15335991 -rw-r--r-- 3 0 Oct 20 03:08 cp-lH
> 15335991 -rw-r--r-- 3 0 Oct 20 03:08 cp-lL
> 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 cp-lP -> file
> 15335994 lrwxrwxrwx 1 4 Oct 20 03:09 cp-r -> file
> 15335991 -rw-r--r-- 3 0 Oct 20 03:08 file
> 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 link -> file
The above look correct.
For comparison on FreeBSD 9.1
1755429 -rw-r--r-- 1 pbrady pbrady - 0 Oct 29 12:16 cp
1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 cp-l
1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 cp-lH
1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 cp-lL
1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 cp-lP
1755430 -rw-r--r-- 1 pbrady pbrady - 0 Oct 29 12:16 cp-r
1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 file
1755428 lrwxr-xr-x 1 pbrady pbrady - 4 Oct 29 12:16 link -> file
And from older 8.10 GNU cp we have:
2368286 -rw-rw-r--. 1 0 Oct 29 18:19 cp
2368262 lrwxrwxrwx. 4 4 Oct 29 18:19 cp-l -> file
2368262 lrwxrwxrwx. 4 4 Oct 29 18:19 cp-lH -> file
2368262 lrwxrwxrwx. 4 4 Oct 29 18:19 cp-lL -> file
2368298 lrwxrwxrwx. 1 4 Oct 29 18:19 cp-lP -> file
2368299 lrwxrwxrwx. 1 4 Oct 29 18:19 cp-r -> file
2368218 -rw-rw-r--. 1 0 Oct 29 18:19 file
2368262 lrwxrwxrwx. 4 4 Oct 29 18:19 link -> file
In both the above we seem to be just getting the default
dereferencing behavior of link(1).
> WDYT?
>
> BTW: tests/cp/same-file.sh now fails with the patch.
Yes better to consolidate the functionality,
before doing the tests to avoid awkward test churn.
> Have a nice day,
> Berny
>
>
>>From fca06077e11d6338b35029bc61fbee5dbfb5ab66 Mon Sep 17 00:00:00 2001
> From: Gian Piero Carrubba <gpiero <at> rm-rf.it>
> Date: Sun, 20 Oct 2013 03:04:41 +0200
> Subject: [PATCH] cp: fix --link --dereference [DRAFT]
>
> * src/copy.c (create_hard_link): Add a bool 'dereference' parameter,
> and canonicalize the given src_name when dereference is true.
> (copy_internal): Add a value for the above new parameter in all
> three calls to create_hard_link: true if the src_name should be
> dereferenced before creating the hard link, false otherwise.
> * src/cp.c (main): after parsing the options, if x.dereference is
> still DEFEF_UNDEFINED and the --links option was specified, initialize
s/--links/--link/
> x.dereference to DEREF_NEVER.
> * NEWS (Changes in behavior): Mention the change.
>
> This fixes http://bugs.gnu.org/15173
> ---
> NEWS | 4 ++++
> src/copy.c | 42 ++++++++++++++++++++++++++++++------------
> src/cp.c | 2 +-
> 3 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 39dd3d6..c1d637c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -70,6 +70,10 @@ GNU coreutils NEWS -*- outline -*-
>
> ** Changes in behavior
>
> + cp --link --dereference now dereferences a symbolic link as source files
> + before creating the hard link in the destination.
> + Previously, it would create a hard link of the symbolic link.
> +
> dd status=none now suppresses all non fatal diagnostic messages,
> not just the transfer counts.
>
> diff --git a/src/copy.c b/src/copy.c
> index f66ab46..fa4d734 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -1558,18 +1558,23 @@ restore_default_fscreatecon_or_die (void)
> _("failed to restore the default file creation context"));
> }
>
> -/* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE and
> - VERBOSE settings. Return true upon success. Otherwise, diagnose
> - the failure and return false.
> - If SRC_NAME is a symbolic link it will not be followed. If the system
> - doesn't support hard links to symbolic links, then DST_NAME will
> - be created as a symbolic link to SRC_NAME. */
> +/* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE,
> + VERBOSE and DEREFERENCE settings. Return true upon success.
> + Otherwise, diagnose the failure and return false.
> + If SRC_NAME is a symbolic link it will not be followed unless DEREFERENCE
> + is true. If the system doesn't support hard links to symbolic links,
> + then DST_NAME will 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)
> {
> + char *src_deref = NULL;
> + if (dereference)
> + src_deref = canonicalize_filename_mode (src_name, CAN_EXISTING);
Probably best avoided as mentioned above.
> /* We want to guarantee that symlinks are not followed. */
> - bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0);
> + bool link_failed = (linkat (AT_FDCWD, dereference ? src_deref : src_name,
> + AT_FDCWD, dst_name, 0) != 0);
>
> /* If the link failed because of an existing destination,
> remove that file and then call link again. */
> @@ -1578,13 +1583,17 @@ create_hard_link (char const *src_name, char const *dst_name,
> if (unlink (dst_name) != 0)
> {
> error (0, errno, _("cannot remove %s"), quote (dst_name));
> + free (src_deref);
> return false;
> }
> 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, dereference ? src_deref : src_name,
> + AT_FDCWD, dst_name, 0) != 0);
> }
>
> + free (src_deref);
> +
> if (link_failed)
> {
> error (0, errno, _("cannot create hard link %s to %s"),
> @@ -1748,7 +1757,10 @@ copy_internal (char const *src_name, char const *dst_name,
> /* 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 +2090,10 @@ copy_internal (char const *src_name, char const *dst_name,
> }
> 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 +2404,10 @@ copy_internal (char const *src_name, char const *dst_name,
> && !(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)))
Lots and copy and paste there.
Maybe use a helper like:
static inline bool _GL_ATTRIBUTE_PURE
should_dereference (const struct cp_options *x, bool cli_arg)
{
return x->dereference == DEREF_ALWAYS
|| (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS
&& cli_arg);
}
> goto un_backup;
> }
> else if (S_ISREG (src_mode)
> diff --git a/src/cp.c b/src/cp.c
> index 7bc8630..6efb152 100644
> --- a/src/cp.c
> +++ b/src/cp.c
> @@ -1135,7 +1135,7 @@ main (int argc, char **argv)
>
> if (x.dereference == DEREF_UNDEFINED)
> {
> - if (x.recursive)
> + if (x.recursive || x.hard_link)
> /* This is compatible with FreeBSD. */
> x.dereference = DEREF_NEVER;
> else
>
Thanks to both of you for looking into this awkward issue.
Pádraig.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Wed, 30 Oct 2013 12:15:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 15173 <at> debbugs.gnu.org (full text, mbox):
> On October 29, 2013 at 7:27 PM Pádraig Brady <P <at> draigBrady.com> wrote:
> On 10/20/2013 02:11 AM, Bernhard Voelker wrote:
> >> I would have expected both 'cp-Hl' and 'cp-Ll' to be hardlinked to
> >> 'file', not to 'link'.
> >
> > I'd also say that the above is an error ... or at least contradicts
> > to what is documented in the code (copy.c, line 2377):
> >
> > Yet cp, invoked with '--link --no-dereference',
> > should not follow the link.
>
> Yes I would think that `ln -L` and `cp --link -L` should
> be equivalent in this regard.
s/ln -L/ln -n/, I guess.
> > Instead of passing 'flags' to linkat (of which I don't know if it works
> > on all platforms), I'd rather dereference the symlink manually:
> >
> > char *src_name_new = canonicalize_filename_mode (src_name, CAN_EXISTING);
>
> Actually since linkat() has defined semantics,
> _and thanks to gnulib, guaranteed semantics_
> we should just leverage the gnulib magic here,
> and use linkat with flags.
Thanks for the clarification. I changed it back to Gian's
version.
> > This is the result:
> >
> > $ : > file; ln -s file link
> > $ for opt in "" -l -lH -lL -lP -r ; do cp $opt link cp$opt ; done
> > $ ls -ldogi link file cp*
>
> Nice basis for a test.
Yes, but it's not enough: I exercised also some combinations with
-lrP -lrH -lrL, and the whole stuff again for a symlink pointing
to a directory.
It got a bit late yesterday, and I couldn't get the things sorted
to present here. All Ican say now is that there are still corner
cases where cp now behaves differently (e.g. "cp -l link2dir dst").
I hope I can get back here with the results this evening.
> > 15335993 -rw-r--r-- 1 0 Oct 20 03:09 cp
> > 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 cp-l -> file
> > 15335991 -rw-r--r-- 3 0 Oct 20 03:08 cp-lH
> > 15335991 -rw-r--r-- 3 0 Oct 20 03:08 cp-lL
> > 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 cp-lP -> file
> > 15335994 lrwxrwxrwx 1 4 Oct 20 03:09 cp-r -> file
> > 15335991 -rw-r--r-- 3 0 Oct 20 03:08 file
> > 15335992 lrwxrwxrwx 3 4 Oct 20 03:08 link -> file
>
> The above look correct.
Great, thanks.
> For comparison on FreeBSD 9.1
>
> 1755429 -rw-r--r-- 1 pbrady pbrady - 0 Oct 29 12:16 cp
> 1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 cp-l
> 1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 cp-lH
> 1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 cp-lL
> 1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 cp-lP
> 1755430 -rw-r--r-- 1 pbrady pbrady - 0 Oct 29 12:16 cp-r
> 1755427 -rw-r--r-- 5 pbrady pbrady - 0 Oct 29 12:16 file
> 1755428 lrwxr-xr-x 1 pbrady pbrady - 4 Oct 29 12:16 link -> file
Interesting.
> > BTW: tests/cp/same-file.sh now fails with the patch.
>
> Yes better to consolidate the functionality,
> before doing the tests to avoid awkward test churn.
yes, regarding from what I got with the tests mentioned above,
I'm still not convinced we're at that point.
> >>From fca06077e11d6338b35029bc61fbee5dbfb5ab66 Mon Sep 17 00:00:00 2001
> > From: Gian Piero Carrubba <gpiero <at> rm-rf.it>
> > Date: Sun, 20 Oct 2013 03:04:41 +0200
> > Subject: [PATCH] cp: fix --link --dereference [DRAFT]
> >
> > * src/copy.c (create_hard_link): Add a bool 'dereference' parameter,
> > and canonicalize the given src_name when dereference is true.
> > (copy_internal): Add a value for the above new parameter in all
> > three calls to create_hard_link: true if the src_name should be
> > dereferenced before creating the hard link, false otherwise.
> > * src/cp.c (main): after parsing the options, if x.dereference is
> > still DEFEF_UNDEFINED and the --links option was specified, initialize
>
> s/--links/--link/
Fixed, thanks.
> > diff --git a/src/copy.c b/src/copy.c
> > index f66ab46..fa4d734 100644
> > --- a/src/copy.c
> > +++ b/src/copy.c
[...]
> > @@ -1748,7 +1757,10 @@ copy_internal (char const *src_name, char const
> > *dst_name,
> > /* 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 +2090,10 @@ copy_internal (char const *src_name, char const
> > *dst_name,
> > }
> > 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 +2404,10 @@ copy_internal (char const *src_name, char const
> > *dst_name,
> > && !(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)))
>
> Lots and copy and paste there.
> Maybe use a helper like:
>
> static inline bool _GL_ATTRIBUTE_PURE
> should_dereference (const struct cp_options *x, bool cli_arg)
> {
> return x->dereference == DEREF_ALWAYS
> || (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS
> && cli_arg);
> }
Good idea!
Thanks again for looking into this. It's really quite complex.
I'll come up with the test (old vs. new) cases soon.
Have a nice day,
Berny
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 31 Oct 2013 02:08:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 10/30/2013 01:13 PM, Bernhard Voelker wrote:
> Thanks again for looking into this. It's really quite complex.
> I'll come up with the test (old vs. new) cases soon.
Okay, here we go:
the attached patch is updated regarding the issues you mentioned.
I furthermore tweaked the NEWS entry:
* instead of just mentioning the --dereference option,
mention both the -L and -H options explicitly.
* add a note that "cp -l dangling" and "cp -l symlink-to-dir"
now create hard links as expected - cp doesn't try to dereference
the src anymore which would lead to different error diagnostics
in the two cases [*].
Then, I created a few test cases to check the changes in behaviour
of cp-8.21 against the changed cp. It runs various combinations of
the options -l, -s, -R, -H, -L, -P and --preserve=links on symlinks
to a file ('filelink'), to a directory ('dirlink') and on a dangling
symlink ('danglink').
The script is already ugly enough, so I omitted checking the
files, dir and links in the subdirectory for changes.
The changes so far - 12 out of 144 - are:
$ grep "DIFFERENCE in Test: " testit.log | sed 's/^.*: //'
cp -L -l filelink ...
cp -L -l --preserve=links filelink ...
cp -L -l -R filelink ...
cp -L -l -R --preserve=links filelink ...
cp -H -l filelink ...
cp -H -l --preserve=links filelink ...
cp -H -l -R filelink ...
cp -H -l -R --preserve=links filelink ...
cp -l dirlink ...
cp -l --preserve=links dirlink ...
cp -l danglink ...
cp -l --preserve=links danglink ...
I think these are all okay.
[*] IMO even the 4 latter changes are okay and don't contradict to
POSIX: --link is not specified by POSIX.
BTW: The failures "can make relative symbolic links only in current directory"
for e.g. "cp -L -s -R dirlink" are interesting but a different case, i.e.,
cp's behavior didn't change.
WDYT?
Finally, the testsuite is still broken (tests/cp/same-file), and new
tests for "cp -l[LH]" are also still missing.
Have a nice day,
Berny
[cp--link--deref-v2.patch (text/x-patch, attachment)]
[testit.log.xz (application/x-xz, attachment)]
[testit.sh (application/x-shellscript, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 31 Oct 2013 12:14:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 10/31/2013 02:07 AM, Bernhard Voelker wrote:
> On 10/30/2013 01:13 PM, Bernhard Voelker wrote:
>> Thanks again for looking into this. It's really quite complex.
>> I'll come up with the test (old vs. new) cases soon.
>
> Okay, here we go:
>
> the attached patch is updated regarding the issues you mentioned.
>
> I furthermore tweaked the NEWS entry:
>
> * instead of just mentioning the --dereference option,
> mention both the -L and -H options explicitly.
>
> * add a note that "cp -l dangling" and "cp -l symlink-to-dir"
> now create hard links as expected - cp doesn't try to dereference
> the src anymore which would lead to different error diagnostics
> in the two cases [*].
>
> Then, I created a few test cases to check the changes in behaviour
> of cp-8.21 against the changed cp. It runs various combinations of
> the options -l, -s, -R, -H, -L, -P and --preserve=links on symlinks
> to a file ('filelink'), to a directory ('dirlink') and on a dangling
> symlink ('danglink').
> The script is already ugly enough, so I omitted checking the
> files, dir and links in the subdirectory for changes.
> The changes so far - 12 out of 144 - are:
>
> $ grep "DIFFERENCE in Test: " testit.log | sed 's/^.*: //'
> cp -L -l filelink ...
> cp -L -l --preserve=links filelink ...
> cp -L -l -R filelink ...
> cp -L -l -R --preserve=links filelink ...
> cp -H -l filelink ...
> cp -H -l --preserve=links filelink ...
> cp -H -l -R filelink ...
> cp -H -l -R --preserve=links filelink ...
So -HL are now honored in this regard
> cp -l dirlink ...
> cp -l --preserve=links dirlink ...
> cp -l danglink ...
> cp -l --preserve=links danglink ...
and by default we don't deref the above.
That all seems consistent with expectations and what we previously discussed.
But...
I've just now read POSIX for cp, and it states:
"If the -R option was not specified, cp shall take actions based on the type
and contents of the file referenced by the symbolic link, and not by the
symbolic link itself, unless the -P option was specified."
This suggests that -HL should only be significant with -R ?
That is a bit surprising TBH. What do you think Eric?
It also suggests that we should hardlink to a symlink only with -P,
i.e. that we should AT_SYMLINK_FOLLOW unless -P is specified ?
That's also a bit surprising, given that POSIX for ln states
that it's implementation defined what's done if neither -P or -L is specified.
I wouldn't be inclined to follow POSIX in that regard.
Also worth comparing is --symbolic-link, which seems closer to POSIX in this regard.
--symbolic-link does need -P to be able to reference dangling links
even though the target of the created symlink is the source symlink rather than its target.
Should we remove the -P requirement to symlink dangling links
as is the case for --link above.
thanks,
Pádraig.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 31 Oct 2013 13:18:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 15173 <at> debbugs.gnu.org (full text, mbox):
> On October 31, 2013 at 1:12 PM Pádraig Brady <P <at> draigBrady.com> wrote:
> That all seems consistent with expectations and what we previously discussed.
>
> But...
>
> I've just now read POSIX for cp, and it states:
>
> "If the -R option was not specified, cp shall take actions based on the type
> and contents of the file referenced by the symbolic link, and not by the
> symbolic link itself, unless the -P option was specified."
>
> This suggests that -HL should only be significant with -R ?
> That is a bit surprising TBH. What do you think Eric?
>
> It also suggests that we should hardlink to a symlink only with -P,
> i.e. that we should AT_SYMLINK_FOLLOW unless -P is specified ?
> That's also a bit surprising, given that POSIX for ln states
> that it's implementation defined what's done if neither -P or -L is specified.
> I wouldn't be inclined to follow POSIX in that regard.
I don't read the POSIX spec that way: there are 2 things to consider:
a) POSIX doesn't say a word about hard links, and
b) the -l,--link option is a GNU extension to conveniently
copy files or trees by creating hard links (only).
I.e. if someone uses -l, then the POSIX semantics does not
apply anymore because we do not copy anymore. Whether the
ln(1) specification does apply more here is another question.
Therefore, I think GNU cp(1) should do what makes most sense
for the user - depending on -LPH or none being used. With the
proposed patch, I think we're getting a bit closer to that.
Have a nice day,
Berny
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 31 Oct 2013 13:56:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
* [Thu, Oct 31, 2013 at 12:12:55PM +0000] Pádraig Brady:
>> On 10/30/2013 01:13 PM, Bernhard Voelker wrote:
>> The changes so far - 12 out of 144 - are:
>>
>> $ grep "DIFFERENCE in Test: " testit.log | sed 's/^.*: //'
>> cp -L -l filelink ...
>> cp -L -l --preserve=links filelink ...
>> cp -L -l -R filelink ...
>> cp -L -l -R --preserve=links filelink ...
>> cp -H -l filelink ...
>> cp -H -l --preserve=links filelink ...
>> cp -H -l -R filelink ...
>> cp -H -l -R --preserve=links filelink ...
>
>So -HL are now honored in this regard
>
>> cp -l dirlink ...
>> cp -l --preserve=links dirlink ...
>> cp -l danglink ...
>> cp -l --preserve=links danglink ...
>
>and by default we don't deref the above.
>
>That all seems consistent with expectations and what we previously
>discussed.
Not what I would expect, and imho not SUSv3 compliant, either with
regard to --preserve=links and -R. More about this later.
>I've just now read POSIX for cp, and it states:
>
> "If the -R option was not specified, cp shall take actions based on the type
> and contents of the file referenced by the symbolic link, and not by the
> symbolic link itself, unless the -P option was specified."
>
>This suggests that -HL should only be significant with -R ?
>That is a bit surprising TBH. What do you think Eric?
Oh well, didn't read it like this. My opinion was, and still is even if
not strong as before, that explicitly using -H/L/preserve=links would
override that _default_ behaviour.
>I wouldn't be inclined to follow POSIX in that regard.
Oh... I was tweaking a bit the last patch posted by Bernhard in order to
let it be POSIX compliant (but now I have to add: "for my interpretation
of POSIX"), but didn't had the time to complete before going to work.
Will probably finish it up anyway this evening or tomorrow, at this
point at least for comparing how far we're currently are from "POSIX".
In the meantime please see attached the results for the tests from
Bernhard. There are a lot more differences in behaviour:
$ fgrep -c "DIFFERENCE in Test: " testit.log
40
but I suspect (I hadn't the time for double checking them) they are all
correct.
My points in short:
- POSIX (my interpretation): without '-r' the link should be
dereferenced, *unless* told otherwise
- POSIX (I think, cannot check now): order of options is important. In
case of conflicting options, the LAST one should be used
- --(no)-preserve=links
To be honest, it isn't clear to me what it stands for. I have assumed
--preserve=links should set DEREF_NEVER and --no-preserve=links should
set DEREF_ALWAYS. Anyway I don't particularly like the implementation,
as now dereferencing settings are both in x->preserve_links and in
x->dereference, some times even with opposite values. And some times a
function checks for x->preserve_links and not for x->dereference, or
vice versa.
If it has a different meaning from what I've supposed, I can't
understand how it should behave.
Frankly, dereferencing in cp seems a bit more complicated as I first
supposed.
Ciao,
Gian Piero.
[testit.log.gz (application/octet-stream, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 31 Oct 2013 14:04:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 15173 <at> debbugs.gnu.org (full text, mbox):
* [Thu, Oct 31, 2013 at 02:54:59PM +0100] Gian Piero Carrubba:
>>I've just now read POSIX for cp, and it states:
>>
>>"If the -R option was not specified, cp shall take actions based on the type
>> and contents of the file referenced by the symbolic link, and not by the
>> symbolic link itself, unless the -P option was specified."
>>
>>This suggests that -HL should only be significant with -R ?
>>That is a bit surprising TBH. What do you think Eric?
>
>Oh well, didn't read it like this. My opinion was, and still is even
>if not strong as before, that explicitly using -H/L/preserve=links
>would override that _default_ behaviour.
Sorry, fast reading got me confused.
Yes, my opinion is that the default behaviour (i.e.: when not using -P
or --preserve=links) should be to dereference the symlink. This does not
mean that -HL are not significant without -R, as for what I remember
about parameters order:
cp -P -L
should dereference the symlinks.
Ciao,
Gian Piero.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 31 Oct 2013 16:48:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 10/31/2013 01:17 PM, Bernhard Voelker wrote:
>> On October 31, 2013 at 1:12 PM Pádraig Brady <P <at> draigBrady.com> wrote:
>> That all seems consistent with expectations and what we previously discussed.
>>
>> But...
>>
>> I've just now read POSIX for cp, and it states:
>>
>> "If the -R option was not specified, cp shall take actions based on the type
>> and contents of the file referenced by the symbolic link, and not by the
>> symbolic link itself, unless the -P option was specified."
>>
>> This suggests that -HL should only be significant with -R ?
>> That is a bit surprising TBH. What do you think Eric?
>>
>> It also suggests that we should hardlink to a symlink only with -P,
>> i.e. that we should AT_SYMLINK_FOLLOW unless -P is specified ?
>> That's also a bit surprising, given that POSIX for ln states
>> that it's implementation defined what's done if neither -P or -L is specified.
>> I wouldn't be inclined to follow POSIX in that regard.
>
> I don't read the POSIX spec that way: there are 2 things to consider:
> a) POSIX doesn't say a word about hard links, and
> b) the -l,--link option is a GNU extension to conveniently
> copy files or trees by creating hard links (only).
>
> I.e. if someone uses -l, then the POSIX semantics does not
> apply anymore because we do not copy anymore. Whether the
> ln(1) specification does apply more here is another question.
>
> Therefore, I think GNU cp(1) should do what makes most sense
> for the user - depending on -LPH or none being used. With the
> proposed patch, I think we're getting a bit closer to that.
OK cool. With that cp and ln will be more consistent with each other anyway.
thanks,
Pádraig.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 31 Oct 2013 17:28:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 15173 <at> debbugs.gnu.org (full text, mbox):
* [Thu, Oct 31, 2013 at 02:17:24PM +0100] Bernhard Voelker:
>> On October 31, 2013 at 1:12 PM Pádraig Brady <P <at> draigBrady.com> wrote:
>> I've just now read POSIX for cp, and it states:
>>
>> "If the -R option was not specified, cp shall take actions based on the type
>> and contents of the file referenced by the symbolic link, and not by the
>> symbolic link itself, unless the -P option was specified."
>>
>> This suggests that -HL should only be significant with -R ?
>> That is a bit surprising TBH. What do you think Eric?
[...]
>I don't read the POSIX spec that way: there are 2 things to consider:
>a) POSIX doesn't say a word about hard links, and
>b) the -l,--link option is a GNU extension to conveniently
> copy files or trees by creating hard links (only).
>
>I.e. if someone uses -l, then the POSIX semantics does not
>apply anymore because we do not copy anymore.
I don't support this argument. Being it an extension could mean that it
couldn't _violate_ POSIX but I don't think it is a good reason for being
_inconsistent_ with POSIX.
>Therefore, I think GNU cp(1) should do what makes most sense
>for the user - depending on -LPH or none being used. With the
>proposed patch, I think we're getting a bit closer to that.
I (strongly) disagree, _especially_ from a user perspective. This is the
same inconsistence I mentioned before about `cp -r`.
Let's say I run:
cp symlink new
so I get a copy of the _dereferenced_ file.
Mmmh, wait a moment... I don't need a copy, a hardlink would be
sufficient:
cp -l symlink new
And now I get a hardlink to the _symlink_. Why ? I just wanted to use
link instead of copy, I didn't mention anything about the dereferencing.
Why should I have different default behaviours about dereferencing when
I use _unrelated_ options ?
So now I have to remember that:
- a plain `cp` dereferences
- `cp -l` doesn't
- `cp -r` doesn't
- whatever new GNU option (not -yet- covered by POSIX) could possibly
dereference, or not, or just for the command line arguments...
Really Bernhard, from a user perspective this is terrible.
Ciao,
Gian Piero.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 31 Oct 2013 21:29:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
* [Thu, Oct 31, 2013 at 02:54:59PM +0100] Gian Piero Carrubba:
>Oh... I was tweaking a bit the last patch posted by Bernhard in order
>to let it be POSIX compliant (but now I have to add: "for my
>interpretation of POSIX"), but didn't had the time to complete before
>going to work. Will probably finish it up anyway this evening or
>tomorrow, at this point at least for comparing how far we're currently
>are from "POSIX".
Here it is.
First of all, sorry for having generated a lot of noise. I completely
misunderstood the meaning of --no-preserve=links. Having get rid of
that, the differences in the results of the tests are a lot less.
$ fgrep "DIFFERENCE in Test: " testit.log | sed 's/^.*: //'
cp -l filelink ...
cp -l --preserve=links filelink ...
cp -L -l filelink ...
cp -L -l --preserve=links filelink ...
cp -L -l -R filelink ...
cp -L -l -R --preserve=links filelink ...
cp -H -l filelink ...
cp -H -l --preserve=links filelink ...
cp -H -l -R filelink ...
cp -H -l -R --preserve=links filelink ...
I think they are all legit (full log and differences summary attached).
* Changes with regard to the last patch posted by Bernhard.
=== Minor changes
- use 'dereference' instead of 'deref' in order to be consistent with
'x->dereference' and for passing the "grep test"[0]
- use 'command_line_arg' instead of 'cli_arg' in order to be consistent
with an already used variable and for passing the "grep test"
- move up the declaration of bool dereference in order to use it for
every invocation of create_hard_link()
- clean up the initialization of 'flags' in create_hard_link()
[0] i.e.: http://jamie-wong.com/2013/07/12/grep-test/
=== Major changes
- dereference by default (i.e.: unless --no-dereference is used) the
source files.
I promise that I will not insist anymore :), but I really think this
is important for consistency from a user point of view.
Ciao,
Gian Piero.
[bug-15173.patch (text/x-diff, attachment)]
[testit-differences-summary.log.gz (application/octet-stream, attachment)]
[testit.log.gz (text/plain, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Sat, 02 Nov 2013 00:08:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 10/31/2013 09:28 PM, Gian Piero Carrubba wrote:
> * [Thu, Oct 31, 2013 at 02:54:59PM +0100] Gian Piero Carrubba:
>> Oh... I was tweaking a bit the last patch posted by Bernhard in order to let it be POSIX compliant (but now I have to add: "for my interpretation of POSIX"), but didn't had the time to complete before going to work. Will probably finish it up anyway this evening or tomorrow, at this point at least for comparing how far we're currently are from "POSIX".
>
> Here it is.
> First of all, sorry for having generated a lot of noise. I completely misunderstood the meaning of --no-preserve=links. Having get rid of that, the differences in the results of the tests are a lot less.
>
> $ fgrep "DIFFERENCE in Test: " testit.log | sed 's/^.*: //'
> cp -l filelink ...
> cp -l --preserve=links filelink ...
> cp -L -l filelink ...
> cp -L -l --preserve=links filelink ...
> cp -L -l -R filelink ...
> cp -L -l -R --preserve=links filelink ...
> cp -H -l filelink ...
> cp -H -l --preserve=links filelink ...
> cp -H -l -R filelink ...
> cp -H -l -R --preserve=links filelink ...
>
> I think they are all legit (full log and differences summary attached).
>
> * Changes with regard to the last patch posted by Bernhard.
>
> === Minor changes
>
> - use 'dereference' instead of 'deref' in order to be consistent with 'x->dereference' and for passing the "grep test"[0]
> - use 'command_line_arg' instead of 'cli_arg' in order to be consistent with an already used variable and for passing the "grep test"
> - move up the declaration of bool dereference in order to use it for every invocation of create_hard_link()
> - clean up the initialization of 'flags' in create_hard_link()
>
> [0] i.e.: http://jamie-wong.com/2013/07/12/grep-test/
>
> === Major changes
>
> - dereference by default (i.e.: unless --no-dereference is used) the source files.
> I promise that I will not insist anymore :), but I really think this is important for consistency from a user point of view.
>
> Ciao,
> Gian Piero.
So far we have Bernhard's slightly less invasive patch
which is an improvement, and behaves as per this table:
Old New BSD
+-------------
ln | S = T
ln -L | T = T
ln -P | S = S
cp -l | S S -
cp -lL | S T -
cp -lP | NS S -
cp -Rl | NS S -
cp -RlL | S T -
cp -RlP | NS S -
NS = New symlink
S = hard link to symlink
T = hard link to symlink target
Furthermore my previously noted POSIX interpretation,
and Gian's preference and patch, is to also
deref in the 'cp -l' case above (to turn it to a T).
There are valid arguments to doing that.
1. Follows naturally from standard cp which would create files in the dest
2. POSIX seems to imply we should operation on target without -P
3. A hardlink to a relative symlink may become invalid when in a different dir
Arguments against doing it would be:
1. It would be a little more invasive given the less specific cp command.
2. would be inconsistent with `ln`. Though not on BSD. Now we're free to
change ln to hardlink to targets rather than to symlinks by default,
and that may be more natural as per the 3 points above.
Though that would be more invasive again.
So I would be 60:40 for going with Gian's addition
of dereferencing with just `cp -l`, and I would be
60:40 against changing the behavior of `ln` even though
it would now be inconsistent with `cp -l` (and still inconsistent with BSD).
thanks,
Pádraig.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 04 Nov 2013 00:49:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 11/02/2013 01:07 AM, Pádraig Brady wrote:
> So I would be 60:40 for going with Gian's addition
> of dereferencing with just `cp -l`, and I would be
> 60:40 against changing the behavior of `ln` even though
> it would now be inconsistent with `cp -l` (and still inconsistent with BSD).
Thanks for the summary.
After playing a bit more, I'd also tend to use Gian's solution:
hard links to symbolic links are probably not that useful, because
* they have the same owner/group as the original symbolic link which
could lead to problems with permissions during removal of the symlink
in a sticky dir like /tmp), and
* the target cannot be changed without creating the symlink again
which is problematic to remember in both places.
BUT I'm not happy at all with the following case:
$ : > file
$ ln -s file filelink
$ src/cp --link filelink dst--link
$ src/cp --link -R filelink dst--link-R
$ ls -ldogi file filelink dst--link dst--link-R
537364 -rw-r--r-- 2 0 Nov 4 01:30 dst--link
537365 lrwxrwxrwx 2 4 Nov 4 01:30 dst--link-R -> file
537364 -rw-r--r-- 2 0 Nov 4 01:30 file
537365 lrwxrwxrwx 2 4 Nov 4 01:30 filelink -> file
That's exactly what Gian was worried about in a different case
of my solution: it *matters* whether the rather unrelated -R option
is specified or not. ;-(
It got late again. I'll have a look tomorrow again - maybe there
are no side effects with the following:
diff --git a/src/cp.c b/src/cp.c
index 7bc8630..78c0a04 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -1135,7 +1135,7 @@ main (int argc, char **argv)
if (x.dereference == DEREF_UNDEFINED)
{
- if (x.recursive)
+ if (x.recursive && ! x.hard_link)
/* This is compatible with FreeBSD. */
x.dereference = DEREF_NEVER;
else
Have a nice day,
Berny
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 04 Nov 2013 09:38:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 11/04/2013 12:48 AM, Bernhard Voelker wrote:
> On 11/02/2013 01:07 AM, Pádraig Brady wrote:
>> So I would be 60:40 for going with Gian's addition
>> of dereferencing with just `cp -l`, and I would be
>> 60:40 against changing the behavior of `ln` even though
>> it would now be inconsistent with `cp -l` (and still inconsistent with BSD).
>
> Thanks for the summary.
>
> After playing a bit more, I'd also tend to use Gian's solution:
> hard links to symbolic links are probably not that useful, because
> * they have the same owner/group as the original symbolic link which
> could lead to problems with permissions during removal of the symlink
> in a sticky dir like /tmp), and
> * the target cannot be changed without creating the symlink again
> which is problematic to remember in both places.
>
> BUT I'm not happy at all with the following case:
>
> $ : > file
> $ ln -s file filelink
> $ src/cp --link filelink dst--link
> $ src/cp --link -R filelink dst--link-R
> $ ls -ldogi file filelink dst--link dst--link-R
> 537364 -rw-r--r-- 2 0 Nov 4 01:30 dst--link
> 537365 lrwxrwxrwx 2 4 Nov 4 01:30 dst--link-R -> file
> 537364 -rw-r--r-- 2 0 Nov 4 01:30 file
> 537365 lrwxrwxrwx 2 4 Nov 4 01:30 filelink -> file
>
> That's exactly what Gian was worried about in a different case
> of my solution: it *matters* whether the rather unrelated -R option
> is specified or not. ;-(
>
> It got late again. I'll have a look tomorrow again - maybe there
> are no side effects with the following:
>
> diff --git a/src/cp.c b/src/cp.c
> index 7bc8630..78c0a04 100644
> --- a/src/cp.c
> +++ b/src/cp.c
> @@ -1135,7 +1135,7 @@ main (int argc, char **argv)
>
> if (x.dereference == DEREF_UNDEFINED)
> {
> - if (x.recursive)
> + if (x.recursive && ! x.hard_link)
> /* This is compatible with FreeBSD. */
> x.dereference = DEREF_NEVER;
> else
Yes I didn't consider -R in the table as I don't see any reason
for it to behave differently when -l is specified (and -l aplies
neither to POSIX or BSD (comment)). So the above adjustment
looks correct to me.
thanks,
Pádraig.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 04 Nov 2013 23:04:02 GMT)
Full text and
rfc822 format available.
Message #53 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 11/04/2013 10:37 AM, Pádraig Brady wrote:
> On 11/04/2013 12:48 AM, Bernhard Voelker wrote:
>> It got late again. I'll have a look tomorrow again - maybe there
>> are no side effects with the following:
>>
>> diff --git a/src/cp.c b/src/cp.c
>> index 7bc8630..78c0a04 100644
>> --- a/src/cp.c
>> +++ b/src/cp.c
>> @@ -1135,7 +1135,7 @@ main (int argc, char **argv)
>>
>> if (x.dereference == DEREF_UNDEFINED)
>> {
>> - if (x.recursive)
>> + if (x.recursive && ! x.hard_link)
>> /* This is compatible with FreeBSD. */
>> x.dereference = DEREF_NEVER;
>> else
>
> Yes I didn't consider -R in the table as I don't see any reason
> for it to behave differently when -l is specified (and -l aplies
> neither to POSIX or BSD (comment)). So the above adjustment
> looks correct to me.
Indeed, that adjustment seems to be the right one - finally. ;-)
I have added it to the patch as well as a new test case. I've added
a "Co-authored-by:"-me line for that.
@Gian: please note that we use "double-blank after-dot" format in
comments. I've fixed that for you.
The patch passes make check and syntax-check.
I've also attached the test result cp-8.21 versus the new one.
These are the differences (I leave out the --preserve=links cases
as these are also affected but unrelated):
COMMAND | 8.21 | NEW
cp -l filelink ... | S | T
cp -l -R filelink ... | S | T
cp -L -l filelink ... | S | T
cp -L -l -R filelink ... | S | T
cp -H -l filelink ... | S | T
cp -H -l -R filelink ... | S | T
cp -l -R dirlink ... | S | NDH
cp -l -R danglink ... | S | ERR_DEREF
NDH = New directory with hard links below
S = hard link to symlink
T = hard link to symlink target
ERR_DEREF = failure because dereferencing fails
Now, the dereferencing behavior of cp --link is not related to -R anymore.
I hope we don't introduce a regression ...
Comments?
Have a nice day,
Berny
[cp--link--deref-v3.patch (text/x-patch, attachment)]
[testit.log.xz (application/x-xz, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 04 Nov 2013 23:34:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 11/04/2013 11:02 PM, Bernhard Voelker wrote:
> On 11/04/2013 10:37 AM, Pádraig Brady wrote:
>> On 11/04/2013 12:48 AM, Bernhard Voelker wrote:
>>> It got late again. I'll have a look tomorrow again - maybe there
>>> are no side effects with the following:
>>>
>>> diff --git a/src/cp.c b/src/cp.c
>>> index 7bc8630..78c0a04 100644
>>> --- a/src/cp.c
>>> +++ b/src/cp.c
>>> @@ -1135,7 +1135,7 @@ main (int argc, char **argv)
>>>
>>> if (x.dereference == DEREF_UNDEFINED)
>>> {
>>> - if (x.recursive)
>>> + if (x.recursive && ! x.hard_link)
>>> /* This is compatible with FreeBSD. */
>>> x.dereference = DEREF_NEVER;
>>> else
>>
>> Yes I didn't consider -R in the table as I don't see any reason
>> for it to behave differently when -l is specified (and -l aplies
>> neither to POSIX or BSD (comment)). So the above adjustment
>> looks correct to me.
>
> Indeed, that adjustment seems to be the right one - finally. ;-)
> I have added it to the patch as well as a new test case. I've added
> a "Co-authored-by:"-me line for that.
>
> @Gian: please note that we use "double-blank after-dot" format in
> comments. I've fixed that for you.
>
> The patch passes make check and syntax-check.
>
> I've also attached the test result cp-8.21 versus the new one.
> These are the differences (I leave out the --preserve=links cases
> as these are also affected but unrelated):
>
> COMMAND | 8.21 | NEW
> cp -l filelink ... | S | T
> cp -l -R filelink ... | S | T
> cp -L -l filelink ... | S | T
> cp -L -l -R filelink ... | S | T
> cp -H -l filelink ... | S | T
> cp -H -l -R filelink ... | S | T
great
> cp -l -R dirlink ... | S | NDH
> cp -l -R danglink ... | S | ERR_DEREF
These are Ok too I think given that
cp -al will not deref and thus not give the errors.
> NDH = New directory with hard links below
> S = hard link to symlink
> T = hard link to symlink target
> ERR_DEREF = failure because dereferencing fails
>
> Now, the dereferencing behavior of cp --link is not related to -R anymore.
> I hope we don't introduce a regression ...
>
> Comments?
Small suggested adjustment to NEWS:
+ cp --link now dereferences a symbolic link as source before creating the
+ hard link in the destination unless the -P,--no-deref option is specified.
+ Previously, it would create a hard link of the symbolic link, even when
+ the dereferencing options -L or -H were specified.
In the test I would do this to minimise writes to disk:
- cp --help > file || framework_failure_
+ touch file || framework_failure_
Very nice test BTW.
thanks!
Pádraig.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Tue, 05 Nov 2013 00:02:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 15173 <at> debbugs.gnu.org (full text, mbox):
>On 11/04/2013 12:48 AM, Bernhard Voelker wrote:
[...]
>> BUT I'm not happy at all with the following case:
>>
>> $ : > file
>> $ ln -s file filelink
>> $ src/cp --link filelink dst--link
>> $ src/cp --link -R filelink dst--link-R
>> $ ls -ldogi file filelink dst--link dst--link-R
>> 537364 -rw-r--r-- 2 0 Nov 4 01:30 dst--link
>> 537365 lrwxrwxrwx 2 4 Nov 4 01:30 dst--link-R -> file
>> 537364 -rw-r--r-- 2 0 Nov 4 01:30 file
>> 537365 lrwxrwxrwx 2 4 Nov 4 01:30 filelink -> file
>>
>> That's exactly what Gian was worried about in a different case
>> of my solution: it *matters* whether the rather unrelated -R option
>> is specified or not. ;-(
Exactly. But this is a problem with the implementation of '-R', and as
such I think it should be fixed there. Please see the just-opened bug
#15806 [0]. Imho, modifying it here would mean special-casing a
special-case. Add we wouldn't gain a lot in terms of consistency ( `cp`
dereferences, `cp -R` doesn't, `cp -lR` does ).
[0] http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15806
* [Mon, Nov 04, 2013 at 09:37:07AM +0000] Pádraig Brady:
>> diff --git a/src/cp.c b/src/cp.c
>> index 7bc8630..78c0a04 100644
>> --- a/src/cp.c
>> +++ b/src/cp.c
>> @@ -1135,7 +1135,7 @@ main (int argc, char **argv)
>>
>> if (x.dereference == DEREF_UNDEFINED)
>> {
>> - if (x.recursive)
>> + if (x.recursive && ! x.hard_link)
>> /* This is compatible with FreeBSD. */
>> x.dereference = DEREF_NEVER;
>> else
>
>Yes I didn't consider -R in the table as I don't see any reason
>for it to behave differently when -l is specified (and -l aplies
>neither to POSIX or BSD (comment)). So the above adjustment
>looks correct to me.
Not sure I've understood what you mean here. If '-R' should act the same
when '-l' is specified, the above change should _not_ be applied. But I
probably misunderstood.
Ciao,
Gian Piero.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 07 Nov 2013 07:08:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[I was hoping that someone with more coreutils history
was jumping in here - adding Jim for that matter.]
On 11/05/2013 01:01 AM, Gian Piero Carrubba wrote:
>> On 11/04/2013 12:48 AM, Bernhard Voelker wrote:
> [...]
>>> BUT I'm not happy at all with the following case:
>>>
>>> $ : > file
>>> $ ln -s file filelink
>>> $ src/cp --link filelink dst--link
>>> $ src/cp --link -R filelink dst--link-R
>>> $ ls -ldogi file filelink dst--link dst--link-R
>>> 537364 -rw-r--r-- 2 0 Nov 4 01:30 dst--link
>>> 537365 lrwxrwxrwx 2 4 Nov 4 01:30 dst--link-R -> file
>>> 537364 -rw-r--r-- 2 0 Nov 4 01:30 file
>>> 537365 lrwxrwxrwx 2 4 Nov 4 01:30 filelink -> file
>>>
>>> That's exactly what Gian was worried about in a different case
>>> of my solution: it *matters* whether the rather unrelated -R option
>>> is specified or not. ;-(
>
> Exactly. But this is a problem with the implementation of '-R', and as
> such I think it should be fixed there. Please see the just-opened bug
> #15806 [0]. Imho, modifying it here would mean special-casing a
> special-case. Add we wouldn't gain a lot in terms of consistency ( `cp`
> dereferences, `cp -R` doesn't, `cp -lR` does ).
>
> [0] http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15806
Okay, so we're leaving the ground of the GNU extension --link here.
cp's dereferencing vs. non-dereferencing behavior with -r seems to be a
longer story, e.g. there have been several commits between Dec 2001 and
Mar 2002. I don't know why and I can't tell a reason, but I have the
impression that the current behavior is the best choice for almost all
situations. On the other side, many scripts I've seen (including mine)
are using the -a option (which is "-dR --preserve=all" with in turn
-d as "--no-dereference --preserve=links") for copying whole directory
tree ... that may be an indication that the default for -r alone is not
that much used.
Anyway, I belief that doing a change aside from this --link patch
would be quite delicate.
Have a nice day,
Berny
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 07 Nov 2013 07:53:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 15173 <at> debbugs.gnu.org (full text, mbox):
* [Thu, Nov 07, 2013 at 08:07:11AM +0100] Bernhard Voelker:
>> [0] http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15806
>
>Okay, so we're leaving the ground of the GNU extension --link here.
Sure. It's a completely separate matter, just with some interactions
(will follow-up on that report as soon as I've enough time: did more
researches but as you noticed the commit history is pretty long and not
always so linear).
>Anyway, I belief that doing a change aside from this --link patch
>would be quite delicate.
I agree. My point here is that I would avoid modifying the behaviour of
'-r' (even if it's considered "wrong") while working on '--link', as imo
> - if (x.recursive)
> + if (x.recursive && ! x.hard_link)
would do. Well... and also that dereference settings in cp should be
reorder a bit (but again it doesn't belong here).
Ciao,
Gian Piero.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 07 Nov 2013 10:15:01 GMT)
Full text and
rfc822 format available.
Message #68 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 11/07/2013 07:07 AM, Bernhard Voelker wrote:
> Okay, so we're leaving the ground of the GNU extension --link here.
Yes.
I think the --link case should be considered separately (with and without -R).
I.E. IMHO your current patch is fine and good.
> cp's dereferencing vs. non-dereferencing behavior with -r seems to be a
> longer story, e.g. there have been several commits between Dec 2001 and
> Mar 2002. I don't know why and I can't tell a reason, but I have the
> impression that the current behavior is the best choice for almost all
> situations. On the other side, many scripts I've seen (including mine)
> are using the -a option (which is "-dR --preserve=all" with in turn
> -d as "--no-dereference --preserve=links") for copying whole directory
> tree ... that may be an indication that the default for -r alone is not
> that much used.
> Anyway, I belief that doing a change aside from this --link patch
> would be quite delicate.
The general -R dereferencing discussion can continue in 15806.
thanks,
Pádraig.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 07 Nov 2013 14:10:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 15173 <at> debbugs.gnu.org (full text, mbox):
* [Thu, Nov 07, 2013 at 10:14:05AM +0000] Pádraig Brady:
>On 11/07/2013 07:07 AM, Bernhard Voelker wrote:
>> Okay, so we're leaving the ground of the GNU extension --link here.
>
>Yes.
>I think the --link case should be considered separately (with and without -R).
>I.E. IMHO your current patch is fine and good.
Fair enough. In this case please also update the documentation.
Maybe something like the following could be enough (please re-word it in order
to let it intelligible) ?
--- old-issue-15173/doc/coreutils.texi 2013-11-07 14:07:35.139997551 +0100
+++ new-issue-15173/doc/coreutils.texi 2013-11-07 14:07:35.143997573 +0100
@@ -8052,8 +8052,8 @@
to corresponding destination directories.
When copying from a symbolic link, @command{cp} normally follows the
-link only when not copying
-recursively. This default can be overridden with the
+link only when not copying recursively or when @option{--link}
+(@option{-l}) is used. This default can be overridden with the
@option{--archive} (@option{-a}), @option{-d}, @option{--dereference}
(@option{-L}), @option{--no-dereference} (@option{-P}), and
@option{-H} options. If more than one of these options is specified,
@@ -8327,7 +8327,8 @@
@cindex recursively copying directories
@cindex non-directories, copying as special files
Copy directories recursively. By default, do not follow symbolic
-links in the source; see the @option{--archive} (@option{-a}), @option{-d},
+links in the source unless used together with the @option{--link}
+(@option{-l}) option; see the @option{--archive} (@option{-a}), @option{-d},
@option{--dereference} (@option{-L}), @option{--no-dereference}
(@option{-P}), and @option{-H} options. Special files are copied by
creating a destination file of the same type as the source; see the
Ciao,
Gian Piero.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 07 Nov 2013 15:26:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On Wed, Nov 6, 2013 at 11:07 PM, Bernhard Voelker
<mail <at> bernhard-voelker.de> wrote:
> [I was hoping that someone with more coreutils history
> was jumping in here - adding Jim for that matter.]
Hi Berny,
I don't have enough context (12 years is a long time) yet, but would guess that
the current behavior was driven by an attempt to be consistent with one of the
*BSD implementations. Does the current behavior conflict with POSIX or
disagree with how other implementations work? I haven't been
following this thread closely.
Jim
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 07 Nov 2013 17:09:01 GMT)
Full text and
rfc822 format available.
Message #77 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 11/07/2013 04:25 PM, Jim Meyering wrote:
> Does the current behavior conflict with POSIX or
> disagree with how other implementations work?
Hi Jim,
no, it doesn't conflict with POSIX, and for the --link case
(this thread) I think we have found consensus.
The problem is - unfortunately in the other bug#15806 [1]
that rm behaves differently regarding the dereferencing (or not)
of symbolic links when the -r option is specified (or not).
The BSDs seem to also dereference without -r.
[1] http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15806
(I hope I got it summarized right - I'm in a hurry.)
Have a nice day,
Berny
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 07 Nov 2013 17:24:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On Thu, Nov 7, 2013 at 9:08 AM, Bernhard Voelker
<mail <at> bernhard-voelker.de> wrote:
> On 11/07/2013 04:25 PM, Jim Meyering wrote:
>> Does the current behavior conflict with POSIX or
>> disagree with how other implementations work?
>
> Hi Jim,
>
> no, it doesn't conflict with POSIX, and for the --link case
> (this thread) I think we have found consensus.
>
> The problem is - unfortunately in the other bug#15806 [1]
> that rm behaves differently regarding the dereferencing (or not)
> of symbolic links when the -r option is specified (or not).
> The BSDs seem to also dereference without -r.
>
> [1] http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15806
Right. Thanks for the summary.
I was referring to the -R-vs-deref issue but hadn't read the original
report carefully. I will follow up on that thread.
Reply sent
to
Bernhard Voelker <mail <at> bernhard-voelker.de>:
You have taken responsibility.
(Thu, 07 Nov 2013 23:00:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Gian Piero Carrubba <gpiero <at> rm-rf.it>:
bug acknowledged by developer.
(Thu, 07 Nov 2013 23:00:04 GMT)
Full text and
rfc822 format available.
Message #85 received at 15173-done <at> debbugs.gnu.org (full text, mbox):
On 11/07/2013 03:09 PM, Gian Piero Carrubba wrote:
> * [Thu, Nov 07, 2013 at 10:14:05AM +0000] Pádraig Brady:
>> I.E. IMHO your current patch is fine and good.
>
> Fair enough. In this case please also update the documentation.
Good idea, thanks.
I pushed it in your name (together with Padraig's notes regarding
NEWS and the test case):
http://git.sv.gnu.org/cgit/coreutils.git/commit/?id=bf6bf52d
I'm hereby marking the bug as done.
Thanks again.
Have a nice day,
Berny
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Fri, 06 Dec 2013 12:24:04 GMT)
Full text and
rfc822 format available.
bug unarchived.
Request was from
Pádraig Brady <P <at> draigBrady.com>
to
control <at> debbugs.gnu.org.
(Mon, 09 Dec 2013 01:26:01 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 09 Dec 2013 02:25:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Sorry if you get multiple copies of this.
The test for this is failing on solaris 10 (NFS)
It does seem that hardlinks to symlinks are supported:
$ touch tfile
$ ln -s tfile tlink
$ src/ln -L tlink tlink-ln-L
$ src/ln -P tlink tlink-ln-P
$ src/ln tlink tlink-ln
$ ls -li tfile tlink*
8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tfile
8551 lrwxrwxrwx 2 padraig csw 5 Dec 9 01:19 tlink -> tfile
8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tlink-ln
8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tlink-ln-L
8551 lrwxrwxrwx 2 padraig csw 5 Dec 9 01:19 tlink-ln-P -> tfile
But we have linkat() emulation in place I think:
$ grep LINK lib/config.h
/* #undef CHOWN_MODIFIES_SYMLINK */
#define GNULIB_AREADLINKAT 1
#define GNULIB_TEST_LINK 1
#define GNULIB_TEST_LINKAT 1
#define GNULIB_TEST_READLINK 1
#define GNULIB_TEST_READLINKAT 1
#define GNULIB_TEST_SYMLINK 1
#define GNULIB_TEST_SYMLINKAT 1
#define GNULIB_TEST_UNLINK 1
#define GNULIB_TEST_UNLINKAT 1
#define HAVE_LINK 1
/* #undef HAVE_LINKAT */
#define HAVE_READLINK 1
/* #undef HAVE_READLINKAT */
#define HAVE_SYMLINK 1
/* #undef HAVE_SYMLINKAT */
#define HAVE_UNLINKAT 1
/* #undef LINKAT_TRAILING_SLASH_BUG */
#define LINK_FOLLOWS_SYMLINKS -1
#define LSTAT_FOLLOWS_SLASHED_SYMLINK 1
#define PIPE_LINK_COUNT_MAX (0)
/* #undef READLINK_TRAILING_SLASH_BUG */
/* #undef RENAME_HARD_LINK_BUG */
/* #undef UNLINK_CANNOT_UNLINK_DIR */
/* #undef UNLINK_PARENT_BUG */
I've attached the verbose log for the test,
as I've not time to look into it at present.
thanks,
Pádraig.
[test-suite.log (text/x-log, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 12 Dec 2013 18:45:01 GMT)
Full text and
rfc822 format available.
Message #95 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 12/09/2013 02:24 AM, Pádraig Brady wrote:
> Sorry if you get multiple copies of this.
>
> The test for this is failing on solaris 10 (NFS)
> It does seem that hardlinks to symlinks are supported:
>
> $ touch tfile
> $ ln -s tfile tlink
> $ src/ln -L tlink tlink-ln-L
> $ src/ln -P tlink tlink-ln-P
> $ src/ln tlink tlink-ln
> $ ls -li tfile tlink*
> 8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tfile
> 8551 lrwxrwxrwx 2 padraig csw 5 Dec 9 01:19 tlink -> tfile
> 8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tlink-ln
> 8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tlink-ln-L
> 8551 lrwxrwxrwx 2 padraig csw 5 Dec 9 01:19 tlink-ln-P -> tfile
>
> But we have linkat() emulation in place I think:
>
> $ grep LINK lib/config.h
...
> /* #undef HAVE_LINKAT */
> #define LINK_FOLLOWS_SYMLINKS -1
> FAIL: tests/cp/link-deref
> =========================
> --- exp Mon Dec 9 01:00:08 2013
> +++ out Mon Dec 9 01:00:08 2013
> @@ -1,1 +1,1 @@
> -cp --link -P dirlink dst|result=0|inode=8436|type=symbolic link|error=
> +cp --link -P dirlink dst|result=0|inode=8467|type=symbolic link|error=
> + rm -f diff.out
> + false
> + ls -lid dirlink dir dst
> 8434 drwxr-xr-x 2 padraig csw 2 Dec 9 01:00 dir
> 8436 lrwxrwxrwx 1 padraig csw 3 Dec 9 01:00 dirlink -> dir
> 8467 lrwxrwxrwx 1 padraig csw 3 Dec 9 01:00 dst -> dir
> + fail=1
So the attached should address this on FreeBSD ast least
where we HAVE_LINKAT so don't need to fallback to
the symlink -> symlink emulation in copy.c
For the above case we'll need to skip parts of the test I think,
depending on the above 2 config vars.
thanks,
Pádraig
[cp-linkat.patch (text/x-patch, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Thu, 12 Dec 2013 23:12:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
So while the above patch is probably correct, it's slightly risky
at this stage before a release. Also the benefits are minimal as
the existing symlink to symlink emulation should be fine on the mentioned systems.
Therefore I'll go with the attached patch which fixes the test to run
on all platforms. The test failed on freebsd, aix and solaris previously,
and I've confirmed it now passes on all those.
cheers,
Pádraig.
[link-deref-compat.patch (text/x-patch, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Fri, 13 Dec 2013 00:52:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 12/13/2013 12:11 AM, Pádraig Brady wrote:
> So while the above patch is probably correct, it's slightly risky
> at this stage before a release. Also the benefits are minimal as
> the existing symlink to symlink emulation should be fine on the mentioned systems.
>
> Therefore I'll go with the attached patch which fixes the test to run
> on all platforms. The test failed on freebsd, aix and solaris previously,
> and I've confirmed it now passes on all those.
Unfortunately, I didn't have time to look into the details of this
issue. As I understood your comment, it is not a FP in the tests but
rather an issue in copy.c on these platforms.
I'm fine with the proposed patch for the time being, i.e. to NOP out
some test cases to avoid FPs- but it then would probably make sense
to document the varying behavior, at least in a FIXME in the test.
Thanks & have a nice day,
Berny
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Fri, 13 Dec 2013 01:07:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 12/13/2013 12:51 AM, Bernhard Voelker wrote:
> On 12/13/2013 12:11 AM, Pádraig Brady wrote:
>> So while the above patch is probably correct, it's slightly risky
>> at this stage before a release. Also the benefits are minimal as
>> the existing symlink to symlink emulation should be fine on the mentioned systems.
>>
>> Therefore I'll go with the attached patch which fixes the test to run
>> on all platforms. The test failed on freebsd, aix and solaris previously,
>> and I've confirmed it now passes on all those.
>
> Unfortunately, I didn't have time to look into the details of this
> issue. As I understood your comment, it is not a FP in the tests but
> rather an issue in copy.c on these platforms.
It's a minor issue in copy.c on some of these platforms (with HAVE_LINKAT).
Improving that would make the test pass on FreeBSD for example, but
we can leave that until the next release.
The main issue is the test expected hardlinks to symlinks to be
always done by cp --link, so the added guard is needed in any case.
> I'm fine with the proposed patch for the time being, i.e. to NOP out
> some test cases to avoid FPs- but it then would probably make sense
> to document the varying behavior, at least in a FIXME in the test.
thanks for the review,
Pádraig.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Fri, 10 Jan 2014 12:24:04 GMT)
Full text and
rfc822 format available.
bug unarchived.
Request was from
Pádraig Brady <P <at> draigBrady.com>
to
control <at> debbugs.gnu.org.
(Mon, 10 Feb 2014 02:14:03 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 10 Feb 2014 02:19:01 GMT)
Full text and
rfc822 format available.
Message #111 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 12/12/2013 06:44 PM, Pádraig Brady wrote:
> On 12/09/2013 02:24 AM, Pádraig Brady wrote:
>> Sorry if you get multiple copies of this.
>>
>> The test for this is failing on solaris 10 (NFS)
>> It does seem that hardlinks to symlinks are supported:
>>
>> $ touch tfile
>> $ ln -s tfile tlink
>> $ src/ln -L tlink tlink-ln-L
>> $ src/ln -P tlink tlink-ln-P
>> $ src/ln tlink tlink-ln
>> $ ls -li tfile tlink*
>> 8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tfile
>> 8551 lrwxrwxrwx 2 padraig csw 5 Dec 9 01:19 tlink -> tfile
>> 8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tlink-ln
>> 8550 -rw-r--r-- 3 padraig csw 0 Dec 9 01:18 tlink-ln-L
>> 8551 lrwxrwxrwx 2 padraig csw 5 Dec 9 01:19 tlink-ln-P -> tfile
>>
>> But we have linkat() emulation in place I think:
>>
>> $ grep LINK lib/config.h
> ...
>> /* #undef HAVE_LINKAT */
>> #define LINK_FOLLOWS_SYMLINKS -1
>
>> FAIL: tests/cp/link-deref
>> =========================
>
>> --- exp Mon Dec 9 01:00:08 2013
>> +++ out Mon Dec 9 01:00:08 2013
>> @@ -1,1 +1,1 @@
>> -cp --link -P dirlink dst|result=0|inode=8436|type=symbolic link|error=
>> +cp --link -P dirlink dst|result=0|inode=8467|type=symbolic link|error=
>> + rm -f diff.out
>> + false
>> + ls -lid dirlink dir dst
>> 8434 drwxr-xr-x 2 padraig csw 2 Dec 9 01:00 dir
>> 8436 lrwxrwxrwx 1 padraig csw 3 Dec 9 01:00 dirlink -> dir
>> 8467 lrwxrwxrwx 1 padraig csw 3 Dec 9 01:00 dst -> dir
>> + fail=1
>
> So the attached should address this on FreeBSD ast least
> where we HAVE_LINKAT so don't need to fallback to
> the symlink -> symlink emulation in copy.c
I still think this is correct, and while I didn't think it
worth the small risk right before the 8.22 release,
I hope to push this rebased patch soon.
thanks,
Pádraig.
[cp-linkat.patch (text/x-patch, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 10 Feb 2014 02:30:02 GMT)
Full text and
rfc822 format available.
Message #114 received at 15173 <at> debbugs.gnu.org (full text, mbox):
Pádraig Brady wrote:
> +#if ! defined HAVE_LINKAT
> && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
> - && x->dereference == DEREF_NEVER))
> + && x->dereference == DEREF_NEVER)
> +#endif
Could you reword that sort of thing so as not to use the #if inside an
expression? Something like this instead, perhaps, earlier in the code:
#if !defined HAVE_LINKAT && LINK_FOLLOWS_SYMLINKS
# define LINKAT_FOLLOWS_SYMLINKS 1
#else
# define LINKAT_FOLLOWS_SYMLINKS 0
#endif
That way, the rest of the code can simply replace LINK_FOLLOWS_SYMLINKS
with LINKAT_FOLLOWS_SYMLINKS, which will make it easier to read.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 10 Feb 2014 04:03:02 GMT)
Full text and
rfc822 format available.
Message #117 received at 15173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 02/10/2014 02:29 AM, Paul Eggert wrote:
> Pádraig Brady wrote:
>> +#if ! defined HAVE_LINKAT
>> && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
>> - && x->dereference == DEREF_NEVER))
>> + && x->dereference == DEREF_NEVER)
>> +#endif
>
> Could you reword that sort of thing so as not to use the #if inside an expression? Something like this instead, perhaps, earlier in the code:
>
> #if !defined HAVE_LINKAT && LINK_FOLLOWS_SYMLINKS
> # define LINKAT_FOLLOWS_SYMLINKS 1
> #else
> # define LINKAT_FOLLOWS_SYMLINKS 0
> #endif
That's slightly confusing as linkat() emulation never
actually symlinks the referent. I went for the more direct:
#if defined HAVE_LINKAT || ! LINK_FOLLOWS_SYMLINKS
# define CAN_HARDLINK_SYMLINKS 1
#else
# define CAN_HARDLINK_SYMLINKS 0
#endif
Full patch is attached.
thanks,
Pádraig.
[cp-linkat.patch (text/x-patch, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 10 Feb 2014 04:15:02 GMT)
Full text and
rfc822 format available.
Message #120 received at 15173 <at> debbugs.gnu.org (full text, mbox):
Thanks, looks good.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#15173; Package
coreutils.
(Mon, 10 Feb 2014 08:59:02 GMT)
Full text and
rfc822 format available.
Message #123 received at 15173 <at> debbugs.gnu.org (full text, mbox):
On 02/10/2014 05:02 AM, Pádraig Brady wrote:
> Full patch is attached.
+1
Have a nice day,
Berny
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Mon, 10 Mar 2014 11:24:14 GMT)
Full text and
rfc822 format available.
bug unarchived.
Request was from
Assaf Gordon <assafgordon <at> gmail.com>
to
control <at> debbugs.gnu.org.
(Sat, 27 Oct 2018 22:09:02 GMT)
Full text and
rfc822 format available.
Merged 15173 23120.
Request was from
Assaf Gordon <assafgordon <at> gmail.com>
to
control <at> debbugs.gnu.org.
(Sat, 27 Oct 2018 22:11:02 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sun, 25 Nov 2018 12:24:05 GMT)
Full text and
rfc822 format available.
This bug report was last modified 5 years and 147 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.