From fed7d8f76de3442f4aede00f8e47c3d142f73e94 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 24 Jul 2020 16:22:33 -0400 Subject: file(DOWNLOAD): Make file argument optional --- Help/command/file.rst | 15 ++++-- Help/release/dev/file-download-optional-file.rst | 5 ++ Source/cmFileCommand.cxx | 63 ++++++++++++++-------- Tests/CMakeTests/CMakeLists.txt | 2 +- Tests/CMakeTests/FileDownloadTest.cmake.in | 13 +++++ Tests/CMakeTests/FileTestScript.cmake | 4 +- .../RunCMake/file/DOWNLOAD-no-save-hash-result.txt | 1 + .../RunCMake/file/DOWNLOAD-no-save-hash-stderr.txt | 4 ++ Tests/RunCMake/file/DOWNLOAD-no-save-hash.cmake | 8 +++ Tests/RunCMake/file/DOWNLOAD-no-save-hash.txt | 0 Tests/RunCMake/file/RunCMakeTest.cmake | 1 + 11 files changed, 86 insertions(+), 30 deletions(-) create mode 100644 Help/release/dev/file-download-optional-file.rst create mode 100644 Tests/RunCMake/file/DOWNLOAD-no-save-hash-result.txt create mode 100644 Tests/RunCMake/file/DOWNLOAD-no-save-hash-stderr.txt create mode 100644 Tests/RunCMake/file/DOWNLOAD-no-save-hash.cmake create mode 100644 Tests/RunCMake/file/DOWNLOAD-no-save-hash.txt diff --git a/Help/command/file.rst b/Help/command/file.rst index 693c059..2cf938b 100644 --- a/Help/command/file.rst +++ b/Help/command/file.rst @@ -776,11 +776,14 @@ Transfer .. code-block:: cmake - file(DOWNLOAD [...]) + file(DOWNLOAD [] [...]) file(UPLOAD [...]) -The ``DOWNLOAD`` mode downloads the given ```` to a local ````. -The ``UPLOAD`` mode uploads a local ```` to a given ````. +The ``DOWNLOAD`` mode downloads the given ```` to a local ````. If +```` is not specified for ``file(DOWNLOAD)``, the file is not saved. This +can be useful if you want to know if a file can be downloaded (for example, to +check that it exists) without actually saving it anywhere. The ``UPLOAD`` mode +uploads a local ```` to a given ````. Options to both ``DOWNLOAD`` and ``UPLOAD`` are: @@ -853,10 +856,12 @@ Additional options to ``DOWNLOAD`` are: Verify that the downloaded content hash matches the expected value, where ``ALGO`` is one of the algorithms supported by ``file()``. - If it does not match, the operation fails with an error. + If it does not match, the operation fails with an error. It is an error to + specify this if ``DOWNLOAD`` is not given a ````. ``EXPECTED_MD5 `` - Historical short-hand for ``EXPECTED_HASH MD5=``. + Historical short-hand for ``EXPECTED_HASH MD5=``. It is an error to + specify this if ``DOWNLOAD`` is not given a ````. Locking ^^^^^^^ diff --git a/Help/release/dev/file-download-optional-file.rst b/Help/release/dev/file-download-optional-file.rst new file mode 100644 index 0000000..f3dc24c --- /dev/null +++ b/Help/release/dev/file-download-optional-file.rst @@ -0,0 +1,5 @@ +file-download-optional-file +--------------------------- + +* The ```` argument is now optional for :command:`file(DOWNLOAD)`. If it + is not specified, the file is not saved. diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index 7101e22..9db8b85 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -1394,8 +1394,10 @@ size_t cmWriteToFileCallback(void* ptr, size_t size, size_t nmemb, void* data) { int realsize = static_cast(size * nmemb); cmsys::ofstream* fout = static_cast(data); - const char* chPtr = static_cast(ptr); - fout->write(chPtr, realsize); + if (fout) { + const char* chPtr = static_cast(ptr); + fout->write(chPtr, realsize); + } return realsize; } @@ -1551,15 +1553,14 @@ bool HandleDownloadCommand(std::vector const& args, { #if !defined(CMAKE_BOOTSTRAP) auto i = args.begin(); - if (args.size() < 3) { - status.SetError("DOWNLOAD must be called with at least three arguments."); + if (args.size() < 2) { + status.SetError("DOWNLOAD must be called with at least two arguments."); return false; } ++i; // Get rid of subcommand std::string url = *i; ++i; - std::string file = *i; - ++i; + std::string file; long timeout = 0; long inactivity_timeout = 0; @@ -1690,6 +1691,8 @@ bool HandleDownloadCommand(std::vector const& args, return false; } curl_headers.push_back(*i); + } else if (file.empty()) { + file = *i; } else { // Do not return error for compatibility reason. std::string err = cmStrCat("Unexpected argument: ", *i); @@ -1697,11 +1700,18 @@ bool HandleDownloadCommand(std::vector const& args, } ++i; } + // Can't calculate hash if we don't save the file. + // TODO Incrementally calculate hash in the write callback as the file is + // being downloaded so this check can be relaxed. + if (file.empty() && hash) { + status.SetError("DOWNLOAD cannot calculate hash if file is not saved."); + return false; + } // If file exists already, and caller specified an expected md5 or sha, // and the existing file already has the expected hash, then simply // return. // - if (cmSystemTools::FileExists(file) && hash.get()) { + if (!file.empty() && cmSystemTools::FileExists(file) && hash.get()) { std::string msg; std::string actualHash = hash->HashFile(file); if (actualHash == expectedHash) { @@ -1716,20 +1726,26 @@ bool HandleDownloadCommand(std::vector const& args, // Make sure parent directory exists so we can write to the file // as we receive downloaded bits from curl... // - std::string dir = cmSystemTools::GetFilenamePath(file); - if (!dir.empty() && !cmSystemTools::FileExists(dir) && - !cmSystemTools::MakeDirectory(dir)) { - std::string errstring = "DOWNLOAD error: cannot create directory '" + dir + - "' - Specify file by full path name and verify that you " - "have directory creation and file write privileges."; - status.SetError(errstring); - return false; + if (!file.empty()) { + std::string dir = cmSystemTools::GetFilenamePath(file); + if (!dir.empty() && !cmSystemTools::FileExists(dir) && + !cmSystemTools::MakeDirectory(dir)) { + std::string errstring = "DOWNLOAD error: cannot create directory '" + + dir + + "' - Specify file by full path name and verify that you " + "have directory creation and file write privileges."; + status.SetError(errstring); + return false; + } } - cmsys::ofstream fout(file.c_str(), std::ios::binary); - if (!fout) { - status.SetError("DOWNLOAD cannot open file for write."); - return false; + cmsys::ofstream fout; + if (!file.empty()) { + fout.open(file.c_str(), std::ios::binary); + if (!fout) { + status.SetError("DOWNLOAD cannot open file for write."); + return false; + } } # if defined(_WIN32) @@ -1791,7 +1807,8 @@ bool HandleDownloadCommand(std::vector const& args, cmFileCommandVectorOfChar chunkDebug; - res = ::curl_easy_setopt(curl, CURLOPT_WRITEDATA, &fout); + res = ::curl_easy_setopt(curl, CURLOPT_WRITEDATA, + file.empty() ? nullptr : &fout); check_curl_result(res, "DOWNLOAD cannot set write data: "); res = ::curl_easy_setopt(curl, CURLOPT_DEBUGDATA, &chunkDebug); @@ -1865,8 +1882,10 @@ bool HandleDownloadCommand(std::vector const& args, // Explicitly flush/close so we can measure the md5 accurately. // - fout.flush(); - fout.close(); + if (!file.empty()) { + fout.flush(); + fout.close(); + } // Verify MD5 sum if requested: // diff --git a/Tests/CMakeTests/CMakeLists.txt b/Tests/CMakeTests/CMakeLists.txt index e32d693..348e6d0 100644 --- a/Tests/CMakeTests/CMakeLists.txt +++ b/Tests/CMakeTests/CMakeLists.txt @@ -35,7 +35,7 @@ AddCMakeTest(CMakeHostSystemInformation "") AddCMakeTest(FileDownload "") set_tests_properties(CMake.FileDownload PROPERTIES PASS_REGULAR_EXPRESSION "file already exists with expected MD5 sum" - FAIL_REGULAR_EXPRESSION "Unexpected status" + FAIL_REGULAR_EXPRESSION "Unexpected status|incorrectly interpreted" ) AddCMakeTest(FileDownloadBadHash "") set_property(TEST CMake.FileDownloadBadHash PROPERTY diff --git a/Tests/CMakeTests/FileDownloadTest.cmake.in b/Tests/CMakeTests/FileDownloadTest.cmake.in index 76c0000..69d9a14 100644 --- a/Tests/CMakeTests/FileDownloadTest.cmake.in +++ b/Tests/CMakeTests/FileDownloadTest.cmake.in @@ -163,3 +163,16 @@ __reportIfWrongStatus("${status}" 0) if(NOT EXISTS file12.png) message(SEND_ERROR "file12.png not downloaded: ${status}") endif() + +message(STATUS "FileDownload:13") +file(DOWNLOAD + ${url} + TIMEOUT ${timeout} + STATUS status + ) +__reportIfWrongStatus("${status}" 0) +if(EXISTS TIMEOUT) + file(REMOVE TIMEOUT) + message(SEND_ERROR "TIMEOUT argument was incorrectly interpreted as a filename") +endif() +message(STATUS "${status}") diff --git a/Tests/CMakeTests/FileTestScript.cmake b/Tests/CMakeTests/FileTestScript.cmake index 145f28a..fc3c28a 100644 --- a/Tests/CMakeTests/FileTestScript.cmake +++ b/Tests/CMakeTests/FileTestScript.cmake @@ -10,7 +10,7 @@ elseif(testname STREQUAL different_not_enough_args) # fail file(DIFFERENT ffff) elseif(testname STREQUAL download_not_enough_args) # fail - file(DOWNLOAD ffff) + file(DOWNLOAD) elseif(testname STREQUAL read_not_enough_args) # fail file(READ ffff) @@ -181,7 +181,7 @@ elseif(testname STREQUAL to_native_path) # pass message("v='${v}'") elseif(testname STREQUAL download_wrong_number_of_args) # fail - file(DOWNLOAD zzzz://bogus/ffff) + file(DOWNLOAD) elseif(testname STREQUAL download_file_with_no_path) # pass file(DOWNLOAD zzzz://bogus/ffff ffff) diff --git a/Tests/RunCMake/file/DOWNLOAD-no-save-hash-result.txt b/Tests/RunCMake/file/DOWNLOAD-no-save-hash-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/file/DOWNLOAD-no-save-hash-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/file/DOWNLOAD-no-save-hash-stderr.txt b/Tests/RunCMake/file/DOWNLOAD-no-save-hash-stderr.txt new file mode 100644 index 0000000..b0f0d19 --- /dev/null +++ b/Tests/RunCMake/file/DOWNLOAD-no-save-hash-stderr.txt @@ -0,0 +1,4 @@ +^CMake Error at DOWNLOAD-no-save-hash\.cmake:[0-9]+ \(file\): + file DOWNLOAD cannot calculate hash if file is not saved\. +Call Stack \(most recent call first\): + CMakeLists\.txt:[0-9]+ \(include\)$ diff --git a/Tests/RunCMake/file/DOWNLOAD-no-save-hash.cmake b/Tests/RunCMake/file/DOWNLOAD-no-save-hash.cmake new file mode 100644 index 0000000..ce959a7 --- /dev/null +++ b/Tests/RunCMake/file/DOWNLOAD-no-save-hash.cmake @@ -0,0 +1,8 @@ +if(NOT "${CMAKE_CURRENT_SOURCE_DIR}" MATCHES "^/") + set(slash /) +endif() +file(DOWNLOAD + "file://${slash}${CMAKE_CURRENT_SOURCE_DIR}/DOWNLOAD-no-save-md5.txt" + EXPECTED_HASH MD5=55555555555555555555555555555555 + STATUS status + ) diff --git a/Tests/RunCMake/file/DOWNLOAD-no-save-hash.txt b/Tests/RunCMake/file/DOWNLOAD-no-save-hash.txt new file mode 100644 index 0000000..e69de29 diff --git a/Tests/RunCMake/file/RunCMakeTest.cmake b/Tests/RunCMake/file/RunCMakeTest.cmake index a4de1d3..8d84943 100644 --- a/Tests/RunCMake/file/RunCMakeTest.cmake +++ b/Tests/RunCMake/file/RunCMakeTest.cmake @@ -11,6 +11,7 @@ run_cmake(DOWNLOAD-netrc-bad) run_cmake(DOWNLOAD-tls-cainfo-not-set) run_cmake(DOWNLOAD-tls-verify-not-set) run_cmake(DOWNLOAD-pass-not-set) +run_cmake(DOWNLOAD-no-save-hash) run_cmake(TOUCH) run_cmake(TOUCH-error-in-source-directory) run_cmake(TOUCH-error-missing-directory) -- cgit v0.12