Be more selective about what packages we remove so that we can detect conflicts.

Right now, the preflight queue unmerges all changed packages prior to
upgrading them. This is done to ensure that any lingering state from
failed package uprevs is cleaned up.

This change updates cros_mark_as_stable to be more selective. This both improves
speed and allows for more conflicts to be detected.

Change-Id: I3134918eb16b41e8882c8af1f1bdf9052bafd9ad

BUG=chromium-os:10127
TEST=Test uprev. Run unit tests.

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

cros_mark_as_stable.py

@@ -78,15 +78,15 @@ def _Print(message):
     Info(message)
 
 
-def _CleanStalePackages(board, package_array):
+def _CleanStalePackages(board, package_atoms):
     """Cleans up stale package info from a previous build."""
-    Info('Cleaning up stale packages %s.' % package_array)
+    Info('Cleaning up stale packages %s.' % package_atoms)
     unmerge_board_cmd = ['emerge-%s' % board, '--unmerge']
-    unmerge_board_cmd.extend(package_array)
+    unmerge_board_cmd.extend(package_atoms)
     RunCommand(unmerge_board_cmd)
 
     unmerge_host_cmd = ['sudo', 'emerge', '--unmerge']
-    unmerge_host_cmd.extend(package_array)
+    unmerge_host_cmd.extend(package_atoms)
     RunCommand(unmerge_host_cmd)
 
     RunCommand(['eclean-%s' % board, '-d', 'packages'], redirect_stderr=True)
@@ -319,15 +319,15 @@ class EBuild(object):
     """Sets up data about an ebuild from its path."""
     from portage.versions import pkgsplit
     unused_path, self.category, self.pkgname, filename = path.rsplit('/', 3)
-    unused_pkgname, version_no_rev, rev = pkgsplit(
+    unused_pkgname, self.version_no_rev, rev = pkgsplit(
         filename.replace('.ebuild', ''))
 
     self.ebuild_path_no_version = os.path.join(
         os.path.dirname(path), self.pkgname)
     self.ebuild_path_no_revision = '%s-%s' % (self.ebuild_path_no_version,
-                                              version_no_rev)
+                                              self.version_no_rev)
     self.current_revision = int(rev.replace('r', ''))
-    self.version = '%s-%s' % (version_no_rev, rev)
+    self.version = '%s-%s' % (self.version_no_rev, rev)
     self.package = '%s/%s' % (self.category, self.pkgname)
     self.ebuild_path = path
 
@@ -454,17 +454,19 @@ class EBuildStableMarker(object):
         OSError: Error occurred while creating a new ebuild.
         IOError: Error occurred while writing to the new revved ebuild file.
     Returns:
