Bug 1531634 - Change how OMNIJAR_NAME is handled for fennec builds. r=nalexander

Fennec has a value of OMNIJAR_NAME that contains a directory, contrary
to other platforms, and relies in post-packaging, pre-unpacking steps to
accommodate with the difference.

With this change, we just make the packaging and unpacking steps aware
of this setup, and make allow them to pack/unpack resources in foo/
under foo/$OMNIJAR_NAME, whether $OMNIJAR_NAME is a file name or a path.

This will, further down the road, allow the packager code to handle jar
logs from PGO instrumentation without munging them.

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

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

python/mozbuild/mozpack/packager/unpack.py

@@ -40,7 +40,7 @@ class UnpackFinder(BaseFinder):
     The UnpackFinder is populated with files from this Finder instance,
     or with files from a FileFinder using the given path as its root.
     '''
-    def __init__(self, source):
+    def __init__(self, source, omnijar_name=None):
         if isinstance(source, BaseFinder):
             self._finder = source
         else:
@@ -48,7 +48,12 @@ class UnpackFinder(BaseFinder):
         self.base = self._finder.base
         self.files = FileRegistry()
         self.kind = 'flat'
-        self.omnijar = None
+        if omnijar_name:
+            self.omnijar = omnijar_name
+        else:
+            # Can't include globally because of bootstrapping issues.
+            from buildconfig import substs
+            self.omnijar = substs.get('OMNIJAR_NAME', 'omni.ja')
         self.jarlogs = {}
         self.compressed = False
 
@@ -59,18 +64,15 @@ class UnpackFinder(BaseFinder):
             if p == 'precomplete':
                 continue
             base = mozpath.dirname(p)
-            # If the file is a zip/jar that is not a .xpi, and contains a
-            # chrome.manifest, it is an omnijar. All the files it contains
-            # go in the directory containing the omnijar. Manifests are merged
-            # if there is a corresponding manifest in the directory.
-            if not p.endswith('.xpi') and self._maybe_zip(f) and \
-                    (mozpath.basename(p) == self.omnijar or
-                     not self.omnijar):
+            # If the file matches the omnijar pattern, it is an omnijar.
+            # All the files it contains go in the directory containing the full
+            # pattern. Manifests are merged if there is a corresponding manifest
+            # in the directory.
+            if self._maybe_zip(f) and mozpath.match(p, '**/%s' % self.omnijar):
                 jar = self._open_jar(p, f)
                 if 'chrome.manifest' in jar:
                     self.kind = 'omni'
-                    self.omnijar = mozpath.basename(p)
-                    self._fill_with_jar(base, jar)
+                    self._fill_with_jar(p[:-len(self.omnijar) - 1], jar)
                     continue
             # If the file is a manifest, scan its entries for some referencing
             # jar: urls. If there are some, the files contained in the jar they
@@ -172,23 +174,23 @@ class UnpackFinder(BaseFinder):
         return mozpath.join(base, jar), entry
 
 
-def unpack_to_registry(source, registry):
+def unpack_to_registry(source, registry, omnijar_name=None):
     '''
     Transform a jar chrome or omnijar packaged directory into a flat package.
 
     The given registry is filled with the flat package.
     '''
-    finder = UnpackFinder(source)
+    finder = UnpackFinder(source, omnijar_name)
     packager = SimplePackager(FlatFormatter(registry))
     for p, f in finder.find('*'):
         packager.add(p, f)
     packager.close()
 
 
-def unpack(source):
+def unpack(source, omnijar_name=None):
     '''
     Transform a jar chrome or omnijar packaged directory into a flat package.
     '''
     copier = FileCopier()
-    unpack_to_registry(source, copier)
+    unpack_to_registry(source, copier, omnijar_name)
     copier.copy(source, skip_if_older=False)

python/mozbuild/mozpack/test/test_packager_formats.py

@@ -238,6 +238,11 @@ RESULT_OMNIJAR['omni.foo'].update({
     )
 })
 
+RESULT_OMNIJAR_WITH_SUBPATH = {
+    k.replace('omni.foo', 'bar/omni.foo'): v
+    for k, v in RESULT_OMNIJAR.items()
+}
+
 CONTENTS_WITH_BASE = {
     'bases': {
         mozpath.join('base/root', b) if b else 'base/root': a
@@ -367,6 +372,14 @@ class TestFormatters(TestErrors, unittest.TestCase):
         self.assertEqual(get_contents(registry), RESULT_OMNIJAR_WITH_BASE)
         self.do_test_contents(formatter, CONTENTS_WITH_BASE)
 
+    def test_omnijar_formatter_with_subpath(self):
+        registry = FileRegistry()
+        formatter = OmniJarFormatter(registry, 'bar/omni.foo')
+
+        fill_formatter(formatter, CONTENTS)
+        self.assertEqual(get_contents(registry), RESULT_OMNIJAR_WITH_SUBPATH)
+        self.do_test_contents(formatter, CONTENTS)
+
     def test_omnijar_is_resource(self):
         def is_resource(base, path):
             registry = FileRegistry()

python/mozbuild/mozpack/test/test_packager_unpack.py

@@ -44,7 +44,8 @@ class TestUnpack(TestWithTmpDir):
         # Unpack that package. Its content is expected to match that of a Flat
         # formatted package.
         registry = FileRegistry()
-        unpack_to_registry(self.tmpdir, registry)
+        unpack_to_registry(self.tmpdir, registry,
+                           getattr(cls, 'OMNIJAR_NAME', None))
         self.assertEqual(get_contents(registry, read_all=True), self.contents)
 
     def test_flat_unpack(self):
@@ -53,12 +54,19 @@ class TestUnpack(TestWithTmpDir):
     def test_jar_unpack(self):
         self._unpack_test(JarFormatter)
 
-    def test_omnijar_unpack(self):
+    @staticmethod
+    def _omni_foo_formatter(name):
         class OmniFooFormatter(OmniJarFormatter):
+            OMNIJAR_NAME = name
             def __init__(self, registry):
-                super(OmniFooFormatter, self).__init__(registry, 'omni.foo')
+                super(OmniFooFormatter, self).__init__(registry, name)
+        return OmniFooFormatter
 
-        self._unpack_test(OmniFooFormatter)
+    def test_omnijar_unpack(self):
+        self._unpack_test(self._omni_foo_formatter('omni.foo'))
+
+    def test_omnijar_subpath_unpack(self):
+        self._unpack_test(self._omni_foo_formatter('bar/omni.foo'))
 
 
 if __name__ == '__main__':

toolkit/moz.configure

@@ -577,12 +577,7 @@ def omnijar_name(toolkit):
     # Fennec's static resources live in the assets/ folder of the
     # APK.  Adding a path to the name here works because we only
     # have one omnijar file in the final package (which is not the
-    # case on desktop), and necessitates some contortions during
-    # packaging so that the resources in the omnijar are considered
-    # as rooted at / and not as rooted at assets/ (which again is
-    # not the case on desktop: there are omnijars rooted at webrtc/,
-    # etc). packager.mk handles changing the rooting of the single
-    # omnijar.
+    # case on desktop).
     return 'assets/omni.ja' if toolkit == 'android' else 'omni.ja'
 
 set_config('OMNIJAR_NAME', omnijar_name)

toolkit/mozapps/installer/packager.mk

@@ -19,11 +19,7 @@ endif
 
 export USE_ELF_HACK ELF_HACK_FLAGS
 
-# Override the value of OMNIJAR_NAME from config.status with the value
-# set earlier in this file.
-
 stage-package: multilocale.txt locale-manifest.in $(MOZ_PKG_MANIFEST) $(MOZ_PKG_MANIFEST_DEPS)
-	OMNIJAR_NAME=$(OMNIJAR_NAME) \
 	NO_PKG_FILES="$(NO_PKG_FILES)" \
 	$(PYTHON) $(MOZILLA_DIR)/toolkit/mozapps/installer/packager.py $(DEFINES) $(ACDEFINES) \
 		--format $(MOZ_PACKAGER_FORMAT) \

toolkit/mozapps/installer/unpack.py

@@ -2,6 +2,7 @@
 # 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/.
 
+import argparse
 import sys
 import os
 from mozpack.packager.unpack import unpack
@@ -9,14 +10,16 @@ import buildconfig
 
 
 def main():
-    if len(sys.argv) != 2:
-        print >>sys.stderr, "Usage: %s directory" % \
-                            os.path.basename(sys.argv[0])
-        sys.exit(1)
+    parser = argparse.ArgumentParser(
+        description='Unpack a Gecko-based application')
+    parser.add_argument('directory', help='Location of the application')
+    parser.add_argument('--omnijar', help='Name of the omnijar')
+
+    options = parser.parse_args(sys.argv[1:])
 
     buildconfig.substs['USE_ELF_HACK'] = False
     buildconfig.substs['PKG_SKIP_STRIP'] = True
-    unpack(sys.argv[1])
+    unpack(sys.argv[1], options.omnijar)
 
 if __name__ == "__main__":
     main()

toolkit/mozapps/installer/upload-files-APK.mk

@@ -57,19 +57,6 @@ INNER_ROBOCOP_PACKAGE=echo 'Testing is disabled - No Android Robocop for you'
 endif
 
 
-# Fennec's OMNIJAR_NAME can include a directory; for example, it might
-# be "assets/omni.ja". This path specifies where the omni.ja file
-# lives in the APK, but should not root the resources it contains
-# under assets/ (i.e., resources should not live at chrome://assets/).
-# packager.py writes /omni.ja in order to be consistent with the
-# layout expected by language repacks. Therefore, we move it to the
-# correct path here, in INNER_MAKE_PACKAGE. See comment about
-# OMNIJAR_NAME in configure.in.
-
-# OMNIJAR_DIR is './' for "omni.ja", 'assets/' for "assets/omni.ja".
-OMNIJAR_DIR := $(dir $(OMNIJAR_NAME))
-OMNIJAR_NAME := $(notdir $(OMNIJAR_NAME))
-
 # We force build an ap_ that does not check dependencies below.
 # Language repacks take advantage of this unchecked dependency ap_ to
 # insert additional resources (translated strings) into the ap_
@@ -109,17 +96,7 @@ repackage_fennec = \
 
 INNER_MAKE_PACKAGE = $(if $(UNPACKAGE),$(repackage_fennec),$(package_fennec))
 
-# Language repacks root the resources contained in assets/omni.ja
-# under assets/, but the repacks expect them to be rooted at /.
-# Therefore, we we move the omnijar back to / so the resources are
-# under the root here, in INNER_UNMAKE_PACKAGE. See comments about
-# OMNIJAR_NAME earlier in this file and in configure.in.
-
 INNER_UNMAKE_PACKAGE = \
   mkdir $(MOZ_PKG_DIR) && \
   ( cd $(MOZ_PKG_DIR) && \
-    $(UNZIP) $(UNPACKAGE) $(ROOT_FILES) && \
-    $(UNZIP) $(UNPACKAGE) $(OMNIJAR_DIR)$(OMNIJAR_NAME) && \
-    $(if $(filter-out ./,$(OMNIJAR_DIR)), \
-      mv $(OMNIJAR_DIR)$(OMNIJAR_NAME) $(OMNIJAR_NAME), \
-      true) )
+    $(UNZIP) $(UNPACKAGE) $(ROOT_FILES) $(OMNIJAR_NAME) )