Bug 835309 - Look at .xpi file contents when unifying them for universal builds. r=gps

python/mozbuild/mozpack/errors.py

@@ -128,5 +128,10 @@ class ErrorCollector(object):
         if count:
             raise AccumulatedErrors()
 
+    @property
+    def count(self):
+        # _count can be None.
+        return self._count if self._count else 0
+
 
 errors = ErrorCollector()

python/mozbuild/mozpack/files.py

@@ -15,7 +15,9 @@ from mozpack.executables import (
 from mozpack.chrome.manifest import ManifestEntry
 from io import BytesIO
 from mozpack.errors import ErrorMessage
+from mozpack.mozjar import JarReader
 import mozpack.path
+from collections import OrderedDict
 
 
 class Dest(object):
@@ -321,13 +323,10 @@ class MinifiedProperties(BaseFile):
                                if not l.startswith('#')))
 
 
-class FileFinder(object):
+class BaseFinder(object):
-    '''
-    Helper to get appropriate BaseFile instances from the file system.
-    '''
     def __init__(self, base, minify=False):
         '''
-        Create a FileFinder for files under the given base directory. The
+        Initializes the instance with a reference base directory. The
         optional minify argument specifies whether file types supporting
         minification (currently only "*.properties") should be minified.
         '''
@@ -339,18 +338,65 @@ class FileFinder(object):
         Yield path, BaseFile_instance pairs for all files under the base
         directory and its subdirectories that match the given pattern. See the
         mozpack.path.match documentation for a description of the handled
-        patterns. Note all files with a name starting with a '.' are ignored
+        patterns.
-        when scanning directories, but are not ignored when explicitely
-        requested.
         '''
         while pattern.startswith('/'):
             pattern = pattern[1:]
-        return self._find(pattern)
+        for p, f in self._find(pattern):
+            yield p, self._minify_file(p, f)
+
+    def __iter__(self):
+        '''
+        Iterates over all files under the base directory (excluding files
+        starting with a '.' and files at any level under a directory starting
+        with a '.').
+            for path, file in finder:
+                ...
+        '''
+        return self.find('')
+
+    def __contains__(self, pattern):
+        raise RuntimeError("'in' operator forbidden for %s. Use contains()." %
+                           self.__class__.__name__)
+
+    def contains(self, pattern):
+        '''
+        Return whether some files under the base directory match the given
+        pattern. See the mozpack.path.match documentation for a description of
+        the handled patterns.
+        '''
+        return any(self.find(pattern))
+
+    def _minify_file(self, path, file):
+        '''
+        Return an appropriate MinifiedSomething wrapper for the given BaseFile
+        instance (file), according to the file type (determined by the given
+        path), if the FileFinder was created with minification enabled.
+        Otherwise, just return the given BaseFile instance.
+        Currently, only "*.properties" files are handled.
+        '''
+        if self._minify and not isinstance(file, ExecutableFile):
+            if path.endswith('.properties'):
+                return MinifiedProperties(file)
+        return file
+
+
+class FileFinder(BaseFinder):
+    '''
+    Helper to get appropriate BaseFile instances from the file system.
+    '''
+    def __init__(self, base, **kargs):
+        '''
+        Create a FileFinder for files under the given base directory.
+        '''
+        BaseFinder.__init__(self, base, **kargs)
 
     def _find(self, pattern):
         '''
         Actual implementation of FileFinder.find(), dispatching to specialized
         member functions depending on what kind of pattern was given.
+        Note all files with a name starting with a '.' are ignored when
+        scanning directories, but are not ignored when explicitely requested.
         '''
         if '*' in pattern:
             return self._find_glob('', mozpack.path.split(pattern))
@@ -384,7 +430,7 @@ class FileFinder(object):
         if is_executable(srcpath):
             yield path, ExecutableFile(srcpath)
         else:
-            yield path, self._minify_file(srcpath, File(srcpath))
+            yield path, File(srcpath)
 
     def _find_glob(self, base, pattern):
         '''
@@ -418,37 +464,35 @@ class FileFinder(object):
                                         pattern[1:]):
                 yield p, f
 