-      True if the revved package is different than the old ebuild.
+      If the revved package is different than the old ebuild, return the full
+      revved package name, including the version number. Otherwise, return None.
     """
     if self._ebuild.is_stable:
-      new_stable_ebuild_path = '%s-r%d.ebuild' % (
+      stable_version_no_rev = self._ebuild.version_no_rev
-          self._ebuild.ebuild_path_no_revision,
-          self._ebuild.current_revision + 1)
     else:
       # If given unstable ebuild, use 0.0.1 rather than 9999.
-      new_stable_ebuild_path = '%s-0.0.1-r%d.ebuild' % (
+      stable_version_no_rev = '0.0.1'
-          self._ebuild.ebuild_path_no_version,
+
-          self._ebuild.current_revision + 1)
+    new_version = '%s-r%d' % (stable_version_no_rev,
+                              self._ebuild.current_revision + 1)
+    new_stable_ebuild_path = '%s-%s.ebuild' % (
+        self._ebuild.ebuild_path_no_version, new_version)
 
     _Print('Creating new stable ebuild %s' % new_stable_ebuild_path)
     unstable_ebuild_path = ('%s-9999.ebuild' %
@@ -480,7 +482,7 @@ class EBuildStableMarker(object):
     if 0 == RunCommand(diff_cmd, exit_code=True, redirect_stdout=True,
                        redirect_stderr=True, print_cmd=gflags.FLAGS.verbose):
       os.unlink(new_stable_ebuild_path)
-      return False
+      return None
     else:
       _Print('Adding new stable ebuild to git')
       _SimpleRunCommand('git add %s' % new_stable_ebuild_path)
@@ -489,7 +491,7 @@ class EBuildStableMarker(object):
         _Print('Removing old ebuild from git')
         _SimpleRunCommand('git rm %s' % old_ebuild_path)
 
-      return True
+      return '%s-%s' % (self._ebuild.package, new_version)
 
   @classmethod
   def CommitChange(cls, message):
@@ -556,16 +558,18 @@ def main(argv):
 
       # Contains the array of packages we actually revved.
       revved_packages = []
+      new_package_atoms = []
       for ebuild in ebuilds:
         try:
           _Print('Working on %s' % ebuild.package)
           worker = EBuildStableMarker(ebuild)
           commit_id = ebuild.GetCommitId()
-          if worker.RevWorkOnEBuild(commit_id):
+          new_package = worker.RevWorkOnEBuild(commit_id)
+          if new_package:
             message = _GIT_COMMIT_MESSAGE % (ebuild.package, commit_id)
             worker.CommitChange(message)
             revved_packages.append(ebuild.package)
-
+            new_package_atoms.append('=%s' % new_package)
         except (OSError, IOError):
           Warning('Cannot rev %s\n' % ebuild.package,
                   'Note you will have to go into %s '
@@ -573,7 +577,7 @@ def main(argv):
           raise
 
       if revved_packages:
-        _CleanStalePackages(gflags.FLAGS.board, 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))

cros_mark_as_stable_unittest.py

@@ -129,6 +129,7 @@ class EBuildTest(mox.MoxTestBase):
     self.mox.ReplayAll()
     fake_ebuild = cros_mark_as_stable.EBuild(fake_ebuild_path)
     self.mox.VerifyAll()
+    self.assertEquals(fake_ebuild.version_no_rev, '0.0.1')
     self.assertEquals(fake_ebuild.ebuild_path_no_revision,
                       '/path/to/test_package/test_package-0.0.1')
     self.assertEquals(fake_ebuild.ebuild_path_no_version,
@@ -144,6 +145,7 @@ class EBuildTest(mox.MoxTestBase):
     fake_ebuild = cros_mark_as_stable.EBuild(fake_ebuild_path)
     self.mox.VerifyAll()
 
+    self.assertEquals(fake_ebuild.version_no_rev, '9999')
     self.assertEquals(fake_ebuild.ebuild_path_no_revision,
                       '/path/to/test_package/test_package-9999')
     self.assertEquals(fake_ebuild.ebuild_path_no_version,
@@ -160,12 +162,14 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
     self.mox.StubOutWithMock(os, 'unlink')
     self.m_ebuild = self.mox.CreateMock(cros_mark_as_stable.EBuild)
     self.m_ebuild.is_stable = True
-    self.m_ebuild.package = 'test_package'
+    self.m_ebuild.package = 'test_package/test_package'
+    self.m_ebuild.version_no_rev = '0.0.1'
     self.m_ebuild.current_revision = 1
     self.m_ebuild.ebuild_path_no_revision = '/path/test_package-0.0.1'
     self.m_ebuild.ebuild_path_no_version = '/path/test_package'
     self.m_ebuild.ebuild_path = '/path/test_package-0.0.1-r1.ebuild'
     self.revved_ebuild_path = '/path/test_package-0.0.1-r2.ebuild'
+    self.unstable_ebuild_path = '/path/test_package-9999.ebuild'
 
   def testRevWorkOnEBuild(self):
     self.mox.StubOutWithMock(cros_mark_as_stable.fileinput, 'input')
@@ -197,8 +201,9 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
 
     self.mox.ReplayAll()
     marker = cros_mark_as_stable.EBuildStableMarker(self.m_ebuild)
-    marker.RevWorkOnEBuild('my_id', redirect_file=m_file)
+    result = marker.RevWorkOnEBuild('my_id', redirect_file=m_file)
     self.mox.VerifyAll()
+    self.assertEqual(result, 'test_package/test_package-0.0.1-r2')
 
   def testRevUnchangedEBuild(self):
     self.mox.StubOutWithMock(cros_mark_as_stable.fileinput, 'input')
@@ -229,8 +234,9 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
 
     self.mox.ReplayAll()
     marker = cros_mark_as_stable.EBuildStableMarker(self.m_ebuild)
-    marker.RevWorkOnEBuild('my_id', redirect_file=m_file)
+    result = marker.RevWorkOnEBuild('my_id', redirect_file=m_file)
     self.mox.VerifyAll()
+    self.assertEqual(result, None)
 
   def testRevMissingEBuild(self):
     self.mox.StubOutWithMock(cros_mark_as_stable.fileinput, 'input')
@@ -239,6 +245,11 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
     self.mox.StubOutWithMock(cros_mark_as_stable, 'Die')
     m_file = self.mox.CreateMock(file)
 
+    revved_ebuild_path = self.m_ebuild.ebuild_path
+    self.m_ebuild.ebuild_path = self.unstable_ebuild_path
+    self.m_ebuild.is_stable = False
+    self.m_ebuild.current_revision = 0
+
     # Prepare mock fileinput.  This tests to make sure both the commit id
     # and keywords are changed correctly.
     mock_file = ['EAPI=2', 'CROS_WORKON_COMMIT=old_id',
@@ -247,25 +258,24 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
     ebuild_9999 = self.m_ebuild.ebuild_path_no_version + '-9999.ebuild'
     cros_mark_as_stable.os.path.exists(ebuild_9999).AndReturn(False)
     cros_mark_as_stable.Die("Missing unstable ebuild: %s" % ebuild_9999)
-    cros_mark_as_stable.shutil.copyfile(ebuild_9999, self.revved_ebuild_path)
+    cros_mark_as_stable.shutil.copyfile(ebuild_9999, revved_ebuild_path)
-    cros_mark_as_stable.fileinput.input(self.revved_ebuild_path,
+    cros_mark_as_stable.fileinput.input(revved_ebuild_path,
                                         inplace=1).AndReturn(mock_file)
     m_file.write('EAPI=2')
     m_file.write('CROS_WORKON_COMMIT="my_id"\n')
     m_file.write('KEYWORDS="x86 arm"')
     m_file.write('src_unpack(){}')
-    diff_cmd = ['diff', '-Bu', self.m_ebuild.ebuild_path,
+    diff_cmd = ['diff', '-Bu', self.unstable_ebuild_path, revved_ebuild_path]
-                self.revved_ebuild_path]
     cros_mark_as_stable.RunCommand(diff_cmd, exit_code=True,
                                    print_cmd=False, redirect_stderr=True,
                                    redirect_stdout=True).AndReturn(1)
-    cros_mark_as_stable._SimpleRunCommand('git add ' + self.revved_ebuild_path)
+    cros_mark_as_stable._SimpleRunCommand('git add ' + revved_ebuild_path)
-    cros_mark_as_stable._SimpleRunCommand('git rm ' + self.m_ebuild.ebuild_path)
 
     self.mox.ReplayAll()
     marker = cros_mark_as_stable.EBuildStableMarker(self.m_ebuild)
-    marker.RevWorkOnEBuild('my_id', redirect_file=m_file)
+    result = marker.RevWorkOnEBuild('my_id', redirect_file=m_file)
     self.mox.VerifyAll()
+    self.assertEqual(result, 'test_package/test_package-0.0.1-r1')
 
 
   def testCommitChange(self):