launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/cp-bug-655614-disabled-arch-indices into lp:~launchpad-pqm/launchpad/production-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #655614 Don't publish indices for disabled arches
  https://bugs.launchpad.net/bugs/655614


CP of the branch that landed in db-devel already.  It prevents re-generated of the Packages and Release files for disabled architectures.

Urgently required before maverick releases.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/cp-bug-655614-disabled-arch-indices/+merge/37946
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/cp-bug-655614-disabled-arch-indices into lp:~launchpad-pqm/launchpad/production-devel.

=== modified file 'lib/lp/archivepublisher/config.py'
--- lib/lp/archivepublisher/config.py	2010-08-23 16:51:11 +0000
+++ lib/lp/archivepublisher/config.py	2010-10-08 10:40:43 +0000
@@ -117,8 +117,9 @@
                 }
 
             for dar in dr.architectures:
-                config_segment["archtags"].append(
-                    dar.architecturetag.encode('utf-8'))
+                if dar.enabled:
+                    config_segment["archtags"].append(
+                        dar.architecturetag.encode('utf-8'))
 
             if dr.lucilleconfig:
                 strio = StringIO(dr.lucilleconfig.encode('utf-8'))

=== modified file 'lib/lp/archivepublisher/ftparchive.py'
--- lib/lp/archivepublisher/ftparchive.py	2010-08-24 15:29:01 +0000
+++ lib/lp/archivepublisher/ftparchive.py	2010-10-08 10:40:43 +0000
@@ -699,6 +699,13 @@
             self.log.debug("Writing file lists for %s" % suite)
             for component, architectures in components.items():
                 for architecture, file_names in architectures.items():
+                    # XXX wgrant 2010-10-06: There must be a better place to
+                    # do this.
+                    series, pocket = (
+                        self.distro.getDistroSeriesAndPocket(suite))
+                    if (architecture != 'source' and
+                        not series.getDistroArchSeries(architecture[7:]).enabled):
+                        continue
                     self.writeFileList(architecture, file_names,
                                        suite, component)
 

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2010-08-24 15:29:01 +0000
+++ lib/lp/archivepublisher/publishing.py	2010-10-08 10:40:43 +0000
@@ -353,8 +353,16 @@
         source_index.close()
 
         for arch in distroseries.architectures:
+            if not arch.enabled:
+                continue
+
             arch_path = 'binary-%s' % arch.architecturetag
 
