Merge topic 'file-download-no-save'

fed7d8f76d file(DOWNLOAD): Make file argument optional

Acked-by: Kitware Robot <[email protected]>
Merge-request: !5060

Help/command/file.rst

@@ -776,11 +776,14 @@ Transfer
 
 .. code-block:: cmake
 
-  file(DOWNLOAD <url> <file> [<options>...])
+  file(DOWNLOAD <url> [<file>] [<options>...])
   file(UPLOAD   <file> <url> [<options>...])
 
-The ``DOWNLOAD`` mode downloads the given ``<url>`` to a local ``<file>``.
-The ``UPLOAD`` mode uploads a local ``<file>`` to a given ``<url>``.
+The ``DOWNLOAD`` mode downloads the given ``<url>`` to a local ``<file>``. If
+``<file>`` 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 ``<file>`` to a given ``<url>``.
 
 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(<HASH>)``.
-  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 ``<file>``.
 
 ``EXPECTED_MD5 <value>``
-  Historical short-hand for ``EXPECTED_HASH MD5=<value>``.
+  Historical short-hand for ``EXPECTED_HASH MD5=<value>``. It is an error to
+  specify this if ``DOWNLOAD`` is not given a ``<file>``.
 
 Locking
 ^^^^^^^

Help/release/dev/file-download-optional-file.rst

@@ -0,0 +1,5 @@
+file-download-optional-file
+---------------------------
+
+* The ``<file>`` argument is now optional for :command:`file(DOWNLOAD)`. If it
+  is not specified, the file is not saved.

Source/cmFileCommand.cxx

@@ -1394,8 +1394,10 @@ size_t cmWriteToFileCallback(void* ptr, size_t size, size_t nmemb, void* data)
 {
   int realsize = static_cast<int>(size * nmemb);
   cmsys::ofstream* fout = static_cast<cmsys::ofstream*>(data);
-  const char* chPtr = static_cast<char*>(ptr);
-  fout->write(chPtr, realsize);
+  if (fout) {
+    const char* chPtr = static_cast<char*>(ptr);
+    fout->write(chPtr, realsize);
+  }
   return realsize;
 }
 
@@ -1551,15 +1553,14 @@ bool HandleDownloadCommand(std::vector<std::string> 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<std::string> 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<std::string> 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<std::string> 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<std::string> 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<std::string> 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:
   //

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

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}")

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)

Tests/RunCMake/file/DOWNLOAD-no-save-hash-result.txt

@@ -0,0 +1 @@
+1

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\)$

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
+  )

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)