Bug 1496995 - Account for all dictionaries when updating built_in_addons.json during l10n repack. r=nalexander
authorMike Hommey <mh+mozilla@glandium.org>
Fri, 18 Jan 2019 21:53:16 +0000
changeset 514478 f51c5057cc00
parent 514477 4145884732ca
child 514479 57dc8bbbc38f
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1496995
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1496995 - Account for all dictionaries when updating built_in_addons.json during l10n repack. r=nalexander All directories are part of the langpack that is being merged in, but when the langpack includes the english dictionary, it is not handled at the same time as other dictionaries, because it is also part of the original application. Instead of trying to catch all places where a dictionary might be added to the final repack, we wrap the formatter so that it tracks all of them wherever they're added from, and updates the built_in_addons.json file accordingly. Differential Revision: https://phabricator.services.mozilla.com/D16785
python/mozbuild/mozpack/files.py
python/mozbuild/mozpack/packager/l10n.py
python/mozbuild/mozpack/test/test_files.py
--- a/python/mozbuild/mozpack/files.py
+++ b/python/mozbuild/mozpack/files.py
@@ -1,15 +1,16 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from __future__ import absolute_import
 
 import errno
+import inspect
 import os
 import platform
 import shutil
 import stat
 import subprocess
 import uuid
 import mozbuild.makeutil as makeutil
 from itertools import chain
@@ -583,17 +584,27 @@ class PreprocessedFile(BaseFile):
         return True
 
 
 class GeneratedFile(BaseFile):
     '''
     File class for content with no previous existence on the filesystem.
     '''
     def __init__(self, content):
-        self.content = content
+        self._content = content
+
+    @property
+    def content(self):
+        if inspect.isfunction(self._content):
+            self._content = self._content()
+        return self._content
+
+    @content.setter
+    def content(self, content):
+        self._content = content
 
     def open(self):
         return BytesIO(self.content)
 
     def read(self):
         return self.content
 
     def size(self):
--- a/python/mozbuild/mozpack/packager/l10n.py
+++ b/python/mozbuild/mozpack/packager/l10n.py
@@ -89,16 +89,47 @@ class LocaleManifestFinder(object):
         sink.add(Component(''), '*')
         sink.close(False)
 
         # Find unique locales used in these manifest entries.
         self.locales = list(set(e.id for e in self.entries
                                 if isinstance(e, ManifestLocale)))
 
 
