launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/bug-805547 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #805547 in Launchpad itself: "Deriving from multiple distroseries fails with duplicate package errors"
  https://bugs.launchpad.net/launchpad/+bug/805547

For more details, see:
https://code.launchpad.net/~rvb/launchpad/bug-805547/+merge/66861

This branch fixes initialise_distroseries so that only the sources from each parent is copied when a packageset's content is copied.

= Test =

./bin/test -vvc test_initialize_distroseries test_copying_packagesets_multiple_parents_same_source

= QA =

1. Make a new distro and series
2. On the initialization page (+initseries):
  * Select natty as a parent
  * Select lucid as a parent
3. Select "zope" packageset from natty
4. Select "mythbuntu" packageset from lucid
5. Click initialise

The initialisation should not fail.

-- 
https://code.launchpad.net/~rvb/launchpad/bug-805547/+merge/66861
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/bug-805547 into lp:launchpad.

=== modified file 'lib/lp/soyuz/scripts/initialize_distroseries.py'
--- lib/lp/soyuz/scripts/initialize_distroseries.py	2011-06-24 12:16:30 +0000
+++ lib/lp/soyuz/scripts/initialize_distroseries.py	2011-07-05 08:09:33 +0000
@@ -315,15 +315,22 @@
         """
         archive_set = getUtility(IArchiveSet)
 
-        spns = []
-        # The overhead from looking up each packageset is mitigated by
-        # this usually running from a job.
-        if self.packagesets:
-            for pkgsetid in self.packagesets:
-                pkgset = self._store.get(Packageset, int(pkgsetid))
-                spns += list(pkgset.getSourcesIncluded())
-
         for parent in self.derivation_parents:
+            spns = []
+            # The overhead from looking up each packageset is mitigated by
+            # this usually running from a job.
+            if self.packagesets:
+                for pkgsetid in self.packagesets:
+                    pkgset = self._store.get(Packageset, int(pkgsetid))
+                    if pkgset.distroseries == parent:
+                        spns += list(pkgset.getSourcesIncluded())
+
+                # Some packagesets where selected but not a single
+                # source from this parent: we skip the copy since
+                # calling copy with spns=[] would copy all the packagesets.
+                if spns == []:
+                    continue
+
             distroarchseries_list = distroarchseries_lists[parent]
             for archive in parent.distribution.all_distro_archives:
                 if archive.purpose not in (

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2011-06-24 12:16:30 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2011-07-05 08:09:33 +0000
@@ -268,6 +268,37 @@
         parent_srcs = test1.getSourcesIncluded(direct_inclusion=True)
         self.assertEqual(parent_srcs, child_srcs)
 
+    def test_copying_packagesets_multiple_parents_same_source(self):
+        # If a source with the same packagename is included in two parents,
+        # only the one from the selected packageset is copied.
+        self.parent, self.parent_das = self.setupParent()
+        self.parent2, self.parent_das = self.setupParent()
+        uploader = self.factory.makePerson()
+        test1 = getUtility(IPackagesetSet).new(
+            u'test1', u'test 1 packageset', self.parent.owner,
+            distroseries=self.parent)
+        test2 = getUtility(IPackagesetSet).new(
+            u'test2', u'test 2 packageset', self.parent2.owner,
+            distroseries=self.parent2)
+        test1.addSources('udev')
+        test2.addSources('udev')
+        getUtility(IArchivePermissionSet).newPackagesetUploader(
+            self.parent.main_archive, uploader, test1)
+        child = self._fullInitialize(
+            [self.parent, self.parent2], packagesets=(str(test1.id),))
+        child_test1 = getUtility(IPackagesetSet).getByName(
+            u'test1', distroseries=child)
+        self.assertEqual(test1.description, child_test1.description)
+        self.assertEqual(child_test1.relatedSets().one(), test1)
+        self.assertEqual(
+            list(child_test1.relatedSets()),
+            [test1])
+        # The contents of the packagesets will have been copied.
+        child_srcs = child_test1.getSourcesIncluded(
+            direct_inclusion=True)
+        parent_srcs = test1.getSourcesIncluded(direct_inclusion=True)
+        self.assertEqual(parent_srcs, child_srcs)
+
     def test_no_cross_distro_perm_copying(self):
         # No cross-distro archivepermissions copying should happen.
         self.parent, self.parent_das = self.setupParent()
@@ -704,7 +735,7 @@
              "and no parent was passed to the initilization method"
              ".").format(child=child),
              ids.check)
-             
+
     def test_copy_method_diff_archive_empty_target(self):
         # If the archives are different and the target archive is
         # empty: use the cloner.