Bug 1489340 - Handle XPT files as blobs of data in packager. r=froydnj

We don't actually ship XPT files anymore, but it's still useful for the
packager code to handle old Firefox versions. But for that, we don't
really need the complexity of "linking" XPT files in a single unit per
directory. We can just as well keep the XPT files intact, as long as we
retain individual `interfaces` manifest entries for each.

And since those entries used to be all merged into one, we now instead
group them all together in manifests (which also happens to make it
easier on unit test changes).

Differential Revision: https://phabricator.services.mozilla.com/D5221

--HG--
extra : moz-landing-system : lando

python/mozbuild/mozpack/copier.py

@@ -14,7 +14,6 @@ from mozpack.files import (
     DeflatedFile,
     Dest,
     ManifestFile,
-    XPTFile,
 )
 import mozpack.path as mozpath
 import errno
@@ -583,7 +582,7 @@ class Jarrer(FileRegistry, BaseFile):
                 # but need to be able to decompress those files, per
                 # UnpackFinder and formatters, we force deflate on them.
                 if compress == JAR_BROTLI and (
-                        isinstance(file, (ManifestFile, XPTFile)) or
+                        isinstance(file, ManifestFile) or
                         mozpath.basename(path) == 'install.rdf'):
                     compress = True
 

python/mozbuild/mozpack/files.py

@@ -12,6 +12,7 @@ import stat
 import subprocess
 import uuid
 import mozbuild.makeutil as makeutil
+from itertools import chain
 from mozbuild.preprocessor import Preprocessor
 from mozbuild.util import FileAvoidWrite
 from mozpack.executables import (
@@ -21,7 +22,10 @@ from mozpack.executables import (
     may_elfhack,
     elfhack,
 )
-from mozpack.chrome.manifest import ManifestEntry
+from mozpack.chrome.manifest import (
+    ManifestEntry,
+    ManifestInterfaces,
+)
 from io import BytesIO
 from mozpack.errors import (
     ErrorMessage,
@@ -624,78 +628,6 @@ class ExtractedTarFile(GeneratedFile):
     def read(self):
         return self.content
 
-class XPTFile(GeneratedFile):
-    '''
-    File class for a linked XPT file. It takes several XPT files as input
-    (using the add() and remove() member functions), and links them at copy()
-    time.
-    '''
-    def __init__(self):
-        self._files = set()
-
-    def add(self, xpt):
-        '''
-        Add the given XPT file (as a BaseFile instance) to the list of XPTs
-        to link.
-        '''
-        assert isinstance(xpt, BaseFile)
-        self._files.add(xpt)
-
-    def remove(self, xpt):
-        '''
-        Remove the given XPT file (as a BaseFile instance) from the list of
-        XPTs to link.
-        '''
-        assert isinstance(xpt, BaseFile)
-        self._files.remove(xpt)
-
-    def copy(self, dest, skip_if_older=True):
-        '''
-        Link the registered XPTs and place the resulting linked XPT at the
-        destination given as a string or a Dest instance. Avoids an expensive
-        XPT linking if the interfaces in an existing destination match those of
-        the individual XPTs to link.
-        skip_if_older is ignored.
-        '''
-        if isinstance(dest, basestring):
-            dest = Dest(dest)
-        assert isinstance(dest, Dest)
-
-        from xpt import xpt_link, Typelib, Interface
-        all_typelibs = [Typelib.read(f.open()) for f in self._files]
-        if dest.exists():
-            # Typelib.read() needs to seek(), so use a BytesIO for dest
-            # content.
-            dest_interfaces = \
-                dict((i.name, i)
-                     for i in Typelib.read(BytesIO(dest.read())).interfaces
-                     if i.iid != Interface.UNRESOLVED_IID)
-            identical = True
-            for f in self._files:
-                typelib = Typelib.read(f.open())
-                for i in typelib.interfaces:
-                    if i.iid != Interface.UNRESOLVED_IID and \
-                            not (i.name in dest_interfaces and
-                                 i == dest_interfaces[i.name]):
-                        identical = False
-                        break
-            if identical:
-                return False
-        s = BytesIO()
-        xpt_link(all_typelibs).write(s)
-        dest.write(s.getvalue())
-        return True
-
-    def open(self):
-        raise RuntimeError("Unsupported")
-
-    def isempty(self):
-        '''
-        Return whether there are XPT files to link.
-        '''
-        return len(self._files) == 0
-
-
 class ManifestFile(BaseFile):
     '''
     File class for a manifest file. It takes individual manifest entries (using
@@ -712,8 +644,11 @@ class ManifestFile(BaseFile):
         currently but could in the future.
     '''
     def __init__(self, base, entries=None):
-        self._entries = entries if entries else []
         self._base = base
+        self._entries = []
+        self._interfaces = []
+        for e in entries or []:
+            self.add(e)
 
     def add(self, entry):
         '''
@@ -721,14 +656,20 @@ class ManifestFile(BaseFile):
         instead of add() time so that they can be more easily remove()d.
         '''
         assert isinstance(entry, ManifestEntry)
-        self._entries.append(entry)
+        if isinstance(entry, ManifestInterfaces):
+            self._interfaces.append(entry)
+        else:
+            self._entries.append(entry)
 
     def remove(self, entry):
         '''
         Remove the given entry from the manifest.
         '''
         assert isinstance(entry, ManifestEntry)
-        self._entries.remove(entry)
+        if isinstance(entry, ManifestInterfaces):
+            self._interfaces.remove(entry)
+        else:
+            self._entries.remove(entry)
 
     def open(self):
         '''
@@ -736,19 +677,20 @@ class ManifestFile(BaseFile):
         the manifest.
         '''
         return BytesIO(''.join('%s\n' % e.rebase(self._base)
-                               for e in self._entries))
+                               for e in chain(self._entries,
+                                              self._interfaces)))
 
     def __iter__(self):
         '''
         Iterate over entries in the manifest file.
         '''