+class L10NRepackFormatterMixin(object):
+    def __init__(self, *args, **kwargs):
+        super(L10NRepackFormatterMixin, self).__init__(*args, **kwargs)
+        self._dictionaries = {}
+
+    def add(self, path, file):
+        if path.endswith('.dic'):
+            base, relpath = self._get_base(path)
+            if relpath.startswith('dictionaries/'):
+                root, ext = mozpath.splitext(mozpath.basename(path))
+                self._dictionaries[root] = path
+        elif path.endswith('/built_in_addons.json'):
+            data = json.load(file.open())
+            data['dictionaries'] = self._dictionaries
+            # The GeneratedFile content is only really generated after
+            # all calls to formatter.add.
+            file = GeneratedFile(lambda: json.dumps(data))
+        super(L10NRepackFormatterMixin, self).add(path, file)
+
+
+def L10NRepackFormatter(klass):
+    class L10NRepackFormatter(L10NRepackFormatterMixin, klass):
+        pass
+    return L10NRepackFormatter
+
+
+FlatFormatter = L10NRepackFormatter(FlatFormatter)
+JarFormatter = L10NRepackFormatter(JarFormatter)
+OmniJarFormatter = L10NRepackFormatter(OmniJarFormatter)
+
+
 def _repack(app_finder, l10n_finder, copier, formatter, non_chrome=set()):
     app = LocaleManifestFinder(app_finder)
     l10n = LocaleManifestFinder(l10n_finder)
 
     # The code further below assumes there's only one locale replaced with
     # another one.
     if len(app.locales) > 1:
         errors.fatal("Multiple app locales aren't supported: " +
@@ -157,17 +188,16 @@ def _repack(app_finder, l10n_finder, cop
             for p in right:
                 paths[p] = p
             for p in left - right:
                 paths[p] = None
 
     # Create a new package, with non localized bits coming from the original
     # package, and localized bits coming from the langpack.
     packager = SimplePackager(formatter)
-    built_in_addons = None
     for p, f in app_finder:
         if is_manifest(p):
             # Remove localized manifest entries.
             for e in [e for e in f if e.localized]:
                 f.remove(e)
         # If the path is one that needs a locale replacement, use the
         # corresponding file from the langpack.
         path = None
@@ -188,18 +218,16 @@ def _repack(app_finder, l10n_finder, cop
                 if base not in non_chrome:
                     finderBase = ""
                     if hasattr(l10n_finder, 'base'):
                         finderBase = l10n_finder.base
                     errors.error("Missing file: %s" %
                                  os.path.join(finderBase, path))
             else:
                 packager.add(path, files[0])
-        elif p.endswith('built_in_addons.json'):
-            built_in_addons = (p, f)
         else:
             packager.add(p, f)
 
     # Add localized manifest entries from the langpack.
     l10n_manifests = []
     for base in set(e.base for e in l10n.entries):
         m = ManifestFile(base, [e for e in l10n.entries if e.base == base])
         path = mozpath.join(base, 'chrome.%s.manifest' % l10n_locale)
@@ -210,35 +238,23 @@ def _repack(app_finder, l10n_finder, cop
         packager.add(path, m)
         # Add a "manifest $path" entry in the top manifest under that base.
         m = ManifestFile(base)
         m.add(Manifest(base, mozpath.relpath(path, base)))
         packager.add(mozpath.join(base, 'chrome.manifest'), m)
 
     packager.close()
 
-    dictionaries = {}
     # Add any remaining non chrome files.
     for pattern in non_chrome:
         for base in bases:
             for p, f in l10n_finder.find(mozpath.join(base, pattern)):
                 if not formatter.contains(p):
-                    if p.startswith('dictionaries/') and p.endswith('.dic'):
-                        base, ext = os.path.splitext(os.path.basename(p))
-                        dictionaries[base] = p
-
                     formatter.add(p, f)
 
-    # Update the built-in add-ons manifest with the new list of dictionaries
-    # from the langpack.
-    if built_in_addons:
-        data = json.load(built_in_addons[1].open())
-        data['dictionaries'] = dictionaries
-        formatter.add(built_in_addons[0], GeneratedFile(json.dumps(data)))
-
     # Resources in `localization` directories are packaged from the source and then
     # if localized versions are present in the l10n dir, we package them as well
     # keeping the source dir resources as a runtime fallback.
     for p, f in l10n_finder.find('**/localization'):
         if not formatter.contains(p):
             formatter.add(p, f)
 
     # Transplant jar preloading information.
--- a/python/mozbuild/mozpack/test/test_files.py
+++ b/python/mozbuild/mozpack/test/test_files.py
@@ -651,16 +651,51 @@ class TestGeneratedFile(TestWithTmpDir):
         f.copy(DestNoWrite(dest))
         self.assertEqual('test', open(dest, 'rb').read())
 
         # Double check that under conditions where a copy occurs, we would get
         # an exception.
         f = GeneratedFile('fooo')
         self.assertRaises(RuntimeError, f.copy, DestNoWrite(dest))
 
+    def test_generated_file_function(self):
+        '''
+        Test GeneratedFile behavior with functions.
+        '''
+        dest = self.tmppath('dest')
+        data = {
+            'num_calls': 0,
+        }
+
+        def content():
+            data['num_calls'] += 1
+            return 'content'
+
+        f = GeneratedFile(content)
+        self.assertEqual(data['num_calls'], 0)
+        f.copy(dest)
+        self.assertEqual(data['num_calls'], 1)
+        self.assertEqual('content', open(dest, 'rb').read())
+        self.assertEqual('content', f.open().read())
+        self.assertEqual('content', f.read())
+        self.assertEqual(len('content'), f.size())
+        self.assertEqual(data['num_calls'], 1)
+
+        f.content = 'modified'
+        f.copy(dest)
+        self.assertEqual(data['num_calls'], 1)
+        self.assertEqual('modified', open(dest, 'rb').read())
+        self.assertEqual('modified', f.open().read())
+        self.assertEqual('modified', f.read())
+        self.assertEqual(len('modified'), f.size())
+
+        f.content = content
+        self.assertEqual(data['num_calls'], 1)
+        self.assertEqual('content', f.read())
+        self.assertEqual(data['num_calls'], 2)
 
 class TestDeflatedFile(TestWithTmpDir):
     def test_deflated_file(self):
         '''
         Check that DeflatedFile.copy yields the proper content in the
         destination file in all situations that trigger different code paths
         (see TestFile.test_file)
         '''