launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/ftparchive-cleanup-old-indexes into lp:launchpad.

Commit message:
Remove old indexes before calling apt-ftparchive.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/ftparchive-cleanup-old-indexes/+merge/285428

Remove old indexes before calling apt-ftparchive.  This copes with the situation where we switch index_compressors to not contain bzip2 any more; without this we'd have outdated Packages.bz2 files left around on the filesystem.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/ftparchive-cleanup-old-indexes into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/model/ftparchive.py'
--- lib/lp/archivepublisher/model/ftparchive.py	2016-02-06 03:43:15 +0000
+++ lib/lp/archivepublisher/model/ftparchive.py	2016-02-09 01:39:00 +0000
@@ -50,9 +50,16 @@
     return (os.path.basename(filename).split("_"))[0]
 
 
-def safe_mkdir(path):
-    """Ensures the path exists, creating it if it doesn't."""
-    if not os.path.exists(path):
+def make_empty_dir(path):
+    """Ensure that the path exists and is an empty directory."""
+    if os.path.isdir(path):
+        for name in os.listdir(path):
+            child_path = os.path.join(path, name)
+            # Directories containing index files should never have
+            # subdirectories.  Guard against expensive mistakes.
+            assert os.path.isfile(child_path)
+            os.unlink(child_path)
+    else:
         os.makedirs(path, 0o755)
 
 
@@ -754,17 +761,20 @@
             distroseries.include_long_descriptions,
             distroseries.index_compressors)
 
-        # XXX: 2006-08-24 kiko: Why do we do this directory creation here?
+        # Make sure all the relevant directories exist and are empty.  Each
+        # of these only contains files generated by apt-ftparchive, and may
+        # contain files left over from previous configurations (e.g.
+        # different compressor types).
         for comp in comps:
             component_path = os.path.join(
                 self._config.distsroot, suite, comp)
-            safe_mkdir(os.path.join(component_path, "source"))
+            make_empty_dir(os.path.join(component_path, "source"))
             if not distroseries.include_long_descriptions:
-                safe_mkdir(os.path.join(component_path, "i18n"))
+                make_empty_dir(os.path.join(component_path, "i18n"))
             for arch in archs:
-                safe_mkdir(os.path.join(component_path, "binary-" + arch))
+                make_empty_dir(os.path.join(component_path, "binary-" + arch))
                 for subcomp in self.publisher.subcomponents:
-                    safe_mkdir(os.path.join(
+                    make_empty_dir(os.path.join(
                         component_path, subcomp, "binary-" + arch))
 
     def writeAptConfig(self, apt_config, suite, comps, archs,

=== modified file 'lib/lp/archivepublisher/tests/test_ftparchive.py'
--- lib/lp/archivepublisher/tests/test_ftparchive.py	2016-02-05 20:28:29 +0000
+++ lib/lp/archivepublisher/tests/test_ftparchive.py	2016-02-09 01:39:00 +0000
@@ -33,6 +33,7 @@
     )
 from lp.soyuz.enums import (
     BinaryPackageFormat,
+    IndexCompressionType,
     PackagePublishingPriority,
     PackagePublishingStatus,
     )
@@ -536,6 +537,63 @@
             os.path.join(self._distsdir, "hoary-test", "main",
                          "source", "Sources.gz")))
 
+    def test_generateConfig_index_compressors_changed(self):
+        # If index_compressors changes between runs, then old compressed
+        # files are removed.
+        publisher = Publisher(
+            self._logger, self._config, self._dp, self._archive)
+        fa = FTPArchiveHandler(
+            self._logger, self._config, self._dp, self._distribution,
+            publisher)
+        fa.createEmptyPocketRequests(fullpublish=True)
+        self._publishDefaultOverrides(fa, "main")
+        self._publishDefaultFileLists(fa, "main")
+        self._addRepositoryFile("main", "tiny", "tiny_0.1.dsc")
+        self._addRepositoryFile("main", "tiny", "tiny_0.1.tar.gz")
+        self._addRepositoryFile("main", "tiny", "tiny_0.1_i386.deb")
+        comp_dir = os.path.join(self._distsdir, "hoary-test", "main")
+        os.makedirs(os.path.join(comp_dir, "uefi"))
+        with open(os.path.join(comp_dir, "uefi", "stuff"), "w"):
+            pass
+
+        # Run the publisher once with gzip and bzip2 compressors.
+        apt_conf = fa.generateConfig(fullpublish=True)
+        with open(apt_conf) as apt_conf_file:
+            self.assertIn(
+                'Packages::Compress "gzip bzip2";', apt_conf_file.read())
+        fa.runApt(apt_conf)
+        self.assertContentEqual(
+            ["Packages.gz", "Packages.bz2"],
+            os.listdir(os.path.join(comp_dir, "binary-i386")))
+        self.assertContentEqual(
+            ["Packages.gz", "Packages.bz2"],
+            os.listdir(os.path.join(
+                comp_dir, "debian-installer", "binary-i386")))
+        self.assertContentEqual(
+            ["Sources.gz", "Sources.bz2"],
+            os.listdir(os.path.join(comp_dir, "source")))
+
+        # Try again, this time with gzip and xz compressors.  There are no
+        # bzip2 leftovers, but other files are left untouched.
+        self._distribution["hoary-test"].index_compressors = [
+            IndexCompressionType.GZIP, IndexCompressionType.XZ]
+        apt_conf = fa.generateConfig(fullpublish=True)
+        with open(apt_conf) as apt_conf_file:
+            self.assertIn(
+                'Packages::Compress "gzip xz";', apt_conf_file.read())
+        fa.runApt(apt_conf)
+        self.assertContentEqual(
+            ["Packages.gz", "Packages.xz"],
+            os.listdir(os.path.join(comp_dir, "binary-i386")))
+        self.assertContentEqual(
+            ["Packages.gz", "Packages.xz"],
+            os.listdir(os.path.join(
+                comp_dir, "debian-installer", "binary-i386")))
+        self.assertContentEqual(
+            ["Sources.gz", "Sources.xz"],
+            os.listdir(os.path.join(comp_dir, "source")))
+        self.assertEqual(["stuff"], os.listdir(os.path.join(comp_dir, "uefi")))
+
     def test_cleanCaches_noop_if_recent(self):
         # cleanCaches does nothing if it was run recently.
         fa = self._setUpFTPArchiveHandler()