From e0228e2b04b3d6bf72304038c8621fc5072e7f4e Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 20 Jan 2014 13:50:31 -0500 Subject: [PATCH] cmake: Improve '-E create_symlink' edge case handling (#14713) The logic added by commit ffc0b5e4 (Overwrite the symlink if it already exists, 2007-02-15) does not recognize and remove existing broken links before replacing them. Improve the logic to remove any existing destination file or link (but not directory). On failure, report an error message explaining why the existing path could not be removed or the new one could not be created. Add a RunCMake.CommandLine test to cover 'cmake -E' cases. Start with test cases covering 'cmake -E create_symlink' behavior on UNIX platforms. --- Source/cmcmd.cxx | 32 ++++++++++--------- Tests/RunCMake/CMakeLists.txt | 1 + ...E_create_symlink-broken-create-check.cmake | 6 ++++ ..._create_symlink-broken-replace-check.cmake | 3 ++ .../E_create_symlink-missing-dir-result.txt | 1 + .../E_create_symlink-missing-dir-stderr.txt | 1 + ...E_create_symlink-no-replace-dir-result.txt | 1 + ...E_create_symlink-no-replace-dir-stderr.txt | 1 + Tests/RunCMake/CommandLine/RunCMakeTest.cmake | 25 +++++++++++++++ 9 files changed, 56 insertions(+), 15 deletions(-) create mode 100644 Tests/RunCMake/CommandLine/E_create_symlink-broken-create-check.cmake create mode 100644 Tests/RunCMake/CommandLine/E_create_symlink-broken-replace-check.cmake create mode 100644 Tests/RunCMake/CommandLine/E_create_symlink-missing-dir-result.txt create mode 100644 Tests/RunCMake/CommandLine/E_create_symlink-missing-dir-stderr.txt create mode 100644 Tests/RunCMake/CommandLine/E_create_symlink-no-replace-dir-result.txt create mode 100644 Tests/RunCMake/CommandLine/E_create_symlink-no-replace-dir-stderr.txt create mode 100644 Tests/RunCMake/CommandLine/RunCMakeTest.cmake diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx index 789196967b..c1576c4fe8 100644 --- a/Source/cmcmd.cxx +++ b/Source/cmcmd.cxx @@ -460,23 +460,25 @@ int cmcmd::ExecuteCMakeCommand(std::vector& 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; } - return cmSystemTools::CreateSymlink(args[2].c_str(), - args[3].c_str())? 0: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 0; } // Internal CMake shared library support. diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index fc730a7700..9646e679cd 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 0000000000..d7e652de73 --- /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 0000000000..c078ae8fc2 --- /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 0000000000..d00491fd7e --- /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 0000000000..7a6d84b2c1 --- /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 0000000000..d00491fd7e --- /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 0000000000..ebed0d67c9 --- /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 0000000000..ee6cd99f8d --- /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()