Have the ability for the PFQ to both rev Chrome and other packages.

This issue seems bigger than it first appears because we had put extra logic to "Delete" the branch if we didn't rev anything.  This CL basically takes out all the Deletes and relies and cros_mark_as_stable clean to clean up any old dirs correctly.  To do this, I had to fix a bug in clean so that we actually delete the stable branch as part of it (if it exists).

EOM

Change-Id: Ic1020451dc1d492d4b6e2a233d1d49cf8c1c16b3

BUG=chromium-os:11329
TEST=Ran x86-generic-pfq w/ dev options + --chrome_rev=latest_release ...built chrome,
and revved packages without error.  Doing more testing still.

Review URL: http://codereview.chromium.org/6291013

bin/cbuildbot.py

@@ -738,17 +738,17 @@ def main():
     if not os.path.isdir(boardpath):
       _SetupBoard(buildroot, board=buildconfig['board'])
 
-    # Perform uprev.  If chrome_uprev is set, rev Chrome ebuilds.
+    # Perform chrome uprev.
     if options.chrome_rev:
       chrome_atom_to_build = _MarkChromeAsStable(buildroot, tracking_branch,
                                                  options.chrome_rev, board)
-      # If we found nothing to rev, we're done here.
+    # Perform other uprevs.
-      if not chrome_atom_to_build:
+    if buildconfig['uprev']:
-        return
-
-    elif buildconfig['uprev']:
       _UprevPackages(buildroot, tracking_branch, revisionfile,
                      buildconfig['board'], rev_overlays)
+    elif not chrome_atom_to_build:
+      # We found nothing to rev, we're done here.
+      return
 
     _EnableLocalAccount(buildroot)
 

bin/cbuildbot_config.py

@@ -76,10 +76,14 @@ config['x86-generic-pre-flight-queue'].update({
   'rev_overlays': 'public',
   'push_overlays': 'public',
 })
