launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24516
[Merge] ~cjwatson/launchpad:built-using-domination into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:built-using-domination into launchpad:master with ~cjwatson/launchpad:built-using-guard-deletion as a prerequisite.
Commit message:
Handle Built-Using references in the dominator
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1868558 in Launchpad itself: "Honour Built-Using field"
https://bugs.launchpad.net/launchpad/+bug/1868558
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/381240
Keep source publications with Built-Using references from active binary publications. This may extend to reinstating the source publication (via a copy) if it had already been superseded or deleted.
It's possible for this to cause confusing effects if a manual deletion races with a build that produces binaries with a Built-Using reference to the deleted source. I've guarded against this as best I can, and hope the remaining cases will be rare, but err on the side of honouring the reference.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:built-using-domination into launchpad:master.
diff --git a/lib/lp/archivepublisher/domination.py b/lib/lp/archivepublisher/domination.py
index a02fd78..1e896e0 100644
--- a/lib/lp/archivepublisher/domination.py
+++ b/lib/lp/archivepublisher/domination.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Archive Domination class.
@@ -68,10 +68,12 @@ from storm.expr import (
And,
Count,
Desc,
+ Or,
Select,
)
from zope.component import getUtility
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.services.database.bulk import load_related
from lp.services.database.constants import UTC_NOW
@@ -84,8 +86,12 @@ from lp.services.database.sqlbase import (
from lp.services.orderingcheck import OrderingCheck
from lp.soyuz.enums import (
BinaryPackageFormat,
+ BinarySourceReferenceType,
PackagePublishingStatus,
)
+from lp.soyuz.interfaces.binarysourcereference import (
+ IBinarySourceReferenceSet,
+ )
from lp.soyuz.interfaces.publishing import (
inactive_publishing_status,
IPublishingSet,
@@ -93,6 +99,7 @@ from lp.soyuz.interfaces.publishing import (
from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
from lp.soyuz.model.binarypackagename import BinaryPackageName
from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
+from lp.soyuz.model.binarysourcereference import BinarySourceReference
from lp.soyuz.model.publishing import (
BinaryPackagePublishingHistory,
SourcePackagePublishingHistory,
@@ -207,19 +214,44 @@ class GeneralizedPublication:
return sorted(publications, cmp=self.compare, reverse=True)
-def find_live_source_versions(sorted_pubs):
+def get_source_versions(source_publications):
+ """List versions for sequence of `SourcePackagePublishingHistory`.
+
+ :param source_publications: An iterable of
+ `SourcePackagePublishingHistory`.
+ :return: A list of the publications' respective versions.
+ """
+ return [pub.sourcepackagerelease.version for pub in source_publications]
+
+
+def find_live_source_versions(sorted_pubs, built_using=None):
"""Find versions out of Published publications that should stay live.
- This particular notion of liveness applies to source domination: the
- latest version stays live, and that's it.
+ This particular notion of liveness applies to source domination:
+ normally, the latest version stays live, and that's it. The exception
+ is if live binary publications have Built-Using fields referring to some
+ source versions, in which case those versions stay live too.
:param sorted_pubs: An iterable of `SourcePackagePublishingHistory`
sorted by descending package version.
+ :param built_using: An optional collection of `BinarySourceReference`s
+ corresponding to published binary publications with Built-Using
+ references; the SPRs that these refer to should stay live.
:return: A list of live versions.
"""
+ if built_using is None:
+ built_using = set()
+ built_using_spr_ids = set(
+ bsr.source_package_release_id for bsr in built_using)
+
# Given the required sort order, the latest version is at the head
# of the list.
- return [sorted_pubs[0].sourcepackagerelease.version]
+ sorted_pubs = list(sorted_pubs)
+ latest = sorted_pubs.pop(0)
+ return get_source_versions(
+ [latest] + [
+ pub for pub in sorted_pubs
+ if pub.sourcepackagereleaseID in built_using_spr_ids])
def get_binary_versions(binary_publications):
@@ -707,19 +739,22 @@ class Dominator:
execute_plan()
- def _composeActiveSourcePubsCondition(self, distroseries, pocket):
+ def _composeRelevantSourcePubsCondition(self, distroseries, pocket,
+ active=True):
"""Compose ORM condition for restricting relevant source pubs."""
SPPH = SourcePackagePublishingHistory
- return And(
- SPPH.status == PackagePublishingStatus.PUBLISHED,
+ clauses = [
SPPH.distroseries == distroseries,
SPPH.archive == self.archive,
SPPH.pocket == pocket,
- )
+ ]
+ if active:
+ clauses.append(SPPH.status == PackagePublishingStatus.PUBLISHED)
+ return And(*clauses)
def findSourcesForDomination(self, distroseries, pocket):
- """Find binary publications that need dominating.
+ """Find source publications that need dominating.
This is only for traditional domination, where the latest published
publication is always kept published. See `find_live_source_versions`
@@ -728,17 +763,32 @@ class Dominator:
To optimize for that logic, `findSourcesForDomination` will ignore
publications that have no other publications competing for the same
binary package. There'd be nothing to do for those cases.
+
+ This also includes source publications whose source package releases
+ have a Built-Using reference pointing to them, whether active or
+ not. These may need to be superseded or kept/reinstated depending
+ on whether binary publications referring to them are active.
"""
SPPH = SourcePackagePublishingHistory
SPR = SourcePackageRelease
+ BSR = BinarySourceReference
- spph_location_clauses = self._composeActiveSourcePubsCondition(
+ spph_location_clauses = self._composeRelevantSourcePubsCondition(
distroseries, pocket)
candidate_source_names = Select(
SPPH.sourcepackagenameID,
And(join_spph_spr(), spph_location_clauses),
group_by=SPPH.sourcepackagenameID,
having=(Count() > 1))
+ built_using_spph_location_clauses = (
+ self._composeRelevantSourcePubsCondition(
+ distroseries, pocket, active=False))
+ built_using_sprs = Select(
+ SPPH.sourcepackagereleaseID,
+ And(
+ SPPH.sourcepackagereleaseID == BSR.source_package_release_id,
+ BSR.reference_type == BinarySourceReferenceType.BUILT_USING,
+ built_using_spph_location_clauses))
# We'll also access the SourcePackageReleases associated with
# the publications we find. Since they're in the join anyway,
@@ -749,8 +799,13 @@ class Dominator:
query = IStore(SPPH).find(
(SPPH, SPR),
join_spph_spr(),
- SPPH.sourcepackagenameID.is_in(candidate_source_names),
- spph_location_clauses)
+ Or(
+ And(
+ SPPH.sourcepackagenameID.is_in(candidate_source_names),
+ spph_location_clauses),
+ And(
+ SPPH.sourcepackagereleaseID.is_in(built_using_sprs),
+ built_using_spph_location_clauses)))
spphs = DecoratedResultSet(query, itemgetter(0))
load_related(SourcePackageName, spphs, ['sourcepackagenameID'])
return spphs
@@ -771,20 +826,55 @@ class Dominator:
sources = self.findSourcesForDomination(distroseries, pocket)
sorted_packages = self._sortPackages(sources, generalization)
supersede = []
+ keep = set()
delete = []
+ # Of the SPRs associated with publications being considered for
+ # domination, find those that have a Built-Using reference pointing
+ # to them from a live binary publication.
+ bsr_set = getUtility(IBinarySourceReferenceSet)
+ built_using = bsr_set.findPublished(
+ self.archive, distroseries, pocket,
+ BinarySourceReferenceType.BUILT_USING,
+ source_package_releases=[
+ pub.sourcepackagerelease for pub in sources])
+
self.logger.debug("Dominating sources...")
for name, pubs in sorted_packages.iteritems():
self.logger.debug("Dominating %s" % name)
assert len(pubs) > 0, "Dominating zero sources!"
- live_versions = find_live_source_versions(pubs)
- cur_supersede, _, cur_delete = self.planPackageDomination(
+ live_versions = find_live_source_versions(pubs, built_using)
+ cur_supersede, cur_keep, cur_delete = self.planPackageDomination(
pubs, live_versions, generalization)
supersede.extend(cur_supersede)
+ keep.update(cur_keep)
delete.extend(cur_delete)
for pub, dominant in supersede:
pub.supersede(dominant, logger=self.logger)
+ for pub in keep:
+ if pub.status not in (
+ PackagePublishingStatus.PENDING,
+ PackagePublishingStatus.PUBLISHED):
+ # The dominator thinks that we should keep this package, but
+ # it isn't currently published. This can happen when a
+ # source package has initially been superseded, but then a
+ # binary package that references it in Built-Using is
+ # published, requiring the source package to be reinstated.
+ # To cope with this, we'll copy the source back into place,
+ # thereby creating a new PENDING publication record for it,
+ # which will cause the publisher to put it back on disk.
+ #
+ # In general we'd prefer that the dominator not undo manual
+ # deletions, but Built-Using often expresses a compliance
+ # requirement, so we intentionally don't check that here.
+ # Instead, SPPH.requestDeletion checks for live
+ # BinarySourceReferences, and the uploader rejects uploads
+ # of builds with Built-Using references to source packages
+ # that have been manually deleted.
+ pub.copyTo(
+ distroseries, pocket, self.archive,
+ creator=getUtility(ILaunchpadCelebrities).janitor)
for pub in delete:
pub.requestDeletion(None)
@@ -804,7 +894,7 @@ class Dominator:
looking_for,
join_spph_spr(),
join_spph_spn(),
- self._composeActiveSourcePubsCondition(distroseries, pocket))
+ self._composeRelevantSourcePubsCondition(distroseries, pocket))
return result.group_by(SourcePackageName.name)
def findPublishedSPPHs(self, distroseries, pocket, package_name):
@@ -817,7 +907,7 @@ class Dominator:
join_spph_spr(),
join_spph_spn(),
SourcePackageName.name == package_name,
- self._composeActiveSourcePubsCondition(distroseries, pocket))
+ self._composeRelevantSourcePubsCondition(distroseries, pocket))
# Sort by descending version (SPR.version has type debversion in
# the database, so this should be a real proper comparison) so
# that _sortPackage will have slightly less work to do later.
diff --git a/lib/lp/archivepublisher/tests/test_dominator.py b/lib/lp/archivepublisher/tests/test_dominator.py
index b88da56..bddb5f6 100755
--- a/lib/lp/archivepublisher/tests/test_dominator.py
+++ b/lib/lp/archivepublisher/tests/test_dominator.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for domination.py."""
@@ -397,6 +397,113 @@ class TestDominator(TestNativePublishingBase):
for pub in overrides_2:
self.assertEqual(PackagePublishingStatus.PUBLISHED, pub.status)
+ def test_dominateSources_keeps_built_using_refs(self):
+ 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)
+ foo_11_src = self.getPubSource(
+ sourcename="foo", version="1.1", architecturehintlist="i386",
+ status=PackagePublishingStatus.PUBLISHED)
+ [foo_11_i386_bin] = self.getPubBinaries(
+ binaryname="foo-bin", status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=True, version="1.1", pub_source=foo_11_src)
+ bar_10_src = self.getPubSource(
+ sourcename="bar", version="1.0", architecturehintlist="i386",
+ status=PackagePublishingStatus.PUBLISHED)
+ [bar_10_i386_bin] = self.getPubBinaries(
+ binaryname="bar-bin", status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=True, version="1.0", pub_source=bar_10_src,
+ built_using="foo (= 1.0)")
+
+ dominator = Dominator(self.logger, self.ubuntutest.main_archive)
+ dominator.judgeAndDominate(
+ foo_10_src.distroseries, foo_10_src.pocket)
+
+ self.checkPublications(
+ [foo_10_src,
+ foo_11_src, foo_11_i386_bin,
+ bar_10_src, bar_10_i386_bin],
+ PackagePublishingStatus.PUBLISHED)
+ self.checkPublication(
+ foo_10_i386_bin, PackagePublishingStatus.SUPERSEDED)
+
+ bar_11_src = self.getPubSource(
+ sourcename="bar", version="1.1", architecturehintlist="i386",
+ status=PackagePublishingStatus.PUBLISHED)
+ [bar_11_i386_bin] = self.getPubBinaries(
+ binaryname="bar-bin", status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=True, version="1.1", pub_source=bar_11_src,
+ built_using="foo (= 1.1)")
+
+ dominator.judgeAndDominate(
+ foo_10_src.distroseries, foo_10_src.pocket)
+
+ self.checkPublications(
+ [foo_11_src, foo_11_i386_bin,
+ bar_11_src, bar_11_i386_bin],
+ PackagePublishingStatus.PUBLISHED)
+ self.checkPublications(
+ [foo_10_src, foo_10_i386_bin,
+ bar_10_src, bar_10_i386_bin],
+ PackagePublishingStatus.SUPERSEDED)
+
+ def test_dominateSources_reinstates_superseded_built_using_refs(self):
+ 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)
+ foo_11_src = self.getPubSource(
+ sourcename="foo", version="1.1", architecturehintlist="i386",
+ status=PackagePublishingStatus.PUBLISHED)
+ [foo_11_i386_bin] = self.getPubBinaries(
+ binaryname="foo-bin", status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=True, version="1.1", pub_source=foo_11_src)
+ bar_10_src = self.getPubSource(
+ sourcename="bar", version="1.0", architecturehintlist="i386",
+ status=PackagePublishingStatus.PUBLISHED)
+
+ dominator = Dominator(self.logger, self.ubuntutest.main_archive)
+ dominator.judgeAndDominate(
+ foo_10_src.distroseries, foo_10_src.pocket)
+
+ self.checkPublications(
+ [foo_11_src, foo_11_i386_bin,
+ bar_10_src],
+ PackagePublishingStatus.PUBLISHED)
+ self.checkPublications(
+ [foo_10_src, foo_10_i386_bin], PackagePublishingStatus.SUPERSEDED)
+
+ [bar_10_i386_bin] = self.getPubBinaries(
+ binaryname="bar-bin", status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=True, version="1.0", pub_source=bar_10_src,
+ built_using="foo (= 1.0)")
+
+ dominator.judgeAndDominate(
+ foo_10_src.distroseries, foo_10_src.pocket)
+
+ self.checkPublications(
+ [foo_11_src, foo_11_i386_bin,
+ bar_10_src, bar_10_i386_bin],
+ PackagePublishingStatus.PUBLISHED)
+ self.checkPublications(
+ [foo_10_src, foo_10_i386_bin], PackagePublishingStatus.SUPERSEDED)
+
+ foo_10_srcs = foo_10_src.archive.getPublishedSources(
+ name="foo", version="1.0", distroseries=foo_10_src.distroseries,
+ pocket=foo_10_src.pocket, exact_match=True)
+ foo_10_i386_bins = foo_10_i386_bin.archive.getAllPublishedBinaries(
+ name="foo-bin", version="1.0",
+ distroarchseries=foo_10_i386_bin.distroarchseries,
+ pocket=foo_10_i386_bin.pocket, exact_match=True)
+ self.assertEqual(2, foo_10_srcs.count())
+ self.assertEqual(1, foo_10_i386_bins.count())
+ self.checkPublication(foo_10_srcs[0], PackagePublishingStatus.PENDING)
+
class TestDomination(TestNativePublishingBase):
"""Test overall domination procedure."""
diff --git a/lib/lp/soyuz/interfaces/binarysourcereference.py b/lib/lp/soyuz/interfaces/binarysourcereference.py
index 5f7c321..837b171 100644
--- a/lib/lp/soyuz/interfaces/binarysourcereference.py
+++ b/lib/lp/soyuz/interfaces/binarysourcereference.py
@@ -47,6 +47,14 @@ class IBinarySourceReference(Interface):
vocabulary=BinarySourceReferenceType,
required=True, readonly=True)
+ # Export IDs for use by the dominator.
+ binary_package_release_id = Int(
+ title=_("The referencing binary package release ID."),
+ required=True, readonly=True)
+ source_package_release_id = Int(
+ title=_("The referencing source package release ID."),
+ required=True, readonly=True)
+
class IBinarySourceReferenceSet(Interface):
"""A set of references from binary packages to source packages."""
Follow ups
