bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS

[George Valkov]

> On 2023-02-12, at 10:36 PM, Pádraig Brady <P@draigBrady.com> wrote:
> 
> On 12/02/2023 14:15, Pádraig Brady wrote:
>> On 11/02/2023 21:53, Paul Eggert wrote:
>>> On 2023-02-10 17:21, George Valkov wrote:
>>> 
>>>> remember to trace all code paths errors.
>>> 
>>> Yes, I've been doing that. However, I don't use macOS and the two of us
>>> are debugging remotely where I write the code and you debug it but
>>> neither of us know how macOS works. Of course it would be much more
>>> efficient if we weren't operating with these obstacles, but here we are.
>>> 
>>> Because the macOS behavior is poorly documented and we lack the source,
>>> if we can't figure this out fairly quickly coreutils should simply stop
>>> using fclonefileat because it's unreliable for users and a time sink for
>>> us. I'm hoping, though, we can get something working with another round
>>> or two.
>>> 
>>> 
>>>> With CLONE_ACL, I get 22 Invalid argument.
>>> 
>>> OK, this means that although Apple has documented CLONE_ACL and it's in
>>> your include files, it doesn't work in your version of macOS, because
>>> either it's not supported by the kernel, or by the filesystem code, or
>>> by the particular filesystem instance, or for some other reason. EINVAL
>>> hints that it's the kernel but that's by no means certain.
>>> 
>>> One possible way to defend against this macOS misbehavior is for cp to
>>> try to use CLONE_ACL, and if that fails with errno == EINVAL try again
>>> without CLONE_ACL. I attempted to implement this in the attached patch;
>>> please give it a try.
>> I tested this on macOS 13.2 and the first fclonefileat(..., CLONE_ACL) 
>> succeeds.
>> I tested with various umask, --no-preserve=mode, -p, ... combinations
>> and didn't find any inconsistencies in permissions with --reflink=never 
>> copies.
>> Actually scratch that. There was an older xcode installed so CLONE_ACL was 0.
>> I've not got access at the moment, so will need to retest later with newer 
>> xcode.
> 
> Good news is that with newer xcode supporting macOS 13.1
> CLONE_ACL is set and that all works as expected, with the
> first fclonefileat() succeeding.

I can confirm that. CLONE_ACL is 4 when building on macOS 12.6.3. I rand the 
compiled sample on the recovery environment, which is equivalent to macOS 13.3. 
The flag is supported, though I did not observe any difference with or without 
it. The sample was ran as root, while the source and destination were set to 
g:staff 600 @.


> I checked with actual ACLs (set with `chmod +a "user:$USER allow readattr" 
> file`),
> and they were copied or dropped as expected. Note ls -l on macOS will list
> ACLs with a "+" rather than a "@", as was mentioned elsewhere in this thread.

@ is shown on files that have extended attributes. I can remove them with
xattr -cr . # recursive
xattr -cr file # single item

+ means ACL

ll -e -@
drwx------@   3 g     staff     96 May 16  2021 Applications/
        com.apple.quarantine       21
drwx------+   8 g     staff    256 May 16  2021 Music/
 0: group:everyone deny delete


>> One divergence with fclonefileat() is that it writes through a dangling 
>> symlink,
>> where a standard --reflink=never copy will fail.
>> I.e. `make TESTS=tests/cp/thru-dangling.sh SUBDIRS=. check` fails.
>> Now there is nothing wrong with writing through the dangling symlink,
>> and in fact standard cp will also with POSIXLY_CORRECT set in the 
>> environment.
>> So I may just need need to adjust the test to work / skip in this case.

The same test fails on macOS 12.6.3. See test-01*.txt.
https://httpstorm.com/share/.openwrt/test/2023-02-06_coreutils-9.1/

I ran your test, followed by make check-TESTS for both original (8 failed) and 
patched (12 filed).


> Another divergence I noticed is wrt umask and setuid handling.
> The following transcript shows that with fclonefileat() we do _not_
> preserve setuid when it is preserved without.
> Also with fclonefileat() the umask is not honored,
> while it is honored without.
> 
> # id -u
> 0
> 
> # echo create setuid file
> # rm src/cp.s; src/cp src/cp src/cp.s; chmod u+s src/cp.s
> 
> # umask; for reflink in always never; do
> >  for preserve in mode timestamps; do
> >   rm -f src/cp.s.$reflink.$preserve
> >   src/cp --reflink=$reflink --preserve=$preserve src/cp.s 
> > src/cp.s.$reflink.$preserve
> >  done
> > done
> > ls -le src/cp.s*
> 0022
> -rwsrwxr-x  1 root  staff  234032 12 Feb 20:21 src/cp.s
> -rwxrwxr-x  1 root  staff  234032 12 Feb 20:21 src/cp.s.always.mode
> -rwxrwxr-x  1 root  staff  234032 12 Feb 20:21 src/cp.s.always.timestamps
> -rwsrwxr-x  1 root  staff  234032 12 Feb 20:21 src/cp.s.never.mode
> -rwxr-xr-x  1 root  staff  234032 12 Feb 20:21 src/cp.s.never.timestamps

The manpage is clonefile.2.txt, use the link above.


Georgi Valkov
httpstorm.com
nano RTOS