From aba48bd6acbf564dcb5b831b26cc01743b5b5f71 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 6 Oct 2022 13:44:27 -0400 Subject: cmSystemTools: Provide quiet link creation methods Offer variants that let the caller handle error messages. --- Source/cmFileCommand.cxx | 20 ++++++++++++++++---- Source/cmSystemTools.cxx | 43 +++++++++++++++++++++++++------------------ Source/cmSystemTools.h | 10 ++++++---- Source/cmcmd.cxx | 7 +++---- 4 files changed, 50 insertions(+), 30 deletions(-) diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index bec9fb2..421ff12 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -2960,11 +2960,23 @@ bool HandleCreateLinkCommand(std::vector const& args, // Check if the command requires a symbolic link. if (arguments.Symbolic) { - completed = static_cast( - cmSystemTools::CreateSymlink(fileName, newFileName, &result)); + cmsys::Status linked = + cmSystemTools::CreateSymlinkQuietly(fileName, newFileName); + if (linked) { + completed = true; + } else { + result = cmStrCat("failed to create symbolic link '", newFileName, + "': ", linked.GetString()); + } } else { - completed = static_cast( - cmSystemTools::CreateLink(fileName, newFileName, &result)); + cmsys::Status linked = + cmSystemTools::CreateLinkQuietly(fileName, newFileName); + if (linked) { + completed = true; + } else { + result = cmStrCat("failed to create link '", newFileName, + "': ", linked.GetString()); + } } // Check if copy-on-error is enabled in the arguments. diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 5737cc1..ee74908 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -3590,8 +3590,19 @@ std::string cmSystemTools::EncodeURL(std::string const& in, bool escapeSlashes) } cmsys::Status cmSystemTools::CreateSymlink(std::string const& origName, - std::string const& newName, - std::string* errorMessage) + std::string const& newName) +{ + cmsys::Status status = + cmSystemTools::CreateSymlinkQuietly(origName, newName); + if (!status) { + cmSystemTools::Error(cmStrCat("failed to create symbolic link '", newName, + "': ", status.GetString())); + } + return status; +} + +cmsys::Status cmSystemTools::CreateSymlinkQuietly(std::string const& origName, + std::string const& newName) { uv_fs_t req; int flags = 0; @@ -3611,20 +3622,23 @@ cmsys::Status cmSystemTools::CreateSymlink(std::string const& origName, #else status = cmsys::Status::POSIX(-err); #endif - std::string e = cmStrCat("failed to create symbolic link '", newName, - "': ", status.GetString()); - if (errorMessage) { - *errorMessage = std::move(e); - } else { - cmSystemTools::Error(e); - } } return status; } cmsys::Status cmSystemTools::CreateLink(std::string const& origName, - std::string const& newName, - std::string* errorMessage) + std::string const& newName) +{ + cmsys::Status status = cmSystemTools::CreateLinkQuietly(origName, newName); + if (!status) { + cmSystemTools::Error( + cmStrCat("failed to create link '", newName, "': ", status.GetString())); + } + return status; +} + +cmsys::Status cmSystemTools::CreateLinkQuietly(std::string const& origName, + std::string const& newName) { uv_fs_t req; int err = @@ -3638,13 +3652,6 @@ cmsys::Status cmSystemTools::CreateLink(std::string const& origName, #else status = cmsys::Status::POSIX(-err); #endif - std::string e = - cmStrCat("failed to create link '", newName, "': ", status.GetString()); - if (errorMessage) { - *errorMessage = std::move(e); - } else { - cmSystemTools::Error(e); - } } return status; } diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index b02a977..87b354c 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -595,14 +595,16 @@ public: /** Create a symbolic link if the platform supports it. Returns whether creation succeeded. */ static cmsys::Status CreateSymlink(std::string const& origName, - std::string const& newName, - std::string* errorMessage = nullptr); + std::string const& newName); + static cmsys::Status CreateSymlinkQuietly(std::string const& origName, + std::string const& newName); /** Create a hard link if the platform supports it. Returns whether creation succeeded. */ static cmsys::Status CreateLink(std::string const& origName, - std::string const& newName, - std::string* errorMessage = nullptr); + std::string const& newName); + static cmsys::Status CreateLinkQuietly(std::string const& origName, + std::string const& newName); /** Get the system name. */ static cm::string_view GetSystemName(); diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx index 00c9bda..16944e6 100644 --- a/Source/cmcmd.cxx +++ b/Source/cmcmd.cxx @@ -1700,16 +1700,15 @@ cmsys::Status cmcmd::SymlinkInternal(std::string const& file, } std::string linktext = cmSystemTools::GetFilenameName(file); #if defined(_WIN32) && !defined(__CYGWIN__) - std::string errorMessage; - cmsys::Status status = - cmSystemTools::CreateSymlink(linktext, link, &errorMessage); + cmsys::Status status = cmSystemTools::CreateSymlinkQuietly(linktext, link); // Creating a symlink will fail with ERROR_PRIVILEGE_NOT_HELD if the user // does not have SeCreateSymbolicLinkPrivilege, or if developer mode is not // active. In that case, we try to copy the file. if (status.GetWindows() == ERROR_PRIVILEGE_NOT_HELD) { status = cmSystemTools::CopyFileAlways(file, link); } else if (!status) { - cmSystemTools::Error(errorMessage); + cmSystemTools::Error(cmStrCat("failed to create symbolic link '", link, + "': ", status.GetString())); } return status; #else -- cgit v0.12 From 85f01a1ec2203187d4cd99c74136774059b89b35 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 6 Oct 2022 12:48:15 -0400 Subject: file(INSTALL): Improve formatting of symlink creation error Avoid printing two error messages. Format paths without wrapping. --- Source/cmFileCopier.cxx | 24 ++++++++++++---------- .../DIRECTORY-symlink-clobber-all-result.txt | 1 + .../DIRECTORY-symlink-clobber-all-stderr.txt | 12 +++++++++++ .../DIRECTORY-symlink-clobber-all-stdout.txt | 12 +++++++++++ .../install/DIRECTORY-symlink-clobber.cmake | 11 ++++++++++ Tests/RunCMake/install/RunCMakeTest.cmake | 4 ++++ 6 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-result.txt create mode 100644 Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stderr.txt create mode 100644 Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt create mode 100644 Tests/RunCMake/install/DIRECTORY-symlink-clobber.cmake diff --git a/Source/cmFileCopier.cxx b/Source/cmFileCopier.cxx index 1667807..6ad7706 100644 --- a/Source/cmFileCopier.cxx +++ b/Source/cmFileCopier.cxx @@ -504,11 +504,12 @@ bool cmFileCopier::InstallSymlinkChain(std::string& fromFile, cmSystemTools::RemoveFile(toFile); cmSystemTools::MakeDirectory(toFilePath); - if (!cmSystemTools::CreateSymlink(symlinkTarget, toFile)) { - std::ostringstream e; - e << this->Name << " cannot create symlink \"" << toFile - << "\": " << cmSystemTools::GetLastSystemError() << "."; - this->Status.SetError(e.str()); + cmsys::Status status = + cmSystemTools::CreateSymlinkQuietly(symlinkTarget, toFile); + if (!status) { + std::string e = cmStrCat(this->Name, " cannot create symlink\n ", + toFile, "\nbecause: ", status.GetString()); + this->Status.SetError(e); return false; } } @@ -557,12 +558,13 @@ bool cmFileCopier::InstallSymlink(const std::string& fromFile, cmSystemTools::MakeDirectory(cmSystemTools::GetFilenamePath(toFile)); // Create the symlink. - if (!cmSystemTools::CreateSymlink(symlinkTarget, toFile)) { - std::ostringstream e; - e << this->Name << " cannot duplicate symlink \"" << fromFile - << "\" at \"" << toFile - << "\": " << cmSystemTools::GetLastSystemError() << "."; - this->Status.SetError(e.str()); + cmsys::Status status = + cmSystemTools::CreateSymlinkQuietly(symlinkTarget, toFile); + if (!status) { + std::string e = + cmStrCat(this->Name, " cannot duplicate symlink\n ", fromFile, + "\nat\n ", toFile, "\nbecause: ", status.GetString()); + this->Status.SetError(e); return false; } } diff --git a/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-result.txt b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stderr.txt b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stderr.txt new file mode 100644 index 0000000..cdde3e0 --- /dev/null +++ b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stderr.txt @@ -0,0 +1,12 @@ +^CMake Error at cmake_install.cmake:[0-9]+ \(file\): + file INSTALL cannot duplicate symlink + + [^ +]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/new/dir + + at + + [^ +]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/dir + + because: diff --git a/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt new file mode 100644 index 0000000..e69ef02 --- /dev/null +++ b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt @@ -0,0 +1,12 @@ +-- Installing: [^ +]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/dir +-- Installing: [^ +]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/dir/file +-- Installing: [^ +]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/lnk +-- Up-to-date: [^ +]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/lnk +-- Up-to-date: [^ +]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/lnk/file +-- Installing: [^ +]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/dir diff --git a/Tests/RunCMake/install/DIRECTORY-symlink-clobber.cmake b/Tests/RunCMake/install/DIRECTORY-symlink-clobber.cmake new file mode 100644 index 0000000..ac7a2cf --- /dev/null +++ b/Tests/RunCMake/install/DIRECTORY-symlink-clobber.cmake @@ -0,0 +1,11 @@ +file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/old/dir) +file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/old/dir/file "") +file(CREATE_LINK dir ${CMAKE_CURRENT_BINARY_DIR}/old/lnk SYMBOLIC) +install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/old/dir DESTINATION dest) +install(FILES ${CMAKE_CURRENT_BINARY_DIR}/old/lnk DESTINATION dest) + +file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/new/lnk) +file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/new/lnk/file "") +file(CREATE_LINK lnk ${CMAKE_CURRENT_BINARY_DIR}/new/dir SYMBOLIC) +install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/new/lnk DESTINATION dest) +install(FILES ${CMAKE_CURRENT_BINARY_DIR}/new/dir DESTINATION dest) diff --git a/Tests/RunCMake/install/RunCMakeTest.cmake b/Tests/RunCMake/install/RunCMakeTest.cmake index 7c12d4a..477ffe0 100644 --- a/Tests/RunCMake/install/RunCMakeTest.cmake +++ b/Tests/RunCMake/install/RunCMakeTest.cmake @@ -175,6 +175,10 @@ run_install_test(FILES-PERMISSIONS) run_install_test(TARGETS-RPATH) run_install_test(InstallRequiredSystemLibraries) +if(UNIX) + run_install_test(DIRECTORY-symlink-clobber) +endif() + if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") run_cmake(TARGETS-RUNTIME_DEPENDENCIES-macos-two-bundle) run_cmake(TARGETS-RUNTIME_DEPENDENCIES-macos-no-framework) -- cgit v0.12 From 1461ae49330595b7e7d59e793107dd809bb56720 Mon Sep 17 00:00:00 2001 From: John Parent Date: Thu, 22 Sep 2022 12:54:22 -0400 Subject: file(INSTALL): Clarify symlink vs dir conflict errors Clarify error reporting in scenario creating a symlink where a directory previously exists. --- Source/cmFileCopier.cxx | 17 ++++++++++++++++- .../install/DIRECTORY-symlink-clobber-all-stderr.txt | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Source/cmFileCopier.cxx b/Source/cmFileCopier.cxx index 6ad7706..70e6089 100644 --- a/Source/cmFileCopier.cxx +++ b/Source/cmFileCopier.cxx @@ -15,7 +15,11 @@ #include "cmValue.h" #ifdef _WIN32 +# include + # include "cmsys/FStream.hxx" +#else +# include #endif #include @@ -561,9 +565,20 @@ bool cmFileCopier::InstallSymlink(const std::string& fromFile, cmsys::Status status = cmSystemTools::CreateSymlinkQuietly(symlinkTarget, toFile); if (!status) { +#ifdef _WIN32 + bool const errorFileExists = status.GetWindows() == ERROR_FILE_EXISTS; +#else + bool const errorFileExists = status.GetPOSIX() == EEXIST; +#endif + std::string reason; + if (errorFileExists && cmSystemTools::FileIsDirectory(toFile)) { + reason = "A directory already exists at that location"; + } else { + reason = status.GetString(); + } std::string e = cmStrCat(this->Name, " cannot duplicate symlink\n ", fromFile, - "\nat\n ", toFile, "\nbecause: ", status.GetString()); + "\nat\n ", toFile, "\nbecause: ", reason); this->Status.SetError(e); return false; } diff --git a/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stderr.txt b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stderr.txt index cdde3e0..d082b8a 100644 --- a/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stderr.txt +++ b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stderr.txt @@ -9,4 +9,4 @@ [^ ]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/dir - because: + because: A directory already exists at that location -- cgit v0.12 From 569fb1893e6d49677e3724b82e578b568aaa1504 Mon Sep 17 00:00:00 2001 From: John Parent Date: Thu, 22 Sep 2022 12:54:22 -0400 Subject: file(INSTALL): Report "Installing:" for a symlink to a directory --- Source/cmFileCopier.cxx | 5 ++++- Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Source/cmFileCopier.cxx b/Source/cmFileCopier.cxx index 70e6089..ef55abf 100644 --- a/Source/cmFileCopier.cxx +++ b/Source/cmFileCopier.cxx @@ -647,7 +647,10 @@ bool cmFileCopier::InstallDirectory(const std::string& source, { // Inform the user about this directory installation. this->ReportCopy(destination, TypeDir, - !cmSystemTools::FileIsDirectory(destination)); + !( // Report "Up-to-date:" for existing directories, + // but not symlinks to them. + cmSystemTools::FileIsDirectory(destination) && + !cmSystemTools::FileIsSymlink(destination))); // check if default dir creation permissions were set mode_t default_dir_mode_v = 0; diff --git a/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt index e69ef02..db8ca10 100644 --- a/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt +++ b/Tests/RunCMake/install/DIRECTORY-symlink-clobber-all-stdout.txt @@ -4,7 +4,7 @@ ]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/dir/file -- Installing: [^ ]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/lnk --- Up-to-date: [^ +-- Installing: [^ ]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/lnk -- Up-to-date: [^ ]*/Tests/RunCMake/install/DIRECTORY-symlink-clobber-build/root-all/dest/lnk/file -- cgit v0.12