From fa518188d8aeffa176109bc960173b06fa1e135e Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 17 Nov 2022 13:19:10 -0500 Subject: cmSystemTools: Remove unused CopySingleFile overload --- Source/cmSystemTools.cxx | 7 ------- Source/cmSystemTools.h | 2 -- 2 files changed, 9 deletions(-) diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 78d230e..75b7668 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -1111,13 +1111,6 @@ bool cmMoveFile(std::wstring const& oldname, std::wstring const& newname, } #endif -bool cmSystemTools::CopySingleFile(const std::string& oldname, - const std::string& newname) -{ - return cmSystemTools::CopySingleFile(oldname, newname, CopyWhen::Always) == - CopyResult::Success; -} - cmSystemTools::CopyResult cmSystemTools::CopySingleFile( std::string const& oldname, std::string const& newname, CopyWhen when, std::string* err) diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index 87b354c..f2b3dd2 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -177,8 +177,6 @@ public: const mode_t* mode = nullptr); /** Copy a file. */ - static bool CopySingleFile(const std::string& oldname, - const std::string& newname); static CopyResult CopySingleFile(std::string const& oldname, std::string const& newname, CopyWhen when, std::string* err = nullptr); -- cgit v0.12 From efa9eec0402adacc7ac8b0dc17660a8bd968b06a Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 17 Nov 2022 13:27:21 -0500 Subject: file(COPY_FILE): Add option to retry on Windows if input access fails On Windows, a file may be inaccessible for a short time after it is created. This occurs for various reasons, including indexing, antivirus tools, and NTFS's asynchronous semantics. Add an `INPUT_MAY_BE_RECENT` option to tell CMake that the input file may have been recently created so that we can retry a few times to read it. --- Help/command/file.rst | 11 ++++++++++- Source/cmFileCommand.cxx | 8 +++++++- Source/cmSystemTools.cxx | 19 ++++++++++++++++++- Source/cmSystemTools.h | 6 ++++++ .../file/COPY_FILE-file-INPUT_MAY_BE_RECENT.cmake | 10 ++++++++++ Tests/RunCMake/file/RunCMakeTest.cmake | 1 + 6 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 Tests/RunCMake/file/COPY_FILE-file-INPUT_MAY_BE_RECENT.cmake diff --git a/Help/command/file.rst b/Help/command/file.rst index 2348937..df895d0 100644 --- a/Help/command/file.rst +++ b/Help/command/file.rst @@ -750,7 +750,8 @@ The options are: file(COPY_FILE [RESULT ] - [ONLY_IF_DIFFERENT]) + [ONLY_IF_DIFFERENT] + [INPUT_MAY_BE_RECENT]) .. versionadded:: 3.21 @@ -769,6 +770,14 @@ The options are: contents are already the same as ```` (this avoids updating ````'s timestamp). +``INPUT_MAY_BE_RECENT`` + .. versionadded:: 3.26 + + Tell CMake that the input file may have been recently created. This is + meaningful only on Windows, where files may be inaccessible for a short + time after they are created. With this option, if permission is denied, + CMake will retry reading the input a few times. + This sub-command has some similarities to :command:`configure_file` with the ``COPYONLY`` option. An important difference is that :command:`configure_file` creates a dependency on the source file, so CMake will be re-run if it changes. diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index 85f528d..cdd0408 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -1408,12 +1408,14 @@ bool HandleCopyFile(std::vector const& args, struct Arguments { + bool InputMayBeRecent = false; bool OnlyIfDifferent = false; std::string Result; }; static auto const parser = cmArgumentParser{} + .Bind("INPUT_MAY_BE_RECENT"_s, &Arguments::InputMayBeRecent) .Bind("ONLY_IF_DIFFERENT"_s, &Arguments::OnlyIfDifferent) .Bind("RESULT"_s, &Arguments::Result); @@ -1456,9 +1458,13 @@ bool HandleCopyFile(std::vector const& args, } else { when = cmSystemTools::CopyWhen::Always; } + cmSystemTools::CopyInputRecent const inputRecent = arguments.InputMayBeRecent + ? cmSystemTools::CopyInputRecent::Yes + : cmSystemTools::CopyInputRecent::No; std::string err; - if (cmSystemTools::CopySingleFile(oldname, newname, when, &err) == + if (cmSystemTools::CopySingleFile(oldname, newname, when, inputRecent, + &err) == cmSystemTools::CopyResult::Success) { if (!arguments.Result.empty()) { status.GetMakefile().AddDefinition(arguments.Result, "0"); diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 75b7668..f94c4d3 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -1113,7 +1113,7 @@ bool cmMoveFile(std::wstring const& oldname, std::wstring const& newname, cmSystemTools::CopyResult cmSystemTools::CopySingleFile( std::string const& oldname, std::string const& newname, CopyWhen when, - std::string* err) + CopyInputRecent inputRecent, std::string* err) { switch (when) { case CopyWhen::Always: @@ -1137,7 +1137,24 @@ cmSystemTools::CopyResult cmSystemTools::CopySingleFile( status = cmsys::SystemTools::CloneFileContent(oldname, newname); if (!status) { // if cloning did not succeed, fall back to blockwise copy +#ifdef _WIN32 + if (inputRecent == CopyInputRecent::Yes) { + // Windows sometimes locks a file immediately after creation. + // Retry a few times. + WindowsFileRetry retry = cmSystemTools::GetWindowsFileRetry(); + while ((status = + cmsys::SystemTools::CopyFileContentBlockwise(oldname, newname), + status.Path == cmsys::SystemTools::CopyStatus::SourcePath && + status.GetPOSIX() == EACCES && --retry.Count)) { + cmSystemTools::Delay(retry.Delay); + } + } else { + status = cmsys::SystemTools::CopyFileContentBlockwise(oldname, newname); + } +#else + static_cast(inputRecent); status = cmsys::SystemTools::CopyFileContentBlockwise(oldname, newname); +#endif } if (!status) { if (err) { diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index f2b3dd2..09f2bf0 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -149,6 +149,11 @@ public: Always, OnlyIfDifferent, }; + enum class CopyInputRecent + { + No, + Yes, + }; enum class CopyResult { Success, @@ -179,6 +184,7 @@ public: /** Copy a file. */ static CopyResult CopySingleFile(std::string const& oldname, std::string const& newname, CopyWhen when, + CopyInputRecent inputRecent, std::string* err = nullptr); enum class Replace diff --git a/Tests/RunCMake/file/COPY_FILE-file-INPUT_MAY_BE_RECENT.cmake b/Tests/RunCMake/file/COPY_FILE-file-INPUT_MAY_BE_RECENT.cmake new file mode 100644 index 0000000..88bf448 --- /dev/null +++ b/Tests/RunCMake/file/COPY_FILE-file-INPUT_MAY_BE_RECENT.cmake @@ -0,0 +1,10 @@ +set(oldname "${CMAKE_CURRENT_BINARY_DIR}/input") +set(newname "${CMAKE_CURRENT_BINARY_DIR}/output") +file(WRITE "${oldname}" "") +file(COPY_FILE "${oldname}" "${newname}" INPUT_MAY_BE_RECENT) +if(NOT EXISTS "${oldname}") + message(FATAL_ERROR "The old name does not exist:\n ${oldname}") +endif() +if(NOT EXISTS "${newname}") + message(FATAL_ERROR "The new name does not exist:\n ${newname}") +endif() diff --git a/Tests/RunCMake/file/RunCMakeTest.cmake b/Tests/RunCMake/file/RunCMakeTest.cmake index 4ad00ff..0c4c174 100644 --- a/Tests/RunCMake/file/RunCMakeTest.cmake +++ b/Tests/RunCMake/file/RunCMakeTest.cmake @@ -59,6 +59,7 @@ run_cmake_script(COPY_FILE-dirlink-to-file-fail) run_cmake_script(COPY_FILE-file-to-file) run_cmake_script(COPY_FILE-file-to-dir-capture) run_cmake_script(COPY_FILE-file-to-dir-fail) +run_cmake_script(COPY_FILE-file-INPUT_MAY_BE_RECENT) run_cmake_script(COPY_FILE-file-ONLY_IF_DIFFERENT-capture) run_cmake_script(COPY_FILE-file-ONLY_IF_DIFFERENT-fail) run_cmake_script(COPY_FILE-file-ONLY_IF_DIFFERENT-no-overwrite) -- cgit v0.12 From d34986036fdc0dfa85a2d85a4fe5bce0368b1187 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 17 Nov 2022 13:49:25 -0500 Subject: ExternalData: Improve robustness on Windows to copy a data object to a file When an external data object was recently created on disk, perhaps fetched by the current process or another process, it may be inaccessible on Windows for a short time. Tell our `file(COPY_FILE)` call to retry a few times to copy the object to the final file path. Hopefully this will resolve our long-standing spurious failures of the `Module.ExternalData` test on Windows. --- Modules/ExternalData.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/ExternalData.cmake b/Modules/ExternalData.cmake index 189374b..6826c7b 100644 --- a/Modules/ExternalData.cmake +++ b/Modules/ExternalData.cmake @@ -945,7 +945,7 @@ function(_ExternalData_link_or_copy src dst) file(CREATE_LINK "${tgt}" "${tmp}" RESULT result COPY_ON_ERROR SYMBOLIC) else() # Create a copy. - file(COPY_FILE "${src}" "${tmp}" RESULT result) + file(COPY_FILE "${src}" "${tmp}" RESULT result INPUT_MAY_BE_RECENT) endif() if(result) file(REMOVE "${tmp}") -- cgit v0.12