From ef399f9bc6b8e2ff917a5f2809346fb81fd954ae Mon Sep 17 00:00:00 2001
From: Brad King <brad.king@kitware.com>
Date: Wed, 15 Feb 2017 13:58:40 -0500
Subject: ctest_update: Refactor internal APIs to support more failure cases

Thread failure of VC tool commands through more APIs so that we can
detect when they fail.  Defer updating of the individual VC tool usage
the future and just return true from them for now.
---
 Source/CTest/cmCTestBZR.cxx           | 14 +++++++++-----
 Source/CTest/cmCTestBZR.h             |  8 ++++----
 Source/CTest/cmCTestGIT.cxx           | 12 ++++++++----
 Source/CTest/cmCTestGIT.h             |  8 ++++----
 Source/CTest/cmCTestGlobalVC.cxx      |  7 ++++---
 Source/CTest/cmCTestGlobalVC.h        |  4 ++--
 Source/CTest/cmCTestHG.cxx            | 12 ++++++++----
 Source/CTest/cmCTestHG.h              |  8 ++++----
 Source/CTest/cmCTestP4.cxx            | 16 ++++++++++------
 Source/CTest/cmCTestP4.h              |  8 ++++----
 Source/CTest/cmCTestSVN.cxx           | 12 ++++++++----
 Source/CTest/cmCTestSVN.h             |  8 ++++----
 Source/CTest/cmCTestUpdateHandler.cxx |  4 ++--
 Source/CTest/cmCTestVC.cxx            | 12 +++++++-----
 Source/CTest/cmCTestVC.h              |  4 ++--
 15 files changed, 80 insertions(+), 57 deletions(-)

diff --git a/Source/CTest/cmCTestBZR.cxx b/Source/CTest/cmCTestBZR.cxx
index b42953b..6769ee5 100644
--- a/Source/CTest/cmCTestBZR.cxx
+++ b/Source/CTest/cmCTestBZR.cxx
@@ -151,22 +151,24 @@ std::string cmCTestBZR::LoadInfo()
   return rev;
 }
 
-void cmCTestBZR::NoteOldRevision()
+bool cmCTestBZR::NoteOldRevision()
 {
   this->OldRevision = this->LoadInfo();
   this->Log << "Revision before update: " << this->OldRevision << "\n";
   cmCTestLog(this->CTest, HANDLER_OUTPUT, "   Old revision of repository is: "
                << this->OldRevision << "\n");
   this->PriorRev.Rev = this->OldRevision;
+  return true;
 }
 
-void cmCTestBZR::NoteNewRevision()
+bool cmCTestBZR::NoteNewRevision()
 {
   this->NewRevision = this->LoadInfo();
   this->Log << "Revision after update: " << this->NewRevision << "\n";
   cmCTestLog(this->CTest, HANDLER_OUTPUT, "   New revision of repository is: "
                << this->NewRevision << "\n");
   this->Log << "URL = " << this->URL << "\n";
+  return true;
 }
 
 class cmCTestBZR::LogParser : public cmCTestVC::OutputLogger,
@@ -386,7 +388,7 @@ bool cmCTestBZR::UpdateImpl()
   return this->RunUpdateCommand(&bzr_update[0], &out, &err);
 }
 
-void cmCTestBZR::LoadRevisions()
+bool cmCTestBZR::LoadRevisions()
 {
   cmCTestLog(this->CTest, HANDLER_OUTPUT,
              "   Gathering version information (one . per revision):\n"
@@ -400,7 +402,7 @@ void cmCTestBZR::LoadRevisions()
     // DoRevision takes care of discarding the information about OldRevision
     revs = this->OldRevision + ".." + this->NewRevision;
   } else {
-    return;
+    return true;
   }
 
   // Run "bzr log" to get all global revisions of interest.
@@ -415,6 +417,7 @@ void cmCTestBZR::LoadRevisions()
     this->RunChild(bzr_log, &out, &err);
   }
   cmCTestLog(this->CTest, HANDLER_OUTPUT, std::endl);
+  return true;
 }
 
 class cmCTestBZR::StatusParser : public cmCTestVC::LineParser
@@ -460,7 +463,7 @@ private:
   }
 };
 
