bug#15806: [cp] -R modifies dereferencing settings

[Gian Piero Carrubba]

Severity: wishlist

Please Cc: me as I'm not subscribed.

POSIX.1-2008 (ed. 2013) [0] states:

    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, unless the -P option was
    specified.

So far, so good. As far as I know, coreutils' cp fully respect this.

    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.

Somewhere in the past, coreutils team has opted for using '-P' 
(DEREF_NEVER) in this case.  Anyway this leads to an inconsistence in 
the behaviour of cp:

    $ touch file ; ln -s file symlink ; for o in "" -r ; do cp $o
    symlink cp$o ; done ; ls -liog
    total 0
    8224776 -rw-r--r-- 1 0 Nov  4 23:15 cp
    8224777 lrwxrwxrwx 1 4 Nov  4 23:15 cp-r -> file
    8224774 -rw-r--r-- 1 0 Nov  4 23:15 file
    8224775 lrwxrwxrwx 1 4 Nov  4 23:15 symlink -> file

What wrong with it ? The problem is that we got a modified dereference 
setting just by using '--recursive', an unrelated option. Moreover, 
recursion and dereferencing are orthogonal features and there's no 
reason one should imply the other.

As for my opinion, there are three possibilities for, at least, reduce 
this perceived inconsistency. 

1. leave `cp -R` acting as it currently does...

...but properly document it. The texinfo doc already cites this 
behaviour, but both the man page and the help (imo, the two most used  
sources while searching for an option) don't. I would suggest adding 
"Implies '--no-dereference'" to the description of the -r/-R option.

2. let `cp -R` act as if `-L` is used.

This is my preferred option, but it's also the most invasive. All the 
scripts using `cp -r` for copying a directory tree without dereferencing 
the symbolic links would fail. The texinfo doc warns against that use 
[1]:

    -- doc/coreutils.texi:8334
    [...] It is not portable to use
    @option{-r} to copy symbolic links or special files.  On some
    non-GNU systems, @option{-r} implies the equivalent of
    @option{-L} and @option{--copy-contents} for historical reasons.
    Also, it is not portable to use @option{-R} to copy symbolic links
    unless you also specify @option{-P}, as POSIX allows
    implementations that dereference symbolic links by default.

so maybe this option could be considered. But honestly, despite being my 
preferred option (i.e.: the one I would implement if I had to write a 
'cp' utility from scratch), I'm not sure I can endorse it as it would be 
highly backward incompatible.

3. let `cp -R` act as if `-H` is used.

This is a sort of compromise between the other two options. It gains 
some consistency ( `cp` and `cp -R` would act the same with regard to 
dereferencing the symlinks passed on the command line ) and would 
preserve a certain level of backward compatibility. I expect the 
percentage of real-case scripts that would be broken by this change to 
be really minimal.
I don't particularly like this option, but if option 2. is judged 
impractical (as it probably is), I personally largely prefer this 
solution over option 1.

The attached preliminary patch implements option 3.

After applying the patch the following tests fail:

= tests/cp/r-vs-symlink.sh
This is expected, as it is the point of the patch. Anyway the comments 
in the test file are suspicious:

    # cp -r should not create symlinks.  Fixed in fileutils-4.1.5.
    # Restored old behavior (whereby cp -r preserves symlinks) in 4.1.6,
    # though now such usage evokes a warning:
    # cp: 'slink': WARNING: using -r to copy symbolic links is not
    # portable

So maybe a similar change (probably option 2. above) was implemented, 
and reverted, in the past ? I wasn't able to find a reference to it in 
the Changelog, so cannot tell why it has been reverted. Anyway the 
warning cited in the comment doesn't seem to exist anymore.

= tests/cp/special-f.sh [2]
This is unexpected. At a first look, the failure should be caused by the 
following test not being fully inclusive:

    -- src/copy.c:1930
    else if (! S_ISDIR (dst_sb.st_mode)
        /* Never unlink dst_name when in move mode.  */
        && ! x->move_mode
        && (x->unlink_dest_before_opening
            || (x->preserve_links && 1 < dst_sb.st_nlink)
            || (x->dereference == DEREF_NEVER
                && !  S_ISREG (src_sb.st_mode))
            ))

I hadn't the time for investigate further, will do in the next days. At 
the moment, however, I suspect the previous dereference settings was 
simply masking and underlying issue.

Ciao,
Gian Piero.

[0] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html

[1] Anyway I'm not sure why it differentiates between '-r' and '-R', 
given that in coreutils' cp they are aliases:

    -- src/copy.c:1045
    case 'r':
    case 'R':
      x.recursive = true;
      break;

[2] A side note about this test. The comment seems to contradict the 
code:

    -- tests/cp/special-f.sh:28
    # Without -f, expect it to fail
    cp -R fifo e || fail=1

As for my impression, it also contradicts POSIX specifications (w/o -f 
it should fail). Anyway this belongs to another bug report.

draft.patch
Description: Text Data