Backed out 4 changesets (bug 1147207) for test_packager_l10n.py failures.
authorRyan VanderMeulen <ryanvm@gmail.com>
Mon, 06 Apr 2015 16:17:23 -0400
changeset 254855 3d0d39c0b228883ee92828473601afb0c6976a8b
parent 254854 e6c47797b6d3376a3fcbb436f210eb7ceaa19c44
child 254856 28e3d59641954fad71af50399ec34994958ff389
push id7885
push userryanvm@gmail.com
push dateMon, 06 Apr 2015 20:23:06 +0000
treeherdermozilla-aurora@28e3d5964195 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1147207
milestone39.0a2
backs outc2980295ee937d2f23fced5eba3e3aea7ed2b566
ad8dd51f2958ac6613890770d2d24f8b268ab8ed
0f02755eabf5a9fe95890fc18e8e651fe697bf9d
095141488e121e137332d8c90e5a1e1a2d17bdb6
Backed out 4 changesets (bug 1147207) for test_packager_l10n.py failures. Backed out changeset c2980295ee93 (bug 1147207) Backed out changeset ad8dd51f2958 (bug 1147207) Backed out changeset 0f02755eabf5 (bug 1147207) Backed out changeset 095141488e12 (bug 1147207)
python/mozbuild/mozpack/files.py
python/mozbuild/mozpack/packager/__init__.py
python/mozbuild/mozpack/packager/l10n.py
python/mozbuild/mozpack/test/test_files.py
python/mozbuild/mozpack/test/test_packager.py
python/mozbuild/mozpack/test/test_packager_l10n.py
toolkit/mozapps/installer/l10n-repack.py
--- a/python/mozbuild/mozpack/files.py
+++ b/python/mozbuild/mozpack/files.py
@@ -880,35 +880,8 @@ class JarFinder(BaseFinder):
             for p in self._files:
                 yield p, DeflatedFile(self._files[p])
         elif pattern in self._files:
             yield pattern, DeflatedFile(self._files[pattern])
         else:
             for p in self._files:
                 if mozpath.basedir(p, [pattern]) == pattern:
                     yield p, DeflatedFile(self._files[p])
-
-
-class ComposedFinder(BaseFinder):
-    '''
-    Composes multiple File Finders in some sort of virtual file system.
-
-    A ComposedFinder is initialized from a dictionary associating paths to
-    *Finder instances.
-
-    Note this could be optimized to be smarter than getting all the files
-    in advance.
-    '''
-    def __init__(self, finders):
-        # Can't import globally, because of the dependency of mozpack.copier
-        # on this module.
-        from mozpack.copier import FileRegistry
-        self.files = FileRegistry()
-
-        for base, finder in sorted(finders.iteritems()):
-            if self.files.contains(base):
-                self.files.remove(base)
-            for p, f in finder.find(''):
-                self.files.add(mozpath.join(base, p), f)
-
-    def find(self, pattern):
-        for p in self.files.match(pattern):
-            yield p, self.files[p]
--- a/python/mozbuild/mozpack/packager/__init__.py
+++ b/python/mozbuild/mozpack/packager/__init__.py
@@ -234,17 +234,17 @@ class SimplePackager(object):
         self._chrome_queue = CallDeque()
         # Queue for formatter.add() calls.
         self._file_queue = CallDeque()
         # All paths containing addons.
         self._addons = set()
         # All manifest paths imported.
         self._manifests = set()
         # All manifest paths included from some other manifest.
-        self._included_manifests = {}
+        self._included_manifests = set()
         self._closed = False
 
     def add(self, path, file):
         '''
         Add the given BaseFile instance with the given path.
         '''
         assert not self._closed
         if is_manifest(path):
@@ -275,50 +275,43 @@ class SimplePackager(object):
                 self._chrome_queue.append(self.formatter.add_manifest,
                                           e.move(e.base))
             elif not isinstance(e, (Manifest, ManifestInterfaces)):
                 self._queue.append(self.formatter.add_manifest, e.move(e.base))
             if isinstance(e, Manifest):
                 if e.flags:
                     errors.fatal('Flags are not supported on ' +
                                  '"manifest" entries')