-
+config['x86-generic-chrome-pre-flight-queue'] = default.copy()
-config['x86-generic-chrome-pre-flight-queue'] = \
+config['x86-generic-chrome-pre-flight-queue'].update({
-    config['x86-generic-pre-flight-queue']
+  'board' : 'x86-generic',
-
+  'uprev' : False,
+  'master' : True,
+  'rev_overlays': 'public',
+  'push_overlays': 'public',
+})
 config['x86-mario-pre-flight-queue'] = default.copy()
 config['x86-mario-pre-flight-queue'].update({
   'board' : 'x86-mario',
@@ -88,7 +92,6 @@ config['x86-mario-pre-flight-queue'].update({
   'rev_overlays': 'both',
   'push_overlays': 'private',
 })
-
 config['x86-mario-pre-flight-branch'] = default.copy()
 config['x86-mario-pre-flight-branch'].update({
   'board' : 'x86-mario',

bin/cros_mark_chrome_as_stable.py

@@ -345,18 +345,12 @@ def main():
   work_branch = cros_mark_as_stable.GitBranch(
       cros_mark_as_stable.STABLE_BRANCH_NAME, options.tracking_branch)
   work_branch.CreateBranch()
-  try:
   chrome_version_atom = MarkChromeEBuildAsStable(
       stable_candidate, unstable_ebuild, chrome_rev, version_to_uprev,
       commit_to_use, overlay_dir, sticky_ebuild)
   # Explicit print to communicate to caller.
   if chrome_version_atom:
     print 'CHROME_VERSION_ATOM=%s' % chrome_version_atom
-    else:
-      work_branch.Delete()
-  except:
-    work_branch.Delete()
-    raise
 
 
 if __name__ == '__main__':

cros_mark_as_stable.py

@@ -166,10 +166,15 @@ def _BuildEBuildDictionary(overlays, all, packages):
         overlays[overlay].append(ebuild)
 
 
-def _CheckOnStabilizingBranch(stable_branch):
+def _DoWeHaveLocalCommits(stable_branch, tracking_branch):
-  """Returns true if the git branch is on the stabilizing branch."""
+  """Returns true if there are local commits."""
   current_branch = _SimpleRunCommand('git branch | grep \*').split()[1]
-  return current_branch == stable_branch
+  if current_branch == stable_branch:
+    current_commit_id = _SimpleRunCommand('git rev-parse HEAD')
+    tracking_commit_id = _SimpleRunCommand('git rev-parse %s' % tracking_branch)
+    return current_commit_id != tracking_commit_id
+  else:
+    return False
 
 
 def _CheckSaneArguments(package_list, command):
@@ -223,7 +228,10 @@ def Clean(tracking_branch):
     tracking_branch:  The tracking branch we want to return to after the call.
   """
   _SimpleRunCommand('git reset HEAD --hard')
-  _SimpleRunCommand('git checkout %s' % tracking_branch)
+  branch = GitBranch(STABLE_BRANCH_NAME, tracking_branch)
+  if branch.Exists():
+    GitBranch.Checkout(branch)
+    branch.Delete()
 
 
 def PushChange(stable_branch, tracking_branch):
@@ -241,8 +249,8 @@ def PushChange(stable_branch, tracking_branch):
   num_retries = 5
 
   # Sanity check to make sure we're on a stabilizing branch before pushing.
-  if not _CheckOnStabilizingBranch(stable_branch):
+  if not _DoWeHaveLocalCommits(stable_branch, tracking_branch):
-    Info('Not on branch %s so no work found to push.  Exiting' % stable_branch)
+    Info('Not work found to push.  Exiting')
     return
 
   description = _SimpleRunCommand('git log --format=format:%s%n%n%b ' +
@@ -274,6 +282,7 @@ def PushChange(stable_branch, tracking_branch):
         raise
 
 
+
 class GitBranch(object):
   """Wrapper class for a git branch."""
 
@@ -283,17 +292,16 @@ class GitBranch(object):
     self.tracking_branch = tracking_branch
 
   def CreateBranch(self):
-    """Creates a new git branch or replaces an existing one."""
+    GitBranch.Checkout(self)
-    if self.Exists():
-      self.Delete()
-    self._Checkout(self.branch_name)
 
-  def _Checkout(self, target, create=True):
+  @classmethod
-    """Function used internally to create and move between branches."""
+  def Checkout(cls, target):
-    if create:
+    """Function used to check out to another GitBranch."""
-      git_cmd = 'git checkout -b %s %s' % (target, self.tracking_branch)
+    if target.branch_name == target.tracking_branch or target.Exists():
+      git_cmd = 'git checkout %s' % target.branch_name
     else:
-      git_cmd = 'git checkout %s' % target
+      git_cmd = 'git checkout -b %s %s' % (target.branch_name,
+                                           target.tracking_branch)
     _SimpleRunCommand(git_cmd)
 
   def Exists(self):
@@ -307,7 +315,8 @@ class GitBranch(object):
 
     Returns True on success.
     """
-    self._Checkout(self.tracking_branch, create=False)
+    tracking_branch = GitBranch(self.tracking_branch, self.tracking_branch)
+    GitBranch.Checkout(tracking_branch)
     delete_cmd = 'git branch -D %s' % self.branch_name
     _SimpleRunCommand(delete_cmd)
 
@@ -570,14 +579,11 @@ def main(argv):
                   'and reset the git repo yourself.' % overlay)
           raise
 
-      if revved_packages:
       _CleanStalePackages(gflags.FLAGS.board, new_package_atoms)
       if gflags.FLAGS.drop_file:
         fh = open(gflags.FLAGS.drop_file, 'w')
         fh.write(' '.join(revved_packages))
         fh.close()
-      else:
-        work_branch.Delete()
 
 
 if __name__ == '__main__':

cros_mark_as_stable_unittest.py

@@ -24,11 +24,12 @@ class NonClassTests(mox.MoxTestBase):
   def testPushChange(self):
     git_log = 'Marking test_one as stable\nMarking test_two as stable\n'
     fake_description = 'Marking set of ebuilds as stable\n\n%s' % git_log
-    self.mox.StubOutWithMock(cros_mark_as_stable, '_CheckOnStabilizingBranch')
+    self.mox.StubOutWithMock(cros_mark_as_stable, '_DoWeHaveLocalCommits')
     self.mox.StubOutWithMock(cros_mark_as_stable.GitBranch, 'CreateBranch')
     self.mox.StubOutWithMock(cros_mark_as_stable.GitBranch, 'Exists')
 
-    cros_mark_as_stable._CheckOnStabilizingBranch(self._branch).AndReturn(True)
+    cros_mark_as_stable._DoWeHaveLocalCommits(
+        self._branch, self._tracking_branch).AndReturn(True)
     cros_mark_as_stable.GitBranch.CreateBranch()
     cros_mark_as_stable.GitBranch.Exists().AndReturn(True)
     cros_mark_as_stable._SimpleRunCommand('git log --format=format:%s%n%n%b ' +
@@ -51,66 +52,46 @@ class GitBranchTest(mox.MoxTestBase):
     mox.MoxTestBase.setUp(self)
     # Always stub RunCommmand out as we use it in every method.
     self.mox.StubOutWithMock(cros_mark_as_stable, '_SimpleRunCommand')
-    self._branch = 'test_branch'
+    self._branch = self.mox.CreateMock(cros_mark_as_stable.GitBranch)
+    self._branch_name = 'test_branch'
+    self._branch.branch_name = self._branch_name
     self._tracking_branch = 'cros/test'
-
+    self._branch.tracking_branch = self._tracking_branch
-  def testCreateBranchNoPrevious(self):
-    # Test init with no previous branch existing.
-    branch = cros_mark_as_stable.GitBranch(self._branch, self._tracking_branch)
-    self.mox.StubOutWithMock(branch, 'Exists')
-    self.mox.StubOutWithMock(branch, '_Checkout')
-    branch.Exists().AndReturn(False)
-    branch._Checkout(self._branch)
-    self.mox.ReplayAll()
-    branch.CreateBranch()
-    self.mox.VerifyAll()
-
-  def testCreateBranchWithPrevious(self):
-    # Test init with previous branch existing.
-    branch = cros_mark_as_stable.GitBranch(self._branch, self._tracking_branch)
-    self.mox.StubOutWithMock(branch, 'Exists')
-    self.mox.StubOutWithMock(branch, 'Delete')
-    self.mox.StubOutWithMock(branch, '_Checkout')
-    branch.Exists().AndReturn(True)
-    branch.Delete()
-    branch._Checkout(self._branch)
-    self.mox.ReplayAll()
-    branch.CreateBranch()
-    self.mox.VerifyAll()
 
   def testCheckoutCreate(self):
     # Test init with no previous branch existing.
+    self._branch.Exists().AndReturn(False)
     cros_mark_as_stable._SimpleRunCommand(
-        'git checkout -b %s %s' % (self._branch, self._tracking_branch))
+        'git checkout -b %s %s' % (self._branch_name, self._tracking_branch))
     self.mox.ReplayAll()
-    branch = cros_mark_as_stable.GitBranch(self._branch, self._tracking_branch)
+    cros_mark_as_stable.GitBranch.Checkout(self._branch)
-    branch._Checkout(self._branch)
     self.mox.VerifyAll()
 
   def testCheckoutNoCreate(self):
     # Test init with previous branch existing.
+    self._branch.Exists().AndReturn(True)
     cros_mark_as_stable._SimpleRunCommand('git checkout %s' % (
-        self._tracking_branch))
+        self._branch_name))
     self.mox.ReplayAll()
-    branch = cros_mark_as_stable.GitBranch(self._branch, self._tracking_branch)
+    cros_mark_as_stable.GitBranch.Checkout(self._branch)
-    branch._Checkout(self._tracking_branch, False)
     self.mox.VerifyAll()
 
   def testDelete(self):
-    branch = cros_mark_as_stable.GitBranch(self._branch, self._tracking_branch)
+    self.mox.StubOutWithMock(cros_mark_as_stable.GitBranch, 'Checkout')
-    self.mox.StubOutWithMock(branch, '_Checkout')
+    branch = cros_mark_as_stable.GitBranch(self._branch_name,
-    branch._Checkout(self._tracking_branch, create=False)
+                                           self._tracking_branch)
-    cros_mark_as_stable._SimpleRunCommand('git branch -D ' + self._branch)
+    cros_mark_as_stable.GitBranch.Checkout(mox.IgnoreArg())
+    cros_mark_as_stable._SimpleRunCommand('git branch -D ' + self._branch_name)
     self.mox.ReplayAll()
     branch.Delete()
     self.mox.VerifyAll()
 
   def testExists(self):
-    branch = cros_mark_as_stable.GitBranch(self._branch, self._tracking_branch)
+    branch = cros_mark_as_stable.GitBranch(self._branch_name,
-
+                                           self._tracking_branch)
     # Test if branch exists that is created
     cros_mark_as_stable._SimpleRunCommand('git branch').AndReturn(
-        '%s %s' % (self._branch, self._tracking_branch))
+        '%s' % self._branch_name)
     self.mox.ReplayAll()
     self.assertTrue(branch.Exists())
     self.mox.VerifyAll()