-        return iter(self._entries)
+        return chain(self._entries, self._interfaces)
 
     def isempty(self):
         '''
         Return whether there are manifest entries to write
         '''
-        return len(self._entries) == 0
+        return len(self._entries) + len(self._interfaces) == 0
 
 
 class MinifiedProperties(BaseFile):

python/mozbuild/mozpack/packager/formats.py

@@ -16,10 +16,7 @@ from mozpack.chrome.manifest import (
 from mozpack.errors import errors
 from urlparse import urlparse
 import mozpack.path as mozpath
-from mozpack.files import (
-    ManifestFile,
-    XPTFile,
-)
+from mozpack.files import ManifestFile
 from mozpack.copier import (
     FileRegistry,
     FileRegistrySubtree,
@@ -50,8 +47,8 @@ The base interface provides the following methods:
     - add(path, content)
         Add the given content (BaseFile instance) at the given virtual path
     - add_interfaces(path, content)
-        Add the given content (BaseFile instance) and link it to other
-        interfaces in the parent directory of the given virtual path.
+        Add the given content (BaseFile instance) as an interface. Equivalent
+        to add(path, content) with the right add_manifest().
     - add_manifest(entry)
         Add a ManifestEntry.
     - contains(path)
@@ -177,15 +174,9 @@ class FlatSubFormatter(object):
         self.copier[path].add(entry)
 
     def add_interfaces(self, path, content):
-        # Interfaces in the same directory are all linked together in an
-        # interfaces.xpt file.
-        interfaces_path = mozpath.join(mozpath.dirname(path),
-                                       'interfaces.xpt')
-        if not self.copier.contains(interfaces_path):
-            self.add_manifest(ManifestInterfaces(mozpath.dirname(path),
-                                                 'interfaces.xpt'))
-            self.copier.add(interfaces_path, XPTFile())
-        self.copier[interfaces_path].add(content)
+        self.copier.add(path, content)
+        self.add_manifest(ManifestInterfaces(mozpath.dirname(path),
+                                             mozpath.basename(path)))
 
     def contains(self, path):
         assert '*' not in path

python/mozbuild/mozpack/test/test_files.py

@@ -27,7 +27,6 @@ from mozpack.files import (
     MinifiedJavaScript,
     MinifiedProperties,
     PreprocessedFile,
-    XPTFile,
 )
 
 # We don't have hglib installed everywhere.
@@ -63,7 +62,6 @@ import mozpack.path as mozpath
 from tempfile import mkdtemp
 from io import BytesIO
 from StringIO import StringIO
-from xpt import Typelib
 
 
 class TestWithTmpDir(unittest.TestCase):
@@ -824,52 +822,6 @@ foo2_xpt = GeneratedFile(
 )
 
 
-def read_interfaces(file):
-    return dict((i.name, i) for i in Typelib.read(file).interfaces)
-
-
-class TestXPTFile(TestWithTmpDir):
-    def test_xpt_file(self):
-        x = XPTFile()
-        x.add(foo_xpt)
-        x.add(bar_xpt)
-        x.copy(self.tmppath('interfaces.xpt'))
-
-        foo = read_interfaces(foo_xpt.open())
-        foo2 = read_interfaces(foo2_xpt.open())
-        bar = read_interfaces(bar_xpt.open())
-        linked = read_interfaces(self.tmppath('interfaces.xpt'))
-        self.assertEqual(foo['foo'], linked['foo'])
-        self.assertEqual(bar['bar'], linked['bar'])
-
-        x.remove(foo_xpt)
-        x.copy(self.tmppath('interfaces2.xpt'))
-        linked = read_interfaces(self.tmppath('interfaces2.xpt'))
-        self.assertEqual(bar['foo'], linked['foo'])
-        self.assertEqual(bar['bar'], linked['bar'])
-
-        x.add(foo_xpt)
-        x.copy(DestNoWrite(self.tmppath('interfaces.xpt')))
-        linked = read_interfaces(self.tmppath('interfaces.xpt'))
-        self.assertEqual(foo['foo'], linked['foo'])
-        self.assertEqual(bar['bar'], linked['bar'])
-
-        x = XPTFile()
-        x.add(foo2_xpt)
-        x.add(bar_xpt)
-        x.copy(self.tmppath('interfaces.xpt'))
-        linked = read_interfaces(self.tmppath('interfaces.xpt'))
-        self.assertEqual(foo2['foo'], linked['foo'])
-        self.assertEqual(bar['bar'], linked['bar'])
-
-        x = XPTFile()
-        x.add(foo_xpt)
-        x.add(foo2_xpt)
-        x.add(bar_xpt)
-        from xpt import DataError
-        self.assertRaises(DataError, x.copy, self.tmppath('interfaces.xpt'))
-
-
 class TestMinifiedProperties(TestWithTmpDir):
     def test_minified_properties(self):
         propLines = [

python/mozbuild/mozpack/test/test_packager_formats.py

@@ -27,11 +27,9 @@ from mozpack.errors import (
     ErrorMessage,
 )
 from mozpack.test.test_files import (
-    MockDest,
     foo_xpt,
     foo2_xpt,
     bar_xpt,
-    read_interfaces,
 )
 import mozpack.path as mozpath
 from test_errors import TestErrors
@@ -94,13 +92,12 @@ RESULT_FLAT = {
     'chrome/f/oo/qux': FILES['chrome/f/oo/qux'],
     'components/components.manifest': [
         'binary-component foo.so',
-        'interfaces interfaces.xpt',
+        'interfaces bar.xpt',
+        'interfaces foo.xpt',
     ],
     'components/foo.so': FILES['components/foo.so'],
-    'components/interfaces.xpt': {
-        'foo': read_interfaces(foo_xpt.open())['foo'],
-        'bar': read_interfaces(bar_xpt.open())['bar'],
-    },
+    'components/foo.xpt': foo_xpt,
+    'components/bar.xpt': bar_xpt,
     'foo': FILES['foo'],
     'app/chrome.manifest': [
         'manifest chrome/chrome.manifest',
@@ -129,12 +126,11 @@ for addon in ('addon0', 'addon1'):
             ],
             'chrome/foo/bar/baz': FILES[mozpath.join(addon, 'chrome/foo/bar/baz')],
             'components/components.manifest': [
-                'interfaces interfaces.xpt',
+                'interfaces bar.xpt',
+                'interfaces foo.xpt',
             ],
-            'components/interfaces.xpt': {
-                'foo': read_interfaces(foo2_xpt.open())['foo'],
-                'bar': read_interfaces(bar_xpt.open())['bar'],
-            },
+            'components/bar.xpt': bar_xpt,
+            'components/foo.xpt': foo2_xpt,
         }.iteritems()
     })
 
@@ -145,14 +141,16 @@ RESULT_JAR = {
         'chrome/chrome.manifest',
         'components/components.manifest',
         'components/foo.so',
-        'components/interfaces.xpt',
+        'components/foo.xpt',
+        'components/bar.xpt',
         'foo',
         'app/chrome.manifest',
         'app/components/components.manifest',
         'app/components/foo.js',
         'addon0/chrome.manifest',
         'addon0/components/components.manifest',
-        'addon0/components/interfaces.xpt',
+        'addon0/components/foo.xpt',
+        'addon0/components/bar.xpt',
     )
 }
 
