← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:built-using-guard-copying into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:built-using-guard-copying into launchpad:master with ~cjwatson/launchpad:built-using-guard-deletion as a prerequisite.

Commit message:
Guard copies of binaries with Built-Using references

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/382792

If binaries have Built-Using references, then we need to make sure that we can resolve those references and keep the corresponding sources published while the binaries are published.  Prevent copies of binaries if any such references can't be resolved in the target publishing context.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:built-using-guard-copying into launchpad:master.
diff --git a/lib/lp/soyuz/interfaces/binarysourcereference.py b/lib/lp/soyuz/interfaces/binarysourcereference.py
index e625cf9..f702959 100644
--- a/lib/lp/soyuz/interfaces/binarysourcereference.py
+++ b/lib/lp/soyuz/interfaces/binarysourcereference.py
@@ -99,3 +99,20 @@ class IBinarySourceReferenceSet(Interface):
             pointing to any of this sequence of `ISourcePackageRelease`s.
         :return: A `ResultSet` of matching `IBinarySourceReference`s.
         """
+
+    def findMissingSources(archive, distroseries, pockets, reference_type,
+                           binary_package_releases=None):
+        """Find references to unpublished sources in a given context.
+
+        :param archive: An `IArchive` to search for source publications.
+        :param distroseries: An `IDistroSeries` to search for source
+            publications.
+        :param pockets: A sequence of `PackagePublishingPocket`s to search
+            for source publications.
+        :param reference_type: A `BinarySourceReferenceType` to search for.
+        :param binary_package_releases: Only return references from any of
+            this sequence of `IBinaryPackageRelease`s.
+        :return: A `ResultSet` of matching `IBinarySourceReference`s where
+            the `source_package_release` is not published in the given
+            publishing context.
+        """
diff --git a/lib/lp/soyuz/model/binarysourcereference.py b/lib/lp/soyuz/model/binarysourcereference.py
index ca3563b..c2b41e0 100644
--- a/lib/lp/soyuz/model/binarysourcereference.py
+++ b/lib/lp/soyuz/model/binarysourcereference.py
@@ -14,7 +14,9 @@ __all__ = [
 import warnings
 
 from debian.deb822 import PkgRelation
+from storm.expr import LeftJoin
 from storm.locals import (
+    And,
     Int,
     Reference,
     )
@@ -35,7 +37,10 @@ from lp.soyuz.interfaces.binarysourcereference import (
     UnparsableBuiltUsing,
     )
 from lp.soyuz.model.distroarchseries import DistroArchSeries
-from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
+from lp.soyuz.model.publishing import (
+    BinaryPackagePublishingHistory,
+    SourcePackagePublishingHistory,
+    )
 
 
 @implementer(IBinarySourceReference)
@@ -175,3 +180,26 @@ class BinarySourceReferenceSet:
                     spr.id for spr in source_package_releases))
         return IStore(BinarySourceReference).find(
             BinarySourceReference, *clauses).config(distinct=True)
+
+    @classmethod
+    def findMissingSources(cls, archive, distroseries, pockets, reference_type,
+                           binary_package_releases):
+        """See `IBinarySourceReferenceSet`."""
+        origin = [
+            BinarySourceReference,
+            LeftJoin(
+                SourcePackagePublishingHistory,
+                And(
+                    BinarySourceReference.source_package_release_id ==
+                        SourcePackagePublishingHistory.sourcepackagereleaseID,
+                    SourcePackagePublishingHistory.archive == archive,
+                    SourcePackagePublishingHistory.distroseries ==
+                        distroseries,
+                    SourcePackagePublishingHistory.pocket.is_in(pockets))),
+            ]
+        return IStore(BinarySourceReference).using(*origin).find(
+            BinarySourceReference,
+            BinarySourceReference.binary_package_release_id.is_in(
+                bpr.id for bpr in binary_package_releases),
+            BinarySourceReference.reference_type == reference_type,
+            SourcePackagePublishingHistory.id == None).config(distinct=True)
diff --git a/lib/lp/soyuz/scripts/packagecopier.py b/lib/lp/soyuz/scripts/packagecopier.py
index 3891af5..03f4c77 100644
--- a/lib/lp/soyuz/scripts/packagecopier.py
+++ b/lib/lp/soyuz/scripts/packagecopier.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 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).
 
 """Package copying utilities."""
@@ -19,10 +19,17 @@ from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from lp.services.database.bulk import load_related
+from lp.soyuz.adapters.archivedependencies import pocket_dependencies
 from lp.soyuz.adapters.overrides import SourceOverride
-from lp.soyuz.enums import SourcePackageFormat
+from lp.soyuz.enums import (
+    BinarySourceReferenceType,
+    SourcePackageFormat,
+    )
 from lp.soyuz.interfaces.archive import CannotCopy
 from lp.soyuz.interfaces.binarypackagebuild import BuildSetStatus
+from lp.soyuz.interfaces.binarysourcereference import (
+    IBinarySourceReferenceSet,
+    )
 from lp.soyuz.interfaces.publishing import (
     active_publishing_status,
     IBinaryPackagePublishingHistory,
@@ -439,6 +446,7 @@ class CopyChecker:
             built_binaries = source.getBuiltBinaries(want_files=True)
             if len(built_binaries) == 0:
                 raise CannotCopy("source has no binaries to be copied")
+
             # Deny copies of binary publications containing files with
             # expiration date set. We only set such value for immediate
             # expiration of old superseded binaries, so no point in
@@ -449,6 +457,22 @@ class CopyChecker:
                     if binary_file.libraryfile.expires is not None:
                         raise CannotCopy('source has expired binaries')
 
+            # Deny copies of binary publications that contain Built-Using
+            # references to sources that do not exist in the target.  The
+            # dominator will not be able to rectify the situation.
+            bsr_set = getUtility(IBinarySourceReferenceSet)
+            missing_sources = bsr_set.findMissingSources(
+                self.archive, series, pocket_dependencies[pocket],
+                BinarySourceReferenceType.BUILT_USING,
+                [binary_pub.binarypackagerelease
+                 for binary_pub in built_binaries])
+            if not missing_sources.is_empty():
+                # XXX cjwatson 2020-04-19: It may also be useful to show the
+                # specific Built-Using references that don't exist.
+                raise CannotCopy(
+                    'source has binaries with Built-Using references that do '
+                    'not exist in the target')
+
         # Check if there is already a source with the same name and version
         # published in the destination archive.
         self._checkArchiveConflicts(source, series)
diff --git a/lib/lp/soyuz/scripts/tests/test_copypackage.py b/lib/lp/soyuz/scripts/tests/test_copypackage.py
index 388fcf7..1db7a47 100644
--- a/lib/lp/soyuz/scripts/tests/test_copypackage.py
+++ b/lib/lp/soyuz/scripts/tests/test_copypackage.py
@@ -1016,6 +1016,7 @@ class CopyCheckerTestCase(TestCaseWithFactory):
             copied_source, copied_source.distroseries, copied_source.pocket,
             None, False)
 
+<<<<<<< lib/lp/soyuz/scripts/tests/test_copypackage.py
     def test_copy_to_another_archive_tracks_source_archive(self):
         """Checks that creating a copy of a package publishing keeps track
         of origin archive.
