packages: Allow for stable and unstable ebuilds in VM uprev handlers
Currently the uprev handler only allows for a single, stable
ebuild. This makes it difficult to modify the ebuild without creating
merge conflicts with the pupr CLs. This CL changes the handler to
stabilize an unstable 9999 ebuild instead of just renaming the stable
ebuild, so changes can be landed in the unstable ebuild and pulled
into the stable ebuild by pupr.
BUG=chromium:953544
TEST=manually tested
Cq-Depend: chrome-internal:3202670
Change-Id: If7b7f2386eded8c0b74b9161843085afbbbf957c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2340622
Tested-by: Fergus Dall <sidereal@google.com>
Reviewed-by: George Engelbrecht <engeg@google.com>
Commit-Queue: Fergus Dall <sidereal@google.com>
diff --git a/service/packages.py b/service/packages.py
index ac089c3..7496b94 100644
--- a/service/packages.py
+++ b/service/packages.py
@@ -403,32 +403,52 @@
package_src_path = os.path.join(constants.SOURCE_ROOT, package_path)
ebuild_paths = list(portage_util.EBuild.List(package_src_path))
- if not ebuild_paths:
- raise UprevError('No ebuilds found for %s' % package)
- elif len(ebuild_paths) > 1:
- raise UprevError('Multiple ebuilds found for %s' % package)
- else:
- ebuild_path = ebuild_paths[0]
+ stable_ebuild = None
+ unstable_ebuild = None
+ for path in ebuild_paths:
+ ebuild = portage_util.EBuild(path)
+ if ebuild.is_stable:
+ stable_ebuild = ebuild
+ else:
+ unstable_ebuild = ebuild
+
+ if stable_ebuild is None:
+ raise UprevError('No stable ebuild found for %s' % package)
+ if unstable_ebuild is None:
+ raise UprevError('No unstable ebuild found for %s' % package)
+ if len(ebuild_paths) > 2:
+ raise UprevError('Found too many ebuilds for %s: '
+ 'expected one stable and one unstable' % package)
version_pin_src_path = os.path.join(constants.SOURCE_ROOT, version_pin_path)
version = osutils.ReadFile(version_pin_src_path).strip()
- new_ebuild_path = os.path.join(package_path,
- '%s-%s-r1.ebuild' % (package, version))
- new_ebuild_src_path = os.path.join(constants.SOURCE_ROOT, new_ebuild_path)
- os.rename(ebuild_path, new_ebuild_src_path)
+
+ # If the new version is the same as the old version, bump the revision number,
+ # otherwise reset it to 1
+ if version == stable_ebuild.version_no_rev:
+ version = '%s-r%d' % (version, stable_ebuild.current_revision + 1)
+ else:
+ version = version + '-r1'
+
+ new_ebuild_src_path = os.path.join(package_src_path,
+ '%s-%s.ebuild' % (package, version))
manifest_src_path = os.path.join(package_src_path, 'Manifest')
- new_ebuild_chroot_path = os.path.join(constants.CHROOT_SOURCE_ROOT,
- new_ebuild_path)
+
+ portage_util.EBuild.MarkAsStable(unstable_ebuild.ebuild_path,
+ new_ebuild_src_path, {})
+ osutils.SafeUnlink(stable_ebuild.ebuild_path)
try:
- portage_util.UpdateEbuildManifest(new_ebuild_chroot_path, chroot=chroot)
+ portage_util.UpdateEbuildManifest(new_ebuild_src_path, chroot=chroot)
except cros_build_lib.RunCommandError as e:
raise EbuildManifestError(
'Unable to update manifest for %s: %s' % (package, e.stderr))
result = UprevVersionedPackageResult()
result.add_result(version,
- [new_ebuild_src_path, ebuild_path, manifest_src_path])
+ [new_ebuild_src_path,
+ stable_ebuild.ebuild_path,
+ manifest_src_path])
return result
diff --git a/service/packages_unittest.py b/service/packages_unittest.py
index 82389d2..a0e4780 100644
--- a/service/packages_unittest.py
+++ b/service/packages_unittest.py
@@ -118,13 +118,15 @@
new_version = '1.2.4'
ebuild_template = 'package-%s-r1.ebuild'
ebuild = ebuild_template % version
+ unstable_ebuild = 'package-9999.ebuild'
version_pin = 'VERSION-PIN'
manifest = 'Manifest'
def test_uprev_ebuild(self):
"""Tests uprev of ebuild with version path"""
file_layout = (
- D(self.package, [self.ebuild, self.version_pin, self.manifest]),
+ D(self.package, [self.ebuild, self.unstable_ebuild,
+ self.version_pin, self.manifest]),
)
cros_test_lib.CreateOnDiskHierarchy(self.tempdir, file_layout)
@@ -132,13 +134,16 @@
version_pin_path = os.path.join(package_path, self.version_pin)
self.WriteTempFile(version_pin_path, self.new_version)
+ ebuild_path = os.path.join(package_path, self.ebuild)
+ self.WriteTempFile(ebuild_path, 'KEYWORDS="*"\n')
+
result = packages.uprev_ebuild_from_pin(package_path, version_pin_path,
chroot=Chroot())
self.assertEqual(len(result.modified), 1,
'unexpected number of results: %s' % len(result.modified))
mod = result.modified[0]
- self.assertEqual(mod.new_version, self.new_version,
+ self.assertEqual(mod.new_version, self.new_version + '-r1',
'unexpected version number: %s' % mod.new_version)
old_ebuild_path = os.path.join(package_path,
@@ -152,6 +157,44 @@
self.assertCommandContains(['ebuild', 'manifest'])
+ def test_uprev_ebuild_same_version(self):
+ """Tests uprev of ebuild with version path when the version has not changed.
+
+ This should result in bumping the revision number.
+ """
+ file_layout = (
+ D(self.package, [self.ebuild, self.unstable_ebuild,
+ self.version_pin, self.manifest]),
+ )
+ cros_test_lib.CreateOnDiskHierarchy(self.tempdir, file_layout)
+
+ package_path = os.path.join(self.tempdir, self.package)
+ version_pin_path = os.path.join(package_path, self.version_pin)
+ self.WriteTempFile(version_pin_path, self.version)
+
+ ebuild_path = os.path.join(package_path, self.ebuild)
+ self.WriteTempFile(ebuild_path, 'KEYWORDS="*"\n')
+
+ result = packages.uprev_ebuild_from_pin(package_path, version_pin_path,
+ chroot=Chroot())
+ self.assertEqual(len(result.modified), 1,
+ 'unexpected number of results: %s' % len(result.modified))
+
+ mod = result.modified[0]
+ self.assertEqual(mod.new_version, self.version + '-r2',
+ 'unexpected version number: %s' % mod.new_version)
+
+ old_ebuild_path = os.path.join(package_path,
+ self.ebuild_template % self.version)
+ new_ebuild_path = os.path.join(package_path,
+ 'package-%s-r2.ebuild' % self.version)
+ manifest_path = os.path.join(package_path, 'Manifest')
+
+ expected_modified_files = [old_ebuild_path, new_ebuild_path, manifest_path]
+ self.assertCountEqual(mod.files, expected_modified_files)
+
+ self.assertCommandContains(['ebuild', 'manifest'])
+
def test_no_ebuild(self):
"""Tests assertion is raised if package has no ebuilds"""
file_layout = (
@@ -167,8 +210,31 @@
packages.uprev_ebuild_from_pin(package_path, version_pin_path,
chroot=Chroot())
- def test_multiple_ebuilds(self):
- """Tests assertion is raised if multiple ebuilds are present for package"""
+ def test_multiple_stable_ebuilds(self):
+ """Tests assertion is raised if multiple stable ebuilds are present"""
+ file_layout = (
+ D(self.package, [self.version_pin, self.ebuild,
+ self.ebuild_template % '1.2.1',
+ self.manifest]),
+ )
+ cros_test_lib.CreateOnDiskHierarchy(self.tempdir, file_layout)
+
+ package_path = os.path.join(self.tempdir, self.package)
+ version_pin_path = os.path.join(package_path, self.version_pin)
+ self.WriteTempFile(version_pin_path, self.new_version)
+
+ ebuild_path = os.path.join(package_path, self.ebuild)
+ self.WriteTempFile(ebuild_path, 'KEYWORDS="*"\n')
+
+ ebuild_path = os.path.join(package_path, self.ebuild_template % '1.2.1')
+ self.WriteTempFile(ebuild_path, 'KEYWORDS="*"\n')
+
+ with self.assertRaises(packages.UprevError):
+ packages.uprev_ebuild_from_pin(package_path, version_pin_path,
+ chroot=Chroot())
+
+ def test_multiple_unstable_ebuilds(self):
+ """Tests assertion is raised if multiple unstable ebuilds are present"""
file_layout = (
D(self.package, [self.version_pin, self.ebuild,
self.ebuild_template % '1.2.1',
