launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/packageset-owner-preservation into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #740892 in Launchpad itself: "Ownership of package sets doesn't get preserved when a new series gets initialized"
  https://bugs.launchpad.net/launchpad/+bug/740892

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/packageset-owner-preservation/+merge/55466

This branch fixes bug #740892, preserving packageset owners when they are inherited within a distribution. Because initialising a new distribution normally crosses a privilege boundary, the new distroseries owner is used in the case of cross-distro initialisations.

19:21 < bigjools> we can preserve it for the same distro, change it for new distros
-- 
https://code.launchpad.net/~wgrant/launchpad/packageset-owner-preservation/+merge/55466
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/packageset-owner-preservation into lp:launchpad.

=== modified file 'lib/lp/soyuz/scripts/initialise_distroseries.py'
--- lib/lp/soyuz/scripts/initialise_distroseries.py	2011-01-15 06:32:40 +0000
+++ lib/lp/soyuz/scripts/initialise_distroseries.py	2011-03-30 04:49:29 +0000
@@ -285,11 +285,17 @@
         parent_to_child = {}
         # Create the packagesets, and any archivepermissions
         for parent_ps in packagesets:
+            # Cross-distro initialisations get packagesets owned by the
+            # distro owner, otherwise the old owner is preserved.
             if self.packagesets and parent_ps.name not in self.packagesets:
                 continue
+            if self.distroseries.distribution == self.parent.distribution:
+                new_owner = parent_ps.owner
+            else:
+                new_owner = self.distroseries.owner
             child_ps = getUtility(IPackagesetSet).new(
                 parent_ps.name, parent_ps.description,
-                self.distroseries.owner, distroseries=self.distroseries,
+                new_owner, distroseries=self.distroseries,
                 related_set=parent_ps)
             self._store.execute("""
                 INSERT INTO Archivepermission

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py	2010-10-14 12:56:31 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py	2011-03-30 04:49:29 +0000
@@ -158,8 +158,11 @@
             child.isSourcePackageFormatPermitted(
             SourcePackageFormat.FORMAT_1_0))
 
-    def _full_initialise(self, arches=(), packagesets=(), rebuild=False):
-        child = self.factory.makeDistroSeries(parent_series=self.parent)
+    def _full_initialise(self, arches=(), packagesets=(), rebuild=False,
+                         distribution=None):
+        child = self.factory.makeDistroSeries(
+            parent_series=self.parent,
+            distribution=distribution)
         ids = InitialiseDistroSeries(child, arches, packagesets, rebuild)
         ids.check()
         ids.initialise()
@@ -233,6 +236,28 @@
                 child.main_archive, 'udev', uploader,
                 distroseries=child))
 
+    def test_packageset_owner_preserved_within_distro(self):
+        # When initialising a new series within a distro, the copied
+        # packagesets have ownership preserved.
+        ps_owner = self.factory.makePerson()
+        ps = getUtility(IPackagesetSet).new(
+            u'ps', u'packageset', ps_owner, distroseries=self.parent)
+        child = self._full_initialise(distribution=self.parent.distribution)
+        child_ps = getUtility(IPackagesetSet).getByName(
+            u'ps', distroseries=child)
+        self.assertEqual(ps_owner, child_ps.owner)
+
+    def test_packageset_owner_not_preserved_cross_distro(self):
+        # In the case of a cross-distro initialisation, the new
+        # packagesets are owned by the new distro owner.
+        ps = getUtility(IPackagesetSet).new(
+            u'ps', u'packageset', self.factory.makePerson(),
+            distroseries=self.parent)
+        child = self._full_initialise()
+        child_ps = getUtility(IPackagesetSet).getByName(
+            u'ps', distroseries=child)
+        self.assertEqual(child.owner, child_ps.owner)
+
     def test_copy_limit_packagesets(self):
         # If a parent series has packagesets, we can decide which ones we
         # want to copy