From 243cfc2f6f467c71e6dc5af0b1fb28980a31a5ae Mon Sep 17 00:00:00 2001
From: Pedro Navarro <pnavarro@netflix.com>
Date: Tue, 21 Jan 2014 17:02:25 -0800
Subject: [PATCH] ctest_update: Handle P4 unknown revisions more robustly

Mark unknown revisions as such and fail instead of reporting revision 0.
Otherwise CTest reports massive file updates between revisions when the
server timeouts while trying to fetch the current revision number.
---
 Source/CTest/cmCTestP4.cxx    | 61 ++++++++++++++++++++---------------
 Tests/CTestUpdateCommon.cmake |  4 ++-
 2 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/Source/CTest/cmCTestP4.cxx b/Source/CTest/cmCTestP4.cxx
index 0058721eb..b09d6f5a3 100644
--- a/Source/CTest/cmCTestP4.cxx
+++ b/Source/CTest/cmCTestP4.cxx
@@ -380,10 +380,16 @@ std::string cmCTestP4::GetWorkingRevision()
   p4_identify.push_back(0);
 
   std::string rev;
-  IdentifyParser out(this, "rev-out> ", rev);
-  OutputLogger err(this->Log, "rev-err> ");
+  IdentifyParser out(this, "p4_changes-out> ", rev);
+  OutputLogger err(this->Log, "p4_changes-err> ");
 
-  RunChild(&p4_identify[0], &out, &err);
+  bool result = RunChild(&p4_identify[0], &out, &err);
+
+  // If there was a problem contacting the server return "<unknown>"
+  if(!result)
+    {
+    return "<unknown>";
+    }
 
   if(rev.empty())
     {
@@ -423,29 +429,24 @@ void cmCTestP4::LoadRevisions()
   // Use 'p4 changes ...@old,new' to get a list of changelists
   std::string range = this->SourceDirectory + "/...";
 
-  if(this->OldRevision != "0")
+  // If any revision is unknown it means we couldn't contact the server.
+  // Do not process updates
+  if(this->OldRevision == "<unknown>" || this->NewRevision == "<unknown>")
     {
-    range.append("@").append(this->OldRevision);
+    cmCTestLog(this->CTest, HANDLER_OUTPUT, "   At least one of the revisions "
+               << "is unknown. No repository changes will be reported.\n");
+    return;
     }
 
-  if(this->NewRevision != "0")
-    {
-    if(this->OldRevision != "0")
-      {
-      range.append(",").append(this->NewRevision);
-      }
-    else
-      {
-      range.append("@").append(this->NewRevision);
-      }
-    }
+  range.append("@").append(this->OldRevision)
+       .append(",").append(this->NewRevision);
 
   p4_changes.push_back("changes");
   p4_changes.push_back(range.c_str());
   p4_changes.push_back(0);
 
-  ChangesParser out(this, "changes-out> ");
-  OutputLogger err(this->Log, "changes-err> ");
+  ChangesParser out(this, "p4_changes-out> ");
+  OutputLogger err(this->Log, "p4_changes-err> ");
 
   ChangeLists.clear();
   this->RunChild(&p4_changes[0], &out, &err);
@@ -464,8 +465,8 @@ void cmCTestP4::LoadRevisions()
     p4_describe.push_back(i->c_str());
     p4_describe.push_back(0);
 
-    DescribeParser outDescribe(this, "describe-out> ");
-    OutputLogger errDescribe(this->Log, "describe-err> ");
+    DescribeParser outDescribe(this, "p4_describe-out> ");
+    OutputLogger errDescribe(this->Log, "p4_describe-err> ");
     this->RunChild(&p4_describe[0], &outDescribe, &errDescribe);
     }
 }
@@ -484,8 +485,8 @@ void cmCTestP4::LoadModifications()
   p4_diff.push_back(source.c_str());
   p4_diff.push_back(0);
 
-  DiffParser out(this, "diff-out> ");
-  OutputLogger err(this->Log, "diff-err> ");
+  DiffParser out(this, "p4_diff-out> ");
+  OutputLogger err(this->Log, "p4_diff-err> ");
   this->RunChild(&p4_diff[0], &out, &err);
 }
 
@@ -503,8 +504,8 @@ bool cmCTestP4::UpdateCustom(const std::string& custom)
     }
   p4_custom.push_back(0);
 
-  OutputLogger custom_out(this->Log, "custom-out> ");
-  OutputLogger custom_err(this->Log, "custom-err> ");
+  OutputLogger custom_out(this->Log, "p4_customsync-out> ");
+  OutputLogger custom_err(this->Log, "p4_customsync-err> ");
 
   return this->RunUpdateCommand(&p4_custom[0], &custom_out, &custom_err);
 }
@@ -518,6 +519,14 @@ bool cmCTestP4::UpdateImpl()
     return this->UpdateCustom(custom);
     }
 
+  // If we couldn't get a revision number before updating, abort.
+  if(this->OldRevision == "<unknown>")
+    {
+    this->UpdateCommandLine = "Unknown current revision";
+    cmCTestLog(this->CTest, ERROR_MESSAGE, "   Unknown current revision\n");
+    return false;
+    }
+
   std::vector<char const*> p4_sync;
   SetP4Options(p4_sync);
 
@@ -552,8 +561,8 @@ bool cmCTestP4::UpdateImpl()
   p4_sync.push_back(source.c_str());
   p4_sync.push_back(0);
 
-  OutputLogger out(this->Log, "sync-out> ");
-  OutputLogger err(this->Log, "sync-err> ");
+  OutputLogger out(this->Log, "p4_sync-out> ");
+  OutputLogger err(this->Log, "p4_sync-err> ");
 
   return this->RunUpdateCommand(&p4_sync[0], &out, &err);
 }
diff --git a/Tests/CTestUpdateCommon.cmake b/Tests/CTestUpdateCommon.cmake
index db4e08dd7..642a61896 100644
--- a/Tests/CTestUpdateCommon.cmake
+++ b/Tests/CTestUpdateCommon.cmake
@@ -54,7 +54,9 @@ function(check_updates build)
   set(EXTRA "${UPDATE_XML_ENTRIES}")
   list(REMOVE_ITEM EXTRA ${ARGN} ${UPDATE_EXTRA} ${UPDATE_MAYBE})
   set(MISSING "${ARGN}" ${UPDATE_EXTRA})
-  list(REMOVE_ITEM MISSING ${UPDATE_XML_ENTRIES})
+  if(NOT "" STREQUAL "${UPDATE_XML_ENTRIES}")
+    list(REMOVE_ITEM MISSING ${UPDATE_XML_ENTRIES})
+  endif()
 
   if(NOT UPDATE_NOT_GLOBAL)
     set(rev_elements Revision PriorRevision ${UPDATE_GLOBAL_ELEMENTS})