Merge topic 'cdash_upload_retry'

05ed82b1 ctest_submit: Update documentation for CDash upload retries
0ce7643a ctest_submit: improve handling of QUIET option
5614a5cd ctest_submit: Allow RETRY_COUNT for CDASH_UPLOAD

14 files changed, 162 insertions, 52 deletions

diff --git a/Help/command/ctest_submit.rst b/Help/command/ctest_submit.rst
index 6830b59..a7d9708 100644
--- a/Help/command/ctest_submit.rst
+++ b/Help/command/ctest_submit.rst
@@ -56,10 +56,16 @@ Submit to CDash Upload API
 
 ::
 
-  ctest_submit(CDASH_UPLOAD <file> [CDASH_UPLOAD_TYPE <type>])
+  ctest_submit(CDASH_UPLOAD <file> [CDASH_UPLOAD_TYPE <type>]
+               [RETRY_COUNT <count>]
+               [RETRY_DELAY <delay>]
+               [QUIET])
 
 This second signature is used to upload files to CDash via the CDash
 file upload API. The api first sends a request to upload to CDash along
 with a content hash of the file. If CDash does not already have the file,
 then it is uploaded. Along with the file, a CDash type string is specified
 to tell CDash which handler to use to process the data.
+
+This signature accepts the ``RETRY_COUNT``, ``RETRY_DELAY``, and ``QUIET``
+options as described above.
diff --git a/Help/release/dev/cdash_upload_retry.rst b/Help/release/dev/cdash_upload_retry.rst
new file mode 100644
index 0000000..2ceb42b
--- /dev/null
+++ b/Help/release/dev/cdash_upload_retry.rst
@@ -0,0 +1,5 @@
+cdash-upload-retry
+-----------------------
+
+* The ``CDASH_UPLOAD`` signature of :command:`ctest_submit` was taught to honor
+  the ``RETRY_COUNT``, ``RETRY_DELAY``, and ``QUIET`` options.
diff --git a/Source/CTest/cmCTestCurl.cxx b/Source/CTest/cmCTestCurl.cxx
index 9a90f54..ab1412d 100644
--- a/Source/CTest/cmCTestCurl.cxx
+++ b/Source/CTest/cmCTestCurl.cxx
@@ -19,6 +19,7 @@ cmCTestCurl::cmCTestCurl(cmCTest* ctest)
   // default is to verify https
   this->VerifyPeerOff = false;
   this->VerifyHostOff = false;
+  this->Quiet = false;
   this->TimeOutSeconds = 0;
   this->Curl = curl_easy_init();
 }
@@ -96,6 +97,13 @@ bool cmCTestCurl::InitCurl()
   }
   // enable HTTP ERROR parsing
   curl_easy_setopt(this->Curl, CURLOPT_FAILONERROR, 1);
+
+  // if there is little to no activity for too long stop submitting
+  if (this->TimeOutSeconds) {
+    curl_easy_setopt(this->Curl, CURLOPT_LOW_SPEED_LIMIT, 1);
+    curl_easy_setopt(this->Curl, CURLOPT_LOW_SPEED_TIME, this->TimeOutSeconds);
+  }
+
   return true;
 }
 
@@ -110,12 +118,7 @@ bool cmCTestCurl::UploadFile(std::string const& local_file,
   }
   /* enable uploading */
   curl_easy_setopt(this->Curl, CURLOPT_UPLOAD, 1);
-  // if there is little to no activity for too long stop submitting
-  if (this->TimeOutSeconds) {
-    ::curl_easy_setopt(this->Curl, CURLOPT_LOW_SPEED_LIMIT, 1);
-    ::curl_easy_setopt(this->Curl, CURLOPT_LOW_SPEED_TIME,
-                       this->TimeOutSeconds);
-  }
+
   /* HTTP PUT please */
   ::curl_easy_setopt(this->Curl, CURLOPT_PUT, 1);
   ::curl_easy_setopt(this->Curl, CURLOPT_VERBOSE, 1);
