launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24515
[Merge] ~cjwatson/launchpad:built-using-guard-deletion into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:built-using-guard-deletion into launchpad:master with ~cjwatson/launchpad:built-using-ui as a prerequisite.
Commit message:
Guard removal of sources referenced by Built-Using
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/381239
Prevent SourcePackagePublishingHistory.requestDeletion from deleting source publications that have Built-Using references from active binary publications in the same archive and suite.
This isn't necessarily complete: in particular, it can miss references from other pockets, and in any case it might race with a build still in progress. The intent of this is not to ensure integrity, but to avoid some easily-detectable mistakes that could cause confusion.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:built-using-guard-deletion into launchpad:master.
diff --git a/lib/lp/soyuz/interfaces/binarysourcereference.py b/lib/lp/soyuz/interfaces/binarysourcereference.py
index 5affa16..5f7c321 100644
--- a/lib/lp/soyuz/interfaces/binarysourcereference.py
+++ b/lib/lp/soyuz/interfaces/binarysourcereference.py
@@ -79,3 +79,18 @@ class IBinarySourceReferenceSet(Interface):
:param reference_type: A `BinarySourceReferenceType` to search for.
:return: A `ResultSet` of matching `IBinarySourceReference`s.
"""
+
+ def findPublished(archive, distroseries, pocket, reference_type,
+ source_package_releases=None):
+ """Find references corresponding to published binary publications.
+
+ :param archive: An `IArchive` to search for binary publications.
+ :param distroseries: An `IDistroSeries` to search for binary
+ publications.
+ :param pocket: A `PackagePublishingPocket` to search for binary
+ publications.
+ :param reference_type: A `BinarySourceReferenceType` to search for.
+ :param source_package_releases: If not None, only return references
+ pointing to any of this sequence of `ISourcePackageRelease`s.
+ :return: A `ResultSet` of matching `IBinarySourceReference`s.
+ """
diff --git a/lib/lp/soyuz/model/binarysourcereference.py b/lib/lp/soyuz/model/binarysourcereference.py
index 1090c1e..23daf86 100644
--- a/lib/lp/soyuz/model/binarysourcereference.py
+++ b/lib/lp/soyuz/model/binarysourcereference.py
@@ -45,7 +45,11 @@ from lp.soyuz.interfaces.binarysourcereference import (
IBinarySourceReferenceSet,
UnparsableBuiltUsing,
)
-from lp.soyuz.model.publishing import SourcePackagePublishingHistory
+from lp.soyuz.model.distroarchseries import DistroArchSeries
+from lp.soyuz.model.publishing import (
+ BinaryPackagePublishingHistory,
+ SourcePackagePublishingHistory,
+ )
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
@@ -187,3 +191,26 @@ class BinarySourceReferenceSet:
BinarySourceReference,
BinarySourceReference.binary_package_release == bpr,
BinarySourceReference.reference_type == reference_type)
+
+ @classmethod
+ def findPublished(cls, archive, distroseries, pocket, reference_type,
+ source_package_releases=None):
+ """See `IBinarySourceReferenceSet`."""
+ clauses = [
+ BinarySourceReference.binary_package_release_id ==
+ BinaryPackagePublishingHistory.binarypackagereleaseID,
+ BinarySourceReference.reference_type == reference_type,
+ BinaryPackagePublishingHistory.status ==
+ PackagePublishingStatus.PUBLISHED,
+ BinaryPackagePublishingHistory.archive == archive,
+ BinaryPackagePublishingHistory.distroarchseries ==
+ DistroArchSeries.id,
+ BinaryPackagePublishingHistory.pocket == pocket,
+ DistroArchSeries.distroseries == distroseries,
+ ]
+ if source_package_releases is not None:
+ clauses.append(
+ BinarySourceReference.source_package_release_id.is_in(
+ spr.id for spr in source_package_releases))
+ return IStore(BinarySourceReference).find(
+ BinarySourceReference, *clauses).config(distinct=True)
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index ae7b414..f4abecc 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -79,6 +79,7 @@ from lp.services.worlddata.model.country import Country
from lp.soyuz.adapters.proxiedsourcefiles import ProxiedSourceLibraryFileAlias
from lp.soyuz.enums import (
BinaryPackageFormat,
+ BinarySourceReferenceType,
PackagePublishingPriority,
PackagePublishingStatus,
PackageUploadStatus,
@@ -87,6 +88,9 @@ from lp.soyuz.interfaces.binarypackagebuild import (
BuildSetStatus,
IBinaryPackageBuildSet,
)
+from lp.soyuz.interfaces.binarysourcereference import (
+ IBinarySourceReferenceSet,
+ )
from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.interfaces.distributionjob import (
IDistroSeriesDifferenceJobSource,
@@ -599,6 +603,23 @@ class SourcePackagePublishingHistory(SQLBase, ArchivePublisherBase):
"Cannot delete publications from suite '%s'" %
self.distroseries.getSuite(self.pocket))
+ # Check for live Built-Using references to this source package.
+ # This isn't necessarily complete: it's there to avoid confusing
+ # consequences of mistakes, not to ensure integrity. (The dominator
+ # will reinstate a source publication if necessary.)
+ # XXX cjwatson 2020-03-23: It would be nice to check for references
+ # in other pockets within this series, since that seems somewhat
+ # likely to come up in practice (e.g. builds in PROPOSED that
+ # declare Built-Using on a source in RELEASE).
+ bsrs = getUtility(IBinarySourceReferenceSet).findPublished(
+ self.archive, self.distroseries, self.pocket,
+ BinarySourceReferenceType.BUILT_USING,
+ source_package_releases=[self.sourcepackagerelease])
+ if not bsrs.is_empty():
+ raise DeletionError(
+ "Cannot delete source publication with a Built-Using "
+ "reference from an active binary publication.")
+
self.setDeleted(removed_by, removal_comment)
if self.archive.is_main:
dsd_job_source = getUtility(IDistroSeriesDifferenceJobSource)
@@ -1696,6 +1717,33 @@ class PublishingSet:
"Cannot delete publications from suite '%s'" %
distroseries.getSuite(pocket))
+ # Check for live Built-Using references to these source packages.
+ # This isn't necessarily complete: it's there to avoid confusing
+ # consequences of mistakes, not to ensure integrity. (The dominator
+ # will reinstate a source publication if necessary.)
+ # XXX cjwatson 2020-03-23: It would be nice to check for references
+ # in other pockets within this series, since that seems somewhat
+ # likely to come up in practice (e.g. builds in PROPOSED that
+ # declare Built-Using on a source in RELEASE).
+ if sources:
+ # XXX cjwatson 2020-03-24: In theory we could consolidate this
+ # into one query, but it's complex and probably not worth it.
+ sprs_by_location = defaultdict(list)
+ for spph in sources:
+ sprs_by_location[
+ (spph.archive, spph.distroseries, spph.pocket)].append(
+ spph.sourcepackagerelease)
+ for (archive, distroseries, pocket), sprs in (
+ sprs_by_location.items()):
+ bsrs = getUtility(IBinarySourceReferenceSet).findPublished(
+ archive, distroseries, pocket,
+ BinarySourceReferenceType.BUILT_USING,
+ source_package_releases=sprs)
+ if not bsrs.is_empty():
+ raise DeletionError(
+ "Cannot delete source publication with a Built-Using "
+ "reference from an active binary publication.")
+
spph_ids = [spph.id for spph in sources]
self.setMultipleDeleted(
SourcePackagePublishingHistory, spph_ids, removed_by,
diff --git a/lib/lp/soyuz/tests/test_binarysourcereference.py b/lib/lp/soyuz/tests/test_binarysourcereference.py
index 47c8f30..57a6d10 100644
--- a/lib/lp/soyuz/tests/test_binarysourcereference.py
+++ b/lib/lp/soyuz/tests/test_binarysourcereference.py
@@ -225,3 +225,88 @@ class TestBinarySourceReference(TestCaseWithFactory):
[],
self.reference_set.findByBinaryPackageRelease(
other_bpr, BinarySourceReferenceType.BUILT_USING))
+
+ def test_findPublished_empty(self):
+ archive = self.factory.makeArchive()
+ self.assertContentEqual(
+ [],
+ self.reference_set.findPublished(
+ archive, archive.distribution.currentseries,
+ PackagePublishingPocket.RELEASE,
+ BinarySourceReferenceType.BUILT_USING))
+
+ def test_findPublished(self):
+ # findPublished finds references corresponding to published binary
+ # publications.
+ archive = self.factory.makeArchive()
+ distroseries = archive.distribution.currentseries
+ das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+ bpphs = [
+ self.factory.makeBinaryPackagePublishingHistory(
+ archive=archive, distroarchseries=das,
+ pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.PUBLISHED)
+ for _ in range(2)]
+ all_sprs = []
+ bsrs = []
+ for bpph in bpphs:
+ spphs = [
+ self.factory.makeSourcePackagePublishingHistory(
+ archive=bpph.archive, distroseries=bpph.distroseries,
+ pocket=bpph.pocket)
+ for _ in range(2)]
+ sprs = [spph.sourcepackagerelease for spph in spphs]
+ all_sprs.extend(sprs)
+ relationship = (
+ "%s (= %s), %s (= %s)" %
+ (sprs[0].name, sprs[0].version, sprs[1].name, sprs[1].version))
+ bsrs.extend(self.reference_set.createFromRelationship(
+ bpph.binarypackagerelease, relationship,
+ BinarySourceReferenceType.BUILT_USING))
+ # Create a few more BPPHs with slight mismatches to ensure that
+ # findPublished matches correctly.
+ other_bpphs = []
+ other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
+ archive=archive, pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.PUBLISHED))
+ other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
+ distroarchseries=das, pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.PUBLISHED))
+ other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
+ archive=archive, distroarchseries=das,
+ pocket=PackagePublishingPocket.PROPOSED,
+ status=PackagePublishingStatus.PUBLISHED))
+ other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
+ archive=archive, distroarchseries=das,
+ pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.SUPERSEDED))
+ for bpph in other_bpphs:
+ spr = self.factory.makeSourcePackagePublishingHistory(
+ archive=bpph.archive, distroseries=bpph.distroseries,
+ pocket=bpph.pocket).sourcepackagerelease
+ relationship = "%s (= %s)" % (spr.name, spr.version)
+ self.reference_set.createFromRelationship(
+ bpph.binarypackagerelease, relationship,
+ BinarySourceReferenceType.BUILT_USING)
+ self.assertThat(
+ self.reference_set.findPublished(
+ archive, distroseries, PackagePublishingPocket.RELEASE,
+ BinarySourceReferenceType.BUILT_USING),
+ MatchesSetwise(*(
+ MatchesStructure.byEquality(
+ binary_package_release=bsr.binary_package_release,
+ source_package_release=bsr.source_package_release,
+ reference_type=BinarySourceReferenceType.BUILT_USING)
+ for bsr in bsrs)))
+ self.assertThat(
+ self.reference_set.findPublished(
+ archive, distroseries, PackagePublishingPocket.RELEASE,
+ BinarySourceReferenceType.BUILT_USING,
+ source_package_releases=[
+ bsr.source_package_release for bsr in bsrs[:2]]),
+ MatchesSetwise(*(
+ MatchesStructure.byEquality(
+ binary_package_release=bsr.binary_package_release,
+ source_package_release=bsr.source_package_release,
+ reference_type=BinarySourceReferenceType.BUILT_USING)
+ for bsr in bsrs[:2])))
diff --git a/lib/lp/soyuz/tests/test_publishing.py b/lib/lp/soyuz/tests/test_publishing.py
index cd7d1ea..cd11285 100644
--- a/lib/lp/soyuz/tests/test_publishing.py
+++ b/lib/lp/soyuz/tests/test_publishing.py
@@ -38,9 +38,13 @@ from lp.services.log.logger import DevNullLogger
from lp.soyuz.enums import (
ArchivePurpose,
BinaryPackageFormat,
+ BinarySourceReferenceType,
PackageUploadStatus,
)
from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
+from lp.soyuz.interfaces.binarysourcereference import (
+ IBinarySourceReferenceSet,
+ )
from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.interfaces.publishing import (
active_publishing_status,
@@ -984,6 +988,26 @@ class TestPublishingSetLite(TestCaseWithFactory):
self.assertRaisesWithContent(
DeletionError, message, pub.api_requestDeletion, self.person)
+ def test_requestDeletion_disallows_live_built_using_refs(self):
+ bpph = self.factory.makeBinaryPackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED)
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ archive=bpph.archive, distroseries=bpph.distroseries,
+ pocket=bpph.pocket, status=PackagePublishingStatus.PUBLISHED)
+ spr = spph.sourcepackagerelease
+ getUtility(IBinarySourceReferenceSet).createFromRelationship(
+ bpph.binarypackagerelease, "%s (= %s)" % (spr.name, spr.version),
+ BinarySourceReferenceType.BUILT_USING)
+ message = (
+ "Cannot delete source publication with a Built-Using reference "
+ "from an active binary publication.")
+ self.assertRaisesWithContent(
+ DeletionError, message, spph.requestDeletion, self.person)
+ self.assertRaisesWithContent(
+ DeletionError, message, spph.api_requestDeletion, self.person)
+ bpph.requestDeletion(self.person)
+ spph.requestDeletion(self.person)
+
def test_requestDeletion_marks_debug_as_deleted(self):
matching_bpph, debug_matching_bpph = (
self.factory.makeBinaryPackagePublishingHistory(