-                self._included_manifests[e.path] = path
+                self._included_manifests.add(e.path)
 
     def get_bases(self, addons=True):
         '''
         Return all paths under which root manifests have been found. Root
         manifests are manifests that are included in no other manifest.
         `addons` indicates whether to include addon bases as well.
         '''
         all_bases = set(mozpath.dirname(m)
-                        for m in self._manifests
-                                 - set(self._included_manifests))
+                        for m in self._manifests - self._included_manifests)
         if not addons:
             all_bases -= self._addons
-        else:
-            # If for some reason some detected addon doesn't have a
-            # non-included manifest.
-            all_bases |= self._addons
         return all_bases
 
     def close(self):
         '''
         Push all instructions to the formatter.
         '''
         self._closed = True
-
-        bases = self.get_bases()
-        broken_bases = sorted(
-            m for m, includer in self._included_manifests.iteritems()
-            if mozpath.basedir(m, bases) != mozpath.basedir(includer, bases))
-        for m in broken_bases:
-            errors.fatal('"%s" is included from "%s", which is outside "%s"' %
-                         (m, self._included_manifests[m],
-                          mozpath.basedir(m, bases)))
-        for base in bases:
+        broken_addons = sorted(m for m in self._included_manifests
+                               if mozpath.dirname(m) in self._addons)
+        if broken_addons:
+            errors.fatal(
+                'Addon base manifest (%s) is included in some other manifest' %
+                ', '.join(broken_addons)
+            )
+        for base in self.get_bases():
             if base:
                 self.formatter.add_base(base, base in self._addons)
         self._chrome_queue.execute()
         self._queue.execute()
         self._file_queue.execute()
 
 
 class SimpleManifestSink(object):
--- a/python/mozbuild/mozpack/packager/l10n.py
+++ b/python/mozbuild/mozpack/packager/l10n.py
@@ -10,20 +10,17 @@ directory.
 import os
 import mozpack.path as mozpath
 from mozpack.packager.formats import (
     FlatFormatter,
     JarFormatter,
     OmniJarFormatter,
 )
 from mozpack.packager import SimplePackager
