Merge topic 'ExternalProject-retry-only-recoverable'

116b06870d ExternalProject: add INACTIVITY_TIMEOUT argument
f24e34975a ExternalProject: retry download on recoverable errors

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !5034

11 files changed, 171 insertions, 34 deletions

diff --git a/Modules/ExternalProject-download.cmake.in b/Modules/ExternalProject-download.cmake.in
index 99fb917..ff8c659 100644
--- a/Modules/ExternalProject-download.cmake.in
+++ b/Modules/ExternalProject-download.cmake.in
@@ -105,54 +105,65 @@ set(retry_number 5)
 
 message(STATUS "Downloading...
    dst='@LOCAL@'
-   timeout='@TIMEOUT_MSG@'"
+   timeout='@TIMEOUT_MSG@'
+   inactivity timeout='@INACTIVITY_TIMEOUT_MSG@'"
 )
-
+set(download_retry_codes 7 6 8 15)
+set(skip_url_list)
+set(status_code)
 foreach(i RANGE ${retry_number})
-  sleep_before_download(${i})
-
+  if(status_code IN_LIST download_retry_codes)
+    sleep_before_download(${i})
+  endif()
   foreach(url @REMOTE@)
-    message(STATUS "Using src='${url}'")
+    if(NOT url IN_LIST skip_url_list)
+      message(STATUS "Using src='${url}'")
 
-    @TLS_VERIFY_CODE@
-    @TLS_CAINFO_CODE@
-    @NETRC_CODE@
-    @NETRC_FILE_CODE@
+      @TLS_VERIFY_CODE@
+      @TLS_CAINFO_CODE@
+      @NETRC_CODE@
+      @NETRC_FILE_CODE@
 
-    file(
+      file(
         DOWNLOAD
         "${url}" "@LOCAL@"
         @SHOW_PROGRESS@
         @TIMEOUT_ARGS@
+        @INACTIVITY_TIMEOUT_ARGS@
         STATUS status
         LOG log
         @USERPWD_ARGS@
         @HTTP_HEADERS_ARGS@
-    )
-
-    list(GET status 0 status_code)
-    list(GET status 1 status_string)
-
-    if(status_code EQUAL 0)
-      check_file_hash(has_hash hash_is_good)
-      if(has_hash AND NOT hash_is_good)
-        message(STATUS "Hash mismatch, removing...")
-        file(REMOVE "@LOCAL@")
+        )
+
+      list(GET status 0 status_code)
+      list(GET status 1 status_string)
+
+      if(status_code EQUAL 0)
+        check_file_hash(has_hash hash_is_good)
+        if(has_hash AND NOT hash_is_good)
+          message(STATUS "Hash mismatch, removing...")
+          file(REMOVE "@LOCAL@")
+        else()
+          message(STATUS "Downloading... done")
+          return()
+        endif()
       else()
-        message(STATUS "Downloading... done")
-        return()
+        string(APPEND logFailedURLs "error: downloading '${url}' failed
+        status_code: ${status_code}
+        status_string: ${status_string}
+        log:
+        --- LOG BEGIN ---
+        ${log}
+        --- LOG END ---
+        "
+        )
+      if(NOT status_code IN_LIST download_retry_codes)
+        list(APPEND skip_url_list "${url}")
+        break()
       endif()
-    else()
-      string(APPEND logFailedURLs "error: downloading '${url}' failed
-       status_code: ${status_code}
-       status_string: ${status_string}
-       log:
-       --- LOG BEGIN ---
-       ${log}
-       --- LOG END ---
-       "
-      )
     endif()
+  endif()
   endforeach()
 endforeach()
 
diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index fcff5dd..96edbef 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -179,6 +179,9 @@ External Project Definition
       ``TIMEOUT <seconds>``
         Maximum time allowed for file download operations.
 
+      ``INACTIVITY_TIMEOUT <seconds>``
+        Terminate the operation after a period of inactivity.
+
       ``HTTP_USERNAME <username>``
         Username for the download operation if authentication is required.
 
@@ -1300,7 +1303,7 @@ function(_ep_write_gitupdate_script script_filename git_EXECUTABLE git_tag git_r
   )
 endfunction()
 
-function(_ep_write_downloadfile_script script_filename REMOTE LOCAL timeout no_progress hash tls_verify tls_cainfo userpwd http_headers netrc netrc_file)
+function(_ep_write_downloadfile_script script_filename REMOTE LOCAL timeout inactivity_timeout no_progress hash tls_verify tls_cainfo userpwd http_headers netrc netrc_file)
   if(timeout)
     set(TIMEOUT_ARGS TIMEOUT ${timeout})
     set(TIMEOUT_MSG "${timeout} seconds")
