Merge ~cjwatson/launchpad:fix-populate-archive-component into launchpad:master

[Colin Watson]

populate-archive: Fix --component option

https://git.launchpad.net/launchpad/commit?id=4c62ec226f broke the `--component` option to `populate-archive.py`, because the old-style `quote` and `sqlvalues` functions no longer work to escape instances of models that use native Storm rather than SQLObject emulation.

We should probably convert all of `PackageCloner` to use the Storm query compiler at some point; but this makes a few more targeted changes to get things working again in the meantime.

[Jürgen Gmach (jugmac00)]

review: Approve

diff --git a/lib/lp/soyuz/model/packagecloner.py b/lib/lp/soyuz/model/packagecloner.py
index 8092672..2a87c1d 100644
--- a/lib/lp/soyuz/model/packagecloner.py
+++ b/lib/lp/soyuz/model/packagecloner.py
@@ -13,7 +13,7 @@ from zope.interface import implementer
 
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import quote, sqlvalues
+from lp.services.database.sqlbase import sqlvalues
 from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.interfaces.packagecloner import IPackageCloner
 from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
@@ -299,8 +299,8 @@ class PackageCloner:
         )
 
         if origin.component is not None:
-            find_newer_packages += " AND secsrc.component = %s" % quote(
-                origin.component
+            find_newer_packages += (
+                " AND secsrc.component = %s" % origin.component.id
             )
         store.execute(find_newer_packages)
 
@@ -342,8 +342,8 @@ class PackageCloner:
         )
 
         if origin.component is not None:
-            find_origin_only_packages += " AND secsrc.component = %s" % quote(
-                origin.component
+            find_origin_only_packages += (
+                " AND secsrc.component = %s" % origin.component.id
             )
         store.execute(find_origin_only_packages)
 
@@ -422,8 +422,8 @@ class PackageCloner:
         )
 
         if destination.component is not None:
-            pop_query += " AND secsrc.component = %s" % quote(
-                destination.component
+            pop_query += (
+                " AND secsrc.component = %s" % destination.component.id
             )
         store.execute(pop_query)
 
@@ -504,7 +504,7 @@ class PackageCloner:
             )
 
         if origin.component:
-            query += "and spph.component = %s" % sqlvalues(origin.component)
+            query += "AND spph.component = %s" % origin.component.id
 
         store.execute(query)
 
diff --git a/lib/lp/soyuz/tests/test_packagecloner.py b/lib/lp/soyuz/tests/test_packagecloner.py
index 5f4c6e1..12a2641 100644
--- a/lib/lp/soyuz/tests/test_packagecloner.py
+++ b/lib/lp/soyuz/tests/test_packagecloner.py
@@ -125,7 +125,8 @@ class PackageClonerTests(TestCaseWithFactory):
     def makeCopyArchive(
         self,
         package_infos,
-        component="main",
+        from_component=None,
+        to_component="main",
         source_pocket=None,
         target_pocket=None,
         processors=None,
@@ -135,11 +136,14 @@ class PackageClonerTests(TestCaseWithFactory):
         copy_archive = self.getTargetArchive(
             distroseries.distribution, processors=processors
         )
-        to_component = getUtility(IComponentSet).ensure(component)
+        if from_component is not None:
+            from_component = getUtility(IComponentSet).ensure(from_component)
+        to_component = getUtility(IComponentSet).ensure(to_component)
         self.copyArchive(
             copy_archive,
             distroseries,
             from_pocket=source_pocket,
+            from_component=from_component,
             to_pocket=target_pocket,
             to_component=to_component,
             processors=processors,
@@ -174,6 +178,7 @@ class PackageClonerTests(TestCaseWithFactory):
         from_archive=None,
         from_distroseries=None,
         from_pocket=None,
+        from_component=None,
         to_pocket=None,
         to_component=None,
         packagesets=None,
@@ -195,16 +200,16 @@ class PackageClonerTests(TestCaseWithFactory):
             from_distroseries.distribution,
             from_distroseries,
             from_pocket,
+            component=from_component,
+            packagesets=packagesets,
         )
         destination = PackageLocation(
             to_archive,
             to_distroseries.distribution,
             to_distroseries,
             to_pocket,
+            component=to_component,
         )
-        origin.packagesets = packagesets
-        if to_component is not None:
-            destination.component = to_component
         cloner = getUtility(IPackageCloner)
         cloner.clonePackages(
             origin,
@@ -277,10 +282,31 @@ class PackageClonerTests(TestCaseWithFactory):
             ),
         ]
         copy_archive, distroseries = self.makeCopyArchive(
-            package_infos, component="main"
+            package_infos, to_component="main"
         )
         self.checkCopiedSources(copy_archive, distroseries, package_infos)
 