-void cmCTestBZR::LoadModifications()
+bool cmCTestBZR::LoadModifications()
 {
   // Run "bzr status" which reports local modifications.
   const char* bzr = this->CommandLineTool.c_str();
@@ -468,4 +471,5 @@ void cmCTestBZR::LoadModifications()
   StatusParser out(this, "status-out> ");
   OutputLogger err(this->Log, "status-err> ");
   this->RunChild(bzr_status, &out, &err);
+  return true;
 }
diff --git a/Source/CTest/cmCTestBZR.h b/Source/CTest/cmCTestBZR.h
index e7af90b..0b62582 100644
--- a/Source/CTest/cmCTestBZR.h
+++ b/Source/CTest/cmCTestBZR.h
@@ -26,16 +26,16 @@ public:
 
 private:
   // Implement cmCTestVC internal API.
-  void NoteOldRevision() CM_OVERRIDE;
-  void NoteNewRevision() CM_OVERRIDE;
+  bool NoteOldRevision() CM_OVERRIDE;
+  bool NoteNewRevision() CM_OVERRIDE;
   bool UpdateImpl() CM_OVERRIDE;
 
   // URL of repository directory checked out in the working tree.
   std::string URL;
 
   std::string LoadInfo();
-  void LoadModifications() CM_OVERRIDE;
-  void LoadRevisions() CM_OVERRIDE;
+  bool LoadModifications() CM_OVERRIDE;
+  bool LoadRevisions() CM_OVERRIDE;
 
   // Parsing helper classes.
   class InfoParser;
diff --git a/Source/CTest/cmCTestGIT.cxx b/Source/CTest/cmCTestGIT.cxx
index d30f6b3..9c53aa1 100644
--- a/Source/CTest/cmCTestGIT.cxx
+++ b/Source/CTest/cmCTestGIT.cxx
@@ -67,19 +67,21 @@ std::string cmCTestGIT::GetWorkingRevision()
   return rev;
 }
 
-void cmCTestGIT::NoteOldRevision()
+bool cmCTestGIT::NoteOldRevision()
 {
   this->OldRevision = this->GetWorkingRevision();
   cmCTestLog(this->CTest, HANDLER_OUTPUT, "   Old revision of repository is: "
                << this->OldRevision << "\n");
   this->PriorRev.Rev = this->OldRevision;
+  return true;
 }
 
-void cmCTestGIT::NoteNewRevision()
+bool cmCTestGIT::NoteNewRevision()
 {
   this->NewRevision = this->GetWorkingRevision();
   cmCTestLog(this->CTest, HANDLER_OUTPUT, "   New revision of repository is: "
                << this->NewRevision << "\n");
+  return true;
 }
 
 std::string cmCTestGIT::FindGitDir()
@@ -607,7 +609,7 @@ private:
 char const cmCTestGIT::CommitParser::SectionSep[SectionCount] = { '\n', '\n',
                                                                   '\0' };
 
-void cmCTestGIT::LoadRevisions()
+bool cmCTestGIT::LoadRevisions()
 {
   // Use 'git rev-list ... | git diff-tree ...' to get revisions.
   std::string range = this->OldRevision + ".." + this->NewRevision;
@@ -634,9 +636,10 @@ void cmCTestGIT::LoadRevisions()
   out.Process("", 1);
 
   cmsysProcess_Delete(cp);
+  return true;
 }
 
-void cmCTestGIT::LoadModifications()
+bool cmCTestGIT::LoadModifications()
 {
   const char* git = this->CommandLineTool.c_str();
 
@@ -660,4 +663,5 @@ void cmCTestGIT::LoadModifications()
        ci != out.Changes.end(); ++ci) {
     this->DoModification(PathModified, ci->Path);
   }
+  return true;
 }
diff --git a/Source/CTest/cmCTestGIT.h b/Source/CTest/cmCTestGIT.h
index a655502..d5a1ee3 100644
--- a/Source/CTest/cmCTestGIT.h
+++ b/Source/CTest/cmCTestGIT.h
@@ -28,8 +28,8 @@ private:
   unsigned int CurrentGitVersion;
   unsigned int GetGitVersion();
   std::string GetWorkingRevision();