@@ -203,7 +201,8 @@ RESULT_OMNIJAR.update({
 RESULT_OMNIJAR.update({
     'omni.foo': {
         'components/components.manifest': [
-            'interfaces interfaces.xpt',
+            'interfaces bar.xpt',
+            'interfaces foo.xpt',
         ],
     },
     'chrome.manifest': [
@@ -234,7 +233,8 @@ RESULT_OMNIJAR['omni.foo'].update({
         'chrome/f/oo/bar/baz',
         'chrome/f/oo/baz',
         'chrome/f/oo/qux',
-        'components/interfaces.xpt',
+        'components/foo.xpt',
+        'components/bar.xpt',
     )
 })
 
@@ -272,11 +272,6 @@ RESULT_JAR_WITH_BASE = result_with_base(RESULT_JAR)
 RESULT_OMNIJAR_WITH_BASE = result_with_base(RESULT_OMNIJAR)
 
 
-class MockDest(MockDest):
-    def exists(self):
-        return False
-
-
 def fill_formatter(formatter, contents):
     for base, is_addon in contents['bases'].items():
         formatter.add_base(base, is_addon)
@@ -284,7 +279,7 @@ def fill_formatter(formatter, contents):
     for manifest in contents['manifests']:
         formatter.add_manifest(manifest)
 
-    for k, v in contents['files'].iteritems():
+    for k, v in sorted(contents['files'].iteritems()):
         if k.endswith('.xpt'):
             formatter.add_interfaces(k, v)
         else:
@@ -294,11 +289,7 @@ def fill_formatter(formatter, contents):
 def get_contents(registry, read_all=False):
     result = {}
     for k, v in registry:
-        if k.endswith('.xpt'):
-            tmpfile = MockDest()
-            registry[k].copy(tmpfile)
-            result[k] = read_interfaces(tmpfile)
-        elif isinstance(v, FileRegistry):
+        if isinstance(v, FileRegistry):
             result[k] = get_contents(v)
         elif isinstance(v, ManifestFile) or read_all:
             result[k] = v.open().read().splitlines()