+    def testCopySingleComponent(self):
+        """Setting the origin's component limits the sources copied."""
+        package_infos = [
+            PackageInfo(
+                "bzr",
+                "2.1",
+                status=PackagePublishingStatus.PUBLISHED,
+                component="universe",
+            ),
+            PackageInfo(
+                "apt",
+                "2.2",
+                status=PackagePublishingStatus.PUBLISHED,
+                component="main",
+            ),
+        ]
+        copy_archive, distroseries = self.makeCopyArchive(
+            package_infos, from_component="main", to_component="main"
+        )
+        self.checkCopiedSources(copy_archive, distroseries, [package_infos[1]])
+
     def testSubsetsBasedOnPackageset(self):
         """Test that --package-set limits the sources copied."""
         package_infos = [
@@ -478,25 +504,37 @@ class PackageClonerTests(TestCaseWithFactory):
         self,
         target_archive,
         target_distroseries,
+        target_component=None,
         source_archive=None,
         source_distroseries=None,
+        source_component=None,
     ):
         """Run a packageSetDiff of two archives."""
         if source_distroseries is None:
             source_distroseries = target_distroseries
         if source_archive is None:
             source_archive = source_distroseries.distribution.main_archive
+        if source_component is not None:
+            source_component = getUtility(IComponentSet).ensure(
+                source_component
+            )
         source_location = PackageLocation(
             source_archive,
             source_distroseries.distribution,
             source_distroseries,
             PackagePublishingPocket.RELEASE,
+            component=source_component,
         )
+        if target_component is not None:
+            target_component = getUtility(IComponentSet).ensure(
+                target_component
+            )
         target_location = PackageLocation(
             target_archive,
             target_distroseries.distribution,
             target_distroseries,
             PackagePublishingPocket.RELEASE,
+            component=target_component,
         )
         cloner = getUtility(IPackageCloner)
         return cloner.packageSetDiff(source_location, target_location)
@@ -612,6 +650,50 @@ class PackageClonerTests(TestCaseWithFactory):
             distroseries.distribution.main_archive,
         )
 
+    def testPackageSetDiffInComponent(self):
+        old_package_infos = [
+            PackageInfo(
+                "bzr",
+                "2.1",
+                status=PackagePublishingStatus.PUBLISHED,
+                component="universe",
+            ),
+            PackageInfo(
+                "apt",
+                "1.2",
+                status=PackagePublishingStatus.PUBLISHED,
+                component="main",
+            ),
+        ]
+        copy_archive, distroseries = self.makeCopyArchive(old_package_infos)
+        new_package_infos = [
+            PackageInfo(
+                "bzr",
+                "2.2",
+                status=PackagePublishingStatus.PUBLISHED,
+                component="universe",
+            ),
+            PackageInfo(
+                "apt",
+                "1.3",
+                status=PackagePublishingStatus.PENDING,
+                component="main",
+            ),
+        ]
+        self.createSourcePublications(new_package_infos, distroseries)
+        diff = self.diffArchives(
+            copy_archive,
+            distroseries,
+            target_component="main",
+            source_component="main",
+        )
+        self.checkPackageDiff(
+            [new_package_infos[1]],
+            [],
+            diff,
+            distroseries.distribution.main_archive,
+        )
+
     def mergeCopy(
         self,
         target_archive,