-  void NoteOldRevision() CM_OVERRIDE;
-  void NoteNewRevision() CM_OVERRIDE;
+  bool NoteOldRevision() CM_OVERRIDE;
+  bool NoteNewRevision() CM_OVERRIDE;
   bool UpdateImpl() CM_OVERRIDE;
 
   std::string FindGitDir();
@@ -39,8 +39,8 @@ private:
   bool UpdateByCustom(std::string const& custom);
   bool UpdateInternal();
 
-  void LoadRevisions() CM_OVERRIDE;
-  void LoadModifications() CM_OVERRIDE;
+  bool LoadRevisions() CM_OVERRIDE;
+  bool LoadModifications() CM_OVERRIDE;
 
   // "public" needed by older Sun compilers
 public:
diff --git a/Source/CTest/cmCTestGlobalVC.cxx b/Source/CTest/cmCTestGlobalVC.cxx
index 08af179..25294b5 100644
--- a/Source/CTest/cmCTestGlobalVC.cxx
+++ b/Source/CTest/cmCTestGlobalVC.cxx
@@ -102,14 +102,15 @@ void cmCTestGlobalVC::WriteXMLGlobal(cmXMLWriter& xml)
 
 bool cmCTestGlobalVC::WriteXMLUpdates(cmXMLWriter& xml)
 {
+  bool result = true;
   cmCTestLog(this->CTest, HANDLER_OUTPUT,
              "   Gathering version information (one . per revision):\n"
              "    "
                << std::flush);
-  this->LoadRevisions();
+  result = this->LoadRevisions() && result;
   cmCTestLog(this->CTest, HANDLER_OUTPUT, std::endl);
 
-  this->LoadModifications();
+  result = this->LoadModifications() && result;
 
   this->WriteXMLGlobal(xml);
 
@@ -119,5 +120,5 @@ bool cmCTestGlobalVC::WriteXMLUpdates(cmXMLWriter& xml)
     this->WriteXMLDirectory(xml, di->first, di->second);
   }
 
-  return true;
+  return result;
 }
diff --git a/Source/CTest/cmCTestGlobalVC.h b/Source/CTest/cmCTestGlobalVC.h
index 9a3757d..9fa5f21 100644
--- a/Source/CTest/cmCTestGlobalVC.h
+++ b/Source/CTest/cmCTestGlobalVC.h
@@ -64,8 +64,8 @@ protected:
   virtual void DoRevision(Revision const& revision,
                           std::vector<Change> const& changes);
   virtual void DoModification(PathStatus status, std::string const& path);
-  virtual void LoadModifications() = 0;
-  virtual void LoadRevisions() = 0;
+  virtual bool LoadModifications() = 0;
+  virtual bool LoadRevisions() = 0;
 
   virtual void WriteXMLGlobal(cmXMLWriter& xml);
   void WriteXMLDirectory(cmXMLWriter& xml, std::string const& path,
diff --git a/Source/CTest/cmCTestHG.cxx b/Source/CTest/cmCTestHG.cxx
index 8443c93..68ebd37 100644
--- a/Source/CTest/cmCTestHG.cxx
+++ b/Source/CTest/cmCTestHG.cxx
@@ -104,19 +104,21 @@ std::string cmCTestHG::GetWorkingRevision()
   return rev;
 }
 
-void cmCTestHG::NoteOldRevision()
+bool cmCTestHG::NoteOldRevision()
 {
   this->OldRevision = this->GetWorkingRevision();
   cmCTestLog(this->CTest, HANDLER_OUTPUT, "   Old revision of repository is: "
                << this->OldRevision << "\n");
   this->PriorRev.Rev = this->OldRevision;
+  return true;
 }
 
-void cmCTestHG::NoteNewRevision()
+bool cmCTestHG::NoteNewRevision()
 {
   this->NewRevision = this->GetWorkingRevision();
   cmCTestLog(this->CTest, HANDLER_OUTPUT, "   New revision of repository is: "
                << this->NewRevision << "\n");
+  return true;
 }
 
 bool cmCTestHG::UpdateImpl()
@@ -262,7 +264,7 @@ private:
   }
 };
 
