From 702f0e31ca689506cfa18b88fad0f2cb5af6658e Mon Sep 17 00:00:00 2001 From: David James Date: Tue, 21 Dec 2010 12:46:46 -0800 Subject: [PATCH] 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 | 42 ++++++++++++++++++--------------- cros_mark_as_stable_unittest.py | 30 +++++++++++++++-------- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/cros_mark_as_stable.py b/cros_mark_as_stable.py index 9eb092cfd6..013ccd1ca9 100755 --- a/cros_mark_as_stable.py +++ b/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' % ( - self._ebuild.ebuild_path_no_revision, - self._ebuild.current_revision + 1) + stable_version_no_rev = self._ebuild.version_no_rev else: # If given unstable ebuild, use 0.0.1 rather than 9999. - new_stable_ebuild_path = '%s-0.0.1-r%d.ebuild' % ( - self._ebuild.ebuild_path_no_version, - self._ebuild.current_revision + 1) + stable_version_no_rev = '0.0.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)) diff --git a/cros_mark_as_stable_unittest.py b/cros_mark_as_stable_unittest.py index 4f9763e7f9..a2fcbd5ad3 100755 --- a/cros_mark_as_stable_unittest.py +++ b/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.fileinput.input(self.revved_ebuild_path, + cros_mark_as_stable.shutil.copyfile(ebuild_9999, 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, - self.revved_ebuild_path] + diff_cmd = ['diff', '-Bu', self.unstable_ebuild_path, 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 rm ' + self.m_ebuild.ebuild_path) + cros_mark_as_stable._SimpleRunCommand('git add ' + revved_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):