Merge ~cjwatson/launchpad:fix-ftparchive-avoid-empty-overrides into launchpad:master

[Colin Watson]

Generate empty override and file list files where necessary

Commit 25ce0c1d87 stopped `FTPArchive.createEmptyPocketRequests` from writing empty override and file list files for all suites/components, instead merely making sure that they exist. However, this broke in the case where a suite/component (or suite/component/architecture) was non-empty and then became empty, because the previous non-empty overrides and file lists were left in place.

To avoid this, ensure that `FTPArchive.publishOverrides` writes override files for all relevant components, and that `FTPArchive.publishFileLists` writes file list files for all relevant components/architectures; these may be empty if there are no publications in the corresponding contexts. This doesn't suffer from the problem that the aforementioned commit was fixing, because these files are put in place atomically.

It's possible that this comes close to making `FTPArchive.createEmptyPocketRequests` entirely unnecessary, but I haven't yet tried to remove it.

[Tom Wardill (twom)]

review: Approve

diff --git a/lib/lp/archivepublisher/model/ftparchive.py b/lib/lp/archivepublisher/model/ftparchive.py
index a13269e..624aebf 100644
--- a/lib/lp/archivepublisher/model/ftparchive.py
+++ b/lib/lp/archivepublisher/model/ftparchive.py
@@ -382,10 +382,9 @@ class FTPArchiveHandler:
 
                 spphs = self.getSourcesForOverrides(distroseries, pocket)
                 bpphs = self.getBinariesForOverrides(distroseries, pocket)
-                self.publishOverrides(
-                    distroseries.getSuite(pocket), spphs, bpphs)
+                self.publishOverrides(distroseries, pocket, spphs, bpphs)
 