-void cmCTestHG::LoadRevisions()
+bool cmCTestHG::LoadRevisions()
 {
   // Use 'hg log' to get revisions in a xml format.
   //
@@ -293,9 +295,10 @@ void cmCTestHG::LoadRevisions()
   OutputLogger err(this->Log, "log-err> ");
   this->RunChild(hg_log, &out, &err);
   out.Process("</log>\n");
+  return true;
 }
 
-void cmCTestHG::LoadModifications()
+bool cmCTestHG::LoadModifications()
 {
   // Use 'hg status' to get modified files.
   const char* hg = this->CommandLineTool.c_str();
@@ -303,4 +306,5 @@ void cmCTestHG::LoadModifications()
   StatusParser out(this, "status-out> ");
   OutputLogger err(this->Log, "status-err> ");
   this->RunChild(hg_status, &out, &err);
+  return true;
 }
diff --git a/Source/CTest/cmCTestHG.h b/Source/CTest/cmCTestHG.h
index a81c347..63baba2 100644
--- a/Source/CTest/cmCTestHG.h
+++ b/Source/CTest/cmCTestHG.h
@@ -26,12 +26,12 @@ public:
 
 private:
   std::string GetWorkingRevision();
-  void NoteOldRevision() CM_OVERRIDE;
-  void NoteNewRevision() CM_OVERRIDE;
+  bool NoteOldRevision() CM_OVERRIDE;
+  bool NoteNewRevision() CM_OVERRIDE;
   bool UpdateImpl() CM_OVERRIDE;
 
-  void LoadRevisions() CM_OVERRIDE;
-  void LoadModifications() CM_OVERRIDE;
+  bool LoadRevisions() CM_OVERRIDE;
+  bool LoadModifications() CM_OVERRIDE;
 
   // Parsing helper classes.
   class IdentifyParser;
diff --git a/Source/CTest/cmCTestP4.cxx b/Source/CTest/cmCTestP4.cxx
index 41b45a8..4f78876 100644
--- a/Source/CTest/cmCTestP4.cxx
+++ b/Source/CTest/cmCTestP4.cxx
@@ -369,24 +369,26 @@ std::string cmCTestP4::GetWorkingRevision()
   return rev;
 }
 
-void cmCTestP4::NoteOldRevision()
+bool cmCTestP4::NoteOldRevision()
 {
   this->OldRevision = this->GetWorkingRevision();
 
   cmCTestLog(this->CTest, HANDLER_OUTPUT, "   Old revision of repository is: "
                << this->OldRevision << "\n");
   this->PriorRev.Rev = this->OldRevision;
+  return true;
 }
 
-void cmCTestP4::NoteNewRevision()
+bool cmCTestP4::NoteNewRevision()
 {
   this->NewRevision = this->GetWorkingRevision();
 
   cmCTestLog(this->CTest, HANDLER_OUTPUT, "   New revision of repository is: "
                << this->NewRevision << "\n");
+  return true;
 }
 
-void cmCTestP4::LoadRevisions()
+bool cmCTestP4::LoadRevisions()
 {
   std::vector<char const*> p4_changes;
   SetP4Options(p4_changes);
@@ -399,7 +401,7 @@ void cmCTestP4::LoadRevisions()
   if (this->OldRevision == "<unknown>" || this->NewRevision == "<unknown>") {
     cmCTestLog(this->CTest, HANDLER_OUTPUT, "   At least one of the revisions "
                  << "is unknown. No repository changes will be reported.\n");
-    return;
+    return false;
   }
 
   range.append("@")
@@ -418,7 +420,7 @@ void cmCTestP4::LoadRevisions()
   this->RunChild(&p4_changes[0], &out, &err);
 
   if (ChangeLists.empty()) {
-    return;
+    return true;
   }
 
   // p4 describe -s ...@1111111,2222222
@@ -435,9 +437,10 @@ void cmCTestP4::LoadRevisions()
     OutputLogger errDescribe(this->Log, "p4_describe-err> ");
     this->RunChild(&p4_describe[0], &outDescribe, &errDescribe);
   }
+  return true;
 }
 
