Support: Reduce stats in fs::copy_file on Darwin

[dexonsmith]

Summary

fs::copy_file() on Darwin has a nice optimization to clone the file when
possible. Change the implementation to use clonefile() directly, instead
of the higher-level copyfile(). The latter does the wrong thing for
symlinks, which requires calling stat first...

With that out of the way, optimistically call clonefile() all the time,
and then for any error that's recoverable try again with copyfile()
(without the COPYFILE_CLONE flag, as before).

[dexonsmith]

Unit test is failing on Windows:

[ RUN      ] FileSystemTest.CopyFileW
C:\ws\w4\llvm-project\premerge-checks\llvm\unittests\Support\Path.cpp(2335): error: fs::create_link(path::filename(Sources[2]), Symlink): did not return errc::success.
error number: 2
error message: no such file or directory
 
[  FAILED  ] FileSystemTest.CopyFileW (874 ms)

Given we don't support creating symlinks on Windows and the behaviour has only changed on Darwin, I'm not sure this is important to debug. Thoughts on making this test #ifndef _WIN32?

[JDevlieghere]

LGTM

In D112250#3079555, @dexonsmith wrote:

Unit test is failing on Windows:

[ RUN      ] FileSystemTest.CopyFileW
C:\ws\w4\llvm-project\premerge-checks\llvm\unittests\Support\Path.cpp(2335): error: fs::create_link(path::filename(Sources[2]), Symlink): did not return errc::success.
error number: 2
error message: no such file or directory
 
[  FAILED  ] FileSystemTest.CopyFileW (874 ms)

Given we don't support creating symlinks on Windows and the behaviour has only changed on Darwin, I'm not sure this is important to debug. Thoughts on making this test #ifndef _WIN32?

Sounds reasonable to me.

[aprantl]

Nice!

[dexonsmith]

Excluding the fs::create_link() logic on Windows to avoid the failing test.

llvm/lib/Support/Unix/Path.inc

 #include <dirent.h>
 #include <pwd.h>
 #include <sys/file.h>
 
 #ifdef __APPLE__
 #include <mach-o/dyld.h>
 #include <sys/attr.h>
 #include <copyfile.h>
+#if __has_include(<sys/clonefile.h>)
+#include <sys/clonefile.h>
+#endif
 #elif defined(__FreeBSD__)
 #include <osreldate.h>
 #if __FreeBSD_version >= 1300057
 #include <sys/auxv.h>
 #else
 #include <machine/elf.h>
 extern char **environ;
 #endif

 
 #ifdef __APPLE__
 /// This implementation tries to perform an APFS CoW clone of the file,
 /// which can be much faster and uses less space.
 /// Unfortunately fcopyfile(3) does not support COPYFILE_CLONE, so the
 /// file descriptor variant of this function still uses the default
 /// implementation.
 std::error_code copy_file(const Twine &From, const Twine &To) {
-  uint32_t Flag = COPYFILE_DATA;
-#if __has_builtin(__builtin_available) && defined(COPYFILE_CLONE)
+  std::string FromS = From.str();
+  std::string ToS = To.str();
+#if __has_builtin(__builtin_available)
   if (__builtin_available(macos 10.12, *)) {
-    bool IsSymlink;
-    if (std::error_code Error = is_symlink_file(From, IsSymlink))
-      return Error;
-    // COPYFILE_CLONE clones the symlink instead of following it
-    // and returns EEXISTS if the target file already exists.
-    if (!IsSymlink && !exists(To))
-      Flag = COPYFILE_CLONE;
+    // Optimistically try to use clonefile() and handle errors, rather than
+    // calling stat() to see if it'll work.
+    //
+    // Note: It's okay if From is a symlink. In contrast to the behaviour of
+    // copyfile() with COPYFILE_CLONE, clonefile() clones targets (not the
+    // symlink itself) unless the flag CLONE_NOFOLLOW is passed.
+    if (!clonefile(FromS.c_str(), ToS.c_str(), 0))
+      return std::error_code();
+
+    auto Errno = errno;
+    switch (Errno) {
+      case EEXIST:  // To already exists.
+      case ENOTSUP: // Device does not support cloning.
+      case EXDEV:   // From and To are on different devices.
+        break;
+      default:
+        // Anything else will also break copyfile().
+        return std::error_code(Errno, std::generic_category());
     }
-#endif
-  int Status =
-      copyfile(From.str().c_str(), To.str().c_str(), /* State */ NULL, Flag);
 
-  if (Status == 0)
+    // TODO: For EEXIST, profile calling fs::generateUniqueName() and
+    // clonefile() in a retry loop (then rename() on success) before falling
+    // back to copyfile(). Depending on the size of the file this could be
+    // cheaper.
+  }
+#endif
+  if (!copyfile(FromS.c_str(), ToS.c_str(), /*State=*/NULL, COPYFILE_DATA))
     return std::error_code();
   return std::error_code(errno, std::generic_category());
 }
 #endif // __APPLE__
 
 } // end namespace fs
 
 } // end namespace sys
 } // end namespace llvm

