launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/copy-policy-manual-overrides into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/copy-policy-manual-overrides/+merge/62930

ISourcePackagePublishingHistory.copyTo() was recently modified to take an override policy object so that it could automatically apply overrides when copying a package to a new publication.

This works well, except in the case where the callsite has manual overrides, as will happen with a PackageCopyJob that has been released by an archive admin with manual overrides.

To accommodate this, copyTo() now takes the override itself rather than the policy.  The single existing callsite (in _do_direct_copy) was fixed to provide this, and a new test was added to make sure it works.  There was no existing test for copying with policies at all, tsk tsk.

The override object is actually a tuple which is rather ugly.  It'll be easy to change it to a real object at some point but not now.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/copy-policy-manual-overrides/+merge/62930
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/copy-policy-manual-overrides into lp:launchpad/db-devel.

=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py	2011-05-19 04:58:58 +0000
+++ lib/lp/soyuz/interfaces/publishing.py	2011-05-30 22:45:59 +0000
@@ -589,14 +589,15 @@
         `IBinaryPackagePublishingHistory`.
         """
 
-    def copyTo(distroseries, pocket, archive, policy=None):
+    def copyTo(distroseries, pocket, archive, overrides=None):
         """Copy this publication to another location.
 
         :param distroseries: The `IDistroSeries` to copy the source
             publication into.
         :param pocket: The `PackagePublishingPocket` to copy into.
         :param archive: The `IArchive` to copy the source publication into.
-        :param policy: The `IOverridePolicy` to apply to the copy.
+        :param overrides: A tuple of override data as returned from a
+            `IOverridePolicy`
 
         :return: a `ISourcePackagePublishingHistory` record representing the
             source in the destination location.

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2011-05-23 09:27:05 +0000
+++ lib/lp/soyuz/model/publishing.py	2011-05-30 22:45:59 +0000
@@ -800,15 +800,12 @@
             section=new_section,
             archive=current.archive)
 
-    def copyTo(self, distroseries, pocket, archive, policy=None):
+    def copyTo(self, distroseries, pocket, archive, overrides=None):
         """See `ISourcePackagePublishingHistory`."""
         component = self.component
         section = self.section
-        if policy is not None:
-            packages = (self.sourcepackagerelease.sourcepackagename,)
-            override = policy.calculateSourceOverrides(
-                archive, distroseries, pocket, packages)
-            [spn, new_component, new_section] = override[0]
+        if overrides is not None:
+            [spn, new_component, new_section] = overrides[0]
             if new_component is not None:
                 component = new_component
             if new_section is not None:

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2011-05-17 12:38:49 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2011-05-30 22:45:59 +0000
@@ -606,14 +606,19 @@
     copies = []
 
     # Copy source if it's not yet copied.
-    policy = archive.getOverridePolicy()
     source_in_destination = archive.getPublishedSources(
         name=source.sourcepackagerelease.name, exact_match=True,
         version=source.sourcepackagerelease.version,
         status=active_publishing_status,
         distroseries=series, pocket=pocket)
+    policy = archive.getOverridePolicy()
     if source_in_destination.is_empty():
-        source_copy = source.copyTo(series, pocket, archive, policy=policy)
+        overrides = None
+        if policy is not None:
+            package_names = (source.sourcepackagerelease.sourcepackagename,)
+            overrides = policy.calculateSourceOverrides(
+                archive, series, pocket, package_names)
+        source_copy = source.copyTo(series, pocket, archive, overrides)
         close_bugs_for_sourcepublication(source_copy)
         copies.append(source_copy)
     else:

=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py	2011-04-05 03:45:57 +0000
+++ lib/lp/soyuz/tests/test_publishing.py	2011-05-30 22:45:59 +0000
@@ -37,6 +37,7 @@
 from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.services.log.logger import DevNullLogger
+from lp.soyuz.adapters.overrides import UnknownOverridePolicy
 from lp.soyuz.enums import (
     ArchivePurpose,
     BinaryPackageFormat,
@@ -981,6 +982,31 @@
         spph2.overrideFromAncestry()
         self.assertEquals(spph2.component.name, 'main')
 
+    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])
+
+        copy = spph.copyTo(
+            spph.distroseries, spph.pocket, target_archive, overrides)
+
+        # 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)
+
+
 class BuildRecordCreationTests(TestNativePublishingBase):
     """Test the creation of build records."""