Bug 1443208 - Express Fennec APK with GENERATED_FILES. r=ted.mielczarek

This small change is actually very significant.  Previously, |mach
package| for mobile/android had two jobs:

1) produce a final APK

2) rebuild parts of the APK that might have been silently modified by
   l10n mechanisms, both from multi-locale builds and single-locale
   repacks

This second part has never been sensible but has been difficult to
alter until recently, since the l10n mechanisms have been out of
mozilla-central and difficult to modify and test.  That's less true
now.

This patch:

a) removes the rebuild parts (the step labeled 2) above (which I
   generally refer to as the "nodeps mechanism")

b) uses the APKs produced by Gradle directly, without the copying
   indirection from m/a/base/Makefile.in

c) does the rebuild for multi-locale builds as an explicit step in the
   appropriate mozharness script

d) does the rebuild for each single-locale repack as another step in
   the existing `installers-%` target in m/a/locales/Makefile.in (it's
   not easy to remove this from the Makefile, since the repackage is
   invoked immediately after (it's the `repackage-zip-$*` target))

The new m/a/gradle.py file will grow additional tasks in tickets to
follow, hence the lock file and pre-factored form.

MozReview-Commit-ID: IKflLdmHR3P

--HG--
extra : rebase_source : fdabe340b6f0896a0ebb9da2951f10753deb5ff5

mobile/android/base/Makefile.in

@@ -43,50 +43,14 @@ chrome-%::
 	  res/raw$(AB_rCD)/browsersearch.json \
 	  AB_CD=$*
 
-gradle_dir := $(topobjdir)/gradle/build/mobile/android
-
-define gradle_command
-$(1): $(2)
-	@$$(TOUCH) $$@
-	$$(topsrcdir)/mach android assemble-app
-endef
-
-# .gradle.deps: $(generated_resources) $(generated_files) FORCE
-$(eval $(call gradle_command,.gradle.deps,$(generated_resources) $(generated_files) FORCE))
-
-GeneratedJNIWrappers.cpp GeneratedJNIWrappers.h GeneratedJNINatives.h : .gradle.deps
+GeneratedJNIWrappers.cpp GeneratedJNIWrappers.h GeneratedJNINatives.h : android_apks
 	$(REPORT_BUILD)
 
-FennecJNIWrappers.cpp FennecJNIWrappers.h FennecJNINatives.h: .gradle.deps
+FennecJNIWrappers.cpp FennecJNIWrappers.h FennecJNINatives.h: android_apks
 	$(REPORT_BUILD)
 
 include $(topsrcdir)/config/rules.mk
 
-gecko.ap_: .gradle.deps ;
-R.txt: .gradle.deps ;
-
-# This tom-foolery provides a target that forces a rebuild of
-# gecko.ap_.  This is used during packaging to ensure that resources
-# are fresh.  The alternative would be complicated.
-
-gecko-nodeps.ap_: .gradle.nodeps
-	cp $(GRADLE_ANDROID_APP_APK) $@
-
-gecko-nodeps/R.txt: .gradle.nodeps ;
-
-# The first of these rules is used during regular builds.  The second
-# writes an ap_ file that is only used during packaging.  It doesn't
-# write the normal ap_, or R.java, since we don't want the packaging
-# step to write anything that would make a further no-op build do
-# work.  See also toolkit/mozapps/installer/packager.mk.
-
-# It's not quite "no dependencies": nodeps means that it doesn't
-# depend on the generated resources that incorporate l10n, principally
-# strings.xml.
-
-# .gradle.nodeps: AndroidManifest.xml generated/preprocessed/org/mozilla/gecko/AppConstants.java ... FORCE
-$(eval $(call gradle_command,.gradle.nodeps,AndroidManifest.xml $(generated_files) FORCE))
-
 # Override the Java settings with some specific android settings
 include $(topsrcdir)/config/android-common.mk
 
@@ -119,9 +83,9 @@ $(ABS_DIST)/fennec/$(OMNIJAR_NAME): FORCE
 
 ifndef MOZILLA_OFFICIAL
 # Targets built very early during a Gradle build.  In automation,
-# these are built before Gradle is invoked by .gradle.deps and
-# gradle-targets is not made at all.  This is required to avoid
-# building gradle-targets with AB_CD=multi during multi-l10n builds.
+# these are built before Gradle is invoked, and gradle-targets is not
+# made at all.  This is required to avoid building gradle-targets with
+# AB_CD=multi during multi-l10n builds.
 gradle-targets: $(generated_resources) $(generated_files)
 
 # Local developers update omni.ja during their builds.  There's a

mobile/android/base/moz.build

@@ -193,3 +193,27 @@ for f in ['res/values/strings.xml',
         'locales/en-US/android_strings.dtd',
         'locales/en-US/sync_strings.dtd',
     ]
+
+# The recursive make backend treats the first output specially: it's passed as
+# an open FileAvoidWrite to the invoked script.  That doesn't work well with
+# the Gradle task that generates all of the outputs, so we add a dummy first
+# output.
+t = ('android_apks',
+     CONFIG['GRADLE_ANDROID_APP_APK'],
+     CONFIG['GRADLE_ANDROID_APP_ANDROIDTEST_APK'])
+
+GENERATED_FILES += [t]
+GENERATED_FILES[t].force = True
+GENERATED_FILES[t].script = '/mobile/android/gradle.py:assemble_app'
+GENERATED_FILES[t].inputs += [
+    '!AndroidManifest.xml',
+    '!generated/preprocessed/org/mozilla/gecko/AdjustConstants.java',
+    '!generated/preprocessed/org/mozilla/gecko/AppConstants.java',
+    '!generated/preprocessed/org/mozilla/gecko/MmaConstants.java',
+    # These all depend on AB_CD, which isn't captured in this definition.  Due
+    # to subtle RecursiveMake details, everything works out.  In the future we
+    # can try to express the APKs themselves as LOCALIZED_GENERATED_FILES.
+    '!res/raw/browsersearch.json',
+    '!res/raw/suggestedsites.json',
+    '!res/values/strings.xml',
+]

mobile/android/gradle.py

@@ -0,0 +1,39 @@
+# 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/.
+
+from __future__ import print_function
+
+import buildconfig
+import subprocess
+
+from mozbuild.util import (
+    ensureParentDir,
+    lock_file,
+)
+import mozpack.path as mozpath
+
+
+def android(verb, *args):
+    # Building the same Gradle root project with multiple concurrent processes
+    # is not well supported, so we use a simple lock file to serialize build
+    # steps.
+    lock_path = '{}/gradle/mach_android.lockfile'.format(buildconfig.topobjdir)
+    ensureParentDir(lock_path)
+    lock_instance = lock_file(lock_path)
+    try:
+        cmd = [
+            mozpath.join(buildconfig.topsrcdir, 'mach'),
+            'android',
+            verb,
+        ]
+        cmd.extend(args)
+        subprocess.check_call(cmd)
+
+        return 0
+    finally:
+        del lock_instance
+
+
+def assemble_app(dummy_output_file, *inputs):
+    return android('assemble-app')

mobile/android/locales/Makefile.in

@@ -72,6 +72,7 @@ installers-%: IS_LANGUAGE_REPACK=1
 installers-%:
 	$(MAKE) clobber-stage
 	$(MAKE) libs-$*
+	$(MAKE) -C $(DEPTH)/mobile/android/base android_apks
 	$(MAKE) package-langpack-$*
 	$(MAKE) repackage-zip-$*
 	@echo 'repackaging done'

testing/mozharness/configs/multi_locale/standalone_mozilla-central.py

@@ -10,8 +10,6 @@ REPO_PATH = "mozilla-central"
 L10N_REPO_PATH = "l10n-central"
 # Currently this is assumed to be a subdirectory of your build dir
 OBJDIR = "objdir-droid"
-# Set this to mobile/xul for XUL Fennec
-ANDROID_DIR = "mobile/android"
 # Absolute path to your mozconfig.
 # By default it looks at "./mozconfig"
 MOZCONFIG = os.path.join(os.getcwd(), "mozconfig")
@@ -22,7 +20,7 @@ config = {
     "objdir": OBJDIR,
     "locales_file": "%s/mobile/locales/l10n-changesets.json" % BUILD_DIR,
     "locales_platform": "android-multilocale",
-    "locales_dir": "%s/locales" % ANDROID_DIR,
+    "locales_dir": "mobile/android/locales",
     "ignore_locales": ["en-US", "multi"],
     "repos": [{
         "repo": "https://hg.mozilla.org/%s" % REPO_PATH,
@@ -43,6 +41,7 @@ config = {
         "backup-objdir",
         "restore-objdir",
         "add-locales",
+        "android-assemble-app",
         "package-multi",
         "summary",
     ],

testing/mozharness/mozharness/mozilla/building/buildbase.py

@@ -1343,6 +1343,7 @@ or run without that action (ie: --no-{action})"
             'multi_locale/android-mozharness-build.json',
             '--pull-locale-source',
             '--add-locales',
+            '--android-assemble-app',
             '--package-multi',
             '--summary',
         ]

testing/mozharness/mozharness/mozilla/l10n/multi_locale_build.py

@@ -83,14 +83,19 @@ class MultiLocaleBuild(LocalesMixin, MercurialScript):
     def __init__(self, require_config_file=True):
         LocalesMixin.__init__(self)
         MercurialScript.__init__(self, config_options=self.config_options,
-                                 all_actions=['clobber', 'pull-build-source',
+                                 all_actions=['clobber',
+                                              'pull-build-source',
                                               'pull-locale-source',
-                                              'build', 'package-en-US',
+                                              'build',
+                                              'package-en-US',
                                               'upload-en-US',
                                               'backup-objdir',
                                               'restore-objdir',
-                                              'add-locales', 'package-multi',
-                                              'upload-multi', 'summary'],
+                                              'add-locales',
+                                              'android-assemble-app',
+                                              'package-multi',
+                                              'upload-multi',
+                                              'summary'],
                                  require_config_file=require_config_file)
 
     def query_l10n_env(self):
@@ -135,6 +140,16 @@ class MultiLocaleBuild(LocalesMixin, MercurialScript):
                                  env=env, error_list=MakefileErrorList):
             self.fatal("Erroring out after the build failed.")
 
+    def android_assemble_app(self):
+        dirs = self.query_abs_dirs()
+
+        command = 'make -C mobile/android/base android_apks'
+        env = self.query_env()
+        if self._process_command(command=command,
+                                 cwd=dirs['abs_objdir'],
+                                 env=env, error_list=MakefileErrorList):
+            self.fatal("Erroring out after assembling Android APKs failed.")
+
     def add_locales(self):
         c = self.config
         dirs = self.query_abs_dirs()

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

@@ -19,8 +19,6 @@ ROOT_FILES := \
   removed-files \
   $(NULL)
 
-GECKO_APP_AP_PATH = $(topobjdir)/mobile/android/base
-
 ifdef ENABLE_TESTS
 INNER_ROBOCOP_PACKAGE=true
 ifeq ($(MOZ_BUILD_APP),mobile/android)
@@ -59,11 +57,9 @@ OMNIJAR_NAME := $(notdir $(OMNIJAR_NAME))
 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_ \
+    --inputs $(GRADLE_ANDROID_APP_APK) \
     --omnijar $(MOZ_PKG_DIR)/$(OMNIJAR_NAME) \
     --lib-dirs $(MOZ_PKG_DIR)/lib \
     --assets-dirs $(MOZ_PKG_DIR)/assets \
@@ -80,12 +76,11 @@ package_fennec = \
 # Re-packaging only replaces Android resources and the omnijar before
 # (re-)signing.
 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_ \
+      $(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))