launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-1565861 into lp:launchpad.

Commit message:
Fix apt-ftparchive integration to not delete non-English Translation-*.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1565861 in Launchpad itself: "xenial's dists/*/*/i18n only contains Translations-en"
  https://bugs.launchpad.net/launchpad/+bug/1565861

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1565861/+merge/290945

Fix apt-ftparchive integration to not delete non-English Translation-*.

r17923 changed FTPArchiveHandler.generateConfigForPocket to ensure that
the index directories were all empty -- including i18n. But i18n also
includes files extracted from DDTP custom uploads, so we can't delete
everything. Instead, just delete Translation-en and its compressed
variants.

NMAF is unaffected, as disabled compressed variants are deleted by
RepositoryIndexFile instead.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-1565861 into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/model/ftparchive.py'
--- lib/lp/archivepublisher/model/ftparchive.py	2016-03-17 14:47:14 +0000
+++ lib/lp/archivepublisher/model/ftparchive.py	2016-04-05 03:44:26 +0000
@@ -3,6 +3,7 @@
 
 from collections import defaultdict
 import os
+import re
 from StringIO import StringIO
 import time
 
@@ -50,11 +51,15 @@
     return (os.path.basename(filename).split("_"))[0]
 
 
-def make_empty_dir(path):
-    """Ensure that the path exists and is an empty directory."""
+def make_clean_dir(path, clean_pattern=".*"):
+    """Ensure that the path exists and is an empty directory.
+
+    :param clean_pattern: a regex of filenames to remove from the directory.
+        If omitted, all files are removed.
+    """
     if os.path.isdir(path):
         for name in os.listdir(path):
-            if name == "by-hash":
+            if name == "by-hash" or not re.match(clean_pattern, name):
                 # Ignore existing by-hash directories; they will be cleaned
                 # up to match the rest of the directory tree later.
                 continue
@@ -772,13 +777,18 @@
         for comp in comps:
             component_path = os.path.join(
                 self._config.distsroot, suite, comp)
-            make_empty_dir(os.path.join(component_path, "source"))
+            make_clean_dir(os.path.join(component_path, "source"))
             if not distroseries.include_long_descriptions:
-                make_empty_dir(os.path.join(component_path, "i18n"))
+                # apt-ftparchive only generates the English
+                # translations; DDTP custom uploads might have put other
+                # files here that we want to keep.
+                make_clean_dir(
+                    os.path.join(component_path, "i18n"),
+                    r'Translation-en(\..*)?$')
             for arch in archs:
-                make_empty_dir(os.path.join(component_path, "binary-" + arch))
+                make_clean_dir(os.path.join(component_path, "binary-" + arch))
                 for subcomp in self.publisher.subcomponents:
-                    make_empty_dir(os.path.join(
+                    make_clean_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-09 01:18:37 +0000
+++ lib/lp/archivepublisher/tests/test_ftparchive.py	2016-04-05 03:44:26 +0000
@@ -555,6 +555,12 @@
         os.makedirs(os.path.join(comp_dir, "uefi"))
         with open(os.path.join(comp_dir, "uefi", "stuff"), "w"):
             pass
+        os.makedirs(os.path.join(comp_dir, "i18n"))
+        for name in ("Translation-de", "Translation-de.gz", "Translation-en",
+                     "Translation-en.Z"):
+            with open(os.path.join(comp_dir, "i18n", name), "w"):
+                pass
+        self._distribution["hoary-test"].include_long_descriptions = False
 
         # Run the publisher once with gzip and bzip2 compressors.
         apt_conf = fa.generateConfig(fullpublish=True)
@@ -572,6 +578,10 @@
         self.assertContentEqual(
             ["Sources.gz", "Sources.bz2"],
             os.listdir(os.path.join(comp_dir, "source")))
+        self.assertContentEqual(
+            ["Translation-de", "Translation-de.gz", "Translation-en.gz",
+             "Translation-en.bz2"],
+            os.listdir(os.path.join(comp_dir, "i18n")))
 
         # Try again, this time with gzip and xz compressors.  There are no
         # bzip2 leftovers, but other files are left untouched.
@@ -593,6 +603,10 @@
             ["Sources.gz", "Sources.xz"],
             os.listdir(os.path.join(comp_dir, "source")))
         self.assertEqual(["stuff"], os.listdir(os.path.join(comp_dir, "uefi")))
+        self.assertContentEqual(
+            ["Translation-de", "Translation-de.gz", "Translation-en.gz",
+             "Translation-en.xz"],
+            os.listdir(os.path.join(comp_dir, "i18n")))
 
     def test_cleanCaches_noop_if_recent(self):
         # cleanCaches does nothing if it was run recently.