Merge lp:~wgrant/launchpad/dont-overrideFromAncestry into lp:launchpad

[William Grant]

Drop overrideFromAncestry, getNearestAncestor, and getAncestry.

Stop calling overrideFromAncestry and getNearestAncestor in do_copy. overrideFromAncestry is redundant with the modern overrides in _do_direct_copy, and getNearestAncestor can be replaced with getPublishedSources. Those two methods and getAncestry can then go away, saving a lot of code.

[Steve Kowalik (stevenk)]

review: Approve (code)

=== modified file 'lib/lp/soyuz/doc/publishing.txt'
--- lib/lp/soyuz/doc/publishing.txt	2013-03-12 01:07:49 +0000
+++ lib/lp/soyuz/doc/publishing.txt	2013-05-08 06:29:28 +0000
@@ -1756,117 +1756,3 @@
     ...         [firefox_source_pub.id, foo_pub.id],
     ...         archive=ubuntu.main_archive)
     >>> print_build_summaries(build_summaries)
-
-
-IPublishing ancestry lookup and override
-========================================
-
-`IPublishing` is implemented by both kinds of package publications we
-have `SourcePackagePublishingHistory` and
-`BinaryPackagePublishingHistory`.
-
-This interface allows, among other features, easy ancestry lookup and
-pre-publication overrides via:
-
- * getAncestry
- * overrideFromAncestry
-
-    # Setup a publishing scenario for illustrating ancestry lookup.
-    >>> ubuntu_source_ancestry = test_publisher.getPubSource(
-    ...     sourcename='testing', version='1.0',
-    ...     architecturehintlist='i386', component='multiverse',
-    ...     status=PackagePublishingStatus.PUBLISHED)
-    >>> ubuntu_binary_ancestry = test_publisher.getPubBinaries(
-    ...     binaryname='testing-bin', pub_source=ubuntu_source_ancestry,
-    ...     status=PackagePublishingStatus.PUBLISHED)[0]
-    >>> ppa_source_ancestry = test_publisher.getPubSource(
-    ...     sourcename='testing', version='1.1', component='main',
-    ...     architecturehintlist='i386', archive=cprov.archive,
-    ...     status=PackagePublishingStatus.PUBLISHED)
-    >>> ppa_binary_ancestry = test_publisher.getPubBinaries(
-    ...     binaryname='testing-bin', pub_source=ppa_source_ancestry,
-    ...     status=PackagePublishingStatus.PUBLISHED)[0]
-    >>> test_source = test_publisher.getPubSource(
-    ...     sourcename='testing', version='2.0', component='universe',
-    ...     architecturehintlist='i386')
-    >>> test_binary = test_publisher.getPubBinaries(
-    ...     binaryname='testing-bin', pub_source=test_source)[0]
-
-We will create a helper function to inspect ancestries. It simply pass
-any given keyword argument to 'test_source' and 'test_binary'
-getAncestry() method.
-
-    >>> def print_ancestries(**kwargs):
-    ...     def print_displayname(pub):
-    ...         if pub is not None:
-    ...             print pub.displayname
-    ...         else:
-    ...             print pub
-    ...     print_displayname(test_source.getAncestry(**kwargs))
-    ...     print_displayname(test_binary.getAncestry(**kwargs))
-
-The 'test_source' and 'test_binary' ancestries in the same context,
-i.e. ubuntu primary archive.
-
-    >>> print_ancestries()
-    testing 1.0 in breezy-autotest
-    testing-bin 1.0 in breezy-autotest i386
-
-Call sites may quickly adjust the context where the ancestries are
-looked up.
-
-    >>> print_ancestries(archive=cprov.archive)
-    testing 1.1 in breezy-autotest
-    testing-bin 1.1 in breezy-autotest i386
-
-    >>> print_ancestries(distroseries=test_source.distroseries)
-    testing 1.0 in breezy-autotest
-    testing-bin 1.0 in breezy-autotest i386
-
-    >>> print_ancestries(pocket=test_source.pocket)
-    testing 1.0 in breezy-autotest
-    testing-bin 1.0 in breezy-autotest i386
-
-The default 'status' filter is set to PUBLISHED ancestries, however
-call sites may decide differently.
-
-    >>> print_ancestries(status=PackagePublishingStatus.PUBLISHED)
-    testing 1.0 in breezy-autotest
-    testing-bin 1.0 in breezy-autotest i386
-
-On the lack of ancestry, None is returned.
-
-    >>> from lp.soyuz.interfaces.publishing import (
-    ...     inactive_publishing_status)
-    >>> print_ancestries(status=inactive_publishing_status)
-    None
-    None
-
-`overrideFromAncestry` operates directly on top of the default
-behavior of `getAncestry`. It looks up the most recent ancestry for a
-publication and override it in place.  If there is no previous publication
-then the package's component is used.
-
-    >>> print test_source.component.name
-    universe
-
-    >>> test_source.overrideFromAncestry()
-    >>> print test_source.component.name
-    multiverse
-
-It works in the same way for binaries.
-
-    >>> multiverse = getUtility(IComponentSet)['multiverse']
-    >>> test_binary.binarypackagerelease.component = multiverse
-
-    >>> print test_binary.component.name
-    universe
-
-    >>> copied_binary = test_binary.copyTo(
-    ...     hoary_test, test_source.pocket, archive=test_binary.archive)[0]
-    >>> print copied_binary.component.name
-    universe
-
-    >>> copied_binary.overrideFromAncestry()
-    >>> print copied_binary.component.name
-    multiverse