-void cmCTestP4::LoadModifications()
+bool cmCTestP4::LoadModifications()
 {
   std::vector<char const*> p4_diff;
   SetP4Options(p4_diff);
@@ -453,6 +456,7 @@ void cmCTestP4::LoadModifications()
   DiffParser out(this, "p4_diff-out> ");
   OutputLogger err(this->Log, "p4_diff-err> ");
   this->RunChild(&p4_diff[0], &out, &err);
+  return true;
 }
 
 bool cmCTestP4::UpdateCustom(const std::string& custom)
diff --git a/Source/CTest/cmCTestP4.h b/Source/CTest/cmCTestP4.h
index eadc4fb..2c04dc6 100644
--- a/Source/CTest/cmCTestP4.h
+++ b/Source/CTest/cmCTestP4.h
@@ -51,13 +51,13 @@ private:
   void SetP4Options(std::vector<char const*>& options);
 
   std::string GetWorkingRevision();
-  void NoteOldRevision() CM_OVERRIDE;
-  void NoteNewRevision() CM_OVERRIDE;
+  bool NoteOldRevision() CM_OVERRIDE;
+  bool NoteNewRevision() CM_OVERRIDE;
   bool UpdateImpl() CM_OVERRIDE;
   bool UpdateCustom(const std::string& custom);
 
-  void LoadRevisions() CM_OVERRIDE;
-  void LoadModifications() CM_OVERRIDE;
+  bool LoadRevisions() CM_OVERRIDE;
+  bool LoadModifications() CM_OVERRIDE;
 
   class ChangesParser;
   class DescribeParser;
diff --git a/Source/CTest/cmCTestSVN.cxx b/Source/CTest/cmCTestSVN.cxx
index 410e0d4..e85d01c 100644
--- a/Source/CTest/cmCTestSVN.cxx
+++ b/Source/CTest/cmCTestSVN.cxx
@@ -97,7 +97,7 @@ std::string cmCTestSVN::LoadInfo(SVNInfo& svninfo)
   return rev;
 }
 
-void cmCTestSVN::NoteOldRevision()
+bool cmCTestSVN::NoteOldRevision()
 {
   this->LoadRepositories();
 
@@ -116,9 +116,10 @@ void cmCTestSVN::NoteOldRevision()
   // Set the global old revision to the one of the root
   this->OldRevision = this->RootInfo->OldRevision;
   this->PriorRev.Rev = this->OldRevision;
+  return true;
 }
 
-void cmCTestSVN::NoteNewRevision()
+bool cmCTestSVN::NoteNewRevision()
 {
   this->LoadRepositories();
 
@@ -153,6 +154,7 @@ void cmCTestSVN::NoteNewRevision()
 
   // Set the global new revision to the one of the root
   this->NewRevision = this->RootInfo->NewRevision;
+  return true;
 }
 
 void cmCTestSVN::GuessBase(SVNInfo& svninfo,
@@ -370,7 +372,7 @@ private:
   }
 };
 
-void cmCTestSVN::LoadRevisions()
+bool cmCTestSVN::LoadRevisions()
 {
   // Get revisions for all the external repositories
   std::list<SVNInfo>::iterator itbeg = this->Repositories.begin();
@@ -379,6 +381,7 @@ void cmCTestSVN::LoadRevisions()
     SVNInfo& svninfo = *itbeg;
     LoadRevisions(svninfo);
   }
+  return true;
 }
 
 void cmCTestSVN::LoadRevisions(SVNInfo& svninfo)
@@ -468,7 +471,7 @@ private:
   }
 };
 
-void cmCTestSVN::LoadModifications()
+bool cmCTestSVN::LoadModifications()
 {
   // Run "svn status" which reports local modifications.
   std::vector<const char*> svn_status;
@@ -476,6 +479,7 @@ void cmCTestSVN::LoadModifications()
   StatusParser out(this, "status-out> ");
   OutputLogger err(this->Log, "status-err> ");
   this->RunSVNCommand(svn_status, &out, &err);
+  return true;
 }
 
 void cmCTestSVN::WriteXMLGlobal(cmXMLWriter& xml)
diff --git a/Source/CTest/cmCTestSVN.h b/Source/CTest/cmCTestSVN.h
index c0348c2..1832fd9 100644
--- a/Source/CTest/cmCTestSVN.h
+++ b/Source/CTest/cmCTestSVN.h
@@ -30,8 +30,8 @@ public:
 private:
   // Implement cmCTestVC internal API.
   void CleanupImpl() CM_OVERRIDE;
