Merge topic 'file-link-copy-fix'

57ca3dc521 file(CREATE_LINK): Fix CMP0205 to not remove content behind directory symlink
a6470b4681 Help/file: Clarify behavior change with CMP0205
b894942d3e file(CREATE_LINK): Revise CMP0205 warning

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

Help/command/file.rst

@@ -606,7 +606,7 @@ Filesystem
 
   Create a link ``<linkname>`` that points to ``<original>``.
   It will be a hard link by default, but providing the ``SYMBOLIC`` option
-  results in a symbolic link instead.  Hard links require that ``original``
+  results in a symbolic link instead.  Hard links require that ``<original>``
   exists and is a file, not a directory.  If ``<linkname>`` already exists,
   it will be overwritten.
 
@@ -619,9 +619,13 @@ Filesystem
   creating the link fails.  It can be useful for handling situations such as
   ``<original>`` and ``<linkname>`` being on different drives or mount points,
   which would make them unable to support a hard link.
-  If the source is a directory, the destination directory will be created if
-  it does not exist.  Contents of the source directory will be copied to the
-  destination directory unless policy :policy:`CMP0205` is not set to ``NEW``.
+
+  .. versionchanged:: 4.3
+
+    If the source is a directory, CMake versions prior to 4.3 will create the
+    destination directory if it does not exist, but not copy any files.
+    With CMake 4.3 and above, the contents of the source directory will be
+    copied recursively to the destination.  See policy :policy:`CMP0205`.
 
 .. signature::
   file(CHMOD <files>... <directories>...

Source/cmFileCommand.cxx

@@ -3255,8 +3255,9 @@ bool HandleCreateLinkCommand(std::vector<std::string> const& args,
   // Check if the new file already exists and remove it.
   if (cmSystemTools::PathExists(newFileName)) {
     cmsys::Status rmStatus;
-    if (cmp0205 == cmPolicies::NEW &&
-        cmSystemTools::FileIsDirectory(newFileName)) {
+    if (cmp0205 == cmPolicies::NEW && arguments.CopyOnError &&
+        cmSystemTools::FileIsDirectory(newFileName) &&
+        !cmSystemTools::FileIsSymlink(newFileName)) {
       rmStatus = cmSystemTools::RepeatedRemoveDirectory(newFileName);
     } else {
       rmStatus = cmSystemTools::RemoveFile(newFileName);
@@ -3295,12 +3296,12 @@ bool HandleCreateLinkCommand(std::vector<std::string> const& args,
     if (sourceIsDirectory) {
       if (cmp0205 == cmPolicies::NEW) {
         needToTry = false;
-      } else if (cmp0205 == cmPolicies::WARN) {
+      } else if (cmp0205 == cmPolicies::WARN && arguments.CopyOnError) {
         status.GetMakefile().IssueMessage(
           MessageType::AUTHOR_WARNING,
           cmStrCat("Path\n  ", fileName,
-                   "\nis directory. Hardlinks creation is not supported for "
-                   "directories.\n",
+                   "\nis a directory. Hard link creation is not supported "
+                   "for directories.\n",
                    cmPolicies::GetPolicyWarning(cmPolicies::CMP0205)));
       }
     }
@@ -3320,13 +3321,13 @@ bool HandleCreateLinkCommand(std::vector<std::string> const& args,
     }
   }
 
-  if (arguments.CopyOnError && cmp0205 == cmPolicies::WARN &&
+  if (cmp0205 == cmPolicies::WARN && arguments.CopyOnError &&
       sourceIsDirectory) {
     status.GetMakefile().IssueMessage(
       MessageType::AUTHOR_WARNING,
       cmStrCat("Path\n  ", fileName,
-               "\nis directory. It will be copied recursively when NEW policy "
-               "behavior applies for CMP0205.\n",
+               "\nis a directory. It will be copied "
+               "recursively when CMP0205 is set to NEW.\n",
                cmPolicies::GetPolicyWarning(cmPolicies::CMP0205)));
   }
 

Tests/RunCMake/file-CREATE_LINK/CMP0205-HardLink-WARN-stderr.txt

@@ -5,7 +5,7 @@
     [^
 ]*[\\|/]file-CREATE_LINK[\\|/]CMP0205
 
-  is directory.  Hardlinks creation is not supported for directories.
+  is a directory.  Hard link creation is not supported for directories.
 
   Policy CMP0205 is not set: file\(CREATE_LINK\) with COPY_ON_ERROR copies
   directory content\.  Run "cmake --help-policy CMP0205" for policy details\.
@@ -24,8 +24,7 @@ CMake Warning \(dev\) at [^
     [^
 ]*[\\|/]file-CREATE_LINK[\\|/]CMP0205
 
-  is directory.  It will be copied recursively when NEW policy behavior
-  applies for CMP0205\.
+  is a directory.  It will be copied recursively when CMP0205 is set to NEW\.
 
   Policy CMP0205 is not set: file\(CREATE_LINK\) with COPY_ON_ERROR copies
   directory content\.  Run "cmake --help-policy CMP0205" for policy details\.

Tests/RunCMake/file-CREATE_LINK/CMP0205-SymLink-WARN-stderr.txt

@@ -5,8 +5,7 @@
     [^
 ]*[\\|/]file-CREATE_LINK[\\|/]CMP0205
 
-  is directory.  It will be copied recursively when NEW policy behavior
-  applies for CMP0205\.
+  is a directory.  It will be copied recursively when CMP0205 is set to NEW\.
 
   Policy CMP0205 is not set: file\(CREATE_LINK\) with COPY_ON_ERROR copies
   directory content\.  Run "cmake --help-policy CMP0205" for policy details\.

Tests/RunCMake/file-CREATE_LINK/CMP0205-common-NEW.cmake

@@ -1,6 +1,15 @@
 cmake_policy(SET CMP0205 NEW)
 include("${CMAKE_CURRENT_LIST_DIR}/CMP0205-common.cmake")
 
+# Note that unlike the tests for CMP0205 OLD and WARN, the resulting files
+# in the source and destination should be the same here regardless of whether
+# COPY_ON_ERROR was actually executed (i.e, whether through the link, or actual
+# files).
+
+if(NOT allFilesSrc)
+  message(SEND_ERROR "Source directory is empty: '${allFilesSrc}'")
+endif()
+
 if(NOT allFilesDst)
   message(SEND_ERROR "Destination directory is empty: '${allFilesDst}'")
 endif()

Tests/RunCMake/file-CREATE_LINK/CMP0205-common-OLD.cmake

@@ -1,6 +1,15 @@
 cmake_policy(SET CMP0205 OLD)
 include("${CMAKE_CURRENT_LIST_DIR}/CMP0205-common.cmake")
 
-if(allFilesDst)
-  message(SEND_ERROR "Directory is not empty: '${allFilesDst}'")
+# We only really care about when COPY_ON_ERROR was actually executed, but we'll
+# test both cases for posterity.
+
+if(NOT madeSymlink)
+  if(allFilesDst)
+    message(SEND_ERROR "Directory is not empty: '${allFilesDst}'")
+  endif()
+else()
+  if(NOT allFilesDst)
+    message(SEND_ERROR "Destination directory is empty: '${allFilesDst}'")
+  endif()
 endif()

Tests/RunCMake/file-CREATE_LINK/CMP0205-common-WARN.cmake

@@ -1,6 +1,15 @@
 # CMP0205 is unset
 include("${CMAKE_CURRENT_LIST_DIR}/CMP0205-common.cmake")
 
-if(allFilesDst)
-  message(SEND_ERROR "Directory is not empty: '${allFilesDst}'")
+# We only really care about when COPY_ON_ERROR was actually executed, but we'll
+# test both cases for posterity.
+
+if(NOT madeSymlink)
+  if(allFilesDst)
+    message(SEND_ERROR "Directory is not empty: '${allFilesDst}'")
+  endif()
+else()
+  if(NOT allFilesDst)
+    message(SEND_ERROR "Destination directory is empty: '${allFilesDst}'")
+  endif()
 endif()

Tests/RunCMake/file-CREATE_LINK/CMP0205-common.cmake

@@ -10,5 +10,30 @@ if(NOT result STREQUAL "0")
   message(SEND_ERROR "COPY_ON_ERROR failed: '${result}'")
 endif()
 
+# When CMP0205 is NEW, we must verify after running command this again that:
+# * on systems which support directory symlinks, the source directory to which
+#   the newly-created link points is not deleted, only the symlink itself.
+# * on systems which do not support directory symlinks, the destination
+#   directory which was created via COPY_ON_ERROR is appropriately deleted
+#   at the beginning of executing this command, before creating the new link
+#   (and copying instead, again).
+cmake_policy(GET CMP0205 _cmp0205)
+if("${maybe_SYMBOLIC}" STREQUAL "SYMBOLIC" AND "${_cmp0205}" STREQUAL "NEW")
+  file(CREATE_LINK
+    ${CMAKE_CURRENT_LIST_DIR}/CMP0205 ${CMAKE_CURRENT_BINARY_DIR}/CMP0205-${link_name}
+    ${maybe_SYMBOLIC}
+    RESULT result
+    COPY_ON_ERROR
+    )
+  if(NOT result STREQUAL "0")
+    message(SEND_ERROR "COPY_ON_ERROR failed: '${result}'")
+  endif()
+endif()
+
+set(madeSymlink OFF)
+if(IS_SYMLINK ${CMAKE_CURRENT_BINARY_DIR}/CMP0205-${link_name})
+  set(madeSymlink ON)
+endif()
+
 file(GLOB_RECURSE allFilesSrc LIST_DIRECTORIES true RELATIVE "${CMAKE_CURRENT_LIST_DIR}/CMP0205" "${CMAKE_CURRENT_LIST_DIR}/CMP0205/*")
 file(GLOB_RECURSE allFilesDst LIST_DIRECTORIES true RELATIVE "${CMAKE_CURRENT_BINARY_DIR}/CMP0205-${link_name}" "${CMAKE_CURRENT_BINARY_DIR}/CMP0205-${link_name}/*")

Tests/RunCMake/file-CREATE_LINK/RunCMakeTest.cmake

@@ -12,23 +12,13 @@ if(NOT WIN32
   run_cmake(CREATE_LINK-SYMBOLIC-noexist)
 endif()
 
+run_cmake_script(CMP0205-SymLink-WARN)
+run_cmake_script(CMP0205-SymLink-OLD)
+run_cmake_script(CMP0205-SymLink-NEW)
+
+# Some older versions of macOS with HFS+ filesystems support directory hard
+# links. Inspect whether this test case is applicable on the current system.
 file(MAKE_DIRECTORY ${RunCMake_BINARY_DIR}/CMP0205-Inspect/Dest)
-
-file(REMOVE_RECURSE ${RunCMake_BINARY_DIR}/CMP0205-Inspect-SymLink)
-file(CREATE_LINK
-  ${RunCMake_BINARY_DIR}/CMP0205-Inspect/Dest ${RunCMake_BINARY_DIR}/CMP0205-Inspect-SymLink
-  SYMBOLIC
-  RESULT SymLink_RESULT
-)
-if(SymLink_RESULT STREQUAL "0")
-  message(STATUS "CMP0205-SymLink-* skipped: directory symbolic link creation works")
-  file(REMOVE ${RunCMake_BINARY_DIR}/CMP0205-Inspect-SymLink)
-else()
-  run_cmake_script(CMP0205-SymLink-WARN)
-  run_cmake_script(CMP0205-SymLink-OLD)
-  run_cmake_script(CMP0205-SymLink-NEW)
-endif()
-
 file(REMOVE_RECURSE ${RunCMake_BINARY_DIR}/CMP0205-Inspect-HardLink)
 file(CREATE_LINK
   ${RunCMake_BINARY_DIR}/CMP0205-Inspect/Dest ${RunCMake_BINARY_DIR}/CMP0205-Inspect-HardLink