@@ -1308,6 +1311,14 @@ function(_ep_write_downloadfile_script script_filename REMOTE LOCAL timeout no_p
     set(TIMEOUT_ARGS "# no TIMEOUT")
     set(TIMEOUT_MSG "none")
   endif()
+  if(inactivity_timeout)
+    set(INACTIVITY_TIMEOUT_ARGS INACTIVITY_TIMEOUT ${inactivity_timeout})
+    set(INACTIVITY_TIMEOUT_MSG "${inactivity_timeout} seconds")
+  else()
+    set(INACTIVITY_TIMEOUT_ARGS "# no INACTIVITY_TIMEOUT")
+    set(INACTIVITY_TIMEOUT_MSG "none")
+  endif()
+
 
   if(no_progress)
     set(SHOW_PROGRESS "")
@@ -2512,6 +2523,7 @@ function(_ep_add_download_command name)
         string(REPLACE ";" "-" fname "${fname}")
         set(file ${download_dir}/${fname})
         get_property(timeout TARGET ${name} PROPERTY _EP_TIMEOUT)
+        get_property(inactivity_timeout TARGET ${name} PROPERTY _EP_INACTIVITY_TIMEOUT)
         get_property(no_progress TARGET ${name} PROPERTY _EP_DOWNLOAD_NO_PROGRESS)
         get_property(tls_verify TARGET ${name} PROPERTY _EP_TLS_VERIFY)
         get_property(tls_cainfo TARGET ${name} PROPERTY _EP_TLS_CAINFO)
@@ -2521,7 +2533,7 @@ function(_ep_add_download_command name)
         get_property(http_password TARGET ${name} PROPERTY _EP_HTTP_PASSWORD)
         get_property(http_headers TARGET ${name} PROPERTY _EP_HTTP_HEADER)
         set(download_script "${stamp_dir}/download-${name}.cmake")
-        _ep_write_downloadfile_script("${download_script}" "${url}" "${file}" "${timeout}" "${no_progress}" "${hash}" "${tls_verify}" "${tls_cainfo}" "${http_username}:${http_password}" "${http_headers}" "${netrc}" "${netrc_file}")
+        _ep_write_downloadfile_script("${download_script}" "${url}" "${file}" "${timeout}" "${inactivity_timeout}" "${no_progress}" "${hash}" "${tls_verify}" "${tls_cainfo}" "${http_username}:${http_password}" "${http_headers}" "${netrc}" "${netrc_file}")
         set(cmd ${CMAKE_COMMAND} -P "${download_script}"
           COMMAND)
         if (no_extract)
diff --git a/Tests/RunCMake/ExternalProject/DownloadInactivityResume.cmake b/Tests/RunCMake/ExternalProject/DownloadInactivityResume.cmake
new file mode 100644
index 0000000..d34482d
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/DownloadInactivityResume.cmake
@@ -0,0 +1,5 @@
+include(ExternalProject)
+ExternalProject_Add(MyProj URL ${SERVER_URL} INACTIVITY_TIMEOUT 2 DOWNLOAD_NO_EXTRACT TRUE
+  CONFIGURE_COMMAND ""
+  BUILD_COMMAND ""
+  INSTALL_COMMAND "")
diff --git a/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout-build-result.txt b/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout-build-result.txt
new file mode 100644
index 0000000..d197c91
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout-build-result.txt
@@ -0,0 +1 @@
+[^0]
diff --git a/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout-build-stdout.txt b/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout-build-stdout.txt
new file mode 100644
index 0000000..3238147
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout-build-stdout.txt
@@ -0,0 +1 @@
+(Timeout was reached)?
diff --git a/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout.cmake b/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout.cmake
new file mode 100644
index 0000000..d34482d
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/DownloadInactivityTimeout.cmake
@@ -0,0 +1,5 @@
+include(ExternalProject)
+ExternalProject_Add(MyProj URL ${SERVER_URL} INACTIVITY_TIMEOUT 2 DOWNLOAD_NO_EXTRACT TRUE
+  CONFIGURE_COMMAND ""
+  BUILD_COMMAND ""
+  INSTALL_COMMAND "")
diff --git a/Tests/RunCMake/ExternalProject/DownloadServer.py b/Tests/RunCMake/ExternalProject/DownloadServer.py
new file mode 100644
index 0000000..ac0769f
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/DownloadServer.py
@@ -0,0 +1,42 @@
+from http.server import HTTPServer, BaseHTTPRequestHandler
+import argparse
+import time
+import subprocess
+import sys
+import os
+args = None
+outerthread = None
+
+class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
+    def do_GET(self):
+        self.send_response(200)
+        self.end_headers()
+        data = b'D'
+
+        if args.speed_limit:
+            slow_deadline = time.time()+args.limit_duration
+
+            while time.time() < slow_deadline:
+                self.wfile.write(data)
+                if args.speed_limit:
+                    time.sleep(1.1)
+
+        data = data * 100
+        self.wfile.write(data)
+        self.close_connection = True
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--speed_limit', help='transfer rate limitation', action='store_true',default=False)
+    parser.add_argument('--limit_duration', help='duration of the transfer rate limitation',default=1, type=float)
+    parser.add_argument('--file', help='file to write the url to connect to')
+    parser.add_argument('--subprocess', action='store_true')
+    args = parser.parse_args()
+    if not args.subprocess:
+        subprocess.Popen([sys.executable]+sys.argv+['--subprocess'],stdin=subprocess.DEVNULL, stderr=subprocess.DEVNULL,stdout=subprocess.DEVNULL)
+    else:
+        httpd = HTTPServer(('localhost', 0), SimpleHTTPRequestHandler)
+        with open(args.file,"w") as f:
+            f.write('http://localhost:{}/test'.format(httpd.socket.getsockname()[1]))
+        httpd.handle_request()
+        os.remove(args.file)
diff --git a/Tests/RunCMake/ExternalProject/DownloadTimeout-build-result.txt b/Tests/RunCMake/ExternalProject/DownloadTimeout-build-result.txt
new file mode 100644
index 0000000..c20fd86
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/DownloadTimeout-build-result.txt
@@ -0,0 +1 @@
+^[^0]
diff --git a/Tests/RunCMake/ExternalProject/DownloadTimeout-build-stderr.txt b/Tests/RunCMake/ExternalProject/DownloadTimeout-build-stderr.txt
new file mode 100644
index 0000000..8d98f9d
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/DownloadTimeout-build-stderr.txt
@@ -0,0 +1 @@
+.*
diff --git a/Tests/RunCMake/ExternalProject/DownloadTimeout.cmake b/Tests/RunCMake/ExternalProject/DownloadTimeout.cmake
new file mode 100644
index 0000000..c90b4ba
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/DownloadTimeout.cmake
@@ -0,0 +1,5 @@
+include(ExternalProject)
+set(source_dir "${CMAKE_CURRENT_BINARY_DIR}/DownloadTimeout")
+file(REMOVE_RECURSE "${source_dir}")
+file(MAKE_DIRECTORY "${source_dir}")
+ExternalProject_Add(MyProj URL "http://cmake.org/invalid_file.tar.gz")
diff --git a/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake b/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
index 0d1da26..c2c77e0 100644
--- a/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
+++ b/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
@@ -1,5 +1,11 @@
+cmake_minimum_required(VERSION 3.12)
 include(RunCMake)
 
+# We do not contact any remote URLs, but may use a local one.
+# Remove any proxy configuration that may change behavior.
+unset(ENV{http_proxy})
+unset(ENV{https_proxy})
+
 run_cmake(IncludeScope-Add)
 run_cmake(IncludeScope-Add_Step)
 run_cmake(NoOptions)
@@ -27,6 +33,50 @@ function(__ep_test_with_build testName)
   run_cmake_command(${testName}-build ${CMAKE_COMMAND} --build .)
 endfunction()
 
+find_package(Python3)
+function(__ep_test_with_build_with_server testName)
+  if(NOT Python3_EXECUTABLE)
+    return()
+  endif()
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${testName}-build)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  set(RunCMake_TEST_TIMEOUT 20)
+  set(RunCMake_TEST_OUTPUT_MERGE TRUE)
+  file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
+  file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
+  set(URL_FILE ${RunCMake_BINARY_DIR}/${testName}.url)
+  if(EXISTS "${URL_FILE}")
+    file(REMOVE "${URL_FILE}")
+  endif()
+  execute_process(
+    COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_LIST_DIR}/DownloadServer.py --file "${URL_FILE}" ${ARGN}
+    OUTPUT_FILE ${RunCMake_BINARY_DIR}/${testName}-python.txt
+    ERROR_FILE ${RunCMake_BINARY_DIR}/${testName}-python.txt
+    RESULT_VARIABLE result
+    TIMEOUT 30
+    )
+  if(NOT result EQUAL 0)
+    message(FATAL_ERROR "Failed to start download server:\n  ${result}")
+  endif()
+
+  foreach(i RANGE 1 8)
+    if(EXISTS ${URL_FILE})
+      break()
+    endif()
+    execute_process(COMMAND ${CMAKE_COMMAND} -E sleep ${i})
+  endforeach()
+
+  if(NOT EXISTS ${URL_FILE})
+    message(FATAL_ERROR "Failed to load download server URL from:\n  ${URL_FILE}")
+  endif()
+
+  file(READ ${URL_FILE} SERVER_URL)
+  message(STATUS "URL : ${URL_FILE} - ${SERVER_URL}")
+  run_cmake_with_options(${testName} ${CMAKE_COMMAND} -DSERVER_URL=${SERVER_URL} )
+  run_cmake_command(${testName}-clean ${CMAKE_COMMAND} --build . --target clean)
+  run_cmake_command(${testName}-build ${CMAKE_COMMAND} --build .)
+endfunction()
+
 __ep_test_with_build(MultiCommand)
 
 set(RunCMake_TEST_OUTPUT_MERGE 1)
@@ -38,6 +88,9 @@ set(RunCMake_TEST_OUTPUT_MERGE 0)
 if(NOT RunCMake_GENERATOR MATCHES "Visual Studio")
   __ep_test_with_build(LogOutputOnFailure)
   __ep_test_with_build(LogOutputOnFailureMerged)
+  __ep_test_with_build(DownloadTimeout)
+  __ep_test_with_build_with_server(DownloadInactivityTimeout --speed_limit --limit_duration 40)
+  __ep_test_with_build_with_server(DownloadInactivityResume --speed_limit --limit_duration 1)
 endif()
 
 # We can't test the substitution when using the old MSYS due to