From 57ca3dc521ef93b30ba51e9a6c871f09fdbb3e56 Mon Sep 17 00:00:00 2001 From: Tyler Yankee Date: Sun, 12 Apr 2026 20:21:59 -0400 Subject: [PATCH] file(CREATE_LINK): Fix CMP0205 to not remove content behind directory symlink When removing the destination directory link when it already exists before making the new link, only the (destination) link should be removed, not the (source) directory itself. When COPY_ON_ERROR is specified and the destination is a directory, only then should CMake remove it, since it could likely be placing a new directory there in its place. With this new behavior, adjust the conditions for the symlink test from commit a73ddd2ddb (file(CREATE_LINK): Implement COPY_ON_ERROR for directories, 2025-10-15, v4.3.0-rc1~599^2). The newly modified test case should be run on systems that do and don't support directory symlinks, with slightly different outcomes that are verified. Fixes: #27747 --- Source/cmFileCommand.cxx | 5 ++-- .../file-CREATE_LINK/CMP0205-common-NEW.cmake | 9 +++++++ .../file-CREATE_LINK/CMP0205-common-OLD.cmake | 13 ++++++++-- .../CMP0205-common-WARN.cmake | 13 ++++++++-- .../file-CREATE_LINK/CMP0205-common.cmake | 25 +++++++++++++++++++ .../file-CREATE_LINK/RunCMakeTest.cmake | 22 +++++----------- 6 files changed, 65 insertions(+), 22 deletions(-) diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index b504e761a7..f79cd39aa6 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -3255,8 +3255,9 @@ bool HandleCreateLinkCommand(std::vector 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); diff --git a/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-NEW.cmake b/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-NEW.cmake index 15e6a361c6..cd8d718526 100644 --- a/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-NEW.cmake +++ b/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() diff --git a/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-OLD.cmake b/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-OLD.cmake index 76add41239..ca65eea57c 100644 --- a/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-OLD.cmake +++ b/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() diff --git a/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-WARN.cmake b/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-WARN.cmake index ee940466b7..4e7dadc307 100644 --- a/Tests/RunCMake/file-CREATE_LINK/CMP0205-common-WARN.cmake +++ b/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() diff --git a/Tests/RunCMake/file-CREATE_LINK/CMP0205-common.cmake b/Tests/RunCMake/file-CREATE_LINK/CMP0205-common.cmake index deb313f9a5..3886a76909 100644 --- a/Tests/RunCMake/file-CREATE_LINK/CMP0205-common.cmake +++ b/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}/*") diff --git a/Tests/RunCMake/file-CREATE_LINK/RunCMakeTest.cmake b/Tests/RunCMake/file-CREATE_LINK/RunCMakeTest.cmake index ed83312207..5d5b4ebf7f 100644 --- a/Tests/RunCMake/file-CREATE_LINK/RunCMakeTest.cmake +++ b/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