Bug 1433285: Extract classes*.dex from .ap_ --with-gradle. r=Build,snorp draft
authorNick Alexander <nalexander@mozilla.com>
Wed, 24 Jan 2018 22:09:48 -0800
changeset 747776 a47154f4840d4dfb1fd1d0628258b1f94900e567
parent 747775 27b20570829c64552fc1d645bf5aa45950110b7a
push id97003
push usernalexander@mozilla.com
push dateFri, 26 Jan 2018 20:25:36 +0000
reviewersBuild, snorp
bugs1433285, 1260241
milestone60.0a1
Bug 1433285: Extract classes*.dex from .ap_ --with-gradle. r=Build,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