Merge topic 'file-RENAME'

9bf40d8027 file(RENAME): Add option to not replace existing path
3600c6cd8c cmSystemTools: Add RenameFile option to not replace destination
c61292726c file(RENAME): Add option to capture error message on failure
0c2dc34504 cmSystemTools: Add RenameFile signature to capture the error message
adc351db8f Tests: Add RunCMake helper to run a plain script

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !5877

25 files changed, 254 insertions, 18 deletions

diff --git a/Help/command/file.rst b/Help/command/file.rst
index 3db605d..9cde90c 100644
--- a/Help/command/file.rst
+++ b/Help/command/file.rst
@@ -38,7 +38,7 @@ Synopsis
 
   `Filesystem`_
     file({`GLOB`_ | `GLOB_RECURSE`_} <out-var> [...] [<globbing-expr>...])
-    file(`RENAME`_ <oldname> <newname>)
+    file(`RENAME`_ <oldname> <newname> [...])
     file({`REMOVE`_ | `REMOVE_RECURSE`_ } [<files>...])
     file(`MAKE_DIRECTORY`_ [<dir>...])
     file({`COPY`_ | `INSTALL`_} <file>... DESTINATION <dir> [...])
@@ -665,11 +665,24 @@ Examples of recursive globbing include::
 
 .. code-block:: cmake
 
-  file(RENAME <oldname> <newname>)
+  file(RENAME <oldname> <newname>
+       [RESULT <result>]
+       [NO_REPLACE])
 
 Move a file or directory within a filesystem from ``<oldname>`` to
 ``<newname>``, replacing the destination atomically.
 
+The options are:
+
+``RESULT <result>``
+  Set ``<result>`` variable to ``0`` on success or an error message otherwise.
+  If ``RESULT`` is not specified and the operation fails, an error is emitted.
+
+``NO_REPLACE``
+  If the ``<newname>`` path already exists, do not replace it.
+  If ``RESULT <result>`` is used, the result variable will be
+  set to ``NO_REPLACE``.  Otherwise, an error is emitted.
+
 .. _REMOVE:
 .. _REMOVE_RECURSE:
 
