Merge topic 'file-copy-error-path'

89b144789d file(COPY_FILE): Report if failure occurred on input or output path
91dd7898ee Merge branch 'upstream-KWSys' into file-copy-error-path
a9c659b1b6 KWSys 2022-11-15 (6c92b9b0)

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Acked-by: Ben Boeckel <ben.boeckel@kitware.com>
Merge-request: !7916

10 files changed, 119 insertions, 42 deletions

diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index ee74908..78d230e 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -1140,7 +1140,7 @@ cmSystemTools::CopyResult cmSystemTools::CopySingleFile(
     return CopyResult::Success;
   }
 
-  cmsys::Status status;
+  cmsys::SystemTools::CopyStatus status;
   status = cmsys::SystemTools::CloneFileContent(oldname, newname);
   if (!status) {
     // if cloning did not succeed, fall back to blockwise copy
@@ -1149,14 +1149,24 @@ cmSystemTools::CopyResult cmSystemTools::CopySingleFile(
   if (!status) {
     if (err) {
       *err = status.GetString();
+      switch (status.Path) {
+        case cmsys::SystemTools::CopyStatus::SourcePath:
+          *err = cmStrCat(*err, " (input)");
+          break;
+        case cmsys::SystemTools::CopyStatus::DestPath:
+          *err = cmStrCat(*err, " (output)");
+          break;
+        default:
+          break;
+      }
     }
     return CopyResult::Failure;
   }
   if (perms) {
-    status = SystemTools::SetPermissions(newname, perm);
-    if (!status) {
+    perms = SystemTools::SetPermissions(newname, perm);
+    if (!perms) {
       if (err) {
-        *err = status.GetString();
+        *err = cmStrCat(perms.GetString(), " (output)");
       }
       return CopyResult::Failure;
     }
diff --git a/Source/kwsys/SystemTools.cxx b/Source/kwsys/SystemTools.cxx
index fdd6b2d..a3ab51a 100644
--- a/Source/kwsys/SystemTools.cxx
+++ b/Source/kwsys/SystemTools.cxx
@@ -2290,8 +2290,8 @@ static std::string FileInDir(const std::string& source, const std::string& dir)
   return new_destination + '/' + SystemTools::GetFilenameName(source);
 }
 
-Status SystemTools::CopyFileIfDifferent(std::string const& source,
-                                        std::string const& destination)
+SystemTools::CopyStatus SystemTools::CopyFileIfDifferent(
+  std::string const& source, std::string const& destination)
 {
   // special check for a destination that is a directory
   // FilesDiffer does not handle file to directory compare
@@ -2308,7 +2308,7 @@ Status SystemTools::CopyFileIfDifferent(std::string const& source,
     }
   }
   // at this point the files must be the same so return true
-  return Status::Success();
+  return CopyStatus{ Status::Success(), CopyStatus::NoPath };
 }
 
 #define KWSYS_ST_BUFFER 4096
@@ -2434,13 +2434,13 @@ bool SystemTools::TextFilesDiffer(const std::string& path1,
   return false;
 }
 
-Status SystemTools::CopyFileContentBlockwise(std::string const& source,
-                                             std::string const& destination)
+SystemTools::CopyStatus SystemTools::CopyFileContentBlockwise(
+  std::string const& source, std::string const& destination)
 {
   // Open files
   kwsys::ifstream fin(source.c_str(), std::ios::in | std::ios::binary);
   if (!fin) {
-    return Status::POSIX_errno();
+    return CopyStatus{ Status::POSIX_errno(), CopyStatus::SourcePath };
   }
 
   // try and remove the destination file so that read only destination files
@@ -2452,7 +2452,7 @@ Status SystemTools::CopyFileContentBlockwise(std::string const& source,
   kwsys::ofstream fout(destination.c_str(),
                        std::ios::out | std::ios::trunc | std::ios::binary);
   if (!fout) {
-    return Status::POSIX_errno();
+    return CopyStatus{ Status::POSIX_errno(), CopyStatus::DestPath };
   }
 
   // This copy loop is very sensitive on certain platforms with
@@ -2481,10 +2481,10 @@ Status SystemTools::CopyFileContentBlockwise(std::string const& source,
   fout.close();
 
   if (!fout) {
-    return Status::POSIX_errno();
+    return CopyStatus{ Status::POSIX_errno(), CopyStatus::DestPath };
   }
 
-  return Status::Success();
+  return CopyStatus{ Status::Success(), CopyStatus::NoPath };
 }
 
 /**
@@ -2499,13 +2499,13 @@ Status SystemTools::CopyFileContentBlockwise(std::string const& source,
  * - The underlying filesystem does not support file cloning
  * - An unspecified error occurred
  */
-Status SystemTools::CloneFileContent(std::string const& source,
-                                     std::string const& destination)
+SystemTools::CopyStatus SystemTools::CloneFileContent(
+  std::string const& source, std::string const& destination)
 {
 #if defined(__linux) && defined(FICLONE)
   int in = open(source.c_str(), O_RDONLY);
   if (in < 0) {
-    return Status::POSIX_errno();
+    return CopyStatus{ Status::POSIX_errno(), CopyStatus::SourcePath };
   }
 
   SystemTools::RemoveFile(destination);
@@ -2513,14 +2513,14 @@ Status SystemTools::CloneFileContent(std::string const& source,
   int out =
     open(destination.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
   if (out < 0) {
-    Status status = Status::POSIX_errno();
+    CopyStatus status{ Status::POSIX_errno(), CopyStatus::DestPath };
     close(in);
     return status;
   }
 
-  Status status = Status::Success();
+  CopyStatus status{ Status::Success(), CopyStatus::NoPath };
   if (ioctl(out, FICLONE, in) < 0) {
-    status = Status::POSIX_errno();
+    status = CopyStatus{ Status::POSIX_errno(), CopyStatus::NoPath };
   }
   close(in);
   close(out);
@@ -2532,40 +2532,41 @@ Status SystemTools::CloneFileContent(std::string const& source,
   // be updated by `copy_file_if_different` and `copy_file`.
   if (copyfile(source.c_str(), destination.c_str(), nullptr,
                COPYFILE_METADATA | COPYFILE_CLONE) < 0) {
-    return Status::POSIX_errno();
+    return CopyStatus{ Status::POSIX_errno(), CopyStatus::NoPath };
   }
 #  if KWSYS_CXX_HAS_UTIMENSAT
   // utimensat is only available on newer Unixes and macOS 10.13+
   if (utimensat(AT_FDCWD, destination.c_str(), nullptr, 0) < 0) {
-    return Status::POSIX_errno();
+    return CopyStatus{ Status::POSIX_errno(), CopyStatus::DestPath };
   }
 #  else
   // fall back to utimes
   if (utimes(destination.c_str(), nullptr) < 0) {
-    return Status::POSIX_errno();
+    return CopyStatus{ Status::POSIX_errno(), CopyStatus::DestPath };
   }
 #  endif
-  return Status::Success();
+  return CopyStatus{ Status::Success(), CopyStatus::NoPath };
 #else
   (void)source;
   (void)destination;
-  return Status::POSIX(ENOSYS);
+  return CopyStatus{ Status::POSIX(ENOSYS), CopyStatus::NoPath };
 #endif
 }
 
 /**
  * Copy a file named by "source" to the file named by "destination".
  */
-Status SystemTools::CopyFileAlways(std::string const& source,
-                                   std::string const& destination)
+SystemTools::CopyStatus SystemTools::CopyFileAlways(
+  std::string const& source, std::string const& destination)
 {
-  Status status;
+  CopyStatus status;
   mode_t perm = 0;
   Status perms = SystemTools::GetPermissions(source, perm);
   std::string real_destination = destination;
 
   if (SystemTools::FileIsDirectory(source)) {
-    status = SystemTools::MakeDirectory(destination);
+    status = CopyStatus{ SystemTools::MakeDirectory(destination),
+                         CopyStatus::DestPath };
     if (!status.IsSuccess()) {
       return status;
     }
@@ -2590,7 +2591,8 @@ Status SystemTools::CopyFileAlways(std::string const& source,
 
     // Create destination directory
     if (!destination_dir.empty()) {
-      status = SystemTools::MakeDirectory(destination_dir);
+      status = CopyStatus{ SystemTools::MakeDirectory(destination_dir),
+                           CopyStatus::DestPath };
       if (!status.IsSuccess()) {
         return status;
       }
@@ -2606,13 +2608,15 @@ Status SystemTools::CopyFileAlways(std::string const& source,
     }
   }
   if (perms) {
-    status = SystemTools::SetPermissions(real_destination, perm);
+    status = CopyStatus{ SystemTools::SetPermissions(real_destination, perm),
+                         CopyStatus::DestPath };
   }
   return status;
 }
 
-Status SystemTools::CopyAFile(std::string const& source,
-                              std::string const& destination, bool always)
+SystemTools::CopyStatus SystemTools::CopyAFile(std::string const& source,
+                                               std::string const& destination,
+                                               bool always)
 {
   if (always) {
     return SystemTools::CopyFileAlways(source, destination);
diff --git a/Source/kwsys/SystemTools.hxx.in b/Source/kwsys/SystemTools.hxx.in
index acce015..56b65fd 100644
--- a/Source/kwsys/SystemTools.hxx.in
+++ b/Source/kwsys/SystemTools.hxx.in
@@ -566,11 +566,34 @@ public:
                               const mode_t* mode = nullptr);
 
   /**
+   * Represent the result of a file copy operation.
+   * This is the result 'Status' and, if the operation failed,
+   * an indication of whether the error occurred on the source
+   * or destination path.
+   */
+  struct CopyStatus : public Status
+  {
+    enum WhichPath
+    {
+      NoPath,
+      SourcePath,
+      DestPath,
+    };
+    CopyStatus() = default;
+    CopyStatus(Status s, WhichPath p)
+      : Status(s)
+      , Path(p)
+    {
+    }
+    WhichPath Path = NoPath;
+  };
+
+  /**
    * Copy the source file to the destination file only
    * if the two files differ.
    */
-  static Status CopyFileIfDifferent(std::string const& source,
-                                    std::string const& destination);
+  static CopyStatus CopyFileIfDifferent(std::string const& source,
+                                        std::string const& destination);
 
   /**
    * Compare the contents of two files.  Return true if different
@@ -588,13 +611,13 @@ public:
   /**
    * Blockwise copy source to destination file
    */
-  static Status CopyFileContentBlockwise(std::string const& source,
-                                         std::string const& destination);
+  static CopyStatus CopyFileContentBlockwise(std::string const& source,
+                                             std::string const& destination);
   /**
    * Clone the source file to the destination file
    */
-  static Status CloneFileContent(std::string const& source,
-                                 std::string const& destination);
+  static CopyStatus CloneFileContent(std::string const& source,
+                                     std::string const& destination);
 
   /**
    * Return true if the two files are the same file
@@ -604,16 +627,17 @@ public:
   /**
    * Copy a file.
    */
-  static Status CopyFileAlways(std::string const& source,
-                               std::string const& destination);
+  static CopyStatus CopyFileAlways(std::string const& source,
+                                   std::string const& destination);
 
   /**
    * Copy a file.  If the "always" argument is true the file is always
    * copied.  If it is false, the file is copied only if it is new or
    * has changed.
    */
-  static Status CopyAFile(std::string const& source,
-                          std::string const& destination, bool always = true);
+  static CopyStatus CopyAFile(std::string const& source,
+                              std::string const& destination,
+                              bool always = true);
 
   /**
    * Copy content directory to another directory with all files and
diff --git a/Tests/RunCMake/file/COPY_FILE-input-missing-result.txt b/Tests/RunCMake/file/COPY_FILE-input-missing-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/file/COPY_FILE-input-missing-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/file/COPY_FILE-input-missing-stderr.txt b/Tests/RunCMake/file/COPY_FILE-input-missing-stderr.txt
new file mode 100644
index 0000000..989925d
--- /dev/null
+++ b/Tests/RunCMake/file/COPY_FILE-input-missing-stderr.txt
@@ -0,0 +1,14 @@
+^CMake Error at [^
+]*/Tests/RunCMake/file/COPY_FILE-input-missing.cmake:[0-9]+ \(file\):
+  file COPY_FILE failed to copy
+
+    [^
+]*/Tests/RunCMake/file/COPY_FILE-input-missing-build/input-missing
+
+  to
+
+    [^
+]*/Tests/RunCMake/file/COPY_FILE-input-missing-build/output
+
+  because: [^
+]+ \(input\)$
diff --git a/Tests/RunCMake/file/COPY_FILE-input-missing.cmake b/Tests/RunCMake/file/COPY_FILE-input-missing.cmake
new file mode 100644
index 0000000..2d2c55e
--- /dev/null
+++ b/Tests/RunCMake/file/COPY_FILE-input-missing.cmake
@@ -0,0 +1,3 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input-missing")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output")
+file(COPY_FILE "${oldname}" "${newname}")
diff --git a/Tests/RunCMake/file/COPY_FILE-output-missing-result.txt b/Tests/RunCMake/file/COPY_FILE-output-missing-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/file/COPY_FILE-output-missing-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/file/COPY_FILE-output-missing-stderr.txt b/Tests/RunCMake/file/COPY_FILE-output-missing-stderr.txt
new file mode 100644
index 0000000..0e7d9f7
--- /dev/null
+++ b/Tests/RunCMake/file/COPY_FILE-output-missing-stderr.txt
@@ -0,0 +1,14 @@
+^CMake Error at [^
+]*/Tests/RunCMake/file/COPY_FILE-output-missing.cmake:[0-9]+ \(file\):
+  file COPY_FILE failed to copy
+
+    [^
+]*/Tests/RunCMake/file/COPY_FILE-output-missing-build/input
+
+  to
+
+    [^
+]*/Tests/RunCMake/file/COPY_FILE-output-missing-build/output-missing/output
+
+  because: [^
+]+ \(output\)$
diff --git a/Tests/RunCMake/file/COPY_FILE-output-missing.cmake b/Tests/RunCMake/file/COPY_FILE-output-missing.cmake
new file mode 100644
index 0000000..22133e7
--- /dev/null
+++ b/Tests/RunCMake/file/COPY_FILE-output-missing.cmake
@@ -0,0 +1,4 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output-missing/output")
+file(WRITE "${oldname}" "")
+file(COPY_FILE "${oldname}" "${newname}")
diff --git a/Tests/RunCMake/file/RunCMakeTest.cmake b/Tests/RunCMake/file/RunCMakeTest.cmake
index db88956..4ad00ff 100644
--- a/Tests/RunCMake/file/RunCMakeTest.cmake
+++ b/Tests/RunCMake/file/RunCMakeTest.cmake
@@ -65,6 +65,8 @@ run_cmake_script(COPY_FILE-file-ONLY_IF_DIFFERENT-no-overwrite)
 run_cmake_script(COPY_FILE-link-to-file)
 run_cmake_script(COPY_FILE-arg-missing)
 run_cmake_script(COPY_FILE-arg-unknown)
+run_cmake_script(COPY_FILE-input-missing)
+run_cmake_script(COPY_FILE-output-missing)
 
 run_cmake_script(RENAME-file-replace)
 run_cmake_script(RENAME-file-to-file)