=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py	2013-05-03 00:07:30 +0000
+++ lib/lp/soyuz/interfaces/publishing.py	2013-05-08 06:29:28 +0000
@@ -221,34 +221,6 @@
             `IBinaryPackagePublishingHistory`.
         """
 
-    def getAncestry(archive=None, distroseries=None, pocket=None,
-                    status=None):
-        """Return the most recent publication of the same source or binary.
-
-        If a suitable ancestry could not be found, None is returned.
-
-        It optionally accepts parameters for adjusting the publishing
-        context, if not given they default to the current context.
-
-        :param archive: optional `IArchive`, defaults to the context archive.
-        :param distroseries: optional `IDistroSeries`, defaults to the
-            context distroseries.
-        :param pocket: optional `PackagePublishingPocket`, defaults to any
-            pocket.
-        :param status: optional `PackagePublishingStatus` or a collection of
-            them, defaults to `PackagePublishingStatus.PUBLISHED`
-        """
-
-    def overrideFromAncestry():
-        """Set the right published component from publishing ancestry.
-
-        Start with the publishing records and fall back to the original
-        uploaded package if necessary.
-
-        :raise: AssertionError if the context publishing record is not in
-            PENDING status.
-        """
-
 
 class IPublishingEdit(Interface):
     """Base interface for writeable Publishing classes."""
@@ -1343,34 +1315,6 @@
         the source_package_pub, allowing the use of the cached results.
         """
 