diff --git a/Help/release/dev/file-RENAME.rst b/Help/release/dev/file-RENAME.rst
new file mode 100644
index 0000000..6c1314d
--- /dev/null
+++ b/Help/release/dev/file-RENAME.rst
@@ -0,0 +1,6 @@
+file-RENAME
+-----------
+
+* The :command:`file(RENAME)` command learned to optionally capture
+  failure in a result variable.  It also gained a ``NO_REPLACE``
+  option to fail if the destination exists.
diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index f674833..065b845 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -1313,8 +1313,9 @@ bool HandleRelativePathCommand(std::vector<std::string> const& args,
 bool HandleRename(std::vector<std::string> const& args,
                   cmExecutionStatus& status)
 {
-  if (args.size() != 3) {
-    status.SetError("RENAME given incorrect number of arguments.");
+  if (args.size() < 3) {
+    status.SetError("RENAME must be called with at least two additional "
+                    "arguments");
     return false;
   }
 
@@ -1330,13 +1331,52 @@ bool HandleRename(std::vector<std::string> const& args,
       cmStrCat(status.GetMakefile().GetCurrentSourceDirectory(), '/', args[2]);
   }
 
-  if (!cmSystemTools::RenameFile(oldname, newname)) {
-    std::string err = cmSystemTools::GetLastSystemError();
-    status.SetError(cmStrCat("RENAME failed to rename\n  ", oldname,
-                             "\nto\n  ", newname, "\nbecause: ", err, "\n"));
+  struct Arguments
+  {
+    bool NoReplace = false;
+    std::string Result;
+  };
+
+  static auto const parser = cmArgumentParser<Arguments>{}
+                               .Bind("NO_REPLACE"_s, &Arguments::NoReplace)
+                               .Bind("RESULT"_s, &Arguments::Result);
+
+  std::vector<std::string> unconsumedArgs;
+  Arguments const arguments =
+    parser.Parse(cmMakeRange(args).advance(3), &unconsumedArgs);
+  if (!unconsumedArgs.empty()) {
+    status.SetError("RENAME unknown argument:\n  " + unconsumedArgs.front());
     return false;
   }
-  return true;
+
+  std::string err;
+  switch (cmSystemTools::RenameFile(oldname, newname,
+                                    arguments.NoReplace
+                                      ? cmSystemTools::Replace::No
+                                      : cmSystemTools::Replace::Yes,
+                                    &err)) {
+    case cmSystemTools::RenameResult::Success:
+      if (!arguments.Result.empty()) {
+        status.GetMakefile().AddDefinition(arguments.Result, "0");
+      }
+      return true;
+    case cmSystemTools::RenameResult::NoReplace:
+      if (!arguments.Result.empty()) {
+        err = "NO_REPLACE";
+      } else {
+        err = "path not replaced";
+      }
+      CM_FALLTHROUGH;
+    case cmSystemTools::RenameResult::Failure:
+      if (!arguments.Result.empty()) {
+        status.GetMakefile().AddDefinition(arguments.Result, err);
+        return true;
+      }
+      break;
+  }
+  status.SetError(cmStrCat("RENAME failed to rename\n  ", oldname, "\nto\n  ",
+                           newname, "\nbecause: ", err, "\n"));
+  return false;
 }
 
 bool HandleRemoveImpl(std::vector<std::string> const& args, bool recurse,
diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index 0807590..db5b1ac 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -149,6 +149,27 @@ static int cm_archive_read_open_file(struct archive* a, const char* file,
 #  define environ (*_NSGetEnviron())
 #endif
 
+namespace {
+void ReportError(std::string* err)
+{
+  if (!err) {
+    return;
+  }
+#ifdef _WIN32
+  LPSTR message = NULL;
+  DWORD size = FormatMessageA(
+    FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
+      FORMAT_MESSAGE_IGNORE_INSERTS,
+    NULL, GetLastError(), MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+    (LPSTR)&message, 0, NULL);
+  *err = std::string(message, size);
+  LocalFree(message);
+#else
+  *err = strerror(errno);
+#endif
+}
+}
+
 bool cmSystemTools::s_RunCommandHideConsole = false;
 bool cmSystemTools::s_DisableRunCommandOutput = false;
 bool cmSystemTools::s_ErrorOccured = false;
@@ -952,14 +973,19 @@ void cmSystemTools::InitializeLibUV()
 
 #ifdef _WIN32
 namespace {
-bool cmMoveFile(std::wstring const& oldname, std::wstring const& newname)
+bool cmMoveFile(std::wstring const& oldname, std::wstring const& newname,
+                cmSystemTools::Replace replace)
 {
   // Not only ignore any previous error, but clear any memory of it.
   SetLastError(0);
 
-  // Use MOVEFILE_REPLACE_EXISTING to replace an existing destination file.
-  return MoveFileExW(oldname.c_str(), newname.c_str(),
-                     MOVEFILE_REPLACE_EXISTING);
+  DWORD flags = 0;
+  if (replace == cmSystemTools::Replace::Yes) {
+    // Use MOVEFILE_REPLACE_EXISTING to replace an existing destination file.
+    flags = flags | MOVEFILE_REPLACE_EXISTING;
+  }
+
+  return MoveFileExW(oldname.c_str(), newname.c_str(), flags);
 }
 }
 #endif
@@ -967,6 +993,14 @@ bool cmMoveFile(std::wstring const& oldname, std::wstring const& newname)
 bool cmSystemTools::RenameFile(const std::string& oldname,
                                const std::string& newname)
 {
+  return cmSystemTools::RenameFile(oldname, newname, Replace::Yes) ==
+    RenameResult::Success;
+}
+
+cmSystemTools::RenameResult cmSystemTools::RenameFile(
+  std::string const& oldname, std::string const& newname, Replace replace,
+  std::string* err)
+{
 #ifdef _WIN32
 #  ifndef INVALID_FILE_ATTRIBUTES
 #    define INVALID_FILE_ATTRIBUTES ((DWORD)-1)
@@ -988,7 +1022,7 @@ bool cmSystemTools::RenameFile(const std::string& oldname,
     oldname_wstr, FILE_ATTRIBUTE_NOT_CONTENT_INDEXED);
 
   DWORD move_last_error = 0;
-  while (!cmMoveFile(oldname_wstr, newname_wstr) && --retry.Count) {
+  while (!cmMoveFile(oldname_wstr, newname_wstr, replace) && --retry.Count) {
     move_last_error = GetLastError();
 
     // There was no error ==> the operation is not yet complete.
@@ -1004,7 +1038,11 @@ bool cmSystemTools::RenameFile(const std::string& oldname,
     // 3) Windows Explorer has an associated directory already opened.
     if (move_last_error != ERROR_ACCESS_DENIED &&
         move_last_error != ERROR_SHARING_VIOLATION) {
-      return false;
+      if (replace == Replace::No && move_last_error == ERROR_ALREADY_EXISTS) {
+        return RenameResult::NoReplace;
+      }
+      ReportError(err);
+      return RenameResult::Failure;
     }
 
     DWORD const attrs = GetFileAttributesW(newname_wstr.c_str());
@@ -1028,10 +1066,31 @@ bool cmSystemTools::RenameFile(const std::string& oldname,
     save_restore_file_attributes.SetPath(newname_wstr);
   }
   SetLastError(move_last_error);
-  return retry.Count > 0;
+  if (retry.Count > 0) {
+    return RenameResult::Success;
+  }
+  if (replace == Replace::No && GetLastError() == ERROR_ALREADY_EXISTS) {
+    return RenameResult::NoReplace;
+  }
+  ReportError(err);
+  return RenameResult::Failure;
 #else
-  /* On UNIX we have an OS-provided call to do this atomically.  */
-  return rename(oldname.c_str(), newname.c_str()) == 0;
+  // On UNIX we have OS-provided calls to create 'newname' atomically.
+  if (replace == Replace::No) {
+    if (link(oldname.c_str(), newname.c_str()) == 0) {
+      return RenameResult::Success;
+    }
+    if (errno == EEXIST) {
+      return RenameResult::NoReplace;
+    }
+    ReportError(err);
+    return RenameResult::Failure;
+  }
+  if (rename(oldname.c_str(), newname.c_str()) == 0) {
+    return RenameResult::Success;
+  }
+  ReportError(err);
+  return RenameResult::Failure;
 #endif
 }
 
diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h
index 5bbbb0c..3cc032c 100644
--- a/Source/cmSystemTools.h
+++ b/Source/cmSystemTools.h
@@ -128,10 +128,25 @@ public:
   static bool SimpleGlob(const std::string& glob,
                          std::vector<std::string>& files, int type = 0);
 
+  enum class Replace
+  {
+    Yes,
+    No,
+  };
+  enum class RenameResult
+  {
+    Success,
+    NoReplace,
+    Failure,
+  };
+
   /** Rename a file or directory within a single disk volume (atomic
       if possible).  */
   static bool RenameFile(const std::string& oldname,
                          const std::string& newname);
+  static RenameResult RenameFile(std::string const& oldname,
+                                 std::string const& newname, Replace replace,
+                                 std::string* err = nullptr);
 
   //! Rename a file if contents are different, delete the source otherwise
   static void MoveFileIfDifferent(const std::string& source,
diff --git a/Tests/RunCMake/RunCMake.cmake b/Tests/RunCMake/RunCMake.cmake
index a26f632..1a69059 100644
--- a/Tests/RunCMake/RunCMake.cmake
+++ b/Tests/RunCMake/RunCMake.cmake
@@ -214,6 +214,11 @@ function(run_cmake_command test)
   run_cmake(${test})
 endfunction()
 
+function(run_cmake_script test)
+  set(RunCMake_TEST_COMMAND ${CMAKE_COMMAND} ${ARGN} -P ${RunCMake_SOURCE_DIR}/${test}.cmake)
+  run_cmake(${test})
+endfunction()
+
 function(run_cmake_with_options test)
   set(RunCMake_TEST_OPTIONS "${ARGN}")
   run_cmake(${test})
diff --git a/Tests/RunCMake/file/RENAME-arg-missing-result.txt b/Tests/RunCMake/file/RENAME-arg-missing-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-arg-missing-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/file/RENAME-arg-missing-stderr.txt b/Tests/RunCMake/file/RENAME-arg-missing-stderr.txt
new file mode 100644
index 0000000..98d2961
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-arg-missing-stderr.txt
@@ -0,0 +1,3 @@
+^CMake Error at [^
+]*/Tests/RunCMake/file/RENAME-arg-missing.cmake:1 \(file\):
+  file RENAME must be called with at least two additional arguments$
diff --git a/Tests/RunCMake/file/RENAME-arg-missing.cmake b/Tests/RunCMake/file/RENAME-arg-missing.cmake
new file mode 100644
index 0000000..2358ce9
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-arg-missing.cmake
@@ -0,0 +1 @@
+file(RENAME "old")
diff --git a/Tests/RunCMake/file/RENAME-arg-unknown-result.txt b/Tests/RunCMake/file/RENAME-arg-unknown-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-arg-unknown-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/file/RENAME-arg-unknown-stderr.txt b/Tests/RunCMake/file/RENAME-arg-unknown-stderr.txt
new file mode 100644
index 0000000..16edb25
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-arg-unknown-stderr.txt
@@ -0,0 +1,5 @@
+^CMake Error at [^
+]*/Tests/RunCMake/file/RENAME-arg-unknown.cmake:1 \(file\):
+  file RENAME unknown argument:
+
+    unknown$
diff --git a/Tests/RunCMake/file/RENAME-arg-unknown.cmake b/Tests/RunCMake/file/RENAME-arg-unknown.cmake
new file mode 100644
index 0000000..3110f70
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-arg-unknown.cmake
@@ -0,0 +1 @@
+file(RENAME "old" "new" unknown)
diff --git a/Tests/RunCMake/file/RENAME-file-NO_REPLACE-capture-stdout.txt b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-capture-stdout.txt
new file mode 100644
index 0000000..a116330
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-capture-stdout.txt
@@ -0,0 +1 @@
+^-- file\(RENAME\) failed with result: NO_REPLACE$
diff --git a/Tests/RunCMake/file/RENAME-file-NO_REPLACE-capture.cmake b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-capture.cmake
new file mode 100644
index 0000000..1ff4178
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-capture.cmake
@@ -0,0 +1,9 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output")
+file(WRITE "${oldname}" "a")
+file(WRITE "${newname}" "b")
+file(RENAME "${oldname}" "${newname}" NO_REPLACE RESULT result)
+message(STATUS "file(RENAME) failed with result: ${result}")
+if(NOT EXISTS "${oldname}")
+  message(FATAL_ERROR "The old name does not still exist:\n ${oldname}")
+endif()
diff --git a/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail-result.txt b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail-stderr.txt b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail-stderr.txt
new file mode 100644
index 0000000..dd7294c
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail-stderr.txt
@@ -0,0 +1,13 @@
+^CMake Error at [^
+]*/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail.cmake:[0-9] \(file\):
+  file RENAME failed to rename
+
+    [^
+]*/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail-build/input
+
+  to
+
+    [^
+]*/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail-build/output
+
+  because: path not replaced$
diff --git a/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail.cmake b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail.cmake
new file mode 100644
index 0000000..c05dd63
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-NO_REPLACE-fail.cmake
@@ -0,0 +1,5 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output")
+file(WRITE "${oldname}" "a")
+file(WRITE "${newname}" "b")
+file(RENAME "${oldname}" "${newname}" NO_REPLACE)
diff --git a/Tests/RunCMake/file/RENAME-file-replace.cmake b/Tests/RunCMake/file/RENAME-file-replace.cmake
new file mode 100644
index 0000000..efbfaed
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-replace.cmake
@@ -0,0 +1,9 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output")
+file(WRITE "${oldname}" "a")
+file(WRITE "${newname}" "b")
+file(RENAME "${oldname}" "${newname}")
+file(READ "${newname}" new)
+if(NOT "${new}" STREQUAL "a")
+  message(FATAL_ERROR "New name:\n  ${newname}\ndoes not contain expected content 'a'.")
+endif()
diff --git a/Tests/RunCMake/file/RENAME-file-to-dir-capture-stdout.txt b/Tests/RunCMake/file/RENAME-file-to-dir-capture-stdout.txt
new file mode 100644
index 0000000..0276a5f
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-to-dir-capture-stdout.txt
@@ -0,0 +1 @@
+^-- file\(RENAME\) failed with result: [A-Za-z]
diff --git a/Tests/RunCMake/file/RENAME-file-to-dir-capture.cmake b/Tests/RunCMake/file/RENAME-file-to-dir-capture.cmake
new file mode 100644
index 0000000..4f817e8
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-to-dir-capture.cmake
@@ -0,0 +1,9 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output")
+file(WRITE "${oldname}" "")
+file(MAKE_DIRECTORY "${newname}")
+file(RENAME "${oldname}" "${newname}" RESULT result)
+message(STATUS "file(RENAME) failed with result: ${result}")
+if(NOT EXISTS "${oldname}")
+  message(FATAL_ERROR "The old name does not still exist:\n ${oldname}")
+endif()
diff --git a/Tests/RunCMake/file/RENAME-file-to-dir-fail-result.txt b/Tests/RunCMake/file/RENAME-file-to-dir-fail-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-to-dir-fail-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/file/RENAME-file-to-dir-fail-stderr.txt b/Tests/RunCMake/file/RENAME-file-to-dir-fail-stderr.txt
new file mode 100644
index 0000000..e4dbc38
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-to-dir-fail-stderr.txt
@@ -0,0 +1,13 @@
+^CMake Error at [^
+]*/Tests/RunCMake/file/RENAME-file-to-dir-fail.cmake:[0-9] \(file\):
+  file RENAME failed to rename
+
+    [^
+]*/Tests/RunCMake/file/RENAME-file-to-dir-fail-build/input
+
+  to
+
+    [^
+]*/Tests/RunCMake/file/RENAME-file-to-dir-fail-build/output
+
+  because: [A-Za-z]
diff --git a/Tests/RunCMake/file/RENAME-file-to-dir-fail.cmake b/Tests/RunCMake/file/RENAME-file-to-dir-fail.cmake
new file mode 100644
index 0000000..61fa644
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-to-dir-fail.cmake
@@ -0,0 +1,5 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output")
+file(WRITE "${oldname}" "")
+file(MAKE_DIRECTORY "${newname}")
+file(RENAME "${oldname}" "${newname}")
diff --git a/Tests/RunCMake/file/RENAME-file-to-file.cmake b/Tests/RunCMake/file/RENAME-file-to-file.cmake
new file mode 100644
index 0000000..dbc411be
--- /dev/null
+++ b/Tests/RunCMake/file/RENAME-file-to-file.cmake
@@ -0,0 +1,10 @@
+set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input")
+set(newname "${CMAKE_CURRENT_BINARY_DIR}/output")
+file(WRITE "${oldname}" "")
+file(RENAME "${oldname}" "${newname}")
+if(EXISTS "${oldname}")
+  message(FATAL_ERROR "The old name still exists:\n ${oldname}")
+endif()
+if(NOT EXISTS "${newname}")
+  message(FATAL_ERROR "The new name does not exist:\n ${newname}")
+endif()
diff --git a/Tests/RunCMake/file/RunCMakeTest.cmake b/Tests/RunCMake/file/RunCMakeTest.cmake
index 22813eb..f74832c 100644
--- a/Tests/RunCMake/file/RunCMakeTest.cmake
+++ b/Tests/RunCMake/file/RunCMakeTest.cmake
@@ -50,6 +50,15 @@ run_cmake(SIZE-error-does-not-exist)
 
 run_cmake(REMOVE-empty)
 
+run_cmake_script(RENAME-file-replace)
+run_cmake_script(RENAME-file-to-file)
+run_cmake_script(RENAME-file-to-dir-capture)
+run_cmake_script(RENAME-file-to-dir-fail)
+run_cmake_script(RENAME-file-NO_REPLACE-capture)
+run_cmake_script(RENAME-file-NO_REPLACE-fail)
+run_cmake_script(RENAME-arg-missing)
+run_cmake_script(RENAME-arg-unknown)
+
 # tests are valid both for GLOB and GLOB_RECURSE
 run_cmake(GLOB-sort-dedup)
 run_cmake(GLOB-error-LIST_DIRECTORIES-not-boolean)