launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15559
[Merge] lp:~wgrant/launchpad/dont-overrideFromAncestry into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/dont-overrideFromAncestry into lp:launchpad.
Commit message:
Drop overrideFromAncestry, getNearestAncestor, and getAncestry.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/dont-overrideFromAncestry/+merge/162933
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.
--
https://code.launchpad.net/~wgrant/launchpad/dont-overrideFromAncestry/+merge/162933
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/dont-overrideFromAncestry into lp:launchpad.
=== 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."""