-    def getNearestAncestor(
-        package_name, archive, distroseries, pocket=None, status=None,
-        binary=False):
-        """Return the ancestor of the given parkage in a particular archive.
-
-        :param package_name: The package name for which we are checking for
-            an ancestor.
-        :type package_name: ``string``
-        :param archive: The archive in which we are looking for an ancestor.
-        :type archive: `IArchive`
-        :param distroseries: The particular series in which we are looking for
-            an ancestor.
-        :type distroseries: `IDistroSeries`
-        :param pocket: An optional pocket to restrict the search.
-        :type pocket: `PackagePublishingPocket`.
-        :param status: An optional status defaulting to PUBLISHED if not
-            provided.
-        :type status: `PackagePublishingStatus`
-        :param binary: An optional argument to look for a binary ancestor
-            instead of the default source.
-        :type binary: ``Boolean``
-
-        :return: The most recent publishing history for the given
-            arguments.
-        :rtype: `ISourcePackagePublishingHistory` or
-            `IBinaryPackagePublishingHistory` or None.
-        """
-
 active_publishing_status = (
     PackagePublishingStatus.PENDING,
     PackagePublishingStatus.PUBLISHED,

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2013-05-04 00:37:58 +0000
+++ lib/lp/soyuz/model/publishing.py	2013-05-08 06:29:28 +0000
@@ -847,41 +847,6 @@
         return getUtility(
             IPublishingSet).getBuildStatusSummaryForSourcePublication(self)
 
-    def getAncestry(self, archive=None, distroseries=None, pocket=None,
-                    status=None):
-        """See `ISourcePackagePublishingHistory`."""
-        if archive is None:
-            archive = self.archive
-        if distroseries is None:
-            distroseries = self.distroseries
-
-        return getUtility(IPublishingSet).getNearestAncestor(
-            self.source_package_name, archive, distroseries, pocket,
-            status)
-
-    def overrideFromAncestry(self):
-        """See `ISourcePackagePublishingHistory`."""
-        # We don't want to use changeOverride here because it creates a
-        # new publishing record. This code can be only executed for pending
-        # publishing records.
-        assert self.status == PackagePublishingStatus.PENDING, (
-            "Cannot override published records.")
-
-        # If there is published ancestry, use its component, otherwise
-        # use the original upload component. Since PPAs only use main,
-        # we don't need to check the ancestry.
-        if not self.archive.is_ppa:
-            ancestry = self.getAncestry()
-            if ancestry is not None:
-                component = ancestry.component
-            else:
-                component = self.sourcepackagerelease.component
-
-            self.component = component
-
-        assert self.component in (
-            self.archive.getComponentsForSeries(self.distroseries))
-
     def sourceFileUrls(self):
         """See `ISourcePackagePublishingHistory`."""
         source_urls = proxied_urls(
@@ -1280,36 +1245,6 @@
         return getUtility(IPublishingSet).copyBinariesTo(
             [self], distroseries, pocket, archive)
 
-    def getAncestry(self, archive=None, distroseries=None, pocket=None,
-                    status=None):
-        """See `IBinaryPackagePublishingHistory`."""
-        if archive is None:
-            archive = self.archive
-        if distroseries is None:
-            distroseries = self.distroarchseries.distroseries
-
-        return getUtility(IPublishingSet).getNearestAncestor(
-            self.binary_package_name, archive, distroseries, pocket,
-            status, binary=True)
-
-    def overrideFromAncestry(self):
-        """See `IBinaryPackagePublishingHistory`."""
-        # We don't want to use changeOverride here because it creates a
-        # new publishing record. This code can be only executed for pending
-        # publishing records.
-        assert self.status == PackagePublishingStatus.PENDING, (
-            "Cannot override published records.")
-
-        # If there is an ancestry, use its component, otherwise use the
-        # original upload component.
-        ancestry = self.getAncestry()
-        if ancestry is not None:
-            component = ancestry.component
-        else:
-            component = self.binarypackagerelease.component
-
-        self.component = component
-
     def _getDownloadCountClauses(self, start_date=None, end_date=None):
         clauses = [
             BinaryPackageReleaseDownloadCount.archive == self.archive,
@@ -2109,27 +2044,6 @@
                 BinaryPackagePublishingHistory, bpph_ids, removed_by,
                 removal_comment=removal_comment)
 
-    def getNearestAncestor(
-        self, package_name, archive, distroseries, pocket=None,
-        status=None, binary=False):
-        """See `IPublishingSet`."""
-        if status is None:
-            status = PackagePublishingStatus.PUBLISHED
-
-        if binary:
-            ancestries = archive.getAllPublishedBinaries(
-                name=package_name, exact_match=True, pocket=pocket,
-                status=status, distroarchseries=distroseries.architectures)
-        else:
-            ancestries = archive.getPublishedSources(
-                name=package_name, exact_match=True, pocket=pocket,
-                status=status, distroseries=distroseries)
-
-        try:
-            return ancestries[0]
-        except IndexError:
-            return None
-
 
 def get_current_source_releases(context_sourcepackagenames, archive_ids_func,
                                 package_clause_func, extra_clauses, key_col):

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2013-04-17 23:19:35 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2013-05-08 06:29:28 +0000
@@ -38,6 +38,7 @@
 from lp.soyuz.interfaces.queue import IPackageUploadCustom
 from lp.soyuz.scripts.custom_uploads_copier import CustomUploadsCopier
 
+
 # XXX cprov 2009-06-12: this function should be incorporated in
 # IPublishing.
 def update_files_privacy(pub_record):
@@ -181,7 +182,7 @@
         # implementations of ancestry lookup:
         # NascentUpload.getSourceAncestry,
         # PackageUploadSource.getSourceAncestryForDiffs, and
-        # PublishingSet.getNearestAncestor, none of which is obviously
+        # Archive.getPublishedSources, none of which is obviously
         # correct here.  Instead of adding a fourth, we should consolidate
         # these.
         ancestries = archive.getPublishedSources(
@@ -476,8 +477,10 @@
         # published in the destination archive.
         self._checkArchiveConflicts(source, series)
 
-        ancestry = source.getAncestry(
-            self.archive, series, pocket, status=active_publishing_status)
+        ancestry = self.archive.getPublishedSources(
+            name=source.source_package_name, exact_match=True,
+            distroseries=series, pocket=pocket,
+            status=active_publishing_status).first()
         if ancestry is not None:
             ancestry_version = ancestry.sourcepackagerelease.version
             copy_version = source.sourcepackagerelease.version
@@ -633,13 +636,12 @@
                 announce_from_person=announce_from_person,
                 previous_version=old_version)
         if not archive.private and has_restricted_files(source):
-            # Fix copies by overriding them according to the current
-            # ancestry and unrestrict files with privacy mismatch.  We must
-            # do this *after* calling notify (which only actually sends mail
-            # on commit), because otherwise the new changelog LFA won't be
-            # visible without a commit, which may not be safe here.
+            # Fix copies by unrestricting files with privacy mismatch.
+            # We must do this *after* calling notify (which only
+            # actually sends mail on commit), because otherwise the new
+            # changelog LFA won't be visible without a commit, which may
+            # not be safe here.
             for pub_record in sub_copies:
-                pub_record.overrideFromAncestry()
                 for changed_file in update_files_privacy(pub_record):
                     if logger is not None:
                         logger.info("Made %s public" % changed_file.filename)

=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py	2013-05-04 00:37:58 +0000
+++ lib/lp/soyuz/tests/test_publishing.py	2013-05-08 06:29:28 +0000
@@ -31,9 +31,7 @@
 from lp.services.database.constants import UTC_NOW
 from lp.services.librarian.interfaces import ILibraryFileAliasSet
 from lp.services.log.logger import DevNullLogger
-from lp.soyuz.adapters.overrides import UnknownOverridePolicy
 from lp.soyuz.enums import (
-    ArchivePurpose,
     BinaryPackageFormat,
     PackageUploadStatus,
     )
@@ -803,200 +801,6 @@
         shutil.rmtree(test_temp_dir)
 
 
-class OverrideFromAncestryTestCase(TestCaseWithFactory):
-    """Test `IPublishing.overrideFromAncestry`.
-
-    When called from a `SourcePackagePublishingHistory` or a
-    `BinaryPackagePublishingHistory` it sets the object target component
-    according to its ancestry if available or falls back to the component
-    it was originally uploaded to.
-    """
-    layer = LaunchpadZopelessLayer
-
-    def setUp(self):
-        TestCaseWithFactory.setUp(self)
-        self.test_publisher = SoyuzTestPublisher()
-        self.test_publisher.prepareBreezyAutotest()
-
-    def test_overrideFromAncestry_only_works_for_pending_records(self):
-        # overrideFromAncestry only accepts PENDING publishing records.
-        source = self.test_publisher.getPubSource()
-
-        forbidden_status = [
-            item
-            for item in PackagePublishingStatus.items
-            if item is not PackagePublishingStatus.PENDING]
-
-        for status in forbidden_status:
-            source.status = status
-            self.layer.commit()
-            self.assertRaisesWithContent(
-                AssertionError,
-                'Cannot override published records.',
-                source.overrideFromAncestry)
-
-    def makeSource(self):
-        """Return a 'source' publication.
-
-        It's pending publication with binaries in a brand new PPA
-        and in 'main' component.
-        """
-        test_archive = self.factory.makeArchive(
-            distribution=self.test_publisher.ubuntutest,
-            purpose=ArchivePurpose.PPA)
-        source = self.test_publisher.getPubSource(archive=test_archive)
-        self.test_publisher.getPubBinaries(pub_source=source)
-        return source
-
-    def copyAndCheck(self, pub_record, series, component_name):
-        """Copy and check if overrideFromAncestry is working as expected.
-
-        The copied publishing record is targeted to the same component
-        as its source, but override_from_ancestry changes it to follow
-        the ancestry or fallback to the SPR/BPR original component.
-        """
-        copied = pub_record.copyTo(
-            series, pub_record.pocket, series.main_archive)
-
-        # Cope with heterogeneous results from copyTo().
-        try:
-            copies = tuple(copied)
-        except TypeError:
-            copies = (copied, )
-
-        for copy in copies:
-            self.assertEqual(pub_record.component, copy.component)
-            copy.overrideFromAncestry()
-            self.layer.commit()
-            self.assertEqual("universe", copy.component.name)
-
-    def test_overrideFromAncestry_fallback_to_source_component(self):
-        # overrideFromAncestry on the lack of ancestry, falls back to the
-        # component the source was originally uploaded to.
-        source = self.makeSource()
-
-        # Adjust the source package release original component.
-        universe = getUtility(IComponentSet)['universe']
-        source.sourcepackagerelease.component = universe
-
-        self.copyAndCheck(source, source.distroseries, 'universe')
-
-    def test_overrideFromAncestry_fallback_to_binary_component(self):
-        # overrideFromAncestry on the lack of ancestry, falls back to the
-        # component the binary was originally uploaded to.
-        binary = self.makeSource().getPublishedBinaries()[0]
-
-        # Adjust the binary package release original component.
-        universe = getUtility(IComponentSet)['universe']
-        removeSecurityProxy(binary.binarypackagerelease).component = universe
-
-        self.copyAndCheck(
-            binary, binary.distroarchseries.distroseries, 'universe')
-
-    def test_overrideFromAncestry_follow_ancestry_source_component(self):
-        # overrideFromAncestry finds and uses the component of the most
-        # recent PUBLISHED publication of the same name in the same
-        #location.
-        source = self.makeSource()
-
-        # Create a published ancestry source in the copy destination
-        # targeted to 'universe' and also 2 other noise source
-        # publications, a pending source target to 'restricted' and
-        # a published, but older, one target to 'multiverse'.
-        self.test_publisher.getPubSource(component='restricted')
-
-        self.test_publisher.getPubSource(
-            component='multiverse', status=PackagePublishingStatus.PUBLISHED)
-
-        self.test_publisher.getPubSource(
-            component='universe', status=PackagePublishingStatus.PUBLISHED)
-
-        # Overridden copy it targeted to 'universe'.
-        self.copyAndCheck(source, source.distroseries, 'universe')
-
-    def test_overrideFromAncestry_follow_ancestry_binary_component(self):
-        # overrideFromAncestry finds and uses the component of the most
-        # recent published publication of the same name in the same
-        # location.
-        binary = self.makeSource().getPublishedBinaries()[0]
-
-        # Create a published ancestry binary in the copy destination
-        # targeted to 'universe'.
-        restricted_source = self.test_publisher.getPubSource(
-            component='restricted')
-        self.test_publisher.getPubBinaries(pub_source=restricted_source)
-
-        multiverse_source = self.test_publisher.getPubSource(
-            component='multiverse')
-        self.test_publisher.getPubBinaries(
-            pub_source=multiverse_source,
-            status=PackagePublishingStatus.PUBLISHED)
-
-        ancestry_source = self.test_publisher.getPubSource(
-            component='universe')
-        self.test_publisher.getPubBinaries(
-            pub_source=ancestry_source,
-            status=PackagePublishingStatus.PUBLISHED)
-
-        # Overridden copy it targeted to 'universe'.
-        self.copyAndCheck(
-            binary, binary.distroarchseries.distroseries, 'universe')
-
-    def test_ppa_override_no_ancestry(self):
-        # Test a PPA publication with no ancestry is 'main'
-        ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
-        spr = self.factory.makeSourcePackageRelease()
-        spph = self.factory.makeSourcePackagePublishingHistory(
-            sourcepackagerelease=spr, archive=ppa)
-        spph.overrideFromAncestry()
-        self.assertEqual("main", spph.component.name)
-
-    def test_ppa_override_with_ancestry(self):
-        # Test a PPA publication with ancestry
-        ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
-        spr = self.factory.makeSourcePackageRelease()
-        # We don't reference the first spph so it doesn't get a name.
-        self.factory.makeSourcePackagePublishingHistory(
-            sourcepackagerelease=spr, archive=ppa)
-        spph2 = self.factory.makeSourcePackagePublishingHistory(
-            sourcepackagerelease=spr, archive=ppa)
-        spph2.overrideFromAncestry()
-        self.assertEqual("main", spph2.component.name)
-
-    def test_copyTo_with_overrides(self):
-        # Specifying overrides with copyTo should result in the new
-        # publication having those overrides.
-        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
-        target_archive = self.factory.makeArchive(
-            purpose=ArchivePurpose.PRIMARY)
-        main_component = getUtility(IComponentSet)['main']
-        spph = self.factory.makeSourcePackagePublishingHistory(
-            archive=archive, component=main_component)
-        name = spph.sourcepackagerelease.sourcepackagename
-
-        # Roll with default overrides to reduce the setup.
-        policy = UnknownOverridePolicy()
-        overrides = policy.calculateSourceOverrides(
-            target_archive, None, None, [name])
-        [override] = overrides
-
-        copy = spph.copyTo(
-            spph.distroseries, spph.pocket, target_archive, override)
-
-        # The component is overridden to the default.
-        self.assertEqual('universe', copy.component.name)
-        # Section has no default so it comes from the old publication.
-        self.assertEqual(spph.section, copy.section)
-
-    def test_copyTo_sets_ancestor(self):
-        # SPPH's ancestor get's populated when a spph is copied over.
-        target_archive = self.factory.makeArchive()
-        spph = self.factory.makeSourcePackagePublishingHistory()
-        copy = spph.copyTo(spph.distroseries, spph.pocket, target_archive)
-
-        self.assertEqual(spph, copy.ancestor)
-
-
 class BuildRecordCreationTests(TestNativePublishingBase):
     """Test the creation of build records."""