Cleanup and reapply uprevs in PublishUprev in CQ.

Currently the CQ reuses old uprevs from the Uprev stage and this is
error-prone because the SHA1s can be out of date. We have robust
logic for fixing this but it only executes when the commit queue
fails. This has the nice side effect of also making the commit queue
more robust to conflicting with other changes, since it resyncs and
reapplies all changes from scratch at the end, rather than trying to
do a complex rebase that could run into conflicts.

This logic is quite well tested and has been in use for a long time,
so it's time we switch to it.

I've also deleted the old UpdateCommitHashesForChanges function. This
logic wasn't working (due to the fact that it was running on the wrong
branch after phobbs' changes) and with my change above, is now
obsolete.

BUG=chromium:549839, chromium:547541
TEST=Unit tests.

Change-Id: I9174504db4f468e04ec7a1c60091c16077be32c1
Reviewed-on: https://chromium-review.googlesource.com/310032
Trybot-Ready: David James <davidjames@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: David James <davidjames@chromium.org>

1 files changed, 11 insertions, 11 deletions

diff --git a/cbuildbot/stages/completion_stages.py b/cbuildbot/stages/completion_stages.py
index 2c6b08d34..b77b2f05b 100644
--- a/cbuildbot/stages/completion_stages.py
+++ b/cbuildbot/stages/completion_stages.py
@@ -18,9 +18,7 @@ from chromite.cbuildbot.stages import generic_stages
 from chromite.cbuildbot.stages import sync_stages
 from chromite.lib import clactions
 from chromite.lib import cros_logging as logging
-from chromite.lib import git
 from chromite.lib import patch as cros_patch
-from chromite.lib import portage_util
 
 
 def GetBuilderSuccessMap(builder_run, overall_success):
@@ -485,11 +483,6 @@ class CommitQueueCompletionStage(MasterSlaveSyncCompletionStage):
   def HandleSuccess(self):
     if self._run.config.master:
       self.sync_stage.pool.SubmitPool(reason=constants.STRATEGY_CQ_SUCCESS)
-      # After submitting the pool, update the commit hashes for uprevved
-      # ebuilds.
-      manifest = git.ManifestCheckout.Cached(self._build_root)
-      portage_util.EBuild.UpdateCommitHashesForChanges(
-          self.sync_stage.pool.changes, self._build_root, manifest)
       if config_lib.IsPFQType(self._run.config.build_type):
         super(CommitQueueCompletionStage, self).HandleSuccess()
 
@@ -830,10 +823,17 @@ class PublishUprevChangesStage(generic_stages.BuilderStage):
     overlays, push_overlays = self._ExtractOverlays()
     assert push_overlays, 'push_overlays must be set to run this stage'
 
-    # If the build failed, we don't want to push our local changes, because
-    # they might include some CLs that failed. Instead, clean up our local
-    # changes and do a fresh uprev.
-    if not self.success:
+    # If we're a commit queue, we should clean out our local changes, resync,
+    # and reapply our uprevs. This is necessary so that 1) we are sure to point
+    # at the remote SHA1s, not our local SHA1s; 2) we can avoid doing a
+    # rebase; 3) in the case of failure, we don't submit the changes that were
+    # committed locally.
+    #
+    # If we're not a commit queue and the build succeeded, we can skip the
+    # cleanup here. This is a cheap trick so that the Chrome PFQ pushes its
+    # earlier uprev from the SyncChrome stage (it would be a bit tricky to
+    # replicate the uprev here, so we'll leave it alone).
+    if config_lib.IsCQType(self._run.config.build_type) or not self.success:
       # Clean up our root and sync down the latest changes that were
       # submitted.
       commands.BuildRootGitCleanup(self._build_root)