Merge topic 'cmake-E-symlink-replace-dangling'

e0228e2b cmake: Improve '-E create_symlink' edge case handling (#14713)

9 files changed, 56 insertions, 15 deletions

diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx
index 7891969..c1576c4 100644
--- a/Source/cmcmd.cxx
+++ b/Source/cmcmd.cxx
@@ -460,23 +460,25 @@ int cmcmd::ExecuteCMakeCommand(std::vector<std::string>& args)
     else if (args[1] == "create_symlink" && args.size() == 4)
       {
       const char* destinationFileName = args[3].c_str();
-      if ( cmSystemTools::FileExists(destinationFileName) )
+      if((cmSystemTools::FileExists(destinationFileName) ||
+          cmSystemTools::FileIsSymlink(destinationFileName)) &&
+         !cmSystemTools::RemoveFile(destinationFileName))
         {
-        if ( cmSystemTools::FileIsSymlink(destinationFileName) )
-          {
-          if ( !cmSystemTools::RemoveFile(destinationFileName) ||
-            cmSystemTools::FileExists(destinationFileName) )
-            {
-            return 0;
-            }
-          }
-        else
-          {
-          return 0;
-          }
+        std::string emsg = cmSystemTools::GetLastSystemError();
+        std::cerr <<
+          "failed to create symbolic link '" << destinationFileName <<
+          "' because existing path cannot be removed: " << emsg << "\n";
+        return 1;
+        }
+      if(!cmSystemTools::CreateSymlink(args[2].c_str(), args[3].c_str()))
+        {
+        std::string emsg = cmSystemTools::GetLastSystemError();
+        std::cerr <<
+          "failed to create symbolic link '" << destinationFileName <<
+          "': " << emsg << "\n";
+        return 1;
         }
-      return cmSystemTools::CreateSymlink(args[2].c_str(),
-                                          args[3].c_str())? 0:1;
+      return 0;
       }
 
     // Internal CMake shared library support.
diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt
index fc730a7..9646e67 100644
--- a/Tests/RunCMake/CMakeLists.txt
+++ b/Tests/RunCMake/CMakeLists.txt
@@ -111,5 +111,6 @@ add_RunCMake_test(File_Generate)
 add_RunCMake_test(ExportWithoutLanguage)
 add_RunCMake_test(target_link_libraries)
 add_RunCMake_test(CheckModules)
+add_RunCMake_test(CommandLine)
 
 add_RunCMake_test(install)
diff --git a/Tests/RunCMake/CommandLine/E_create_symlink-broken-create-check.cmake b/Tests/RunCMake/CommandLine/E_create_symlink-broken-create-check.cmake
new file mode 100644
index 0000000..d7e652d
--- /dev/null
+++ b/Tests/RunCMake/CommandLine/E_create_symlink-broken-create-check.cmake
@@ -0,0 +1,6 @@
+if(NOT IS_SYMLINK ${RunCMake_TEST_BINARY_DIR}/L)
+  set(RunCMake_TEST_FAILED "Symlink 'L' incorrectly not created!")
+endif()
+if(EXISTS ${RunCMake_TEST_BINARY_DIR}/L)
+  set(RunCMake_TEST_FAILED "Symlink 'L' not broken!")
+endif()
diff --git a/Tests/RunCMake/CommandLine/E_create_symlink-broken-replace-check.cmake b/Tests/RunCMake/CommandLine/E_create_symlink-broken-replace-check.cmake
new file mode 100644
index 0000000..c078ae8
--- /dev/null
+++ b/Tests/RunCMake/CommandLine/E_create_symlink-broken-replace-check.cmake
@@ -0,0 +1,3 @@
+if(NOT IS_DIRECTORY ${RunCMake_TEST_BINARY_DIR}/L)
+  set(RunCMake_TEST_FAILED "Symlink 'L' not replaced correctly!")
+endif()
diff --git a/Tests/RunCMake/CommandLine/E_create_symlink-missing-dir-result.txt b/Tests/RunCMake/CommandLine/E_create_symlink-missing-dir-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/CommandLine/E_create_symlink-missing-dir-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/CommandLine/E_create_symlink-missing-dir-stderr.txt b/Tests/RunCMake/CommandLine/E_create_symlink-missing-dir-stderr.txt
new file mode 100644
index 0000000..7a6d84b
--- /dev/null
+++ b/Tests/RunCMake/CommandLine/E_create_symlink-missing-dir-stderr.txt
@@ -0,0 +1 @@
+failed to create symbolic link 'missing-dir/L':
diff --git a/Tests/RunCMake/CommandLine/E_create_symlink-no-replace-dir-result.txt b/Tests/RunCMake/CommandLine/E_create_symlink-no-replace-dir-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/CommandLine/E_create_symlink-no-replace-dir-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/CommandLine/E_create_symlink-no-replace-dir-stderr.txt b/Tests/RunCMake/CommandLine/E_create_symlink-no-replace-dir-stderr.txt
new file mode 100644
index 0000000..ebed0d6
--- /dev/null
+++ b/Tests/RunCMake/CommandLine/E_create_symlink-no-replace-dir-stderr.txt
@@ -0,0 +1 @@
+failed to create symbolic link '\.' because existing path cannot be removed:
diff --git a/Tests/RunCMake/CommandLine/RunCMakeTest.cmake b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake
new file mode 100644
index 0000000..ee6cd99
--- /dev/null
+++ b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake
@@ -0,0 +1,25 @@
+include(RunCMake)
+
+if(UNIX)
+  run_cmake_command(E_create_symlink-missing-dir
+    ${CMAKE_COMMAND} -E create_symlink T missing-dir/L
+    )
+
+  # Use a single build tree for a few tests without cleaning.
+  set(RunCMake_TEST_BINARY_DIR
+    ${RunCMake_BINARY_DIR}/E_create_symlink-broken-build)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
+  run_cmake_command(E_create_symlink-broken-create
+    ${CMAKE_COMMAND} -E create_symlink T L
+    )
+  run_cmake_command(E_create_symlink-broken-replace
+    ${CMAKE_COMMAND} -E create_symlink . L
+    )
+  unset(RunCMake_TEST_BINARY_DIR)
+  unset(RunCMake_TEST_NO_CLEAN)
+
+  run_cmake_command(E_create_symlink-no-replace-dir
+    ${CMAKE_COMMAND} -E create_symlink T .
+    )
+endif()