Merge lp:~wgrant/launchpad/bug-655614-disabled-arch-indices into lp:launchpad/db-devel

[William Grant]

Don't publish indices for disabled architectures.

Both the publication methods (NMAF and a-f) still publish indices for disabled architectures every time the pocket is dirtied. This is pointless pollution, as these indices will be empty.

This branch fixes both publishers to not publish any indices for disabled archs, and also to exclude them from the series Release file. It adds tests to confirm that neither publisher writes disabled indices, or even creates directories for them.

The revolting hack in lib/lp/archivepublisher/ftparchive.py is largely stolen from another part of the publisher, and can't be avoided without a substantial refactor.

[Julian Edwards (julian-edwards)]

review: Approve

[Julian Edwards (julian-edwards)]

I am testing and landing. If it works on on dogfood I'll cherry pick too, thanks for writing this William.

=== 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-07 14:15:57 +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-07 14:15:57 +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-07 14:15:57 +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-07 14:15:57 +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.