[libc++] Use COPYFILE_CLONE from the macOS copyfile(3) API to support APFS clones

[ldionne]

Summary

On macOS, copyfile supports cloning files instead of copying them, which
is more efficient. This is a best effort flag -- if cloning can't be
performed, it falls back to a normal copy of the file. The idea for this
patch comes from https://reviews.llvm.org/D60802.

[ldionne]

I'm not sure this change is valid because now we're also copying things like attributes, ACL and even the stat of the file. But I wanted to have that discussion in public here for documentation.

[dexonsmith]

+aprantl

[aprantl]

According to the documentation COPYFILE_CLONE will clone a symlink rather than its contents, whereas COPY_FILE_DATA will follow the link. That's why I'm checking for symlinks in https://reviews.llvm.org/D60802. I don't know whether copy_file_impl_copyfile makes any guarantees.

[aprantl]

I assume the availability check isn't necessary because libcxx won't back-deploy to 10.11?

libcxx/src/filesystem/operations.cpp

681

It looks like you may be able to pass nullptr as the third parameter if you don't need the state afterwards.

[ldionne]

In D60880#1472059, @aprantl wrote:

According to the documentation COPYFILE_CLONE will clone a symlink rather than its contents, whereas COPY_FILE_DATA will follow the link. That's why I'm checking for symlinks in https://reviews.llvm.org/D60802. I don't know whether copy_file_impl_copyfile makes any guarantees.

I looked at the code that calls copyfile_impl, and it gives an error if the source file is not a regular file here. I assumed it meant it wasn't a symlink, but maybe I made a mistake.

I assume the availability check isn't necessary because libcxx won't back-deploy to 10.11?

This _is_ the dylib, so we're not back-deploying it to 10.11. If it were in the headers, then we'd need to check that the underlying system supports this functionality, yes.

libcxx/src/filesystem/operations.cpp

681

Hmm, I think that's correct:

If the state parameter is the return value from copyfile_state_alloc(), then copyfile() and fcopyfile() will use the information from the state object; if it is NULL, then both functions will work normally, but less control will be available to the caller.

So we could get rid of the CopyFileState helper and instead just pass nullptr -- is that your suggestion?

[aprantl]

I found one more quirks while debugging why https://reviews.llvm.org/D60802 caused a clang modules crash reproducer test to fail:

COPYFILE_DATA will succeed if the destination file already exists, whereas COPYFILE_CLONE returns EEXIST.

[aprantl]

libcxx/src/filesystem/operations.cpp

681

Yes.

[EricWF]

I'm not sure this change is valid because now we're also copying things like attributes, ACL and even the stat of the file.

That seems to be the correct behavior. According to [fs.op.copy.file]

copy the contents and attributes of the file from resolves to, to the file to resolves to [...]

In D60880#1472274, @aprantl wrote:

I found one more quirks while debugging why https://reviews.llvm.org/D60802 caused a clang modules crash reproducer test to fail:

COPYFILE_DATA will succeed if the destination file already exists, whereas COPYFILE_CLONE returns EEXIST.

That shouldn't be the case for fcopyfile. Since the dest file is specified by a file descriptor.

[EricWF]

This LGTM.

I think we should have already read through symlinks and other possible gotcha's.

[ldionne]

Ah! It turns out that fcopyfile does not support the COPYFILE_CLONE option at all:

COPYFILE_CLONE Try to clone the file instead.
This is a best try flag i.e. if cloning fails, fallback to copying the file. [...] Recursive copying however is supported, see below for more information. (This is only applicable for the copyfile() function.)

See the bit in bold. Apparently, this applies to the option as a whole, not only the last sentence.

[aprantl]

Would it be feasible to use regular copyfile here? I don't know the surrounding code well enough...

libcxx/src/filesystem/operations.cpp

struct CopyFileState {
     ~CopyFileState() { copyfile_state_free(state); }
 
   private:
     CopyFileState(CopyFileState const&) = delete;
     CopyFileState& operator=(CopyFileState const&) = delete;
   };
 
   CopyFileState cfs;
-  if (fcopyfile(read_fd.fd, write_fd.fd, cfs.state, COPYFILE_DATA) < 0) {
+  if (fcopyfile(read_fd.fd, write_fd.fd, cfs.state, COPYFILE_CLONE) < 0) {
     ec = capture_errno();
     return false;
   }
 
   ec.clear();
   return true;
 }
 #endif