-    def __iter__(self):
-        '''
-        Iterates over all files under the base directory (excluding files
-        starting with a '.' and files at any level under a directory starting
-        with a '.').
-            for path, file in finder:
-                ...
-        '''
-        return self.find('')
 
-    def __contains__(self, pattern):
+class JarFinder(BaseFinder):
-        raise RuntimeError("'in' operator forbidden for %s. Use contains()." %
+    '''
-                           self.__class__.__name__)
+    Helper to get appropriate DeflatedFile instances from a JarReader.
+    '''
+    def __init__(self, base, reader, **kargs):
+        '''
+        Create a JarFinder for files in the given JarReader. The base argument
+        is used as an indication of the Jar file location.
+        '''
+        assert isinstance(reader, JarReader)
+        BaseFinder.__init__(self, base, **kargs)
+        self._files = OrderedDict((f.filename, f) for f in reader)
 
-    def contains(self, pattern):
+    def _find(self, pattern):
         '''
-        Return whether some files under the base directory match the given
+        Actual implementation of JarFinder.find(), dispatching to specialized
-        pattern. See the mozpack.path.match documentation for a description of
+        member functions depending on what kind of pattern was given.
-        the handled patterns.
         '''
-        return any(self.find(pattern))
+        if '*' in pattern:
-
+            for p in self._files:
-    def _minify_file(self, path, file):
+                if mozpack.path.match(p, pattern):
-        '''
+                    yield p, DeflatedFile(self._files[p])
-        Return an appropriate MinifiedSomething wrapper for the given BaseFile
+        elif pattern == '':
-        instance (file), according to the file type (determined by the given
+            for p in self._files:
-        path), if the FileFinder was created with minification enabled.
+                yield p, DeflatedFile(self._files[p])
-        Otherwise, just return the given BaseFile instance.
+        elif pattern in self._files:
-        Currently, only "*.properties" files are handled.
+            yield pattern, DeflatedFile(self._files[pattern])
-        '''
+        else:
-        if self._minify:
+            for p in self._files:
-            if path.endswith('.properties'):
+                if mozpack.path.basedir(p, [pattern]) == pattern:
-                return MinifiedProperties(file)
+                    yield p, DeflatedFile(self._files[p])
-        return file

python/mozbuild/mozpack/test/test_copier.py

@@ -59,6 +59,7 @@ class TestFileRegistry(MatchTestTemplate, unittest.TestCase):
         self.registry.remove('bar')
         self.assertEqual(self.registry.paths(), [])
 
+        self.prepare_match_test()
         self.do_match_test()
         self.assertTrue(self.checked)
         self.assertEqual(self.registry.paths(), [

python/mozbuild/mozpack/test/test_files.py

@@ -11,6 +11,7 @@ from mozpack.files import (
     XPTFile,
     MinifiedProperties,
     FileFinder,
+    JarFinder,
 )
 from mozpack.mozjar import (
     JarReader,
@@ -486,7 +487,7 @@ class TestMinifiedProperties(TestWithTmpDir):
 
 
 class MatchTestTemplate(object):
-    def do_match_test(self):
+    def prepare_match_test(self, with_dotfiles=False):
         self.add('bar')
         self.add('foo/bar')
         self.add('foo/baz')
@@ -494,7 +495,11 @@ class MatchTestTemplate(object):
         self.add('foo/qux/bar')
         self.add('foo/qux/2/test')
         self.add('foo/qux/2/test2')
+        if with_dotfiles:
+            self.add('foo/.foo')
+            self.add('foo/.bar/foo')
 
+    def do_match_test(self):
         self.do_check('', [
             'bar', 'foo/bar', 'foo/baz', 'foo/qux/1', 'foo/qux/bar',
             'foo/qux/2/test', 'foo/qux/2/test2'
@@ -533,6 +538,33 @@ class MatchTestTemplate(object):
         self.do_check('**/barbaz', [])
         self.do_check('f**/bar', ['foo/bar'])
 
+    def do_finder_test(self, finder):
+        self.assertTrue(finder.contains('foo/.foo'))
+        self.assertTrue(finder.contains('foo/.bar'))
+        self.assertTrue('foo/.foo' in [f for f, c in
+                                       finder.find('foo/.foo')])
+        self.assertTrue('foo/.bar/foo' in [f for f, c in
+                                           finder.find('foo/.bar')])
+        self.assertEqual(sorted([f for f, c in finder.find('foo/.*')]),
+                         ['foo/.bar/foo', 'foo/.foo'])
+        for pattern in ['foo', '**', '**/*', '**/foo', 'foo/*']:
+            self.assertFalse('foo/.foo' in [f for f, c in
+                                            finder.find(pattern)])
+            self.assertFalse('foo/.bar/foo' in [f for f, c in
+                                                finder.find(pattern)])
+            self.assertEqual(sorted([f for f, c in finder.find(pattern)]),
+                             sorted([f for f, c in finder
+                                     if mozpack.path.match(f, pattern)]))
+
+
+def do_check(test, finder, pattern, result):
+    if result:
+        test.assertTrue(finder.contains(pattern))
+    else:
+        test.assertFalse(finder.contains(pattern))
+    test.assertEqual(sorted(list(f for f, c in finder.find(pattern))),
+                     sorted(result))
+
 
 class TestFileFinder(MatchTestTemplate, TestWithTmpDir):
     def add(self, path):
@@ -540,34 +572,30 @@ class TestFileFinder(MatchTestTemplate, TestWithTmpDir):
         open(self.tmppath(path), 'wb').write(path)
 
     def do_check(self, pattern, result):
-        if result:
+        do_check(self, self.finder, pattern, result)
-            self.assertTrue(self.finder.contains(pattern))
-        else:
-            self.assertFalse(self.finder.contains(pattern))
-        self.assertEqual(sorted(list(f for f, c in self.finder.find(pattern))),
-                         sorted(result))
 
     def test_file_finder(self):
+        self.prepare_match_test(with_dotfiles=True)
         self.finder = FileFinder(self.tmpdir)
         self.do_match_test()
-        self.add('foo/.foo')
+        self.do_finder_test(self.finder)
-        self.add('foo/.bar/foo')
+
-        self.assertTrue(self.finder.contains('foo/.foo'))
+
-        self.assertTrue(self.finder.contains('foo/.bar'))
+class TestJarFinder(MatchTestTemplate, TestWithTmpDir):
-        self.assertTrue('foo/.foo' in [f for f, c in
+    def add(self, path):
-                                       self.finder.find('foo/.foo')])
+        self.jar.add(path, path, compress=True)
-        self.assertTrue('foo/.bar/foo' in [f for f, c in
+
-                                           self.finder.find('foo/.bar')])
+    def do_check(self, pattern, result):
-        self.assertEqual(sorted([f for f, c in self.finder.find('foo/.*')]),
+        do_check(self, self.finder, pattern, result)
-                         ['foo/.bar/foo', 'foo/.foo'])
+
-        for pattern in ['foo', '**', '**/*', '**/foo', 'foo/*']:
+    def test_jar_finder(self):
-            self.assertFalse('foo/.foo' in [f for f, c in
+        self.jar = JarWriter(file=self.tmppath('test.jar'))
-                                            self.finder.find(pattern)])
+        self.prepare_match_test()
-            self.assertFalse('foo/.bar/foo' in [f for f, c in
+        self.jar.finish()
-                                                self.finder.find(pattern)])
+        reader = JarReader(file=self.tmppath('test.jar'))
-            self.assertEqual(sorted([f for f, c in self.finder.find(pattern)]),
+        self.finder = JarFinder(self.tmppath('test.jar'), reader)
-                             sorted([f for f, c in self.finder
+        self.do_match_test()
-                                     if mozpack.path.match(f, pattern)]))
+
 
 if __name__ == '__main__':
     mozunit.main()

python/mozbuild/mozpack/test/test_unify.py

@@ -9,8 +9,17 @@ from mozpack.unify import (
 import mozunit
 from mozpack.test.test_files import TestWithTmpDir
 from mozpack.copier import ensure_parent_dir
+from mozpack.files import FileFinder
+from mozpack.mozjar import JarWriter
+from mozpack.test.test_files import MockDest
+from cStringIO import StringIO
 import os
-from mozpack.errors import ErrorMessage
+import sys
+from mozpack.errors import (
+    ErrorMessage,
+    AccumulatedErrors,
+    errors,
+)
 
 
 class TestUnified(TestWithTmpDir):
@@ -36,7 +45,8 @@ class TestUnifiedFinder(TestUnified):
         self.create_one('b', 'test/foo', 'b\nc\na\n')
         self.create_both('test/bar', 'a\nb\nc\n')
 
-        finder = UnifiedFinder(self.tmppath('a'), self.tmppath('b'),
+        finder = UnifiedFinder(FileFinder(self.tmppath('a')),
+                               FileFinder(self.tmppath('b')),
                                sorted=['test'])
         self.assertEqual(sorted([(f, c.open().read())
                                  for f, c in finder.find('foo')]),
@@ -73,7 +83,8 @@ class TestUnifiedBuildFinder(TestUnified):
                             '</body>',
                             '</html>',
                         ]))
-        finder = UnifiedBuildFinder(self.tmppath('a'), self.tmppath('b'))
+        finder = UnifiedBuildFinder(FileFinder(self.tmppath('a')),
+                                    FileFinder(self.tmppath('b')))
         self.assertEqual(sorted([(f, c.open().read()) for f, c in
                                  finder.find('**/chrome.manifest')]),
                          [('chrome.manifest', 'a\nb\nc\n'),
@@ -92,6 +103,25 @@ class TestUnifiedBuildFinder(TestUnified):
                              '</html>',
                          ]))])
 
+        xpi = MockDest()
+        with JarWriter(fileobj=xpi, compress=True) as jar:
+            jar.add('foo', 'foo')
+            jar.add('bar', 'bar')
+        foo_xpi = xpi.read()
+        self.create_both('foo.xpi', foo_xpi)
+
+        with JarWriter(fileobj=xpi, compress=True) as jar:
+            jar.add('foo', 'bar')
+        self.create_one('a', 'bar.xpi', foo_xpi)
+        self.create_one('b', 'bar.xpi', xpi.read())
+
+        errors.out = StringIO()
+        with self.assertRaises(AccumulatedErrors), errors.accumulate():
+            self.assertEqual([(f, c.open().read()) for f, c in
+                              finder.find('*.xpi')],
+                             [('foo.xpi', foo_xpi)])
+        errors.out = sys.stderr
+
 
 if __name__ == '__main__':
     mozunit.main()

python/mozbuild/mozpack/unify.py

@@ -3,7 +3,8 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from mozpack.files import (
-    FileFinder,
+    BaseFinder,
+    JarFinder,
     ExecutableFile,
     BaseFile,
     GeneratedFile,
@@ -13,6 +14,7 @@ from mozpack.executables import (
     may_strip,
     strip,
 )
+from mozpack.mozjar import JarReader
 from mozpack.errors import errors
 from tempfile import mkstemp
 import mozpack.path
@@ -67,66 +69,70 @@ class UnifiedExecutableFile(BaseFile):
                 os.unlink(f)
 
 
-class UnifiedFinder(FileFinder):
+class UnifiedFinder(BaseFinder):
     '''
     Helper to get unified BaseFile instances from two distinct trees on the
     file system.
     '''
-    def __init__(self, base1, base2, sorted=[], **kargs):
+    def __init__(self, finder1, finder2, sorted=[], **kargs):
         '''
-        Initialize a UnifiedFinder. base1 and base2 are the base directories
+        Initialize a UnifiedFinder. finder1 and finder2 are BaseFinder
-        for the two trees from which files are picked. UnifiedFinder.find()
+        instances from which files are picked. UnifiedFinder.find() will act as
-        will act as FileFinder.find() but will error out when matches can only
+        FileFinder.find() but will error out when matches can only be found in
-        be found in one of the two trees and not the other. It will also error
+        one of the two trees and not the other. It will also error out if
-        out if matches can be found on both ends but their contents are not
+        matches can be found on both ends but their contents are not identical.
-        identical.
 
         The sorted argument gives a list of mozpack.path.match patterns. File
         paths matching one of these patterns will have their contents compared
         with their lines sorted.
         '''
-        self._base1 = FileFinder(base1, **kargs)
+        assert isinstance(finder1, BaseFinder)
-        self._base2 = FileFinder(base2, **kargs)
+        assert isinstance(finder2, BaseFinder)
+        self._finder1 = finder1
+        self._finder2 = finder2
         self._sorted = sorted
+        BaseFinder.__init__(self, finder1.base, **kargs)
 
     def _find(self, path):
         '''
         UnifiedFinder.find() implementation.
         '''
         files1 = OrderedDict()
-        for p, f in self._base1.find(path):
+        for p, f in self._finder1.find(path):
             files1[p] = f
         files2 = set()
-        for p, f in self._base2.find(path):
+        for p, f in self._finder2.find(path):
             files2.add(p)
             if p in files1:
                 if may_unify_binary(files1[p]) and \
                         may_unify_binary(f):
                     yield p, UnifiedExecutableFile(files1[p].path, f.path)
                 else:
+                    err = errors.count
                     unified = self.unify_file(p, files1[p], f)
                     if unified:
                         yield p, unified
-                    else:
+                    elif err == errors.count:
                         self._report_difference(p, files1[p], f)
             else:
-                errors.error('File missing in %s: %s' % (self._base1.base, p))
+                errors.error('File missing in %s: %s' %
+                             (self._finder1.base, p))
         for p in [p for p in files1 if not p in files2]:
-            errors.error('File missing in %s: %s' % (self._base2.base, p))
+            errors.error('File missing in %s: %s' % (self._finder2.base, p))
 
     def _report_difference(self, path, file1, file2):
         '''
         Report differences between files in both trees.
         '''
         errors.error("Can't unify %s: file differs between %s and %s" %
-                     (path, self._base1.base, self._base2.base))
+                     (path, self._finder1.base, self._finder2.base))
         if not isinstance(file1, ExecutableFile) and \
                 not isinstance(file2, ExecutableFile):
             from difflib import unified_diff
             for line in unified_diff(file1.open().readlines(),
                                      file2.open().readlines(),
-                                     os.path.join(self._base1.base, path),
+                                     os.path.join(self._finder1.base, path),
-                                     os.path.join(self._base2.base, path)):
+                                     os.path.join(self._finder2.base, path)):
                 errors.out.write(line)
 
     def unify_file(self, path, file1, file2):
@@ -152,8 +158,8 @@ class UnifiedBuildFinder(UnifiedFinder):
     "*.manifest" files to differ in their order, and unifies "buildconfig.html"
     files by merging their content.
     '''
-    def __init__(self, base1, base2, **kargs):
+    def __init__(self, finder1, finder2, **kargs):
-        UnifiedFinder.__init__(self, base1, base2,
+        UnifiedFinder.__init__(self, finder1, finder2,
                                sorted=['**/*.manifest'], **kargs)
 
     def unify_file(self, path, file1, file2):
@@ -171,4 +177,15 @@ class UnifiedBuildFinder(UnifiedFinder):
                 ['<hr> </hr>\n'] +
                 content2[content2.index('<h1>about:buildconfig</h1>\n') + 1:]
             ))
+        if path.endswith('.xpi'):
+            finder1 = JarFinder(os.path.join(self._finder1.base, path),
+                                JarReader(fileobj=file1.open()))
+            finder2 = JarFinder(os.path.join(self._finder2.base, path),
+                                JarReader(fileobj=file2.open()))
+            unifier = UnifiedFinder(finder1, finder2, sorted=self._sorted)
+            err = errors.count
+            all(unifier.find(''))
+            if err == errors.count:
+                return file1
+            return None
         return UnifiedFinder.unify_file(self, path, file1, file2)

toolkit/mozapps/installer/packager.py

@@ -294,7 +294,8 @@ def main():
 
     with errors.accumulate():
         if args.unify:
-            finder = UnifiedBuildFinder(args.source, args.unify,
+            finder = UnifiedBuildFinder(FileFinder(args.source),
+                                        FileFinder(args.unify),
                                         minify=args.minify)
         else:
             finder = FileFinder(args.source, minify=args.minify)