-from mozpack.files import (
-    ComposedFinder,
-    ManifestFile,
-)
+from mozpack.files import ManifestFile
 from mozpack.copier import (
     FileCopier,
     Jarrer,
 )
 from mozpack.chrome.manifest import (
     ManifestLocale,
     ManifestEntryWithRelPath,
     is_manifest,
@@ -32,62 +29,32 @@ from mozpack.chrome.manifest import (
 )
 from mozpack.errors import errors
 from mozpack.packager.unpack import UnpackFinder
 from createprecomplete import generate_precomplete
 
 
 class LocaleManifestFinder(object):
     def __init__(self, finder):
-        entries = self.entries = []
-        bases = self.bases = ['']
-
-        class MockFormatter(object):
-            def add_interfaces(self, path, content):
-                pass
-
-            def add(self, path, content):
-                pass
-
-            def add_manifest(self, entry):
-                if entry.localized:
-                    entries.append(entry)
-
-            def add_base(self, base, addon=False):
-                bases.append(base)
-
-        # SimplePackager rejects "manifest foo.manifest" entries with
-        # additional flags (such as "manifest foo.manifest application=bar").
-        # Those type of entries are used by language packs to work as addons,
-        # but are not necessary for the purpose of l10n repacking. So we wrap
-        # the finder in order to remove those entries.
-        class WrapFinder(object):
-            def __init__(self, finder):
-                self._finder = finder
-
-            def find(self, pattern):
-                for p, f in self._finder.find(pattern):
-                    if isinstance(f, ManifestFile):
-                        unwanted = [
-                            e for e in f._entries
-                            if isinstance(e, Manifest) and e.flags
-                        ]
-                        if unwanted:
-                            f = ManifestFile(
-                                f._base,
-                                [e for e in f._entries if e not in unwanted])
-                    yield p, f
-
-        sink = SimpleManifestSink(WrapFinder(finder), MockFormatter())
-        sink.add(Component(''), '*')
-        sink.close(False)
-
+        # Read all manifest entries
+        manifests = dict((p, m) for p, m in finder.find('**/*.manifest')
+                         if is_manifest(p))
+        assert all(isinstance(m, ManifestFile)
+                   for m in manifests.itervalues())
+        self.entries = [e for m in manifests.itervalues()
+                        for e in m if e.localized]
         # Find unique locales used in these manifest entries.
         self.locales = list(set(e.id for e in self.entries
                                 if isinstance(e, ManifestLocale)))
+        # Find all paths whose manifest are included by no other manifest.
+        includes = set(mozpath.join(e.base, e.relpath)
+                       for m in manifests.itervalues()
+                       for e in m if isinstance(e, Manifest))
+        self.bases = [mozpath.dirname(p)
+                      for p in set(manifests.keys()) - includes]
 
 
 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.
@@ -193,45 +160,19 @@ def _repack(app_finder, l10n_finder, cop
                     formatter.add(p, f)
 
     # Transplant jar preloading information.
     for path, log in app_finder.jarlogs.iteritems():
         assert isinstance(copier[path], Jarrer)
         copier[path].preload([l.replace(locale, l10n_locale) for l in log])
 
 
-def repack(source, l10n, extra_l10n={}, non_resources=[], non_chrome=set()):
-    '''
-    Replace localized data from the `source` directory with localized data
-    from `l10n` and `extra_l10n`.
-
-    The `source` argument points to a directory containing a packaged
-    application (in omnijar, jar or flat form).
-    The `l10n` argument points to a directory containing the main localized
-    data (usually in the form of a language pack addon) to use to replace
-    in the packaged application.
-    The `extra_l10n` argument contains a dict associating relative paths in
-    the source to separate directories containing localized data for them.
-    This can be used to point at different language pack addons for different
-    parts of the package application.
-    The `non_resources` argument gives a list of relative paths in the source
-    that should not be added in an omnijar in case the packaged application
-    is in that format.
-    The `non_chrome` argument gives a list of file/directory patterns for
-    localized files that are not listed in a chrome.manifest.
-    '''
+def repack(source, l10n, non_resources=[], non_chrome=set()):
     app_finder = UnpackFinder(source)
     l10n_finder = UnpackFinder(l10n)
-    if extra_l10n:
-        finders = {
-            '': l10n_finder,
-        }
-        for base, path in extra_l10n.iteritems():
-            finders[base] = UnpackFinder(path)
-        l10n_finder = ComposedFinder(finders)
     copier = FileCopier()
     if app_finder.kind == 'flat':
         formatter = FlatFormatter(copier)
     elif app_finder.kind == 'jar':
         formatter = JarFormatter(copier, optimize=app_finder.optimizedjars)
     elif app_finder.kind == 'omni':
         formatter = OmniJarFormatter(copier, app_finder.omnijar,
                                      optimize=app_finder.optimizedjars,
--- a/python/mozbuild/mozpack/test/test_files.py
+++ b/python/mozbuild/mozpack/test/test_files.py
@@ -5,17 +5,16 @@
 from mozbuild.util import ensureParentDir
 
 from mozpack.errors import (
     ErrorMessage,
     errors,
 )
 from mozpack.files import (
     AbsoluteSymlinkFile,
-    ComposedFinder,
     DeflatedFile,
     Dest,
     ExistingFile,
     FileFinder,
     File,
     GeneratedFile,
     JarFinder,
     ManifestFile,
@@ -848,20 +847,16 @@ class MatchTestTemplate(object):
             'foo/qux/1', 'foo/qux/bar', 'foo/qux/2/test', 'foo/qux/2/test2'
         ])
         self.do_check('foo/q*ux', [
             'foo/qux/1', 'foo/qux/bar', 'foo/qux/2/test', 'foo/qux/2/test2'
         ])
         self.do_check('foo/*/2/test*', ['foo/qux/2/test', 'foo/qux/2/test2'])
         self.do_check('**/bar', ['bar', 'foo/bar', 'foo/qux/bar'])
         self.do_check('foo/**/test', ['foo/qux/2/test'])
-        self.do_check('foo', [
-            'foo/bar', 'foo/baz', 'foo/qux/1', 'foo/qux/bar',
-            'foo/qux/2/test', 'foo/qux/2/test2'
-        ])
         self.do_check('foo/**', [
             'foo/bar', 'foo/baz', 'foo/qux/1', 'foo/qux/bar',
             'foo/qux/2/test', 'foo/qux/2/test2'
         ])
         self.do_check('**/2/test*', ['foo/qux/2/test', 'foo/qux/2/test2'])
         self.do_check('**/foo', [
             'foo/bar', 'foo/baz', 'foo/qux/1', 'foo/qux/bar',
             'foo/qux/2/test', 'foo/qux/2/test2'
@@ -965,41 +960,10 @@ class TestJarFinder(MatchTestTemplate, T
         self.jar = JarWriter(file=self.tmppath('test.jar'))
         self.prepare_match_test()
         self.jar.finish()
         reader = JarReader(file=self.tmppath('test.jar'))
         self.finder = JarFinder(self.tmppath('test.jar'), reader)
         self.do_match_test()
 
 
-class TestComposedFinder(MatchTestTemplate, TestWithTmpDir):
-    def add(self, path, content=None):
-        # Put foo/qux files under $tmp/b.
-        if path.startswith('foo/qux/'):
-            real_path = mozpath.join('b', path[8:])
-        else:
-            real_path = mozpath.join('a', path)
-        ensureParentDir(self.tmppath(real_path))
-        if not content:
-            content = path
-        open(self.tmppath(real_path), 'wb').write(content)
-
-    def do_check(self, pattern, result):
-        if '*' in pattern:
-            return
-        do_check(self, self.finder, pattern, result)
-
-    def test_composed_finder(self):
-        self.prepare_match_test()
-        # Also add files in $tmp/a/foo/qux because ComposedFinder is
-        # expected to mask foo/qux entirely with content from $tmp/b.
-        ensureParentDir(self.tmppath('a/foo/qux/hoge'))
-        open(self.tmppath('a/foo/qux/hoge'), 'wb').write('hoge')
-        open(self.tmppath('a/foo/qux/bar'), 'wb').write('not the right content')
-        self.finder = ComposedFinder({
-            '': FileFinder(self.tmppath('a')),
-            'foo/qux': FileFinder(self.tmppath('b')),
-        })
-        self.do_match_test()
-
-
 if __name__ == '__main__':
     mozunit.main()
--- a/python/mozbuild/mozpack/test/test_packager.py
+++ b/python/mozbuild/mozpack/test/test_packager.py
@@ -206,68 +206,16 @@ class TestSimplePackager(unittest.TestCa
             (('manifest', 5), 'add', 'foo/bar/foo.html', foo_html),
             (('manifest', 5), 'add', 'foo/bar/bar.html', bar_html),
             (('manifest', 9), 'add', 'addon/install.rdf', install_rdf),
         ])
 
         self.assertEqual(packager.get_bases(), set(['', 'addon', 'qux']))
         self.assertEqual(packager.get_bases(addons=False), set(['', 'qux']))
 
-    def test_simple_packager_manifest_consistency(self):
-        formatter = MockFormatter()
-        # bar/ is detected as an addon because of install.rdf, but top-level
-        # includes a manifest inside bar/.
-        packager = SimplePackager(formatter)
-        packager.add('base.manifest', GeneratedFile(
-            'manifest foo/bar.manifest\n'
-            'manifest bar/baz.manifest\n'
-        ))
-        packager.add('foo/bar.manifest', GeneratedFile('resource bar bar'))
-        packager.add('bar/baz.manifest', GeneratedFile('resource baz baz'))
-        packager.add('bar/install.rdf', GeneratedFile(''))
-
-        with self.assertRaises(ErrorMessage) as e:
-            packager.close()
-
-        self.assertEqual(e.exception.message,
-            'Error: "bar/baz.manifest" is included from "base.manifest", '
-            'which is outside "bar"')
-
-        # bar/ is detected as a separate base because of chrome.manifest that
-        # is included nowhere, but top-level includes another manifest inside
-        # bar/.
-        packager = SimplePackager(formatter)
-        packager.add('base.manifest', GeneratedFile(
-            'manifest foo/bar.manifest\n'
-            'manifest bar/baz.manifest\n'
-        ))
-        packager.add('foo/bar.manifest', GeneratedFile('resource bar bar'))
-        packager.add('bar/baz.manifest', GeneratedFile('resource baz baz'))
-        packager.add('bar/chrome.manifest', GeneratedFile('resource baz baz'))
-
-        with self.assertRaises(ErrorMessage) as e:
-            packager.close()
-
-        self.assertEqual(e.exception.message,
-            'Error: "bar/baz.manifest" is included from "base.manifest", '
-            'which is outside "bar"')
-
-        # bar/ is detected as a separate base because of chrome.manifest that
-        # is included nowhere, but chrome.manifest includes baz.manifest from
-        # the same directory. This shouldn't error out.
-        packager = SimplePackager(formatter)
-        packager.add('base.manifest', GeneratedFile(
-            'manifest foo/bar.manifest\n'
-        ))
-        packager.add('foo/bar.manifest', GeneratedFile('resource bar bar'))
-        packager.add('bar/baz.manifest', GeneratedFile('resource baz baz'))
-        packager.add('bar/chrome.manifest',
-                     GeneratedFile('manifest baz.manifest'))
-        packager.close()
-
 
 class TestSimpleManifestSink(unittest.TestCase):
     def test_simple_manifest_parser(self):
         formatter = MockFormatter()
         foobar = GeneratedFile('foobar')
         foobaz = GeneratedFile('foobaz')
         fooqux = GeneratedFile('fooqux')
         foozot = GeneratedFile('foozot')
--- a/python/mozbuild/mozpack/test/test_packager_l10n.py
+++ b/python/mozbuild/mozpack/test/test_packager_l10n.py
@@ -60,17 +60,17 @@ class TestL10NRepack(unittest.TestCase):
         foo_l10n = GeneratedFile('foo_l10n')
         qux_l10n = GeneratedFile('qux_l10n')
         baz_l10n = GeneratedFile('baz_l10n')
         barbaz_l10n = GeneratedFile('barbaz_l10n')
         lst_l10n = GeneratedFile('foo\nqux')
         l10n_finder = MockFinder({
             'chrome/qux-l10n/qux.properties': qux_l10n,
             'chrome/qux-l10n/baz/baz.properties': baz_l10n,
-            'chrome/chrome.manifest': ManifestFile('chrome', [
+            'chrome/chrome.manifest': ManifestFile(' chrome', [
                 ManifestLocale('chrome', 'qux', 'x-test', 'qux-l10n/'),
             ]),
             'chrome.manifest':
             ManifestFile('', [Manifest('', 'chrome/chrome.manifest')]),
             'dict/bb': dict_bb,
             'dict/cc': dict_cc,
             'app/chrome/bar-l10n/barbaz.dtd': barbaz_l10n,
             'app/chrome/chrome.manifest': ManifestFile('app/chrome', [
--- a/toolkit/mozapps/installer/l10n-repack.py
+++ b/toolkit/mozapps/installer/l10n-repack.py
@@ -23,38 +23,26 @@ NON_CHROME = set([
     'update.locale',
     'updater.ini',
     'extensions/langpack-*@*',
     'distribution/extensions/langpack-*@*',
     'chrome/**/searchplugins/*.xml',
 ])
 
 
-def valid_extra_l10n(arg):
-    if '=' not in arg:
-        raise ValueError('Invalid value')
-    return tuple(arg.split('=', 1))
-
-
 def main():
     parser = ArgumentParser()
     parser.add_argument('build',
                         help='Directory containing the build to repack')
     parser.add_argument('l10n',
                         help='Directory containing the staged langpack')
-    parser.add_argument('extra_l10n', nargs='*', metavar='BASE=PATH',
-                        type=valid_extra_l10n,
-                        help='Extra directories with staged localized files '
-                             'to be considered under the given base in the '
-                             'repacked build')
     parser.add_argument('--non-resource', nargs='+', metavar='PATTERN',
                         default=[],
                         help='Extra files not to be considered as resources')
     args = parser.parse_args()
 
     buildconfig.substs['USE_ELF_HACK'] = False
     buildconfig.substs['PKG_SKIP_STRIP'] = True
-    l10n.repack(args.build, args.l10n, extra_l10n=dict(args.extra_l10n),
-                non_resources=args.non_resource, non_chrome=NON_CHROME)
+    l10n.repack(args.build, args.l10n, args.non_resource, NON_CHROME)
 
 
 if __name__ == "__main__":
     main()