@@ -1045,6 +1046,102 @@ class CopyCheckerTestCase(TestCaseWithFactory):
                     archive=target_archive,
                     copied_from_archive=binary.archive),
             ]))
+=======
+    def test_checkCopy_cannot_copy_dangling_built_using_references(self):
+        # checkCopy() raises CannotCopy if the copy includes binaries and
+        # the binaries contain Built-Using references to sources that do not
+        # exist in the target.
+
+        # Create testing sources and binaries.
+        source = self.test_publisher.getPubSource()
+        built_using_source = self.test_publisher.getPubSource(
+            sourcename='built-using')
+        built_using_relationship = '%s (= %s)' % (
+            built_using_source.sourcepackagerelease.name,
+            built_using_source.sourcepackagerelease.version)
+        self.test_publisher.getPubBinaries(
+            pub_source=source, built_using=built_using_relationship)
+
+        # Create a fresh PPA which will be the destination copy.
+        archive = self.factory.makeArchive(
+            distribution=self.test_publisher.ubuntutest,
+            purpose=ArchivePurpose.PPA)
+        series = source.distroseries
+        pocket = source.pocket
+
+        # Now source-only copies are allowed.
+        copy_checker = CopyChecker(archive, include_binaries=False)
+        self.assertIsNone(
+            copy_checker.checkCopy(
+                source, series, pocket, check_permissions=False))
+
+        # Copies with binaries are denied.
+        copy_checker = CopyChecker(archive, include_binaries=True)
+        self.assertRaisesWithContent(
+            CannotCopy,
+            'source has binaries with Built-Using references that do not '
+            'exist in the target',
+            copy_checker.checkCopy,
+            source, series, pocket, check_permissions=False)
+
+    def test_checkCopy_can_copy_resolvable_built_using_references(self):
+        # checkCopy() allows copying binaries with Built-Using references to
+        # sources that exist in the target, even if no longer published or
+        # in a pocket that the target depends on.
+
+        # Create testing sources and binaries.
+        source = self.test_publisher.getPubSource()
+        published_source = self.test_publisher.getPubSource(
+            sourcename='published')
+        superseded_source = self.test_publisher.getPubSource(
+            sourcename='superseded')
+        release_pocket_source = self.test_publisher.getPubSource(
+            sourcename='release-pocket')
+        relationships = [
+            '%s (= %s)' % (
+                spph.sourcepackagerelease.name,
+                spph.sourcepackagerelease.version)
+            for spph in (
+                published_source, superseded_source, release_pocket_source)]
+        self.test_publisher.getPubBinaries(
+            built_using=', '.join(relationships),
+            status=PackagePublishingStatus.PUBLISHED, pub_source=source)
+        target_series = self.factory.makeDistroSeries(
+            distribution=source.distroseries.distribution)
+        getUtility(ISourcePackageFormatSelectionSet).add(
+            target_series, SourcePackageFormat.FORMAT_1_0)
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=target_series, archive=source.archive,
+            sourcepackagerelease=published_source.sourcepackagerelease,
+            pocket=PackagePublishingPocket.PROPOSED,
+            status=PackagePublishingStatus.PUBLISHED)
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=target_series, archive=source.archive,
+            sourcepackagerelease=superseded_source.sourcepackagerelease,
+            pocket=PackagePublishingPocket.PROPOSED,
+            status=PackagePublishingStatus.SUPERSEDED)
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=target_series, archive=source.archive,
+            sourcepackagerelease=release_pocket_source.sourcepackagerelease,
+            pocket=PackagePublishingPocket.RELEASE,
+            status=PackagePublishingStatus.PUBLISHED)
+
+        # Copies of binaries are permitted.
+        copy_checker = CopyChecker(source.archive, include_binaries=True)
+        copy_checker.checkCopy(
+            source, target_series, PackagePublishingPocket.PROPOSED,
+            check_permissions=False)
+
+        # Since some of the sources were only published in PROPOSED, copies
+        # of binaries to RELEASE that refer to them are denied.
+        self.assertRaisesWithContent(
+            CannotCopy,
+            'source has binaries with Built-Using references that do not '
+            'exist in the target',
+            copy_checker.checkCopy,
+            source, target_series, PackagePublishingPocket.RELEASE,
+            check_permissions=False)
+>>>>>>> lib/lp/soyuz/scripts/tests/test_copypackage.py
 
 
 class BaseDoCopyTests:
