Browse Source

Merge topic 'file-COPY_FILE-retry'

d34986036f ExternalData: Improve robustness on Windows to copy a data object to a file
efa9eec040 file(COPY_FILE): Add option to retry on Windows if input access fails
fa518188d8 cmSystemTools: Remove unused CopySingleFile overload

Acked-by: Kitware Robot <[email protected]>
Acked-by: buildbot <[email protected]>
Merge-request: !7934
Brad King 2 years ago
parent
commit
4391808712

+ 10 - 1
Help/command/file.rst

@@ -750,7 +750,8 @@ The options are:
 
   file(COPY_FILE <oldname> <newname>
        [RESULT <result>]
-       [ONLY_IF_DIFFERENT])
+       [ONLY_IF_DIFFERENT]
+       [INPUT_MAY_BE_RECENT])
 
 .. versionadded:: 3.21
 
@@ -769,6 +770,14 @@ The options are:
   contents are already the same as ``<oldname>`` (this avoids updating
   ``<newname>``'s timestamp).
 
+``INPUT_MAY_BE_RECENT``
+  .. versionadded:: 3.26
+
+  Tell CMake that the input file may have been recently created.  This is
+  meaningful only on Windows, where files may be inaccessible for a short
+  time after they are created.  With this option, if permission is denied,
+  CMake will retry reading the input a few times.
+
 This sub-command has some similarities to :command:`configure_file` with the
 ``COPYONLY`` option.  An important difference is that :command:`configure_file`
 creates a dependency on the source file, so CMake will be re-run if it changes.

+ 1 - 1
Modules/ExternalData.cmake

@@ -945,7 +945,7 @@ function(_ExternalData_link_or_copy src dst)
     file(CREATE_LINK "${tgt}" "${tmp}" RESULT result COPY_ON_ERROR SYMBOLIC)
   else()
     # Create a copy.
-    file(COPY_FILE "${src}" "${tmp}" RESULT result)
+    file(COPY_FILE "${src}" "${tmp}" RESULT result INPUT_MAY_BE_RECENT)
   endif()
   if(result)
     file(REMOVE "${tmp}")

+ 7 - 1
Source/cmFileCommand.cxx

@@ -1408,12 +1408,14 @@ bool HandleCopyFile(std::vector<std::string> const& args,
 
   struct Arguments
   {
+    bool InputMayBeRecent = false;
     bool OnlyIfDifferent = false;
     std::string Result;
   };
 
   static auto const parser =
     cmArgumentParser<Arguments>{}
+      .Bind("INPUT_MAY_BE_RECENT"_s, &Arguments::InputMayBeRecent)
       .Bind("ONLY_IF_DIFFERENT"_s, &Arguments::OnlyIfDifferent)
       .Bind("RESULT"_s, &Arguments::Result);
 
@@ -1456,9 +1458,13 @@ bool HandleCopyFile(std::vector<std::string> const& args,
   } else {
     when = cmSystemTools::CopyWhen::Always;
   }
+  cmSystemTools::CopyInputRecent const inputRecent = arguments.InputMayBeRecent
+    ? cmSystemTools::CopyInputRecent::Yes
+    : cmSystemTools::CopyInputRecent::No;
 
   std::string err;
-  if (cmSystemTools::CopySingleFile(oldname, newname, when, &err) ==
+  if (cmSystemTools::CopySingleFile(oldname, newname, when, inputRecent,
+                                    &err) ==
       cmSystemTools::CopyResult::Success) {
     if (!arguments.Result.empty()) {
       status.GetMakefile().AddDefinition(arguments.Result, "0");

+ 18 - 8
Source/cmSystemTools.cxx

@@ -1111,16 +1111,9 @@ bool cmMoveFile(std::wstring const& oldname, std::wstring const& newname,
 }
 #endif
 
-bool cmSystemTools::CopySingleFile(const std::string& oldname,
-                                   const std::string& newname)
-{
-  return cmSystemTools::CopySingleFile(oldname, newname, CopyWhen::Always) ==
-    CopyResult::Success;
-}
-
 cmSystemTools::CopyResult cmSystemTools::CopySingleFile(
   std::string const& oldname, std::string const& newname, CopyWhen when,
-  std::string* err)
+  CopyInputRecent inputRecent, std::string* err)
 {
   switch (when) {
     case CopyWhen::Always:
@@ -1144,7 +1137,24 @@ cmSystemTools::CopyResult cmSystemTools::CopySingleFile(
   status = cmsys::SystemTools::CloneFileContent(oldname, newname);
   if (!status) {
     // if cloning did not succeed, fall back to blockwise copy
+#ifdef _WIN32
+    if (inputRecent == CopyInputRecent::Yes) {
+      // Windows sometimes locks a file immediately after creation.
+      // Retry a few times.
+      WindowsFileRetry retry = cmSystemTools::GetWindowsFileRetry();
+      while ((status =
+                cmsys::SystemTools::CopyFileContentBlockwise(oldname, newname),
+              status.Path == cmsys::SystemTools::CopyStatus::SourcePath &&
+                status.GetPOSIX() == EACCES && --retry.Count)) {
+        cmSystemTools::Delay(retry.Delay);
+      }
+    } else {
+      status = cmsys::SystemTools::CopyFileContentBlockwise(oldname, newname);
+    }
+#else
+    static_cast<void>(inputRecent);
     status = cmsys::SystemTools::CopyFileContentBlockwise(oldname, newname);
+#endif
   }
   if (!status) {
     if (err) {

+ 6 - 2
Source/cmSystemTools.h

@@ -149,6 +149,11 @@ public:
     Always,
     OnlyIfDifferent,
   };
+  enum class CopyInputRecent
+  {
+    No,
+    Yes,
+  };
   enum class CopyResult
   {
     Success,
@@ -177,10 +182,9 @@ public:
                                          const mode_t* mode = nullptr);
 
   /** Copy a file. */
-  static bool CopySingleFile(const std::string& oldname,
-                             const std::string& newname);
   static CopyResult CopySingleFile(std::string const& oldname,
                                    std::string const& newname, CopyWhen when,
+                                   CopyInputRecent inputRecent,
                                    std::string* err = nullptr);
 
   enum class Replace

+ 10 - 0
Tests/RunCMake/file/COPY_FILE-file-INPUT_MAY_BE_RECENT.cmake

@@ -0,0 +1,10 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output")
+file(WRITE "${oldname}" "")
+file(COPY_FILE "${oldname}" "${newname}" INPUT_MAY_BE_RECENT)
+if(NOT EXISTS "${oldname}")
+  message(FATAL_ERROR "The old name does not exist:\n ${oldname}")
+endif()
+if(NOT EXISTS "${newname}")
+  message(FATAL_ERROR "The new name does not exist:\n ${newname}")
+endif()

+ 1 - 0
Tests/RunCMake/file/RunCMakeTest.cmake

@@ -59,6 +59,7 @@ run_cmake_script(COPY_FILE-dirlink-to-file-fail)
 run_cmake_script(COPY_FILE-file-to-file)
 run_cmake_script(COPY_FILE-file-to-dir-capture)
 run_cmake_script(COPY_FILE-file-to-dir-fail)
+run_cmake_script(COPY_FILE-file-INPUT_MAY_BE_RECENT)
 run_cmake_script(COPY_FILE-file-ONLY_IF_DIFFERENT-capture)
 run_cmake_script(COPY_FILE-file-ONLY_IF_DIFFERENT-fail)
 run_cmake_script(COPY_FILE-file-ONLY_IF_DIFFERENT-no-overwrite)