+            # XXX wgrant 2010-10-06 bug=655690: Using FTPArchiveHandler
+            # for NMAF is wrong.
+            self.apt_handler.requestReleaseFile(
+                suite_name, component.name, arch_path)
+
             self.log.debug("Generating Packages for %s" % arch_path)
 
             package_index_root = os.path.join(
@@ -386,15 +394,10 @@
             package_index.close()
             di_index.close()
 
-        # Inject static requests for Release files into self.apt_handler
-        # in a way which works for NoMoreAptFtpArchive without changing
-        # much of the rest of the code, specially D_writeReleaseFiles.
+        # XXX wgrant 2010-10-06 bug=655690: Using FTPArchiveHandler
+        # is wrong here too.
         self.apt_handler.requestReleaseFile(
             suite_name, component.name, 'source')
-        for arch in distroseries.architectures:
-            arch_name = "binary-" + arch.architecturetag
-            self.apt_handler.requestReleaseFile(
-                suite_name, component.name, arch_name)
 
     def cannotModifySuite(self, distroseries, pocket):
         """Return True if the distroseries is stable and pocket is release."""

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2010-08-31 11:11:09 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2010-10-08 10:40:43 +0000
@@ -22,7 +22,10 @@
 from canonical.launchpad.ftests.keys_for_tests import gpgkeysdir
 from canonical.launchpad.interfaces.gpghandler import IGPGHandler
 from canonical.zeca.ftests.harness import ZecaTestSetup
-from lp.archivepublisher.config import getPubConfig
+from lp.archivepublisher.config import (
+    Config,
+    getPubConfig,
+    )
 from lp.archivepublisher.diskpool import DiskPool
 from lp.archivepublisher.interfaces.archivesigningkey import (
     IArchiveSigningKey,
@@ -767,7 +770,7 @@
         self.checkDirtyPockets(publisher, expected=allowed_suites)
 
     def assertReleaseFileRequested(self, publisher, suite_name,
-                                   component_name, arch_name):
+                                   component_name, archs):
         """Assert the given context will have it's Release file regenerated.
 
         Check if a request for the given context is correctly stored in:
@@ -779,12 +782,10 @@
         self.assertTrue(
             component_name in suite,
             'Component %s/%s not requested' % (suite_name, component_name))
-        self.assertTrue(
-            arch_name in suite[component_name],
-            'Arch %s/%s/%s not requested' % (
-            suite_name, component_name, arch_name))
+        self.assertEquals(archs, suite[component_name])
 
-    def checkAllRequestedReleaseFiles(self, publisher):
+    def checkAllRequestedReleaseFiles(self, publisher,
+                                      architecturetags=('hppa', 'i386')):
         """Check if all expected Release files are going to be regenerated.
 
         'source', 'binary-i386' and 'binary-hppa' Release Files should be
@@ -795,19 +796,17 @@
         self.assertEqual(available_components,
                          ['main', 'multiverse', 'restricted', 'universe'])
 
-        available_archs = ['binary-%s' % a.architecturetag
-                           for a in self.breezy_autotest.architectures]
-        self.assertEqual(available_archs, ['binary-hppa', 'binary-i386'])
+        available_archs = ['binary-%s' % arch for arch in architecturetags]
 
         # XXX cprov 20071210: Include the artificial component 'source' as a
         # location to check for generated indexes. Ideally we should
         # encapsulate this task in publishing.py and this common method
         # in tests as well.
-        dists = ['source'] + available_archs
+        all_archs = set(available_archs)
+        all_archs.add('source')
         for component in available_components:
-            for dist in dists:
-                self.assertReleaseFileRequested(
-                    publisher, 'breezy-autotest', component, dist)
+            self.assertReleaseFileRequested(
+                publisher, 'breezy-autotest', component, all_archs)
 
     def _getReleaseFileOrigin(self, contents):
         origin_header = 'Origin: '
@@ -1082,6 +1081,73 @@
         # The Label: field should be set to the archive displayname
         self.assertEqual(release_contents[1], 'Label: Partner archive')
 
+    def assertIndicesForArchitectures(self, publisher, present, absent):
+        """Assert that the correct set of archs has indices.
+
+        Checks that the architecture tags in 'present' have Packages and
+        Release files and are in the series' Release file, and confirms
+        that those in 'absent' are not.
+        """
+
+        self.checkAllRequestedReleaseFiles(
+            publisher, architecturetags=present)
+
+        arch_template = os.path.join(
+            publisher._config.distsroot, 'breezy-autotest/main/binary-%s')
+        release_template = os.path.join(arch_template, 'Release')
+        packages_template = os.path.join(arch_template, 'Packages')
+        release_content = open(os.path.join(
+            publisher._config.distsroot,
+            'breezy-autotest/Release')).read()
+
+        for arch in present:
+            self.assertTrue(os.path.exists(arch_template % arch))
+            self.assertTrue(os.path.exists(release_template % arch))
+            self.assertTrue(os.path.exists(packages_template % arch))
+            self.assertTrue(arch in release_content)
+
+        for arch in absent:
+            self.assertFalse(os.path.exists(arch_template % arch))
+            self.assertFalse(arch in release_content)
+
+    def testNativeNoIndicesForDisabledArchitectures(self):
+        """Test that no indices are created for disabled archs."""
+        self.getPubBinaries()
+
+        ds = self.ubuntutest.getSeries('breezy-autotest')
+        ds.getDistroArchSeries('i386').enabled = False
+        self.config = Config(self.ubuntutest)
+
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool,
+            self.ubuntutest.main_archive)
+
+        publisher.A_publish(False)
+        publisher.C_writeIndexes(False)
+        publisher.D_writeReleaseFiles(False)
+
+        self.assertIndicesForArchitectures(
+            publisher, present=['hppa'], absent=['i386'])
+
+    def testAptFtparchiveNoIndicesForDisabledArchitectures(self):
+        """Test that no indices are created for disabled archs."""
+        self.getPubBinaries()
+
+        ds = self.ubuntutest.getSeries('breezy-autotest')
+        ds.getDistroArchSeries('i386').enabled = False
+        self.config = Config(self.ubuntutest)
+
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool,
+            self.ubuntutest.main_archive)
+
+        publisher.A_publish(False)
+        publisher.C_doFTPArchive(False)
+        publisher.D_writeReleaseFiles(False)
+
+        self.assertIndicesForArchitectures(
+            publisher, present=['hppa'], absent=['i386'])
+
     def testWorldAndGroupReadablePackagesAndSources(self):
         """Test Packages.gz and Sources.gz files are world and group readable.