From a9c659b1b6165b1bf2729fec5b0b929a9f2db078 Mon Sep 17 00:00:00 2001 From: KWSys Upstream Date: Tue, 15 Nov 2022 07:51:35 -0500 Subject: KWSys 2022-11-15 (6c92b9b0) Code extracted from: https://gitlab.kitware.com/utils/kwsys.git at commit 6c92b9b0e7a57380a86c5dc4a50fe633b72fe66a (master). Upstream Shortlog ----------------- Brad King (1): 30e10c87 SystemTools: Report with copy operation failures which path failed --- SystemTools.cxx | 60 +++++++++++++++++++++++++++++------------------------- SystemTools.hxx.in | 44 ++++++++++++++++++++++++++++++--------- 2 files changed, 66 insertions(+), 38 deletions(-) diff --git a/SystemTools.cxx b/SystemTools.cxx index fdd6b2d..a3ab51a 100644 --- a/SystemTools.cxx +++ b/SystemTools.cxx @@ -2290,8 +2290,8 @@ static std::string FileInDir(const std::string& source, const std::string& dir) return new_destination + '/' + SystemTools::GetFilenameName(source); } -Status SystemTools::CopyFileIfDifferent(std::string const& source, - std::string const& destination) +SystemTools::CopyStatus SystemTools::CopyFileIfDifferent( + std::string const& source, std::string const& destination) { // special check for a destination that is a directory // FilesDiffer does not handle file to directory compare @@ -2308,7 +2308,7 @@ Status SystemTools::CopyFileIfDifferent(std::string const& source, } } // at this point the files must be the same so return true - return Status::Success(); + return CopyStatus{ Status::Success(), CopyStatus::NoPath }; } #define KWSYS_ST_BUFFER 4096 @@ -2434,13 +2434,13 @@ bool SystemTools::TextFilesDiffer(const std::string& path1, return false; } -Status SystemTools::CopyFileContentBlockwise(std::string const& source, - std::string const& destination) +SystemTools::CopyStatus SystemTools::CopyFileContentBlockwise( + std::string const& source, std::string const& destination) { // Open files kwsys::ifstream fin(source.c_str(), std::ios::in | std::ios::binary); if (!fin) { - return Status::POSIX_errno(); + return CopyStatus{ Status::POSIX_errno(), CopyStatus::SourcePath }; } // try and remove the destination file so that read only destination files @@ -2452,7 +2452,7 @@ Status SystemTools::CopyFileContentBlockwise(std::string const& source, kwsys::ofstream fout(destination.c_str(), std::ios::out | std::ios::trunc | std::ios::binary); if (!fout) { - return Status::POSIX_errno(); + return CopyStatus{ Status::POSIX_errno(), CopyStatus::DestPath }; } // This copy loop is very sensitive on certain platforms with @@ -2481,10 +2481,10 @@ Status SystemTools::CopyFileContentBlockwise(std::string const& source, fout.close(); if (!fout) { - return Status::POSIX_errno(); + return CopyStatus{ Status::POSIX_errno(), CopyStatus::DestPath }; } - return Status::Success(); + return CopyStatus{ Status::Success(), CopyStatus::NoPath }; } /** @@ -2499,13 +2499,13 @@ Status SystemTools::CopyFileContentBlockwise(std::string const& source, * - The underlying filesystem does not support file cloning * - An unspecified error occurred */ -Status SystemTools::CloneFileContent(std::string const& source, - std::string const& destination) +SystemTools::CopyStatus SystemTools::CloneFileContent( + std::string const& source, std::string const& destination) { #if defined(__linux) && defined(FICLONE) int in = open(source.c_str(), O_RDONLY); if (in < 0) { - return Status::POSIX_errno(); + return CopyStatus{ Status::POSIX_errno(), CopyStatus::SourcePath }; } SystemTools::RemoveFile(destination); @@ -2513,14 +2513,14 @@ Status SystemTools::CloneFileContent(std::string const& source, int out = open(destination.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR); if (out < 0) { - Status status = Status::POSIX_errno(); + CopyStatus status{ Status::POSIX_errno(), CopyStatus::DestPath }; close(in); return status; } - Status status = Status::Success(); + CopyStatus status{ Status::Success(), CopyStatus::NoPath }; if (ioctl(out, FICLONE, in) < 0) { - status = Status::POSIX_errno(); + status = CopyStatus{ Status::POSIX_errno(), CopyStatus::NoPath }; } close(in); close(out); @@ -2532,40 +2532,41 @@ Status SystemTools::CloneFileContent(std::string const& source, // be updated by `copy_file_if_different` and `copy_file`. if (copyfile(source.c_str(), destination.c_str(), nullptr, COPYFILE_METADATA | COPYFILE_CLONE) < 0) { - return Status::POSIX_errno(); + return CopyStatus{ Status::POSIX_errno(), CopyStatus::NoPath }; } # if KWSYS_CXX_HAS_UTIMENSAT // utimensat is only available on newer Unixes and macOS 10.13+ if (utimensat(AT_FDCWD, destination.c_str(), nullptr, 0) < 0) { - return Status::POSIX_errno(); + return CopyStatus{ Status::POSIX_errno(), CopyStatus::DestPath }; } # else // fall back to utimes if (utimes(destination.c_str(), nullptr) < 0) { - return Status::POSIX_errno(); + return CopyStatus{ Status::POSIX_errno(), CopyStatus::DestPath }; } # endif - return Status::Success(); + return CopyStatus{ Status::Success(), CopyStatus::NoPath }; #else (void)source; (void)destination; - return Status::POSIX(ENOSYS); + return CopyStatus{ Status::POSIX(ENOSYS), CopyStatus::NoPath }; #endif } /** * Copy a file named by "source" to the file named by "destination". */ -Status SystemTools::CopyFileAlways(std::string const& source, - std::string const& destination) +SystemTools::CopyStatus SystemTools::CopyFileAlways( + std::string const& source, std::string const& destination) { - Status status; + CopyStatus status; mode_t perm = 0; Status perms = SystemTools::GetPermissions(source, perm); std::string real_destination = destination; if (SystemTools::FileIsDirectory(source)) { - status = SystemTools::MakeDirectory(destination); + status = CopyStatus{ SystemTools::MakeDirectory(destination), + CopyStatus::DestPath }; if (!status.IsSuccess()) { return status; } @@ -2590,7 +2591,8 @@ Status SystemTools::CopyFileAlways(std::string const& source, // Create destination directory if (!destination_dir.empty()) { - status = SystemTools::MakeDirectory(destination_dir); + status = CopyStatus{ SystemTools::MakeDirectory(destination_dir), + CopyStatus::DestPath }; if (!status.IsSuccess()) { return status; } @@ -2606,13 +2608,15 @@ Status SystemTools::CopyFileAlways(std::string const& source, } } if (perms) { - status = SystemTools::SetPermissions(real_destination, perm); + status = CopyStatus{ SystemTools::SetPermissions(real_destination, perm), + CopyStatus::DestPath }; } return status; } -Status SystemTools::CopyAFile(std::string const& source, - std::string const& destination, bool always) +SystemTools::CopyStatus SystemTools::CopyAFile(std::string const& source, + std::string const& destination, + bool always) { if (always) { return SystemTools::CopyFileAlways(source, destination); diff --git a/SystemTools.hxx.in b/SystemTools.hxx.in index acce015..56b65fd 100644 --- a/SystemTools.hxx.in +++ b/SystemTools.hxx.in @@ -566,11 +566,34 @@ public: const mode_t* mode = nullptr); /** + * Represent the result of a file copy operation. + * This is the result 'Status' and, if the operation failed, + * an indication of whether the error occurred on the source + * or destination path. + */ + struct CopyStatus : public Status + { + enum WhichPath + { + NoPath, + SourcePath, + DestPath, + }; + CopyStatus() = default; + CopyStatus(Status s, WhichPath p) + : Status(s) + , Path(p) + { + } + WhichPath Path = NoPath; + }; + + /** * Copy the source file to the destination file only * if the two files differ. */ - static Status CopyFileIfDifferent(std::string const& source, - std::string const& destination); + static CopyStatus CopyFileIfDifferent(std::string const& source, + std::string const& destination); /** * Compare the contents of two files. Return true if different @@ -588,13 +611,13 @@ public: /** * Blockwise copy source to destination file */ - static Status CopyFileContentBlockwise(std::string const& source, - std::string const& destination); + static CopyStatus CopyFileContentBlockwise(std::string const& source, + std::string const& destination); /** * Clone the source file to the destination file */ - static Status CloneFileContent(std::string const& source, - std::string const& destination); + static CopyStatus CloneFileContent(std::string const& source, + std::string const& destination); /** * Return true if the two files are the same file @@ -604,16 +627,17 @@ public: /** * Copy a file. */ - static Status CopyFileAlways(std::string const& source, - std::string const& destination); + static CopyStatus CopyFileAlways(std::string const& source, + std::string const& destination); /** * Copy a file. If the "always" argument is true the file is always * copied. If it is false, the file is copied only if it is new or * has changed. */ - static Status CopyAFile(std::string const& source, - std::string const& destination, bool always = true); + static CopyStatus CopyAFile(std::string const& source, + std::string const& destination, + bool always = true); /** * Copy content directory to another directory with all files and -- cgit v0.12 From 89b144789d969e4af06de515df2595b9a043431f Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 14 Nov 2022 19:32:07 -0500 Subject: file(COPY_FILE): Report if failure occurred on input or output path When we know whether a failure was associated with the input or the output path, include this information in the error message. --- Source/cmSystemTools.cxx | 18 ++++++++++++++---- Tests/RunCMake/file/COPY_FILE-input-missing-result.txt | 1 + Tests/RunCMake/file/COPY_FILE-input-missing-stderr.txt | 14 ++++++++++++++ Tests/RunCMake/file/COPY_FILE-input-missing.cmake | 3 +++ .../RunCMake/file/COPY_FILE-output-missing-result.txt | 1 + .../RunCMake/file/COPY_FILE-output-missing-stderr.txt | 14 ++++++++++++++ Tests/RunCMake/file/COPY_FILE-output-missing.cmake | 4 ++++ Tests/RunCMake/file/RunCMakeTest.cmake | 2 ++ 8 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 Tests/RunCMake/file/COPY_FILE-input-missing-result.txt create mode 100644 Tests/RunCMake/file/COPY_FILE-input-missing-stderr.txt create mode 100644 Tests/RunCMake/file/COPY_FILE-input-missing.cmake create mode 100644 Tests/RunCMake/file/COPY_FILE-output-missing-result.txt create mode 100644 Tests/RunCMake/file/COPY_FILE-output-missing-stderr.txt create mode 100644 Tests/RunCMake/file/COPY_FILE-output-missing.cmake diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index ee74908..78d230e 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -1140,7 +1140,7 @@ cmSystemTools::CopyResult cmSystemTools::CopySingleFile( return CopyResult::Success; } - cmsys::Status status; + cmsys::SystemTools::CopyStatus status; status = cmsys::SystemTools::CloneFileContent(oldname, newname); if (!status) { // if cloning did not succeed, fall back to blockwise copy @@ -1149,14 +1149,24 @@ cmSystemTools::CopyResult cmSystemTools::CopySingleFile( if (!status) { if (err) { *err = status.GetString(); + switch (status.Path) { + case cmsys::SystemTools::CopyStatus::SourcePath: + *err = cmStrCat(*err, " (input)"); + break; + case cmsys::SystemTools::CopyStatus::DestPath: + *err = cmStrCat(*err, " (output)"); + break; + default: + break; + } } return CopyResult::Failure; } if (perms) { - status = SystemTools::SetPermissions(newname, perm); - if (!status) { + perms = SystemTools::SetPermissions(newname, perm); + if (!perms) { if (err) { - *err = status.GetString(); + *err = cmStrCat(perms.GetString(), " (output)"); } return CopyResult::Failure; } diff --git a/Tests/RunCMake/file/COPY_FILE-input-missing-result.txt b/Tests/RunCMake/file/COPY_FILE-input-missing-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/file/COPY_FILE-input-missing-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/file/COPY_FILE-input-missing-stderr.txt b/Tests/RunCMake/file/COPY_FILE-input-missing-stderr.txt new file mode 100644 index 0000000..989925d --- /dev/null +++ b/Tests/RunCMake/file/COPY_FILE-input-missing-stderr.txt @@ -0,0 +1,14 @@ +^CMake Error at [^ +]*/Tests/RunCMake/file/COPY_FILE-input-missing.cmake:[0-9]+ \(file\): + file COPY_FILE failed to copy + + [^ +]*/Tests/RunCMake/file/COPY_FILE-input-missing-build/input-missing + + to + + [^ +]*/Tests/RunCMake/file/COPY_FILE-input-missing-build/output + + because: [^ +]+ \(input\)$ diff --git a/Tests/RunCMake/file/COPY_FILE-input-missing.cmake b/Tests/RunCMake/file/COPY_FILE-input-missing.cmake new file mode 100644 index 0000000..2d2c55e --- /dev/null +++ b/Tests/RunCMake/file/COPY_FILE-input-missing.cmake @@ -0,0 +1,3 @@ +set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input-missing") +set(newname "${CMAKE_CURRENT_BINARY_DIR}/output") +file(COPY_FILE "${oldname}" "${newname}") diff --git a/Tests/RunCMake/file/COPY_FILE-output-missing-result.txt b/Tests/RunCMake/file/COPY_FILE-output-missing-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/file/COPY_FILE-output-missing-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/file/COPY_FILE-output-missing-stderr.txt b/Tests/RunCMake/file/COPY_FILE-output-missing-stderr.txt new file mode 100644 index 0000000..0e7d9f7 --- /dev/null +++ b/Tests/RunCMake/file/COPY_FILE-output-missing-stderr.txt @@ -0,0 +1,14 @@ +^CMake Error at [^ +]*/Tests/RunCMake/file/COPY_FILE-output-missing.cmake:[0-9]+ \(file\): + file COPY_FILE failed to copy + + [^ +]*/Tests/RunCMake/file/COPY_FILE-output-missing-build/input + + to + + [^ +]*/Tests/RunCMake/file/COPY_FILE-output-missing-build/output-missing/output + + because: [^ +]+ \(output\)$ diff --git a/Tests/RunCMake/file/COPY_FILE-output-missing.cmake b/Tests/RunCMake/file/COPY_FILE-output-missing.cmake new file mode 100644 index 0000000..22133e7 --- /dev/null +++ b/Tests/RunCMake/file/COPY_FILE-output-missing.cmake @@ -0,0 +1,4 @@ +set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input") +set(newname "${CMAKE_CURRENT_BINARY_DIR}/output-missing/output") +file(WRITE "${oldname}" "") +file(COPY_FILE "${oldname}" "${newname}") diff --git a/Tests/RunCMake/file/RunCMakeTest.cmake b/Tests/RunCMake/file/RunCMakeTest.cmake index db88956..4ad00ff 100644 --- a/Tests/RunCMake/file/RunCMakeTest.cmake +++ b/Tests/RunCMake/file/RunCMakeTest.cmake @@ -65,6 +65,8 @@ run_cmake_script(COPY_FILE-file-ONLY_IF_DIFFERENT-no-overwrite) run_cmake_script(COPY_FILE-link-to-file) run_cmake_script(COPY_FILE-arg-missing) run_cmake_script(COPY_FILE-arg-unknown) +run_cmake_script(COPY_FILE-input-missing) +run_cmake_script(COPY_FILE-output-missing) run_cmake_script(RENAME-file-replace) run_cmake_script(RENAME-file-to-file) -- cgit v0.12