diff --git a/lib/lp/soyuz/tests/test_binarysourcereference.py b/lib/lp/soyuz/tests/test_binarysourcereference.py
index 0c4db7f..b8ebcef 100644
--- a/lib/lp/soyuz/tests/test_binarysourcereference.py
+++ b/lib/lp/soyuz/tests/test_binarysourcereference.py
@@ -306,3 +306,62 @@ class TestBinarySourceReference(TestCaseWithFactory):
                     source_package_release=bsr.source_package_release,
                     reference_type=BinarySourceReferenceType.BUILT_USING)
                 for bsr in [bsrs[0], bsrs[2]])))
+
+    def test_findMissingSources(self):
+        # findMissingSources finds references whose source publications
+        # aren't present in the given publishing context.
+        archive = self.factory.makeArchive()
+        distroseries = archive.distribution.currentseries
+        pockets = (
+            PackagePublishingPocket.RELEASE, PackagePublishingPocket.PROPOSED)
+        spphs = [
+            self.factory.makeSourcePackagePublishingHistory(
+                archive=archive, distroseries=distroseries, pocket=pocket)
+            for pocket in pockets]
+        bprs = []
+        for spph in spphs:
+            build = self.factory.makeBinaryPackageBuild(
+                distroarchseries=self.factory.makeDistroArchSeries(
+                    distroseries=spph.distroseries),
+                archive=spph.archive, pocket=spph.pocket)
+            bprs.append(self.factory.makeBinaryPackageRelease(build=build))
+        for bpr, spph in zip(bprs, spphs):
+            self.reference_set.createFromSourcePackageReleases(
+                bpr, [spph.sourcepackagerelease],
+                BinarySourceReferenceType.BUILT_USING)
+        self.assertTrue(
+            self.reference_set.findMissingSources(
+                archive, distroseries, pockets,
+                BinarySourceReferenceType.BUILT_USING, bprs).is_empty())
+        # Try searching with slight mismatches; findMissingSources should
+        # return appropriate results since the necessary SPPHs aren't
+        # present.
+        self.assertThat(
+            self.reference_set.findMissingSources(
+                self.factory.makeArchive(), distroseries, pockets,
+                BinarySourceReferenceType.BUILT_USING, bprs),
+            MatchesSetwise(*(
+                MatchesStructure.byEquality(
+                    binary_package_release=bpr,
+                    source_package_release=spph.sourcepackagerelease,
+                    reference_type=BinarySourceReferenceType.BUILT_USING)
+                for bpr, spph in zip(bprs, spphs))))
+        self.assertThat(
+            self.reference_set.findMissingSources(
+                archive, self.factory.makeDistroSeries(), pockets,
+                BinarySourceReferenceType.BUILT_USING, bprs),
+            MatchesSetwise(*(
+                MatchesStructure.byEquality(
+                    binary_package_release=bpr,
+                    source_package_release=spph.sourcepackagerelease,
+                    reference_type=BinarySourceReferenceType.BUILT_USING)
+                for bpr, spph in zip(bprs, spphs))))
+        self.assertThat(
+            self.reference_set.findMissingSources(
+                archive, distroseries, [PackagePublishingPocket.PROPOSED],
+                BinarySourceReferenceType.BUILT_USING, bprs),
+            MatchesSetwise(
+                MatchesStructure.byEquality(
+                    binary_package_release=bprs[0],
+                    source_package_release=spphs[0].sourcepackagerelease,
+                    reference_type=BinarySourceReferenceType.BUILT_USING)))