launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05240
[Merge] lp:~julian-edwards/launchpad/arch-all-domination-bug-34086 into lp:launchpad
Julian Edwards has proposed merging lp:~julian-edwards/launchpad/arch-all-domination-bug-34086 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #34086 in Launchpad itself: "removal of arch-all packages while there are arch-specific packages dependent on it results in uninstallable binaries"
https://bugs.launchpad.net/launchpad/+bug/34086
For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/arch-all-domination-bug-34086/+merge/78931
Fix bug 34086 - "removal of arch-all packages while there are arch-specific packages dependent on it results in uninstallable binaries"
The simplest case of this can be described thusly:
* source foo 1.0 enters the archive and creates
* foo-bin 1.0 (i386) (depends on foo-common 1.0)
* foo-common 1.0 (arch-indep)
* source foo 1.1 is uploaded
* foo-common 1.1 finishes building and supersedes foo-bin 1.0
* foo-bin 1.0 is now uninstallable as its dependencies can't be met
The fix works by making sure that any arch-indep binaries do not get superseded until all the binaries built by the same source are also superseded. This means that we can now have multiple live versions of arch-indep binaries in the archive index.
--
https://code.launchpad.net/~julian-edwards/launchpad/arch-all-domination-bug-34086/+merge/78931
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/arch-all-domination-bug-34086 into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/domination.py'
--- lib/lp/archivepublisher/domination.py 2011-09-23 13:00:49 +0000
+++ lib/lp/archivepublisher/domination.py 2011-10-13 12:06:33 +0000
@@ -153,6 +153,7 @@
class.
"""
def __init__(self, is_source=True):
+ self.is_source = is_source
if is_source:
self.traits = SourcePublicationTraits
else:
@@ -204,6 +205,27 @@
self.logger = logger
self.archive = archive
+ def _checkArchIndep(self, publication):
+ """Return True if the binary publication can be superseded.
+
+ If the publication is an arch-indep binary, we can only supersede
+ it if all the binaries from the same source are also superseded,
+ else those binaries may become uninstallable.
+ See bug 34086.
+ """
+ binary = publication.binarypackagerelease
+ if not binary.architecturespecific:
+ # getOtherPublicationsForSameSource returns PENDING in
+ # addition to PUBLISHED binaries, and we rely on this since
+ # they must also block domination.
+ others = publication.getOtherPublicationsForSameSource()
+ if others.any():
+ # Don't dominate this arch:all binary as there are
+ # other arch-specific binaries from the same build
+ # that are still active.
+ return False
+ return True
+
def dominatePackage(self, publications, live_versions, generalization):
"""Dominate publications for a single package.
@@ -260,7 +282,9 @@
pub.supersede(current_dominant, logger=self.logger)
self.logger.debug2(
"Superseding older publication for version %s.", version)
- elif version in live_versions:
+ elif (version in live_versions or
+ (not generalization.is_source and
+ not self._checkArchIndep(pub))):
# This publication stays active; if any publications
# that follow right after this are to be superseded,
# this is the release that they are superseded by.
@@ -359,27 +383,6 @@
self.logger.debug("Beginning superseded processing...")
- # XXX: dsilvers 2005-09-22 bug=55030:
- # Need to make binaries go in groups but for now this'll do.
- # An example of the concrete problem here is:
- # - Upload foo-1.0, which builds foo and foo-common (arch all).
- # - Upload foo-1.1, ditto.
- # - foo-common-1.1 is built (along with the i386 binary for foo)
- # - foo-common-1.0 is superseded
- # Foo is now uninstallable on any architectures which don't yet
- # have a build of foo-1.1, as the foo-common for foo-1.0 is gone.
-
- # Essentially we ideally don't want to lose superseded binaries
- # unless the entire group is ready to be made pending removal.
- # In this instance a group is defined as all the binaries from a
- # given build. This assumes we've copied the arch_all binaries
- # from whichever build provided them into each arch-specific build
- # which we publish. If instead we simply publish the arch-all
- # binaries from another build then instead we should scan up from
- # the binary to its source, and then back from the source to each
- # binary published in *this* distroarchseries for that source.
- # if the binaries as a group (in that definition) are all superseded
- # then we can consider them eligible for removal.
for pub_record in binary_records:
binpkg_release = pub_record.binarypackagerelease
self.logger.debug(
@@ -450,19 +453,19 @@
generalization = GeneralizedPublication(is_source=False)
for distroarchseries in distroseries.architectures:
- self.logger.debug(
+ self.logger.info(
"Performing domination across %s/%s (%s)",
distroseries.name, pocket.title,
distroarchseries.architecturetag)
- bpph_location_clauses = And(
+ bpph_location_clauses = [
BinaryPackagePublishingHistory.status ==
PackagePublishingStatus.PUBLISHED,
BinaryPackagePublishingHistory.distroarchseries ==
distroarchseries,
BinaryPackagePublishingHistory.archive == self.archive,
- BinaryPackagePublishingHistory.pocket == pocket,
- )
+ BinaryPackagePublishingHistory.pocket == pocket
+ ]
candidate_binary_names = Select(
BinaryPackageName.id,
And(
@@ -474,18 +477,43 @@
),
group_by=BinaryPackageName.id,
having=Count(BinaryPackagePublishingHistory.id) > 1)
- binaries = IStore(BinaryPackagePublishingHistory).find(
+ main_clauses = [
BinaryPackagePublishingHistory,
BinaryPackageRelease.id ==
- BinaryPackagePublishingHistory.binarypackagereleaseID,
+ BinaryPackagePublishingHistory.binarypackagereleaseID,
BinaryPackageRelease.binarypackagenameID.is_in(
candidate_binary_names),
BinaryPackageRelease.binpackageformat !=
- BinaryPackageFormat.DDEB,
- bpph_location_clauses)
- self.logger.debug("Dominating binaries...")
- self._dominatePublications(
- self._sortPackages(binaries, generalization), generalization)
+ BinaryPackageFormat.DDEB,
+ bpph_location_clauses]
+
+ # Arch-indep binaries need to be done last as they depend on
+ # arch-specific binaries being superseded.
+ arch_specific_clauses = []
+ arch_specific_clauses.extend(main_clauses)
+ arch_specific_clauses.append(
+ BinaryPackageRelease.architecturespecific == True)
+ arch_specific_clauses.extend(bpph_location_clauses)
+ self.logger.info("Finding arch-specific binaries...")
+ arch_specific_bins = IStore(BinaryPackagePublishingHistory).find(
+ *arch_specific_clauses)
+ self.logger.info("Dominating arch-specific binaries...")
+ self._dominatePublications(
+ self._sortPackages(arch_specific_bins, generalization),
+ generalization)
+
+ arch_indep_clauses = []
+ arch_indep_clauses.extend(main_clauses)
+ arch_indep_clauses.append(
+ BinaryPackageRelease.architecturespecific == False)
+ arch_indep_clauses.extend(bpph_location_clauses)
+ self.logger.info("Finding arch-indep binaries...")
+ arch_indep_bins = IStore(BinaryPackagePublishingHistory).find(
+ *arch_indep_clauses)
+ self.logger.info("Dominating arch-indep binaries...")
+ self._dominatePublications(
+ self._sortPackages(arch_indep_bins, generalization),
+ generalization)
def _composeActiveSourcePubsCondition(self, distroseries, pocket):
"""Compose ORM condition for restricting relevant source pubs."""
=== modified file 'lib/lp/archivepublisher/tests/test_dominator.py'
--- lib/lp/archivepublisher/tests/test_dominator.py 2011-09-26 11:05:27 +0000
+++ lib/lp/archivepublisher/tests/test_dominator.py 2011-10-13 12:06:33 +0000
@@ -23,7 +23,9 @@
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
from lp.services.log.logger import DevNullLogger
-from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.enums import (
+ PackagePublishingStatus,
+ )
from lp.soyuz.interfaces.publishing import ISourcePackagePublishingHistory
from lp.soyuz.tests.test_publishing import TestNativePublishingBase
from lp.testing import (
@@ -169,6 +171,69 @@
dominator._dominatePublications,
pubs, GeneralizedPublication(True))
+ def test_archall_domination(self):
+ # Arch-all binaries should not be dominated when a new source
+ # version builds an updated arch-all binary, because slower builds
+ # of other architectures will leave the previous version
+ # uninstallable if they depend on the arch-all binary.
+ # See https://bugs.launchpad.net/launchpad/+bug/34086
+
+ # Set up a source, "foo" which builds "foo-bin" and foo-common
+ # (which is arch-all).
+ foo_10_src = self.getPubSource(
+ sourcename="foo", version="1.0", architecturehintlist="i386",
+ status=PackagePublishingStatus.PUBLISHED)
+ [foo_10_i386_bin] = self.getPubBinaries(
+ binaryname="foo-bin", status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=True, version="1.0", pub_source=foo_10_src)
+ [build] = foo_10_src.getBuilds()
+ bpr = self.factory.makeBinaryPackageRelease(
+ binarypackagename="foo-common", version="1.0", build=build,
+ architecturespecific=False)
+ foo_10_all_bins = self.publishBinaryInArchive(
+ bpr, self.ubuntutest.main_archive, pocket=foo_10_src.pocket,
+ status=PackagePublishingStatus.PUBLISHED)
+
+ # Now, make version 1.1 of foo and add a foo-common but not foo-bin
+ # (imagine that it's not finished building yet).
+ foo_11_src = self.getPubSource(
+ sourcename="foo", version="1.1", architecturehintlist="all",
+ status=PackagePublishingStatus.PUBLISHED)
+ foo_11_all_bins = self.getPubBinaries(
+ binaryname="foo-common", status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=False, version="1.1", pub_source=foo_11_src)
+
+ dominator = Dominator(self.logger, self.ubuntutest.main_archive)
+ dominator.judgeAndDominate(
+ foo_10_src.distroseries, foo_10_src.pocket)
+
+ # The source will be superseded.
+ self.checkPublication(foo_10_src, PackagePublishingStatus.SUPERSEDED)
+ # The arch-specific has no dominant, so it's still published
+ self.checkPublication(
+ foo_10_i386_bin, PackagePublishingStatus.PUBLISHED)
+ # The arch-indep has a dominant but must not be superseded yet
+ # since the arch-specific is still published.
+ self.checkPublications(
+ foo_10_all_bins, PackagePublishingStatus.PUBLISHED)
+
+ # Now creating a newer foo-bin should see those last two
+ # publications superseded.
+ [build2] = foo_11_src.getBuilds()
+ foo_11_bin = self.factory.makeBinaryPackageRelease(
+ binarypackagename="foo-bin", version="1.1", build=build2,
+ architecturespecific=True)
+ self.publishBinaryInArchive(
+ foo_11_bin, self.ubuntutest.main_archive,
+ pocket=foo_10_src.pocket,
+ status=PackagePublishingStatus.PUBLISHED)
+ dominator.judgeAndDominate(
+ foo_10_src.distroseries, foo_10_src.pocket)
+ self.checkPublication(
+ foo_10_i386_bin, PackagePublishingStatus.SUPERSEDED)
+ self.checkPublications(
+ foo_10_all_bins, PackagePublishingStatus.SUPERSEDED)
+
class TestDomination(TestNativePublishingBase):
"""Test overall domination procedure."""
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2011-09-27 07:53:02 +0000
+++ lib/lp/soyuz/model/publishing.py 2011-10-13 12:06:33 +0000
@@ -33,6 +33,7 @@
Desc,
LeftJoin,
Or,
+ Select,
Sum,
)
from storm.store import Store
@@ -1118,6 +1119,62 @@
section=self.section,
priority=self.priority)
+ def getOtherPublicationsForSameSource(self, include_archindep=False):
+ """Return all the other published or pending binaries for this
+ source.
+
+ For example if source package foo builds:
+ foo - i386
+ foo - amd64
+ foo-common - arch-all (published in i386 and amd64)
+ then if this publication is the arch-all amd64, return foo(i386),
+ foo(amd64). If include_archindep is True then also return
+ foo-common (i386)
+
+ :param include_archindep: If True, return architecture independent
+ publications too. Defaults to False.
+
+ :return: an iterable of `BinaryPackagePublishingHistory`
+ """
+ # Avoid circular wotsits.
+ from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
+ from lp.soyuz.model.distroarchseries import DistroArchSeries
+ from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+ source_select = Select(
+ SourcePackageRelease.id,
+ And(
+ BinaryPackageBuild.source_package_release_id ==
+ SourcePackageRelease.id,
+ BinaryPackageRelease.build == BinaryPackageBuild.id,
+ BinaryPackagePublishingHistory.binarypackagereleaseID ==
+ BinaryPackageRelease.id,
+ BinaryPackagePublishingHistory.id == self.id,
+ ))
+ pubs = [
+ BinaryPackageBuild.source_package_release_id ==
+ SourcePackageRelease.id,
+ SourcePackageRelease.id.is_in(source_select),
+ BinaryPackageRelease.build == BinaryPackageBuild.id,
+ BinaryPackagePublishingHistory.binarypackagereleaseID ==
+ BinaryPackageRelease.id,
+ BinaryPackagePublishingHistory.archiveID == self.archive.id,
+ BinaryPackagePublishingHistory.distroarchseriesID ==
+ DistroArchSeries.id,
+ DistroArchSeries.distroseriesID == self.distroseries.id,
+ BinaryPackagePublishingHistory.pocket == self.pocket,
+ BinaryPackagePublishingHistory.status.is_in(
+ active_publishing_status),
+ BinaryPackagePublishingHistory.id != self.id
+ ]
+
+ if not include_archindep:
+ pubs.append(BinaryPackageRelease.architecturespecific == True)
+
+ return IMasterStore(BinaryPackagePublishingHistory).find(
+ BinaryPackagePublishingHistory,
+ *pubs
+ )
+
def supersede(self, dominant=None, logger=None):
"""See `IBinaryPackagePublishingHistory`."""
# At this point only PUBLISHED (ancient versions) or PENDING (
=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py 2011-09-23 07:47:04 +0000
+++ lib/lp/soyuz/tests/test_publishing.py 2011-10-13 12:06:33 +0000
@@ -1471,6 +1471,138 @@
self.assertEquals(spph.ancestor.displayname, ancestor.displayname)
+class TestGetOtherPublicationsForSameSource(TestNativePublishingBase):
+ """Test parts of the BinaryPackagePublishingHistory model.
+
+ See also lib/lp/soyuz/doc/publishing.txt
+ """
+
+ layer = LaunchpadZopelessLayer
+
+ def _makeMixedSingleBuildPackage(self, version="1.0"):
+ # Set up a source with a build that generated four binaries,
+ # two of them an arch-all.
+ foo_src_pub = self.getPubSource(
+ sourcename="foo", version=version, architecturehintlist="i386",
+ status=PackagePublishingStatus.PUBLISHED)
+ [foo_bin_pub] = self.getPubBinaries(
+ binaryname="foo-bin", status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=True, version=version,
+ pub_source=foo_src_pub)
+ # Now need to grab the build for the source so we can add
+ # more binaries to it.
+ [build] = foo_src_pub.getBuilds()
+ foo_one_common = self.factory.makeBinaryPackageRelease(
+ binarypackagename="foo-one-common", version=version, build=build,
+ architecturespecific=False)
+ foo_one_common_pubs = self.publishBinaryInArchive(
+ foo_one_common, self.ubuntutest.main_archive,
+ pocket=foo_src_pub.pocket,
+ status=PackagePublishingStatus.PUBLISHED)
+ foo_two_common = self.factory.makeBinaryPackageRelease(
+ binarypackagename="foo-two-common", version=version, build=build,
+ architecturespecific=False)
+ foo_two_common_pubs = self.publishBinaryInArchive(
+ foo_two_common, self.ubuntutest.main_archive,
+ pocket=foo_src_pub.pocket,
+ status=PackagePublishingStatus.PUBLISHED)
+ foo_three = self.factory.makeBinaryPackageRelease(
+ binarypackagename="foo-three", version=version, build=build,
+ architecturespecific=True)
+ [foo_three_pub] = self.publishBinaryInArchive(
+ foo_three, self.ubuntutest.main_archive,
+ pocket=foo_src_pub.pocket,
+ status=PackagePublishingStatus.PUBLISHED)
+ # So now we have source foo, which has arch specific binaries
+ # foo-bin and foo-three, and arch:all binaries foo-one-common and
+ # foo-two-common. The latter two will have multiple publications,
+ # one for each DAS in the series.
+ return (
+ foo_src_pub, foo_bin_pub, foo_one_common_pubs,
+ foo_two_common_pubs, foo_three_pub)
+
+ def test_getOtherPublicationsForSameSource(self):
+ (foo_src_pub, foo_bin_pub, foo_one_common_pubs, foo_two_common_pubs,
+ foo_three_pub) = self._makeMixedSingleBuildPackage()
+
+ foo_one_common_pub = foo_one_common_pubs[0]
+ others = foo_one_common_pub.getOtherPublicationsForSameSource()
+ others = list(others)
+
+ self.assertEqual(2, len(others))
+ self.assertNotIn(foo_one_common_pub, others)
+ self.assertIn(foo_three_pub, others)
+ self.assertIn(foo_bin_pub, others)
+
+ def test_getOtherPublicationsForSameSource_include_archindep(self):
+ (foo_src_pub, foo_bin_pub, foo_one_common_pubs, foo_two_common_pubs,
+ foo_three_pub) = self._makeMixedSingleBuildPackage()
+
+ foo_one_common_pub = foo_one_common_pubs[0]
+ others = foo_one_common_pub.getOtherPublicationsForSameSource(
+ include_archindep=True)
+ others = list(others)
+
+ # We expect all publications created above to be returned,
+ # except the one we use to call the method on.
+ self.assertEqual(5, len(others))
+ self.assertIn(foo_three_pub, others)
+ self.assertIn(foo_bin_pub, others)
+ for pub in foo_one_common_pubs[1:] + foo_two_common_pubs:
+ self.assertIn(pub, others)
+
+ def test_getOtherPublicationsForSameSource_extra_builds(self):
+ (foo_src_pub, foo_bin_pub, foo_one_common_pubs, foo_two_common_pubs,
+ foo_three_pub) = self._makeMixedSingleBuildPackage()
+
+ # Add an additional build in a different architecture.
+ z80 = self.factory.makeDistroArchSeries(
+ distroseries=foo_src_pub.distroseries, architecturetag="z80")
+ build2 = self.factory.makeBinaryPackageBuild(
+ source_package_release=foo_src_pub.sourcepackagerelease,
+ distroarchseries=z80, archive=foo_src_pub.archive)
+ foo_one_z80 = self.factory.makeBinaryPackageRelease(
+ binarypackagename="foo-one-z80", version="1.0", build=build2,
+ architecturespecific=True)
+ [foo_one_z80_pub] = self.publishBinaryInArchive(
+ foo_one_z80, self.ubuntutest.main_archive,
+ pocket=foo_src_pub.pocket,
+ status=PackagePublishingStatus.PUBLISHED)
+
+ # The publication for the new build should be returned.
+ foo_one_common_pub = foo_one_common_pubs[0]
+ others = foo_one_common_pub.getOtherPublicationsForSameSource()
+ others = list(others)
+ self.assertIn(foo_one_z80_pub, others)
+
+ def test_getOtherPublicationsForSameSource_inactive(self):
+ # Check that inactive publications are not returned.
+ (foo_src_pub, foo_bin_pub, foo_one_common_pubs, foo_two_common_pubs,
+ foo_three_pub) = self._makeMixedSingleBuildPackage()
+ foo_bin_pub.status = PackagePublishingStatus.SUPERSEDED
+ foo_three_pub.status = PackagePublishingStatus.SUPERSEDED
+ foo_one_common_pub = foo_one_common_pubs[0]
+ others = foo_one_common_pub.getOtherPublicationsForSameSource()
+ others = list(others)
+
+ self.assertEqual(0, len(others))
+
+ def test_getOtherPublicationsForSameSource_multiple_versions(self):
+ # Check that only the right versions are returned.
+ (foo_src_pub, foo_bin_pub, foo_one_common_pubs, foo_two_common_pubs,
+ foo_three_pub) = self._makeMixedSingleBuildPackage(version="1.0")
+ self._makeMixedSingleBuildPackage(version="1.1")
+
+ foo_one_common_pub = foo_one_common_pubs[0]
+ others = foo_one_common_pub.getOtherPublicationsForSameSource()
+ others = list(others)
+
+ self.assertEqual(2, len(others))
+ self.assertNotIn(foo_one_common_pub, others)
+ self.assertIn(foo_three_pub, others)
+ self.assertIn(foo_bin_pub, others)
+
+
class TestGetBuiltBinaries(TestNativePublishingBase):
"""Test SourcePackagePublishingHistory.getBuiltBinaries() works."""