-    def publishOverrides(self, suite,
+    def publishOverrides(self, distroseries, pocket,
                          source_publications, binary_publications):
         """Output a set of override files for use in apt-ftparchive.
 
@@ -408,8 +407,15 @@ class FTPArchiveHandler:
         # test_ftparchive.
         from lp.archivepublisher.publishing import FORMAT_TO_SUBCOMPONENT
 
+        suite = distroseries.getSuite(pocket)
+
         # overrides[component][src/bin] = sets of tuples
         overrides = defaultdict(lambda: defaultdict(set))
+        # Ensure that we generate overrides for all the expected components,
+        # even if they're currently empty.
+        for component in self.publisher.archive.getComponentsForSeries(
+                distroseries):
+            overrides[component.name]
 
         def updateOverride(packagename, component, section, archtag=None,
                            priority=None, binpackageformat=None,
@@ -666,16 +672,25 @@ class FTPArchiveHandler:
                         continue
                 spps = self.getSourceFiles(distroseries, pocket)
                 pps = self.getBinaryFiles(distroseries, pocket)
-                self.publishFileLists(distroseries.getSuite(pocket), spps, pps)
+                self.publishFileLists(distroseries, pocket, spps, pps)
 
-    def publishFileLists(self, suite, sourcefiles, binaryfiles):
+    def publishFileLists(self, distroseries, pocket, sourcefiles, binaryfiles):
         """Collate the set of source files and binary files provided and
         write out all the file list files for them.
 
         listroot/distroseries_component_source
         listroot/distroseries_component_binary-archname
         """
+        suite = distroseries.getSuite(pocket)
+
         filelist = defaultdict(lambda: defaultdict(list))
+        # Ensure that we generate file lists for all the expected components
+        # and architectures, even if they're currently empty.
+        for component in self.publisher.archive.getComponentsForSeries(
+                distroseries):
+            filelist[component.name]["source"]
+            for das in distroseries.enabled_architectures:
+                filelist[component.name]["binary-%s" % das.architecturetag]
 
         def updateFileList(sourcepackagename, filename, component,
                            architecturetag=None):
@@ -698,7 +713,6 @@ class FTPArchiveHandler:
             updateFileList(*file_details)
 
         self.log.debug("Writing file lists for %s" % suite)
-        series, pocket = self.distro.getDistroSeriesAndPocket(suite)
         for component, architectures in six.iteritems(filelist):
             for architecture, file_names in six.iteritems(architectures):
                 # XXX wgrant 2010-10-06: There must be a better place to do
@@ -708,7 +722,7 @@ class FTPArchiveHandler:
                 else:
                     # The "[7:]" strips the "binary-" prefix off the
                     # architecture names we get here.
-                    das = series.getDistroArchSeries(architecture[7:])
+                    das = distroseries.getDistroArchSeries(architecture[7:])
                     enabled = das.enabled
                 if enabled:
                     self.writeFileList(
@@ -740,7 +754,8 @@ class FTPArchiveHandler:
             with open(new_path, 'w') as f:
                 files[subcomp].sort(key=package_name)
                 f.write("\n".join(files[subcomp]))
-                f.write("\n")
+                if files[subcomp]:
+                    f.write("\n")
             os.rename(new_path, final_path)
 
     #
diff --git a/lib/lp/archivepublisher/tests/test_ftparchive.py b/lib/lp/archivepublisher/tests/test_ftparchive.py
index 3a9665a..94b68fc 100755
--- a/lib/lp/archivepublisher/tests/test_ftparchive.py
+++ b/lib/lp/archivepublisher/tests/test_ftparchive.py
@@ -186,13 +186,17 @@ class TestFTPArchive(TestCaseWithFactory):
             'tiny', component, section, 'i386',
             PackagePublishingPriority.EXTRA, binpackageformat,
             phased_update_percentage)])
-        fa.publishOverrides('hoary-test', source_overrides, binary_overrides)
+        fa.publishOverrides(
+            self._distribution['hoary-test'], PackagePublishingPocket.RELEASE,
+            source_overrides, binary_overrides)
 
     def _publishDefaultFileLists(self, fa, component):
         source_files = FakeSelectResult([('tiny', 'tiny_0.1.dsc', component)])
         binary_files = FakeSelectResult(
             [('tiny', 'tiny_0.1_i386.deb', component, 'binary-i386')])
-        fa.publishFileLists('hoary-test', source_files, binary_files)
+        fa.publishFileLists(
+            self._distribution['hoary-test'], PackagePublishingPocket.RELEASE,
+            source_files, binary_files)
 
     def test_createEmptyPocketRequests_preserves_existing(self):
         # createEmptyPocketRequests leaves existing override and file list
@@ -362,6 +366,29 @@ class TestFTPArchive(TestCaseWithFactory):
             self.assertEqual(
                 ["tiny\textra\tdevel"], result_file.read().splitlines())
 
+    def test_publishOverrides_empties_missing_components(self):
+        # publishOverrides writes empty overrides files for components that
+        # have no publications.
+        fa = self._setUpFTPArchiveHandler()
+        empty_overrides = []
+        for component in ("restricted", "universe", "multiverse"):
+            empty_overrides.extend([
+                "override.hoary-test.%s" % component,
+                "override.hoary-test.%s.src" % component,
+                "override.hoary-test.extra.%s" % component,
+                ])
+        for override in empty_overrides:
+            write_file(
+                os.path.join(self._overdir, override), b"previous contents\n")
+
+        self._publishDefaultOverrides(fa, "main")
+
+        self._verifyFile("override.hoary-test.main", self._overdir)
+        self._verifyFile("override.hoary-test.main.src", self._overdir)
+        self._verifyFile("override.hoary-test.extra.main", self._overdir)
+        for override in empty_overrides:
+            self._verifyEmpty(os.path.join(self._overdir, override))
+
     def test_generateOverrides(self):
         # generateOverrides generates all the overrides from start to finish.
         self._distribution = getUtility(IDistributionSet).getByName('ubuntu')
@@ -459,6 +486,27 @@ class TestFTPArchive(TestCaseWithFactory):
         self._verifyFile("hoary-test_main_source", self._listdir)
         self._verifyFile("hoary-test_main_binary-i386", self._listdir)
 
+    def test_publishFileLists_empties_missing_components(self):
+        # publishFileLists writes empty file list files for components that
+        # have no publications.
+        fa = self._setUpFTPArchiveHandler()
+        empty_filelists = []
+        for component in ("restricted", "universe", "multiverse"):
+            empty_filelists.extend([
+                "hoary-test_%s_source" % component,
+                "hoary-test_%s_binary-i386" % component,
+                ])
+        for filelist in empty_filelists:
+            write_file(
+                os.path.join(self._listdir, filelist), b"previous contents\n")
+
+        self._publishDefaultFileLists(fa, "main")
+
+        self._verifyFile("hoary-test_main_source", self._listdir)
+        self._verifyFile("hoary-test_main_binary-i386", self._listdir)
+        for filelist in empty_filelists:
+            self._verifyEmpty(os.path.join(self._listdir, filelist))
+
     def test_generateConfig(self):
         # Generate apt-ftparchive configuration file and run it.
 
@@ -727,12 +775,16 @@ class TestFTPArchive(TestCaseWithFactory):
             "bin%d" % i, "main", "misc", "i386",
             PackagePublishingPriority.EXTRA, BinaryPackageFormat.DEB, None)
             for i in range(50)])
-        fa.publishOverrides("hoary-test", source_overrides, binary_overrides)
+        fa.publishOverrides(
+            self._distribution["hoary-test"], PackagePublishingPocket.RELEASE,
+            source_overrides, binary_overrides)
         source_files = FakeSelectResult([("tiny", "tiny_0.1.dsc", "main")])
         binary_files = FakeSelectResult([(
             "bin%d" % i, "bin%d_1_i386.deb" % i, "main", "binary-i386")
             for i in range(50)])
-        fa.publishFileLists("hoary-test", source_files, binary_files)
+        fa.publishFileLists(
+            self._distribution["hoary-test"], PackagePublishingPocket.RELEASE,
+            source_files, binary_files)
         self._addRepositoryFile("main", "tiny", "tiny_0.1.dsc")
         for i in range(50):
             self._addRepositoryFile(