Merge lp:~julian-edwards/launchpad/copy-archive-fix-component into lp:launchpad

[Julian Edwards]

= Summary =
Creation of COPY (rebuild) archives doesn't respect the specified component.

This fix is required for an urgent cherry pick as the Linaro guys need a
rebuild for ARM set up.

== Pre-implementation notes ==
Chatted to Muharem who identified the culprit code which is an SQL query that
ignores the component specified for the copy operation.

== Implementation details ==
Add the component to the query if it's been specified. Also fix a bunch of
tests that relied on the broken behaviour.

== Tests ==
bin/test -cvv test_populatearchive

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/model/packagecloner.py
  lib/lp/soyuz/scripts/tests/test_populatearchive.py

[Jeroen T. Vermeulen (jtv)]

Looks good.

review: Approve

[Francis J. Lacoste (flacoste)]

review: Approve (release-critical)

[James Westby (james-w)]

Hi Julian,

You should fix the docstring of the test that asserts the behaviour
that we don't want.

57 When copying you specify a component, but that component doesn't
58 limit the packages copied. We create a source in main and one in
59 universe, and then copy with --component main, and expect to see
60 both sources in the copy.

Thanks,

James

=== modified file 'lib/lp/soyuz/model/packagecloner.py'
--- lib/lp/soyuz/model/packagecloner.py	2010-01-12 08:36:58 +0000
+++ lib/lp/soyuz/model/packagecloner.py	2010-06-30 15:09:31 +0000
@@ -305,7 +305,7 @@
             to be copied.
         """
         store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-        store.execute('''
+        query = '''
             INSERT INTO SourcePackagePublishingHistory (
                 sourcepackagerelease, distroseries, status, component,
                 section, archive, datecreated, datepublished, pocket)
@@ -321,7 +321,12 @@
                 UTC_NOW, destination.pocket, origin.distroseries,
                 PackagePublishingStatus.PENDING,
                 PackagePublishingStatus.PUBLISHED,
-                origin.pocket, origin.archive))
+                origin.pocket, origin.archive)
+
+        if origin.component:
+            query += "and spph.component = %s" % sqlvalues(origin.component)
+
+        store.execute(query)
 
     def packageSetDiff(self, origin, destination, logger=None):
         """Please see `IPackageCloner`."""

=== modified file 'lib/lp/soyuz/scripts/tests/test_populatearchive.py'
--- lib/lp/soyuz/scripts/tests/test_populatearchive.py	2010-06-22 11:14:02 +0000
+++ lib/lp/soyuz/scripts/tests/test_populatearchive.py	2010-06-30 15:09:31 +0000
@@ -119,7 +119,6 @@
             '--to-distribution', distro_name, '--to-suite', 'hoary',
             '--to-archive', archive_name, '--to-user', 'salgado', '--reason',
             '"copy archive from %s"' % datetime.ctime(datetime.utcnow()),
-            '--component', 'main'
             ]
 
         # Start archive population now!
@@ -206,7 +205,7 @@
     def getScript(self, test_args=None):
         """Return an ArchivePopulator instance."""
         if test_args is None:
-           test_args = []
+            test_args = []
         script = ArchivePopulator("test copy archives", test_args=test_args)
         script.logger = QuietFakeLogger()
         script.txn = self.layer.txn
@@ -502,24 +501,24 @@
             architectures=["386", "amd64"])
         self.checkBuilds(copy_archive, [package_info, package_info])
 
-    def testCopyArchiveCopiesAllComponents(self):
-        """Test that packages from all components are copied.
+    def testCopyArchiveCopiesRightComponents(self):
+        """Test that packages from the right components are copied.
 
-        When copying you specify a component, but that component doesn't
+        When copying you specify a component, that component should
         limit the packages copied. We create a source in main and one in
         universe, and then copy with --component main, and expect to see
-        both sources in the copy.
+        only main in the copy.
         """
-        package_infos = [
-            PackageInfo(
+        package_info_universe = PackageInfo(
                 "bzr", "2.1", status=PackagePublishingStatus.PUBLISHED,
-                component="universe"),
-            PackageInfo(
+                component="universe")
+        package_info_main = PackageInfo(
                 "apt", "2.2", status=PackagePublishingStatus.PUBLISHED,
-                component="main")]
-        copy_archive, distroseries = self.makeCopyArchive(package_infos,
-            component="main")
-        self.checkBuilds(copy_archive, package_infos)
+                component="main")
+        package_infos_both = [package_info_universe, package_info_main]
+        copy_archive, distroseries = self.makeCopyArchive(
+            package_infos_both, component="main")
+        self.checkBuilds(copy_archive, [package_info_main])
 
     def testCopyFromPPA(self):
         """Test we can create a copy archive with a PPA as the source."""
@@ -545,7 +544,7 @@
         archive_name = self.getTargetArchiveName(distroseries.distribution)
         copy_archive = self.copyArchive(
             distroseries, archive_name, owner, from_user=ppa_owner_name,
-            from_archive=ppa_name)
+            from_archive=ppa_name, component=package_info.component)
         self.checkCopiedSources(
             copy_archive, distroseries, [package_info])