@@ -157,13 +160,14 @@ bool cmCTestCurl::UploadFile(std::string const& local_file,
 
   if (!responseData.empty()) {
     response = std::string(responseData.begin(), responseData.end());
-    cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "Curl response: ["
-                 << response << "]\n");
+    cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
+                       "Curl response: [" << response << "]\n", this->Quiet);
   }
   std::string curlDebug;
   if (!debugData.empty()) {
     curlDebug = std::string(debugData.begin(), debugData.end());
-    cmCTestLog(this->CTest, DEBUG, "Curl debug: [" << curlDebug << "]\n");
+    cmCTestOptionalLog(this->CTest, DEBUG,
+                       "Curl debug: [" << curlDebug << "]\n", this->Quiet);
   }
   if (response.empty()) {
     cmCTestLog(this->CTest, ERROR_MESSAGE, "No response from server.\n"
@@ -177,9 +181,10 @@ bool cmCTestCurl::HttpRequest(std::string const& url,
                               std::string const& fields, std::string& response)
 {
   response = "";
-  cmCTestLog(this->CTest, DEBUG, "HttpRequest\n"
-               << "url: " << url << "\n"
-               << "fields " << fields << "\n");
+  cmCTestOptionalLog(this->CTest, DEBUG, "HttpRequest\n"
+                       << "url: " << url << "\n"
+                       << "fields " << fields << "\n",
+                     this->Quiet);
   if (!this->InitCurl()) {
     cmCTestLog(this->CTest, ERROR_MESSAGE, "Initialization of curl failed");
     return false;
@@ -202,13 +207,16 @@ bool cmCTestCurl::HttpRequest(std::string const& url,
 
   if (!responseData.empty()) {
     response = std::string(responseData.begin(), responseData.end());
-    cmCTestLog(this->CTest, DEBUG, "Curl response: [" << response << "]\n");
+    cmCTestOptionalLog(this->CTest, DEBUG,
+                       "Curl response: [" << response << "]\n", this->Quiet);
   }
   if (!debugData.empty()) {
     std::string curlDebug = std::string(debugData.begin(), debugData.end());
-    cmCTestLog(this->CTest, DEBUG, "Curl debug: [" << curlDebug << "]\n");
+    cmCTestOptionalLog(this->CTest, DEBUG,
+                       "Curl debug: [" << curlDebug << "]\n", this->Quiet);
   }
-  cmCTestLog(this->CTest, DEBUG, "Curl res: " << res << "\n");
+  cmCTestOptionalLog(this->CTest, DEBUG, "Curl res: " << res << "\n",
+                     this->Quiet);
   return (res == 0);
 }
 
diff --git a/Source/CTest/cmCTestCurl.h b/Source/CTest/cmCTestCurl.h
index 17fbcff..cdce393 100644
--- a/Source/CTest/cmCTestCurl.h
+++ b/Source/CTest/cmCTestCurl.h
@@ -25,6 +25,7 @@ public:
   void SetCurlOptions(std::vector<std::string> const& args);
   void SetUseHttp10On() { this->UseHttp10 = true; }
   void SetTimeOutSeconds(int s) { this->TimeOutSeconds = s; }
+  void SetQuiet(bool b) { this->Quiet = b; }
   std::string Escape(std::string const& source);
 
 protected:
@@ -40,6 +41,7 @@ private:
   bool VerifyHostOff;
   bool VerifyPeerOff;
   bool UseHttp10;
+  bool Quiet;
   int TimeOutSeconds;
 };
 
diff --git a/Source/CTest/cmCTestSubmitCommand.cxx b/Source/CTest/cmCTestSubmitCommand.cxx
index 14b875f..5cf4ddc 100644
--- a/Source/CTest/cmCTestSubmitCommand.cxx
+++ b/Source/CTest/cmCTestSubmitCommand.cxx
@@ -157,6 +157,7 @@ bool cmCTestSubmitCommand::InitialPass(std::vector<std::string> const& args,
 bool cmCTestSubmitCommand::CheckArgumentKeyword(std::string const& arg)
 {
   if (this->CDashUpload) {
+    // Arguments specific to the CDASH_UPLOAD signature.
     if (arg == "CDASH_UPLOAD") {
       this->ArgumentDoing = ArgumentDoingCDashUpload;
       return true;
@@ -167,7 +168,7 @@ bool cmCTestSubmitCommand::CheckArgumentKeyword(std::string const& arg)
       return true;
     }
   } else {
-    // Look for arguments specific to this command.
+    // Arguments that cannot be used with CDASH_UPLOAD.
     if (arg == "PARTS") {
       this->ArgumentDoing = ArgumentDoingParts;
       this->PartsMentioned = true;
@@ -179,21 +180,21 @@ bool cmCTestSubmitCommand::CheckArgumentKeyword(std::string const& arg)
       this->FilesMentioned = true;
       return true;
     }
+  }
+  // Arguments used by both modes.
+  if (arg == "RETRY_COUNT") {
+    this->ArgumentDoing = ArgumentDoingRetryCount;
+    return true;
+  }
 
-    if (arg == "RETRY_COUNT") {
-      this->ArgumentDoing = ArgumentDoingRetryCount;
-      return true;
-    }
-
-    if (arg == "RETRY_DELAY") {
-      this->ArgumentDoing = ArgumentDoingRetryDelay;
-      return true;
-    }
+  if (arg == "RETRY_DELAY") {
+    this->ArgumentDoing = ArgumentDoingRetryDelay;
+    return true;
+  }
 
-    if (arg == "INTERNAL_TEST_CHECKSUM") {
-      this->InternalTest = true;
-      return true;
-    }
+  if (arg == "INTERNAL_TEST_CHECKSUM") {
+    this->InternalTest = true;
+    return true;
   }
 
   // Look for other arguments.
diff --git a/Source/CTest/cmCTestSubmitHandler.cxx b/Source/CTest/cmCTestSubmitHandler.cxx
index 79c3663..5e5119d 100644
--- a/Source/CTest/cmCTestSubmitHandler.cxx
+++ b/Source/CTest/cmCTestSubmitHandler.cxx
@@ -1007,6 +1007,7 @@ int cmCTestSubmitHandler::HandleCDashUploadFile(std::string const& file,
     return -1;
   }
   cmCTestCurl curl(this->CTest);
+  curl.SetQuiet(this->Quiet);
   std::string curlopt(this->CTest->GetCTestConfiguration("CurlOptions"));
   std::vector<std::string> args;
   cmSystemTools::ExpandListArgument(curlopt, args);
@@ -1022,6 +1023,30 @@ int cmCTestSubmitHandler::HandleCDashUploadFile(std::string const& file,
                "Only http and https are supported for CDASH_UPLOAD\n");
     return -1;
   }
+  bool internalTest = cmSystemTools::IsOn(this->GetOption("InternalTest"));
+
+  // Get RETRY_COUNT and RETRY_DELAY values if they were set.
+  std::string retryDelayString = this->GetOption("RetryDelay") == CM_NULLPTR
+    ? ""
+    : this->GetOption("RetryDelay");
+  std::string retryCountString = this->GetOption("RetryCount") == CM_NULLPTR
+    ? ""
+    : this->GetOption("RetryCount");
+  unsigned long retryDelay = 0;
+  if (retryDelayString != "") {
+    if (!cmSystemTools::StringToULong(retryDelayString.c_str(), &retryDelay)) {
+      cmCTestLog(this->CTest, WARNING, "Invalid value for 'RETRY_DELAY' : "
+                   << retryDelayString << std::endl);
+    }
+  }
+  unsigned long retryCount = 0;
+  if (retryCountString != "") {
+    if (!cmSystemTools::StringToULong(retryCountString.c_str(), &retryCount)) {
+      cmCTestLog(this->CTest, WARNING, "Invalid value for 'RETRY_DELAY' : "
+                   << retryCountString << std::endl);
+    }
+  }
+
   char md5sum[33];
   md5sum[32] = 0;
   cmSystemTools::ComputeFileMD5(file, md5sum);
@@ -1058,7 +1083,33 @@ int cmCTestSubmitHandler::HandleCDashUploadFile(std::string const& file,
                                 << "\nfile: " << file << "\n",
                      this->Quiet);
   std::string response;
-  if (!curl.HttpRequest(url, fields, response)) {
+
+  bool requestSucceeded = curl.HttpRequest(url, fields, response);
+  if (!internalTest && !requestSucceeded) {
+    // If request failed, wait and retry.
+    for (unsigned long i = 0; i < retryCount; i++) {
+      cmCTestOptionalLog(this->CTest, HANDLER_OUTPUT,
+                         "   Request failed, waiting " << retryDelay
+                                                       << " seconds...\n",
+                         this->Quiet);
+
+      double stop = cmSystemTools::GetTime() + static_cast<double>(retryDelay);
+      while (cmSystemTools::GetTime() < stop) {
+        cmSystemTools::Delay(100);
+      }
+
+      cmCTestOptionalLog(this->CTest, HANDLER_OUTPUT,
+                         "   Retry request: Attempt "
+                           << (i + 1) << " of " << retryCount << std::endl,
+                         this->Quiet);
+
+      requestSucceeded = curl.HttpRequest(url, fields, response);
+      if (requestSucceeded) {
+        break;
+      }
+    }
+  }
+  if (!internalTest && !requestSucceeded) {
     cmCTestLog(this->CTest, ERROR_MESSAGE, "Error in HttpRequest\n"
                  << response);
     return -1;
@@ -1068,30 +1119,32 @@ int cmCTestSubmitHandler::HandleCDashUploadFile(std::string const& file,
                      this->Quiet);
   Json::Value json;
   Json::Reader reader;
-  if (!reader.parse(response, json)) {
+  if (!internalTest && !reader.parse(response, json)) {
     cmCTestLog(this->CTest, ERROR_MESSAGE, "error parsing json string ["
                  << response << "]\n"
                  << reader.getFormattedErrorMessages() << "\n");
     return -1;
   }
-  if (json["status"].asInt() != 0) {
+  if (!internalTest && json["status"].asInt() != 0) {
     cmCTestLog(this->CTest, ERROR_MESSAGE,
                "Bad status returned from CDash: " << json["status"].asInt());
     return -1;
   }
-  if (json["datafilesmd5"].isArray()) {
-    int datares = json["datafilesmd5"][0].asInt();
-    if (datares == 1) {
-      cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
-                         "File already exists on CDash, skip upload " << file
-                                                                      << "\n",
-                         this->Quiet);
-      return 0;
+  if (!internalTest) {
+    if (json["datafilesmd5"].isArray()) {
+      int datares = json["datafilesmd5"][0].asInt();
+      if (datares == 1) {
+        cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
+                           "File already exists on CDash, skip upload "
+                             << file << "\n",
+                           this->Quiet);
+        return 0;
+      }
+    } else {
+      cmCTestLog(this->CTest, ERROR_MESSAGE,
+                 "bad datafilesmd5 value in response " << response << "\n");
+      return -1;
     }
-  } else {
-    cmCTestLog(this->CTest, ERROR_MESSAGE,
-               "bad datafilesmd5 value in response " << response << "\n");
-    return -1;
   }
 
   std::string upload_as = cmSystemTools::GetFilenameName(file);
@@ -1100,7 +1153,40 @@ int cmCTestSubmitHandler::HandleCDashUploadFile(std::string const& file,
        << "md5=" << md5sum << "&"
        << "filename=" << curl.Escape(upload_as) << "&"
        << "buildid=" << json["buildid"].asString();
-  if (!curl.UploadFile(file, url, fstr.str(), response)) {
+
+  bool uploadSucceeded = false;
+  if (!internalTest) {
+    uploadSucceeded = curl.UploadFile(file, url, fstr.str(), response);
+  }
+
+  if (!uploadSucceeded) {
+    // If upload failed, wait and retry.
+    for (unsigned long i = 0; i < retryCount; i++) {
+      cmCTestOptionalLog(this->CTest, HANDLER_OUTPUT,
+                         "   Upload failed, waiting " << retryDelay
+                                                      << " seconds...\n",
+                         this->Quiet);
+
+      double stop = cmSystemTools::GetTime() + static_cast<double>(retryDelay);
+      while (cmSystemTools::GetTime() < stop) {
+        cmSystemTools::Delay(100);
+      }
+
+      cmCTestOptionalLog(this->CTest, HANDLER_OUTPUT,
+                         "   Retry upload: Attempt "
+                           << (i + 1) << " of " << retryCount << std::endl,
+                         this->Quiet);
+
+      if (!internalTest) {
+        uploadSucceeded = curl.UploadFile(file, url, fstr.str(), response);
+      }
+      if (uploadSucceeded) {
+        break;
+      }
+    }
+  }
+
+  if (!uploadSucceeded) {
     cmCTestLog(this->CTest, ERROR_MESSAGE, "error uploading to CDash. "
                  << file << " " << url << " " << fstr.str());
     return -1;
diff --git a/Tests/RunCMake/ctest_submit/CDashUploadRETRY_DELAY-result.txt b/Tests/RunCMake/ctest_submit/CDashUploadMissingFile-result.txt
index b57e2de..b57e2de 100644
--- a/Tests/RunCMake/ctest_submit/CDashUploadRETRY_DELAY-result.txt
+++ b/Tests/RunCMake/ctest_submit/CDashUploadMissingFile-result.txt
diff --git a/Tests/RunCMake/ctest_submit/CDashUploadMissingFile-stderr.txt b/Tests/RunCMake/ctest_submit/CDashUploadMissingFile-stderr.txt
new file mode 100644
index 0000000..364ecde
--- /dev/null
+++ b/Tests/RunCMake/ctest_submit/CDashUploadMissingFile-stderr.txt
@@ -0,0 +1 @@
+Upload file not found: 'bad-upload'
diff --git a/Tests/RunCMake/ctest_submit/CDashUploadRETRY_COUNT-stderr.txt b/Tests/RunCMake/ctest_submit/CDashUploadRETRY_COUNT-stderr.txt
deleted file mode 100644
index 21621d4..0000000
--- a/Tests/RunCMake/ctest_submit/CDashUploadRETRY_COUNT-stderr.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-CMake Error at .*/Tests/RunCMake/ctest_submit/CDashUploadRETRY_COUNT/test.cmake:[0-9]+ \(ctest_submit\):
-  ctest_submit called with unknown argument "RETRY_COUNT".
diff --git a/Tests/RunCMake/ctest_submit/CDashUploadRETRY_DELAY-stderr.txt b/Tests/RunCMake/ctest_submit/CDashUploadRETRY_DELAY-stderr.txt
deleted file mode 100644
index f726674..0000000
--- a/Tests/RunCMake/ctest_submit/CDashUploadRETRY_DELAY-stderr.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-CMake Error at .*/Tests/RunCMake/ctest_submit/CDashUploadRETRY_DELAY/test.cmake:[0-9]+ \(ctest_submit\):
-  ctest_submit called with unknown argument "RETRY_DELAY".
diff --git a/Tests/RunCMake/ctest_submit/CDashUploadRETRY_COUNT-result.txt b/Tests/RunCMake/ctest_submit/CDashUploadRetry-result.txt
index b57e2de..b57e2de 100644
--- a/Tests/RunCMake/ctest_submit/CDashUploadRETRY_COUNT-result.txt
+++ b/Tests/RunCMake/ctest_submit/CDashUploadRetry-result.txt
diff --git a/Tests/RunCMake/ctest_submit/CDashUploadRetry-stderr.txt b/Tests/RunCMake/ctest_submit/CDashUploadRetry-stderr.txt
new file mode 100644
index 0000000..6780c17
--- /dev/null
+++ b/Tests/RunCMake/ctest_submit/CDashUploadRetry-stderr.txt
@@ -0,0 +1 @@
+error uploading to CDash.
diff --git a/Tests/RunCMake/ctest_submit/CDashUploadRetry-stdout.txt b/Tests/RunCMake/ctest_submit/CDashUploadRetry-stdout.txt
new file mode 100644
index 0000000..a5459ac
--- /dev/null
+++ b/Tests/RunCMake/ctest_submit/CDashUploadRetry-stdout.txt
@@ -0,0 +1,4 @@
+   Upload failed, waiting 1 seconds...
+   Retry upload: Attempt 1 of 2
+   Upload failed, waiting 1 seconds...
+   Retry upload: Attempt 2 of 2
diff --git a/Tests/RunCMake/ctest_submit/RunCMakeTest.cmake b/Tests/RunCMake/ctest_submit/RunCMakeTest.cmake
index a81bc96..e104f8a 100644
--- a/Tests/RunCMake/ctest_submit/RunCMakeTest.cmake
+++ b/Tests/RunCMake/ctest_submit/RunCMakeTest.cmake
@@ -21,9 +21,9 @@ run_ctest_submit(PARTSCDashUpload PARTS Configure CDASH_UPLOAD)
 run_ctest_submit(PARTSCDashUploadType PARTS Configure CDASH_UPLOAD_TYPE)
 run_ctest_submit(CDashUploadPARTS CDASH_UPLOAD bad-upload PARTS)
 run_ctest_submit(CDashUploadFILES CDASH_UPLOAD bad-upload FILES)
-run_ctest_submit(CDashUploadRETRY_COUNT CDASH_UPLOAD bad-upload RETRY_COUNT)
-run_ctest_submit(CDashUploadRETRY_DELAY CDASH_UPLOAD bad-upload RETRY_DELAY)
 run_ctest_submit(CDashUploadNone CDASH_UPLOAD)
+run_ctest_submit(CDashUploadMissingFile CDASH_UPLOAD bad-upload)
+run_ctest_submit(CDashUploadRetry CDASH_UPLOAD ${CMAKE_CURRENT_LIST_FILE} CDASH_UPLOAD_TYPE foo RETRY_COUNT 2 RETRY_DELAY 1 INTERNAL_TEST_CHECKSUM)
 run_ctest_submit(CDashSubmitQuiet QUIET)
 
 function(run_ctest_CDashUploadFTP)