Merge topic 'robust-ep-download-verify'

a432b93b file DOWNLOAD: Display the curl result status when a hash mismatch occurs.
1cb9ef81 file DOWNLOAD: Test non-zero return status.
f73f0fb3 file DOWNLOAD: Add test for bad hash.
b5e2265f ExternalProject: Reattempt download when verification fails.

5 files changed, 56 insertions, 7 deletions

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index 63f1180..1c1bd2f 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -585,13 +585,30 @@ message(STATUS \"downloading... done\")
 endfunction()
 
 
-function(_ep_write_verifyfile_script script_filename local hash)
+function(_ep_write_verifyfile_script script_filename local hash retries download_script)
   if("${hash}" MATCHES "${_ep_hash_regex}")
     set(algo "${CMAKE_MATCH_1}")
     string(TOLOWER "${CMAKE_MATCH_2}" expect_value)
     set(script_content "set(expect_value \"${expect_value}\")
-file(${algo} \"\${file}\" actual_value)
-if(\"\${actual_value}\" STREQUAL \"\${expect_value}\")
+set(attempt 0)
+set(succeeded 0)
+while(\${attempt} LESS ${retries} OR \${attempt} EQUAL ${retries} AND NOT \${succeeded})
+  file(${algo} \"\${file}\" actual_value)
+  if(\"\${actual_value}\" STREQUAL \"\${expect_value}\")
+    set(succeeded 1)
+  elseif(\${attempt} LESS ${retries})
+    message(STATUS \"${algo} hash of \${file}
+does not match expected value
+  expected: \${expect_value}
+    actual: \${actual_value}
+Retrying download.
+\")
+    file(REMOVE \"\${file}\")
+    execute_process(COMMAND ${CMAKE_COMMAND} -P \"${download_script}\")
+  endif()
+endwhile()
+
+if(\${succeeded})
   message(STATUS \"verifying file... done\")
 else()
   message(FATAL_ERROR \"error: ${algo} hash of
@@ -1394,6 +1411,8 @@ function(_ep_add_download_command name)
     set(repository "external project URL")
     set(module "${url}")
     set(tag "${hash}")
+    set(retries 0)
+    set(download_script "")
     configure_file(
       "${CMAKE_ROOT}/Modules/RepositoryInfo.txt.in"
       "${stamp_dir}/${name}-urlinfo.txt"
@@ -1423,16 +1442,17 @@ function(_ep_add_download_command name)
         get_property(timeout TARGET ${name} PROPERTY _EP_TIMEOUT)
         get_property(tls_verify TARGET ${name} PROPERTY _EP_TLS_VERIFY)
         get_property(tls_cainfo TARGET ${name} PROPERTY _EP_TLS_CAINFO)
-        _ep_write_downloadfile_script("${stamp_dir}/download-${name}.cmake"
-          "${url}" "${file}" "${timeout}" "${hash}" "${tls_verify}" "${tls_cainfo}")
-        set(cmd ${CMAKE_COMMAND} -P ${stamp_dir}/download-${name}.cmake
+        set(download_script "${stamp_dir}/download-${name}.cmake")
+        _ep_write_downloadfile_script("${download_script}" "${url}" "${file}" "${timeout}" "${hash}" "${tls_verify}" "${tls_cainfo}")
+        set(cmd ${CMAKE_COMMAND} -P "${download_script}"
           COMMAND)
+        set(retries 3)
         set(comment "Performing download step (download, verify and extract) for '${name}'")
       else()
         set(file "${url}")
         set(comment "Performing download step (verify and extract) for '${name}'")
       endif()
-      _ep_write_verifyfile_script("${stamp_dir}/verify-${name}.cmake" "${file}" "${hash}")
+      _ep_write_verifyfile_script("${stamp_dir}/verify-${name}.cmake" "${file}" "${hash}" "${retries}" "${download_script}")
       list(APPEND cmd ${CMAKE_COMMAND} -P ${stamp_dir}/verify-${name}.cmake
         COMMAND)
       _ep_write_extractfile_script("${stamp_dir}/extract-${name}.cmake" "${name}" "${file}" "${source_dir}")
diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index 311763b..e79bc6c 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -2983,6 +2983,8 @@ cmFileCommand::HandleDownloadCommand(std::vector<std::string> const& args)
         << "  for file: [" << file << "]" << std::endl
         << "    expected hash: [" << expectedHash << "]" << std::endl
         << "      actual hash: [" << actualHash << "]" << std::endl
+        << "           status: [" << (int)res << ";\""
+          << ::curl_easy_strerror(res) << "\"]" << std::endl
         ;
       this->SetError(oss.str().c_str());
       return false;
diff --git a/Tests/CMakeTests/CMakeLists.txt b/Tests/CMakeTests/CMakeLists.txt
index 41714f6..ce36830 100644
--- a/Tests/CMakeTests/CMakeLists.txt
+++ b/Tests/CMakeTests/CMakeLists.txt
@@ -38,6 +38,10 @@ AddCMakeTest(FileDownload "")
 set_property(TEST CMake.FileDownload PROPERTY
   PASS_REGULAR_EXPRESSION "file already exists with expected MD5 sum"
   )
+AddCMakeTest(FileDownloadBadHash "")
+set_property(TEST CMake.FileDownloadBadHash PROPERTY
+  WILL_FAIL TRUE
+  )
 
 AddCMakeTest(FileUpload "")
 
diff --git a/Tests/CMakeTests/FileDownloadBadHashTest.cmake.in b/Tests/CMakeTests/FileDownloadBadHashTest.cmake.in
new file mode 100644
index 0000000..4a47c06
--- /dev/null
+++ b/Tests/CMakeTests/FileDownloadBadHashTest.cmake.in
@@ -0,0 +1,10 @@
+set(url "file://@CMAKE_CURRENT_SOURCE_DIR@/FileDownloadInput.png")
+set(dir "@CMAKE_CURRENT_BINARY_DIR@/downloads")
+
+file(DOWNLOAD
+  ${url}
+  ${dir}/file3.png
+  TIMEOUT 2
+  STATUS status
+  EXPECTED_HASH SHA1=5555555555555555555555555555555555555555
+  )
diff --git a/Tests/CMakeTests/FileDownloadTest.cmake.in b/Tests/CMakeTests/FileDownloadTest.cmake.in
index 91086c6..83ade2b 100644
--- a/Tests/CMakeTests/FileDownloadTest.cmake.in
+++ b/Tests/CMakeTests/FileDownloadTest.cmake.in
@@ -94,3 +94,16 @@ file(DOWNLOAD
   EXPECTED_MD5 d16778650db435bda3a8c3435c3ff5d1
   )
 message(STATUS "${status}")
+
+message(STATUS "FileDownload:11")
+file(DOWNLOAD
+  badhostname.png
+  ${dir}/file11.png
+  TIMEOUT 2
+  STATUS status
+  )
+message(STATUS "${status}")
+list(GET status 0 status_code)
+if(NOT ${status_code} EQUAL 6)
+  message(SEND_ERROR "error: expected status code 6 for bad host name, got: ${status_code}")
+endif()