Bug 1433285: Extract classes*.dex from .ap_ --with-gradle. r=gps,snorp
authorNick Alexander <nalexander@mozilla.com>
Wed, 24 Jan 2018 22:09:48 -0800
changeset 401508 53fa60c5e8b41f30e410f8cb5e7611fb08c8fc6f
parent 401507 2afca7ece7b741ef5a1ef51c355d3ef44b943469
child 401509 7297a1e8c4ff85c1c9d64abbacc11d59f99bee3e
push id33346
push useraiakab@mozilla.com
push dateTue, 30 Jan 2018 21:59:39 +0000
treeherdermozilla-central@5fc179e245d5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps, snorp
bugs1433285, 1260241
milestone60.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 1433285: Extract classes*.dex from .ap_ --with-gradle. r=gps,snorp Right now, we only expect classes.dex, and even --with-gradle we copy it out of $topobjdir/mobile/android/base. This commit changes that for --with-gradle: we only take classes.dex from the given .ap_ file, and we also handle multiple classesN.dex files (for future multi-dex support). The moz.build system stays the same. This avoids an issue with newer Android-Gradle plugins, where the classes.dex produced could be either in dex/ or in dexMerger/, depending on whether any external libraries needed merging. By extracting classes.dex from the .ap_ file, we don't need to know what Gradle build steps actually occur. The classes.dex in the package-manifest.in has been irrelevant since Bug 1260241. MozReview-Commit-ID: FozKwjTcMzU
mobile/android/base/Makefile.in
mobile/android/installer/package-manifest.in
python/mozbuild/mozbuild/action/package_fennec_apk.py
toolkit/mozapps/installer/upload-files-APK.mk
--- a/mobile/android/base/Makefile.in
+++ b/mobile/android/base/Makefile.in
@@ -247,27 +247,23 @@ define gradle_command
 $(1): $(2)
 	@$$(TOUCH) $$@
 	$$(topsrcdir)/mach android assemble-app
 endef
 
 # .gradle.deps: .aapt.deps FORCE
 $(eval $(call gradle_command,.gradle.deps,.aapt.deps FORCE))
 
-classes.dex: .gradle.deps
-	$(REPORT_BUILD)
-	cp $(gradle_dir)/app/intermediates/transforms/dex/officialPhoton/debug/folders/1000/1f/main/classes.dex $@
-
 GeneratedJNIWrappers.cpp GeneratedJNIWrappers.h GeneratedJNINatives.h : .gradle.deps
 	$(REPORT_BUILD)
 
 FennecJNIWrappers.cpp FennecJNIWrappers.h FennecJNINatives.h: .gradle.deps
 	$(REPORT_BUILD)
 
-else
+else # !MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE
 classes.dex: .proguard.deps
 	$(REPORT_BUILD)
 	$(DX) --dex --output=classes.dex --force-jumbo jars-proguarded
 
 ifdef MOZ_DISABLE_PROGUARD
   PROGUARD_PASSES=0
 else
   ifdef MOZ_DEBUG
@@ -496,17 +492,16 @@ ifdef MOZ_BUILD_MOBILE_ANDROID_WITH_GRAD
 # strings.xml.
 
 # .gradle.nodeps: AndroidManifest.xml generated/preprocessed/org/mozilla/gecko/AppConstants.java ... FORCE
 $(eval $(call gradle_command,.gradle.nodeps,AndroidManifest.xml $(constants_PP_JAVAFILES) FORCE))
 
 .aapt.nodeps: .gradle.nodeps FORCE
 	@$(TOUCH) $@
 	cp $(GRADLE_ANDROID_APP_APK) gecko-nodeps.ap_
-	cp $(gradle_dir)/app/intermediates/transforms/dex/officialPhoton/debug/folders/1000/1f/main/classes.dex classes.dex
 else
 # .aapt.nodeps: AndroidManifest.xml FORCE
 $(eval $(call aapt_command,.aapt.nodeps,AndroidManifest.xml FORCE,gecko-nodeps.ap_,gecko-nodeps/,gecko-nodeps/))
 endif
 
 # Override the Java settings with some specific android settings
 include $(topsrcdir)/config/android-common.mk
 
@@ -612,18 +607,20 @@ libs:: FennecJNIWrappers.cpp
 	  echo '* To update generated code in the tree, please run  *' && \
 	  echo && \
 	  echo '  make -C $(CURDIR) update-fennec-wrappers' && \
 	  echo && \
 	  echo '* Repeat the build, and check in any changes.       *' && \
 	  echo '*****************************************************' && \
 	  exit 1)
 
+ifndef MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE
 libs:: classes.dex
 	$(INSTALL) classes.dex $(FINAL_TARGET)
+endif # MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE
 
 ifndef MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE
 
 # Generate Java binder interfaces from AIDL files.
 GECKOVIEW_AIDLS = \
   org/mozilla/gecko/IGeckoEditableChild.aidl \
   org/mozilla/gecko/IGeckoEditableParent.aidl \
   org/mozilla/gecko/gfx/ISurfaceAllocator.aidl \
--- a/mobile/android/installer/package-manifest.in
+++ b/mobile/android/installer/package-manifest.in
@@ -76,17 +76,16 @@
 @BINPATH@/@MOZ_CHILD_PROCESS_NAME@
 
 #ifdef MOZ_ANDROID_GOOGLE_VR
 @BINPATH@/@DLL_PREFIX@gvr@DLL_SUFFIX@
 #endif
 
 [xpcom]
 @BINPATH@/package-name.txt
-@BINPATH@/classes.dex
 
 [browser]
 ; [Base Browser Files]
 @BINPATH@/application.ini
 @BINPATH@/platform.ini
 @BINPATH@/blocklist.xml
 
 ; [Components]
