bug#9446: Bug in cp (or strange behaviour and unclear documentation)

[Pádraig Brady]

On 09/06/2011 08:07 AM, Jim Meyering wrote:
> Torbjorn Granlund wrote:
>> On FreeBSD (tested on 8.1, but exact version is unlikely relevant)
>> copying a file from a ZFS file system to a tmpfs or tmpmfs filesystem,
>> the -p option causes cp to return with a non-zero exit status.
>>
>> Sample run:
>>
>> $ cp -p /etc/profile /tmp/foo || echo non-zero exit
>> cp: preserving permissions for `/tmp/foo': Invalid argument
>> non-zero exit
>>
>> Some debugging shows that the function acl_access_nontrivial returns 1,
>> caused by the tag ACL_EVERYONE.  (It is present in every file by
>> default.)
>>
>> ACLs are not supported on FreeBSD's tmpfs or tmpmfs file system types.
>>
>> Perhaps this failure is intentional?  Then it should be clearly
>> documented for the -p/--preserve option.  I have read it carefully, and
>> I cannot see support for the observed behaviour.
>>
>> Consider what happens under -p when copying a file owned by somebody
>> else.  The used and group owners can then not (typically) be preserved.
>> This is not considered an error in cp.
>>
>> Similarly, when copying a file with the setuid bit set, and the user is
>> changed (per above) the setuid bit is not preserved, and this is also
>> not considered an error by cp.
>>
>> The failure to preserve (partial) ACL data should IMHO not be considered
>> an error, except perhaps under some --preserve=blah options, where blah
>> denoted preservation of exactly this.
> 
> Hi!
> Long time ;-)
> 
> ACL-preservation has always been lumped in with the
> permission-preservation that you get with cp's "-p".
> However, that is not POSIX-mandated.
> 
> This is the responsible code in copy.c:
> 
>   if (x->preserve_mode || x->move_mode)
>     {
>       if (copy_acl (src_name, -1, dst_name, -1, src_mode) != 0
>           && x->require_preserve)
>         return false;
>     }
> 
> However, upon copy_acl failure, rather than simply returning false, it
> might be ok to return true for EINVAL, or even to perform a relatively
> expensive test (the first time for each FS) to see if the failure is
> due to lack of ACL support in the destination file system.
> 
> I'm less inclined to add a separate --preserve=acl option, because
> that would involve a behavior change (of not copying ACLs by default).

I agree. It's like ACLs span both --preserve=mode and --preserve=xattrs (if 
possible).
The trivial ACLs are already made optional within copy_acl()
assuming I suppose that the dest will record this info anyway.

So what to do if other ACLs can't be preserved.
Ideally I think we'd like to:
  1. Warn once per cp invocation about unpreserved non trivial ACLs
  2. With --verbose, warn per file, which ACLs were not preserved.
  3. if !ACL_NOT_WELL_SUPPORTED(errno) exit 1

As an interim measure we could just do 3, which would give
many non specific "preserving permissions" errors,
but at least exit appropriately.

cheers,
Pádraig.