llvm/unittests/Support/Path.cpp

 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Error.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #ifdef _WIN32
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/Windows/WindowsSupport.h"
 #include <windows.h>
ASSERT_NO_ERROR(fs::unlockFile(FD));
     ASSERT_THAT_EXPECTED(L3, Succeeded());
   }
 
   ASSERT_NO_ERROR(fs::tryLockFile(FD));
   ASSERT_NO_ERROR(fs::unlockFile(FD));
 }
 #endif
 
+TEST_F(FileSystemTest, CopyFile) {
+  unittest::TempDir RootTestDirectory("CopyFileTest", /*Unique=*/true);
+
+  SmallVector<std::string> Data;
+  SmallVector<SmallString<128>> Sources;
+  for (int I = 0, E = 3; I != E; ++I) {
+    Data.push_back(Twine(I).str());
+    Sources.emplace_back(RootTestDirectory.path());
+    path::append(Sources.back(), "source" + Data.back() + ".txt");
+    createFileWithData(Sources.back(), /*ShouldExistBefore=*/false,
+                       fs::CD_CreateNew, Data.back());
+  }
+
+  // Copy the first file to a non-existing file.
+  SmallString<128> Destination(RootTestDirectory.path());
+  path::append(Destination, "destination");
+  ASSERT_FALSE(fs::exists(Destination));
+  fs::copy_file(Sources[0], Destination);
+  verifyFileContents(Destination, Data[0]);
+
+  // Copy the second file to an existing file.
+  fs::copy_file(Sources[1], Destination);
+  verifyFileContents(Destination, Data[1]);
+
+  // Note: The remaining logic is targeted at a potential failure case related
+  // to file cloning and symlinks on Darwin. On Windows, fs::create_link() does
+  // not return success here so the test is skipped.
+#if !defined(_WIN32)
+  // Set up a symlink to the third file.
+  SmallString<128> Symlink(RootTestDirectory.path());
+  path::append(Symlink, "symlink");
+  ASSERT_NO_ERROR(fs::create_link(path::filename(Sources[2]), Symlink));
+  verifyFileContents(Symlink, Data[2]);
+
+  // fs::getUniqueID() should follow symlinks. Otherwise, this isn't good test
+  // coverage.
+  fs::UniqueID SymlinkID;
+  fs::UniqueID Data2ID;
+  ASSERT_NO_ERROR(fs::getUniqueID(Symlink, SymlinkID));
+  ASSERT_NO_ERROR(fs::getUniqueID(Sources[2], Data2ID));
+  ASSERT_EQ(SymlinkID, Data2ID);
+
+  // Copy the third file through the symlink.
+  fs::copy_file(Symlink, Destination);
+  verifyFileContents(Destination, Data[2]);
+
+  // Confirm the destination is not a link to the original file, and not a
+  // symlink.
+  bool IsDestinationSymlink;
+  ASSERT_NO_ERROR(fs::is_symlink_file(Destination, IsDestinationSymlink));
+  ASSERT_FALSE(IsDestinationSymlink);
+  fs::UniqueID DestinationID;
+  ASSERT_NO_ERROR(fs::getUniqueID(Destination, DestinationID));
+  ASSERT_NE(SymlinkID, DestinationID);
+#endif
+}
+
 } // anonymous namespace