--- a/python/mozbuild/mozbuild/action/package_fennec_apk.py
+++ b/python/mozbuild/mozbuild/action/package_fennec_apk.py
@@ -28,21 +28,44 @@ def package_fennec_apk(inputs=[], omni_j
                        lib_dirs=[],
                        assets_dirs=[],
                        features_dirs=[],
                        root_files=[],
                        verbose=False):
     jarrer = Jarrer(optimize=False)
 
     # First, take input files.  The contents of the later files overwrites the
-    # content of earlier files.
+    # content of earlier files.  Multidexing requires special care: we want a
+    # coherent set of classesN.dex files, so we only take DEX files from a
+    # single input.  This avoids taking, say, classes{1,2,3}.dex from the first
+    # input and only classes{1,2}.dex from the second input, leading to
+    # (potentially) duplicated symbols at runtime.
+    last_input_with_dex_files = None
     for input in inputs:
         jar = JarReader(input)
         for file in jar:
             path = file.filename
+
+            if mozpath.match(path, '/classes*.dex'):
+                last_input_with_dex_files = input
+                continue
+
+            if jarrer.contains(path):
+                jarrer.remove(path)
+            jarrer.add(path, DeflatedFile(file), compress=file.compressed)
+
+    # If we have an input with DEX files, take them all here.
+    if last_input_with_dex_files:
+        jar = JarReader(last_input_with_dex_files)
+        for file in jar:
+            path = file.filename
+
+            if not mozpath.match(path, '/classes*.dex'):
+                continue
+
             if jarrer.contains(path):
                 jarrer.remove(path)
             jarrer.add(path, DeflatedFile(file), compress=file.compressed)
 
     def add(path, file, compress=None):
         abspath = os.path.abspath(file.path)
         if verbose:
             print('Packaging %s from %s' % (path, file.path))
@@ -110,16 +133,20 @@ def package_fennec_apk(inputs=[], omni_j
 
     for root_file in root_files:
         add(os.path.basename(root_file), File(root_file))
 
     if omni_ja:
         add(mozpath.join('assets', 'omni.ja'), File(omni_ja), compress=False)
 
     if classes_dex:
+        if buildconfig.substs.get('MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE'):
+            raise ValueError("Fennec APKs built --with-gradle "
+                             "should never specify classes.dex")
+
         add('classes.dex', File(classes_dex))
 
     return jarrer
 
 
 def main(args):
     parser = argparse.ArgumentParser()
     parser.add_argument('--verbose', '-v', default=False, action='store_true',
@@ -138,17 +165,17 @@ def main(args):
                         help='Optional assets/ dirs to pack into APK file.')
     parser.add_argument('--features-dirs', nargs='*', default=[],
                         help='Optional features/ dirs to pack into APK file.')
     parser.add_argument('--root-files', nargs='*', default=[],
                         help='Optional files to pack into APK file root.')
     args = parser.parse_args(args)
 
     if buildconfig.substs.get('OMNIJAR_NAME') != 'assets/omni.ja':
-        raise ValueError("Don't know how package Fennec APKs when "
+        raise ValueError("Don't know how to package Fennec APKs when "
                          " OMNIJAR_NAME is not 'assets/omni.jar'.")
 
     jarrer = package_fennec_apk(inputs=args.inputs,
                                 omni_ja=args.omnijar,
                                 classes_dex=args.classes_dex,
                                 lib_dirs=args.lib_dirs,
                                 assets_dirs=args.assets_dirs,
                                 features_dirs=args.features_dirs,
--- a/toolkit/mozapps/installer/upload-files-APK.mk
+++ b/toolkit/mozapps/installer/upload-files-APK.mk
@@ -66,17 +66,17 @@ PKG_SUFFIX = .apk
 
 INNER_FENNEC_PACKAGE = \
   $(MAKE) -C $(GECKO_APP_AP_PATH) gecko-nodeps.ap_ && \
   $(PYTHON) -m mozbuild.action.package_fennec_apk \
     --verbose \
     --inputs \
       $(GECKO_APP_AP_PATH)/gecko-nodeps.ap_ \
     --omnijar $(MOZ_PKG_DIR)/$(OMNIJAR_NAME) \
-    --classes-dex $(GECKO_APP_AP_PATH)/classes.dex \
+    $(if $(MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE),,--classes-dex $(GECKO_APP_AP_PATH)/classes.dex) \
     --lib-dirs $(MOZ_PKG_DIR)/lib \
     --assets-dirs $(MOZ_PKG_DIR)/assets \
     --features-dirs $(MOZ_PKG_DIR)/features \
     --root-files $(foreach f,$(ROOT_FILES),$(MOZ_PKG_DIR)/$(f)) \
     --output $(PACKAGE:.apk=-unsigned-unaligned.apk) && \
   $(call RELEASE_SIGN_ANDROID_APK,$(PACKAGE:.apk=-unsigned-unaligned.apk),$(PACKAGE))
 
 # Packaging produces many optional artifacts.
@@ -89,17 +89,17 @@ package_fennec = \
 repackage_fennec = \
   $(MAKE) -C $(GECKO_APP_AP_PATH) gecko-nodeps.ap_ && \
   $(PYTHON) -m mozbuild.action.package_fennec_apk \
     --verbose \
     --inputs \
       $(UNPACKAGE) \
       $(GECKO_APP_AP_PATH)/gecko-nodeps.ap_ \
     --omnijar $(MOZ_PKG_DIR)/$(OMNIJAR_NAME) \
-    --classes-dex $(GECKO_APP_AP_PATH)/classes.dex \
+    $(if $(MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE),,--classes-dex $(GECKO_APP_AP_PATH)/classes.dex) \
     --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