Bug 1443208 - Express Fennec APK with GENERATED_FILES. r?ted.mielczarek draft
authorNick Alexander <nalexander@mozilla.com>
Tue, 20 Mar 2018 12:41:49 -0700
changeset 770124 ad81361c6616eb89893fac22c75c0eaa6ad3d645
parent 770123 d79cbc50d50457505f12025055844a53d67f6eb5
push id103337
push usernalexander@mozilla.com
push dateTue, 20 Mar 2018 20:05:24 +0000
reviewersted.mielczarek
bugs1443208
milestone61.0a1
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
mobile/android/base/Makefile.in
mobile/android/base/moz.build
mobile/android/gradle.py
mobile/android/locales/Makefile.in
testing/mozharness/configs/multi_locale/standalone_mozilla-central.py
testing/mozharness/mozharness/mozilla/building/buildbase.py
testing/mozharness/mozharness/mozilla/l10n/multi_locale_build.py
toolkit/mozapps/installer/upload-files-APK.mk
--- a/mobile/android/base/Makefile.in
+++ b/mobile/android/base/Makefile.in
@@ -38,60 +38,24 @@ include $(topsrcdir)/config/AB_rCD.mk
 chrome-%:: AB_CD=$*
 chrome-%::
 	$(MAKE) \
 	  res/values$(AB_rCD)/strings.xml \
 	  res/raw$(AB_rCD)/suggestedsites.json \
 	  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
 
 update-generated-wrappers:
 	@cp $(CURDIR)/GeneratedJNIWrappers.cpp \
 	    $(CURDIR)/GeneratedJNIWrappers.h \
 	    $(CURDIR)/GeneratedJNINatives.h $(topsrcdir)/widget/android
 	@echo Updated generated JNI code
@@ -114,19 +78,19 @@ update-fennec-wrappers:
 	$(MAKE) -C ../../../faster
 	$(MAKE) -C ../installer stage-package
 	$(MKDIR) -p $(@D)
 	rsync --update $(DIST)/fennec/$(notdir $(OMNIJAR_NAME)) $@
 	$(RM) $(DIST)/fennec/$(notdir $(OMNIJAR_NAME))
 
 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
 # chicken-and-egg problem here.
 gradle-omnijar: $(abspath $(DIST)/fennec/$(OMNIJAR_NAME))
 else
 # In automation, omni.ja is built only during packaging.
 gradle-omnijar:
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -188,8 +188,32 @@ for f in ['res/values/strings.xml',
     strings = LOCALIZED_GENERATED_FILES[f]
     strings.script = '/python/mozbuild/mozbuild/action/generate_strings_xml.py'
     strings.inputs = [
         'strings.xml.in',
         # The `locales/en-US/` will be rewritten to the locale-specific path.
         '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',
+]
new file mode 100644
--- /dev/null
+++ b/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')
--- a/mobile/android/locales/Makefile.in
+++ b/mobile/android/locales/Makefile.in
@@ -67,16 +67,17 @@ langpack: langpack-$(AB_CD)
 
 # This is a generic target that will make a langpack and repack tarball
 # builds. It is called from the tinderbox scripts. Alter it with caution.
 
 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'
 
 # When we unpack fennec on MacOS X the platform.ini and application.ini are in slightly
 # different locations that on all other platforms
 ifeq (Darwin, $(OS_ARCH))
 GECKO_PLATFORM_INI_PATH='$(STAGEDIST)/platform.ini'
--- a/testing/mozharness/configs/multi_locale/standalone_mozilla-central.py
+++ b/testing/mozharness/configs/multi_locale/standalone_mozilla-central.py
@@ -5,29 +5,27 @@ BUILD_DIR = "mozilla-central"
 # e.g. "releases/mozilla-aurora"
 REPO_PATH = "mozilla-central"
 # This is where the l10n repos are (everything after https://hg.mozilla.org/)
 # for mozilla-central, that's "l10n-central".
 # For mozilla-aurora, that's "releases/l10n/mozilla-aurora"
 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")
 
 config = {
     "work_dir": ".",
     "log_name": "multilocale",
     "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,
         "branch": "default",
         "dest": BUILD_DIR,
     }],
     "vcs_share_base": "/builds/hg-shared",
     "l10n_repos": [],
@@ -38,12 +36,13 @@ config = {
     "mozconfig": MOZCONFIG,
     "default_actions": [
         "pull-locale-source",
         "build",
         "package-en-US",
         "backup-objdir",
         "restore-objdir",
         "add-locales",
+        "android-assemble-app",
         "package-multi",
         "summary",
     ],
 }
--- a/testing/mozharness/mozharness/mozilla/building/buildbase.py
+++ b/testing/mozharness/mozharness/mozilla/building/buildbase.py
@@ -1338,16 +1338,17 @@ or run without that action (ie: --no-{ac
             sys.executable,
             multil10n_path,
             '--config-file',
             'multi_locale/%s_%s.json' % (branch, multi_config_pf),
             '--config-file',
             'multi_locale/android-mozharness-build.json',
             '--pull-locale-source',
             '--add-locales',
+            '--android-assemble-app',
             '--package-multi',
             '--summary',
         ]
 
         self.run_command(cmd, env=self.query_build_env(), cwd=base_work_dir,
                          halt_on_failure=True)
 
         package_cmd = [
--- a/testing/mozharness/mozharness/mozilla/l10n/multi_locale_build.py
+++ b/testing/mozharness/mozharness/mozilla/l10n/multi_locale_build.py
@@ -78,24 +78,29 @@ class MultiLocaleBuild(LocalesMixin, Mer
          "default": "l10n",
          "help": "Specify the l10n dir name"
          }
     ]]
 
     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):
         return self.query_env()
 
     def clobber(self):
         c = self.config
         if c['work_dir'] != '.':
@@ -130,16 +135,26 @@ class MultiLocaleBuild(LocalesMixin, Mer
 
         mach = os.path.join(dirs['abs_mozilla_dir'], 'mach')
         env = self.query_env()
         if self._process_command(command=[sys.executable, mach, 'build'],
                                  cwd=dirs['abs_mozilla_dir'],
                                  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()
         locales = self.query_locales()
 
         for locale in locales:
             command = 'make chrome-%s L10NBASEDIR=%s' % (locale, dirs['abs_l10n_dir'])
             status = self._process_command(command=command,
--- a/toolkit/mozapps/installer/upload-files-APK.mk
+++ b/toolkit/mozapps/installer/upload-files-APK.mk
@@ -14,18 +14,16 @@ include $(MOZILLA_DIR)/config/android-co
 ROOT_FILES := \
   application.ini \
   package-name.txt \
   ua-update.json \
   platform.ini \
   removed-files \
   $(NULL)
 
-GECKO_APP_AP_PATH = $(topobjdir)/mobile/android/base
-
 ifdef ENABLE_TESTS
 INNER_ROBOCOP_PACKAGE=true
 ifeq ($(MOZ_BUILD_APP),mobile/android)
 UPLOAD_EXTRA_FILES += robocop.apk
 
 # Robocop/Robotium tests and Fennec need to be signed with the same
 # key, which means release signing them all.
 
@@ -54,43 +52,40 @@ OMNIJAR_NAME := $(notdir $(OMNIJAR_NAME)
 # 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
 
 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 \
     --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.
 package_fennec = \
   $(INNER_FENNEC_PACKAGE) && \
   $(INNER_ROBOCOP_PACKAGE)
 
 # 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))
 
 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 /.