-  void NoteOldRevision() CM_OVERRIDE;
-  void NoteNewRevision() CM_OVERRIDE;
+  bool NoteOldRevision() CM_OVERRIDE;
+  bool NoteNewRevision() CM_OVERRIDE;
   bool UpdateImpl() CM_OVERRIDE;
 
   bool RunSVNCommand(std::vector<char const*> const& parameters,
@@ -78,8 +78,8 @@ private:
 
   std::string LoadInfo(SVNInfo& svninfo);
   void LoadRepositories();
-  void LoadModifications() CM_OVERRIDE;
-  void LoadRevisions() CM_OVERRIDE;
+  bool LoadModifications() CM_OVERRIDE;
+  bool LoadRevisions() CM_OVERRIDE;
   void LoadRevisions(SVNInfo& svninfo);
 
   void GuessBase(SVNInfo& svninfo, std::vector<Change> const& changes);
diff --git a/Source/CTest/cmCTestUpdateHandler.cxx b/Source/CTest/cmCTestUpdateHandler.cxx
index 0998d59..2b5683a 100644
--- a/Source/CTest/cmCTestUpdateHandler.cxx
+++ b/Source/CTest/cmCTestUpdateHandler.cxx
@@ -198,7 +198,7 @@ int cmCTestUpdateHandler::ProcessHandler()
   xml.Element("UpdateType",
               cmCTestUpdateHandlerUpdateToString(this->UpdateType));
 
-  vc->WriteXML(xml);
+  bool loadedMods = vc->WriteXML(xml);
 
   int localModifications = 0;
   int numUpdated = vc->GetPathCount(cmCTestVC::PathUpdated);
@@ -246,7 +246,7 @@ int cmCTestUpdateHandler::ProcessHandler()
   xml.EndElement(); // UpdateReturnStatus
   xml.EndElement(); // Update
   xml.EndDocument();
-  return updated ? numUpdated : -1;
+  return updated && loadedMods ? numUpdated : -1;
 }
 
 int cmCTestUpdateHandler::DetectVCS(const char* dir)
diff --git a/Source/CTest/cmCTestVC.cxx b/Source/CTest/cmCTestVC.cxx
index 444c43d..26c9bb5 100644
--- a/Source/CTest/cmCTestVC.cxx
+++ b/Source/CTest/cmCTestVC.cxx
@@ -147,23 +147,25 @@ bool cmCTestVC::Update()
   // just note the current version and finish
   if (!cmSystemTools::IsOn(
         this->CTest->GetCTestConfiguration("UpdateVersionOnly").c_str())) {
-    this->NoteOldRevision();
+    result = this->NoteOldRevision() && result;
     this->Log << "--- Begin Update ---\n";
-    result = this->UpdateImpl();
+    result = this->UpdateImpl() && result;
     this->Log << "--- End Update ---\n";
   }
-  this->NoteNewRevision();
+  result = this->NoteNewRevision() && result;
   return result;
 }
 
-void cmCTestVC::NoteOldRevision()
+bool cmCTestVC::NoteOldRevision()
 {
   // We do nothing by default.
+  return true;
 }
 
-void cmCTestVC::NoteNewRevision()
+bool cmCTestVC::NoteNewRevision()
 {
   // We do nothing by default.
+  return true;
 }
 
 bool cmCTestVC::UpdateImpl()
diff --git a/Source/CTest/cmCTestVC.h b/Source/CTest/cmCTestVC.h
index 2681ba0..a1c1673 100644
--- a/Source/CTest/cmCTestVC.h
+++ b/Source/CTest/cmCTestVC.h
@@ -67,9 +67,9 @@ public:
 protected:
   // Internal API to be implemented by subclasses.
   virtual void CleanupImpl();
-  virtual void NoteOldRevision();
+  virtual bool NoteOldRevision();
   virtual bool UpdateImpl();
-  virtual void NoteNewRevision();
+  virtual bool NoteNewRevision();
   virtual bool WriteXMLUpdates(cmXMLWriter& xml);
 
 #if defined(__SUNPRO_CC) && __SUNPRO_CC <= 0x510
-- 
cgit v0.12