Bug 1531634 - Change how OMNIJAR_NAME is handled for fennec builds. r=nalexander
authorMike Hommey <mh+mozilla@glandium.org>
Fri, 01 Mar 2019 21:49:47 +0000
changeset 519912 339a645ac6f0cb55000280d6ff544c6d8a733680
parent 519911 85fce02180b629cd6db4261c5730b61f5dc5a53e
child 519913 159c31166c7a5ddcd61191729d1247484123de9d
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1531634
milestone67.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 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
python/mozbuild/mozpack/packager/unpack.py
python/mozbuild/mozpack/test/test_packager_formats.py
python/mozbuild/mozpack/test/test_packager_unpack.py
toolkit/moz.configure
toolkit/mozapps/installer/packager.mk
toolkit/mozapps/installer/unpack.py
toolkit/mozapps/installer/upload-files-APK.mk
--- a/python/mozbuild/mozpack/packager/unpack.py
+++ b/python/mozbuild/mozpack/packager/unpack.py
@@ -35,47 +35,49 @@ class UnpackFinder(BaseFinder):
     This means that for example, paths like chrome/browser/content/... match
     files under jar:chrome/browser.jar!/content/... in case of jar chrome
     format.
 
     The only argument to the constructor is a Finder instance or a path.
     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:
             self._finder = FileFinder(source)
         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
 
         jars = set()
 
         for p, f in self._finder.find('*'):
             # Skip the precomplete file, which is generated at packaging time.
             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
             # point to, go under a directory named after the jar.
             if is_manifest(p):
                 m = self.files[p] if self.files.contains(p) \
                     else ManifestFile(base)
                 for e in parse_manifest(self.base, p, f.open()):
@@ -167,28 +169,28 @@ class UnpackFinder(BaseFinder):
         base = entry.base
         jar, relpath = urlparse(relpath).path.split('!', 1)
         entry = entry.rebase(mozpath.join(base, 'jar:%s!' % jar)) \
             .move(mozpath.join(base, mozpath.splitext(jar)[0])) \
             .rebase(base)
         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)
--- a/python/mozbuild/mozpack/test/test_packager_formats.py
+++ b/python/mozbuild/mozpack/test/test_packager_formats.py
@@ -233,16 +233,21 @@ RESULT_OMNIJAR['omni.foo'].update({
         'chrome/f/oo/bar/baz',
         'chrome/f/oo/baz',
         'chrome/f/oo/qux',
         'components/foo.xpt',
         'components/bar.xpt',
     )
 })
 
+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
         for b, a in CONTENTS['bases'].iteritems()
     },
     'manifests': [
         m.move(mozpath.join('base/root', m.base))
         for m in CONTENTS['manifests']
@@ -362,16 +367,24 @@ class TestFormatters(TestErrors, unittes
     def test_omnijar_formatter_with_base(self):
         registry = FileRegistry()
         formatter = OmniJarFormatter(registry, 'omni.foo')
 
         fill_formatter(formatter, CONTENTS_WITH_BASE)
         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()
             f = OmniJarFormatter(registry, 'omni.foo', non_resources=[
                 'defaults/messenger/mailViews.dat',
                 'defaults/foo/*',
                 '*/dummy',
             ])
--- a/python/mozbuild/mozpack/test/test_packager_unpack.py
+++ b/python/mozbuild/mozpack/test/test_packager_unpack.py
@@ -39,27 +39,35 @@ class TestUnpack(TestWithTmpDir):
     def _unpack_test(self, cls):
         # Format a package with the given formatter class
         copier = self._get_copier(cls)
         copier.copy(self.tmpdir)
 
         # 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):
         self._unpack_test(FlatFormatter)
 
     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__':
     mozunit.main()
--- a/toolkit/moz.configure
+++ b/toolkit/moz.configure
@@ -572,22 +572,17 @@ def jar_maker_format(host, build_project
 
 set_config('MOZ_JAR_MAKER_FILE_FORMAT', jar_maker_format)
 
 @depends(toolkit)
 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)
 
 project_flag('MOZ_PLACES',
              help='Build Places if required',
              set_as_define=True)
 
--- a/toolkit/mozapps/installer/packager.mk
+++ b/toolkit/mozapps/installer/packager.mk
@@ -14,21 +14,17 @@ libs:: make-package
 endif
 
 ifdef MOZ_AUTOMATION
 RUN_FIND_DUPES ?= $(MOZ_AUTOMATION)
 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) \
 		$(addprefix --removals ,$(MOZ_PKG_REMOVALS)) \
 		$(if $(filter-out 0,$(MOZ_PKG_FATAL_WARNINGS)),,--ignore-errors) \
 		$(if $(MOZ_PACKAGER_MINIFY),--minify) \
 		$(if $(MOZ_PACKAGER_MINIFY_JS),--minify-js \
 		  $(addprefix --js-binary ,$(JS_BINARY)) \
--- a/toolkit/mozapps/installer/unpack.py
+++ b/toolkit/mozapps/installer/unpack.py
@@ -1,22 +1,25 @@
 # 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/.
 
+import argparse
 import sys
 import os
 from mozpack.packager.unpack import unpack
 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()
--- a/toolkit/mozapps/installer/upload-files-APK.mk
+++ b/toolkit/mozapps/installer/upload-files-APK.mk
@@ -52,29 +52,16 @@ UPLOAD_EXTRA_FILES += robocop.apk
 INNER_ROBOCOP_PACKAGE= \
   $(call RELEASE_SIGN_ANDROID_APK,$(GRADLE_ANDROID_APP_ANDROIDTEST_APK),$(ABS_DIST)/robocop.apk)
 endif
 else
 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_
 # without the build system's participation.  This can do the wrong
 # thing if there are resource changes in between build time and
 # package time.
 PKG_SUFFIX = .apk
 
@@ -104,22 +91,12 @@ repackage_fennec = \
       $(UNPACKAGE) \
       $(GRADLE_ANDROID_APP_APK) \
     --omnijar $(MOZ_PKG_DIR)/$(OMNIJAR_NAME) \
     --output $(PACKAGE:.apk=-unsigned-unaligned.apk) && \
   $(call RELEASE_SIGN_ANDROID_APK,$(PACKAGE:.apk=-unsigned-unaligned.apk),$(PACKAGE))
 
 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) )