launchpad-reviewers team mailing list archive

[Chris Johnston <chris.johnston@xxxxxxxxxxxxx>]

Chris Johnston has proposed merging lp:~cjohnston/launchpad/proper-pkg-link-copying into lp:launchpad.

Requested reviews:
  William Grant (wgrant): code

For more details, see:
https://code.launchpad.net/~cjohnston/launchpad/proper-pkg-link-copying/+merge/228000
-- 
https://code.launchpad.net/~cjohnston/launchpad/proper-pkg-link-copying/+merge/228000
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/soyuz/scripts/initialize_distroseries.py'
--- lib/lp/soyuz/scripts/initialize_distroseries.py	2014-07-21 22:45:01 +0000
+++ lib/lp/soyuz/scripts/initialize_distroseries.py	2014-07-25 00:24:39 +0000
@@ -510,10 +510,9 @@
         for parent in self.derivation_parents:
             spns = self.source_names_by_parent.get(parent.id, None)
             if spns is not None and len(spns) == 0:
-                # 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
-                # from this parent.
+                # Some packagesets may have been selected but not a single
+                # source from this parent. We will not copy any records from
+                # this parent.
                 continue
             # spns=None means no packagesets selected so we need to consider
             # all sources.
@@ -612,7 +611,13 @@
         # We iterate over the parents and copy into the child in
         # sequence to avoid creating duplicates.
         for parent_id in self.derivation_parent_ids:
-            self._store.execute("""
+            spns = self.source_names_by_parent.get(parent_id, None)
+            if spns is not None and len(spns) == 0:
+                # Some packagesets may have been selected but not a single
+                # source from this parent. We will not copy any links for this
+                # parent
+                continue
+            sql = ("""
                 INSERT INTO
                     Packaging(
                         distroseries, sourcepackagename, productseries,
@@ -634,25 +639,40 @@
                     -- Select only the packaging links that are in the parent
                     -- that are not in the child.
                     ChildSeries.id = %s
-                    AND Packaging.sourcepackagename in (
-                        SELECT sourcepackagename
-                        FROM Packaging
-                        WHERE distroseries in (
-                            SELECT id
-                            FROM Distroseries
-                            WHERE id = %s
-                            )
-                        EXCEPT
-                        SELECT sourcepackagename
-                        FROM Packaging
-                        WHERE distroseries in (
-                            SELECT id
-                            FROM Distroseries
-                            WHERE id = ChildSeries.id
-                            )
-                        )
-                """ % sqlvalues(
-                    parent_id, self.distroseries.id, parent_id))
+                """ % sqlvalues(parent_id, self.distroseries.id))
+            sql_filter = ("""
+                AND Packaging.sourcepackagename in (
+                SELECT
+                    Sourcepackagename.id
+                FROM
+                    Sourcepackagename
+                WHERE
+                    Sourcepackagename.name IN %s
+                )
+                """ % sqlvalues(spns))
+            sql_end = ("""
+                AND Packaging.sourcepackagename in (
+                SELECT Packaging.sourcepackagename
+                FROM Packaging
+                WHERE distroseries in (
+                    SELECT id
+                    FROM Distroseries
+                    WHERE id = %s
+                    )
+                EXCEPT
+                SELECT Packaging.sourcepackagename
+                FROM Packaging
+                WHERE distroseries in (
+                    SELECT id
+                    FROM Distroseries
+                    WHERE id = ChildSeries.id
+                    )
+                )
+                """ % sqlvalues(parent_id))
+            if spns is not None:
+                self._store.execute(sql + sql_filter + sql_end)
+            else:
+                self._store.execute(sql + sql_end)
 
     def _copy_packagesets(self):
         """Copy packagesets from the parent distroseries."""

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2014-07-23 01:37:37 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2014-07-25 00:24:39 +0000
@@ -172,6 +172,31 @@
             pocket=PackagePublishingPocket.RELEASE)
         return parent, parent_das, parent_das2, source
 
+    def setupPackagingTesting(self):
+        # Setup the environment for testing the packaging links
+        self.parent, self.parent_das = self.setupParent()
+        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.parent.owner,
+            distroseries=self.parent)
+        packages_test1 = ['udev', 'chromium', 'libc6']
+        packages_test2 = ['postgresql', 'vim']
+        for pkg in packages_test1:
+            test1.addSources(pkg)
+            sp = self.parent.getSourcePackage(pkg)
+            product_series = self.factory.makeProductSeries()
+            product_series.setPackaging(
+                self.parent, sp.sourcepackagename, self.parent.owner)
+        for pkg in packages_test2:
+            test2.addSources(pkg)
+            sp = self.parent.getSourcePackage(pkg)
+            product_series = self.factory.makeProductSeries()
+            product_series.setPackaging(
+                self.parent, sp.sourcepackagename, self.parent.owner)
+        return packages_test1, packages_test2
+
 
 class TestInitializeDistroSeries(InitializationHelperTestCase):
 
@@ -910,6 +935,47 @@
             len(packages_test1) + len(packages_test2))
         self.assertEqual(child.binarycount, 4)  # Chromium is FTBFS
 
+    def test_copy_packaging_links_some(self):
+        # Test that when copying some packagesets from the parent, only
+        # the packaging links for the copied packages are copied.
+        packages_test1, packages_test2 = self.setupPackagingTesting()
+        packageset1 = getUtility(IPackagesetSet).getByName(
+            self.parent, u'test1')
+        child = self._fullInitialize(
+            [self.parent], packagesets=(str(packageset1.id),))
+        packagings = child.getMostRecentlyLinkedPackagings()
+        names = [
+            packaging.sourcepackagename.name for packaging in packagings]
+        self.assertEqual(
+            0, child.getPrioritizedUnlinkedSourcePackages().count())
+        self.assertEqual(set(packages_test1), set(names))
+
+    def test_copy_packaging_links_empty(self):
+        # Test that when copying no packagesets from the parent, none of
+        # the packaging links for the packages are copied.
+        self.setupPackagingTesting()
+        child = self._fullInitialize(
+            [self.parent], packagesets=[])
+        packagings = child.getMostRecentlyLinkedPackagings()
+        names = [
+            packaging.sourcepackagename.name for packaging in packagings]
+        self.assertEqual(
+            0, child.getPrioritizedUnlinkedSourcePackages().count())
+        self.assertEqual([], names)
+
+    def test_copy_packaging_links_none(self):
+        # Test that when copying all packagesets from the parent, all of
+        # the packaging links are copied.
+        packages_test1, packages_test2 = self.setupPackagingTesting()
+        child = self._fullInitialize(
+            [self.parent], packagesets=None)
+        packagings = child.getMostRecentlyLinkedPackagings()
+        names = [
+            packaging.sourcepackagename.name for packaging in packagings]
+        self.assertEqual(
+            0, child.getPrioritizedUnlinkedSourcePackages().count())
+        self.assertEqual(set(packages_test1 + packages_test2), set(names))
+
     def test_rebuild_flag(self):
         # No binaries will get copied if we specify rebuild=True.
         self.parent, self.parent_das = self.setupParent()