Bug 1509573 - Part 1: Use |mach build ...| rather than special Make target. r=snorp
authorNick Alexander <nalexander@mozilla.com>
Tue, 18 Dec 2018 23:54:08 +0000
changeset 511198 43ddf4c3ae27c4b721151545033e79775894636e
parent 511197 6ba198e2a59e3e4ccfc5adf3638843a004c75f7d
child 511199 6fd3033d7cb7d3b8a58e85f0f1dcd70f16b25306
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1509573
milestone66.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 1509573 - Part 1: Use |mach build ...| rather than special Make target. r=snorp This uses |mach build mobile/android/base/...| rather than a custom target, reducing Make magic and making it a little easier to reason about the Gradle build. This is somewhat rearranging deckchairs, but the more that gets out of Make and into moz.build, the simpler our lives become. The shared `onlyIf` Gradle guard will be used to make it very clear when certain tasks are being skipped, as we move details about Gecko binaries to depend on the Gradle task execution graph. I also took the opportunity to improve the task logging. Differential Revision: https://phabricator.services.mozilla.com/D12798
build.gradle
mobile/android/app/build.gradle
mobile/android/base/Makefile.in
mobile/android/base/moz.build
mobile/android/gradle.py
mobile/android/gradle/with_gecko_binaries.gradle
--- a/build.gradle
+++ b/build.gradle
@@ -85,52 +85,87 @@ buildscript {
     if (gradle.mozconfig.substs.MOZ_ANDROID_GOOGLE_PLAY_SERVICES) {
         ext.google_play_services_version = '15.0.1'
     }
 
     dependencies {
         classpath 'org.mozilla.apilint:apilint:0.1.5'
         classpath 'com.android.tools.build:gradle:3.1.4'
         classpath 'com.getkeepsafe.dexcount:dexcount-gradle-plugin:0.8.2'
+        classpath 'org.apache.commons:commons-exec:1.3'
         classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
     }
 }
 
-if ('multi' == System.env.AB_CD) {
+// A stream that processes bytes line by line, prepending a tag before sending
+// each line to Gradle's logging.
+class TaggedLogOutputStream extends org.apache.commons.exec.LogOutputStream {
+    String tag
+    Logger logger
+
+    TaggedLogOutputStream(tag, logger) {
+        this.tag = tag
+        this.logger = logger
+    }
+
+    void processLine(String line, int level) {
+        logger.lifecycle("${this.tag} ${line}")
+    }
+}
+
+ext.geckoBinariesOnlyIf = { task ->
+    // Never for official builds.
+    if (mozconfig.substs.MOZILLA_OFFICIAL) {
+        rootProject.logger.lifecycle("Skipping task ${task.path} because: MOZILLA_OFFICIAL")
+        return false
+    }
+
     // Multi-l10n builds set `AB_CD=multi`, which isn't a valid locale.  This
     // causes the
     //
-    // |mach build| > |mach gradle| > |make gradle-targets| > AndroidManifest.xml > strings.xml > multi/brand.dtd
+    // |mach build| > |mach gradle| >
+    // |mach build mobile/android/base/generated_android_code_and_resources| >
+    // AndroidManifest.xml > strings.xml > multi/brand.dtd
     //
     // dependency chain to fail, since multi isn't a real locale.  To avoid
     // this, if Gradle is invoked with AB_CD=multi, we don't invoke Make at all.
-    task generateCodeAndResources()
-} else if (System.env.IS_LANGUAGE_REPACK == '1') {
+    if ('multi' == System.env.AB_CD) {
+        rootProject.logger.lifecycle("Skipping task ${task.path} because: AB_CD=multi")
+        return false
+    }
+
     // Single-locale l10n repacks set `IS_LANGUAGE_REPACK=1` and handle resource
     // and code generation themselves.
-    task generateCodeAndResources()
-} else {
-    task generateCodeAndResources(type:Exec) {
-        workingDir "${topobjdir}"
+    if ('1' == System.env.IS_LANGUAGE_REPACK) {
+        rootProject.logger.lifecycle("Skipping task ${task.path} because: IS_LANGUAGE_REPACK")
+        return false
+    }
 
-        commandLine mozconfig.substs.GMAKE
-        args '-C'
-        args "${topobjdir}/mobile/android/base"
-        args 'gradle-targets'
+    rootProject.logger.lifecycle("Executing task ${task.path}")
+    return true
+}
+
+task machBuildGeneratedAndroidCodeAndResources(type: Exec) {
+    onlyIf rootProject.ext.geckoBinariesOnlyIf
+
+    workingDir "${topsrcdir}"
 
-        // Only show the output if something went wrong.
-        ignoreExitValue = true
-        standardOutput = new ByteArrayOutputStream()
-        errorOutput = standardOutput
-        doLast {
-            if (execResult.exitValue != 0) {
-                throw new GradleException("Process '${commandLine}' finished with non-zero exit value ${execResult.exitValue}:\n\n${standardOutput.toString()}")
-            }
-        }
+    commandLine mozconfig.substs.PYTHON
+    args "${topsrcdir}/mach"
+    args 'build'
+    args 'mobile/android/base/generated_android_code_and_resources'
+
+    // Add `-v` if we're running under `--info` (or `--debug`).
+    if (project.logger.isEnabled(LogLevel.INFO)) {
+        args '-v'
     }
+
+    // `path` is like `:machBuildGeneratedAndroidCodeAndResources`.
+    standardOutput = new TaggedLogOutputStream("${path}>", logger)
+    errorOutput = standardOutput
 }
 
 afterEvaluate {
     subprojects { project ->
         if (project.name != 'thirdparty') {
             tasks.withType(JavaCompile) {
                 // Add compiler args for all code except third-party code.
                 options.compilerArgs += [
@@ -158,20 +193,20 @@ afterEvaluate {
                 }
             }
         }
 
         if (!hasProperty('android')) {
             return
         }
         android.applicationVariants.all {
-            preBuild.dependsOn rootProject.generateCodeAndResources
+            preBuild.dependsOn rootProject.machBuildGeneratedAndroidCodeAndResources
         }
         android.libraryVariants.all {
-            preBuild.dependsOn rootProject.generateCodeAndResources
+            preBuild.dependsOn rootProject.machBuildGeneratedAndroidCodeAndResources
         }
     }
 }
 
 apply plugin: 'idea'
 
 idea {
     project {
--- a/mobile/android/app/build.gradle
+++ b/mobile/android/app/build.gradle
@@ -333,17 +333,17 @@ android.applicationVariants.all { varian
     // `sourceSets` task, for reasons unknown.
     variant.registerGeneratedResFolders(project.files(syncPreprocessedRes.destinationDir).builtBy(syncPreprocessedRes))
 
     // It's not easy -- see the backout in Bug 1242213 -- to change the
     // <manifest> package for Fennec.  Gradle has grown a mechanism to achieve
     // what we want for Fennec, however, with applicationId.  To use the same
     // manifest as moz.build, we replace the package with org.mozilla.gecko (the
     // eventual package) here.
-    def rewriteManifestPackage = task("rewriteManifestPackageFor${variant.name.capitalize()}", type: Copy, dependsOn: rootProject.generateCodeAndResources) {
+    def rewriteManifestPackage = task("rewriteManifestPackageFor${variant.name.capitalize()}", type: Copy, dependsOn: rootProject.machBuildGeneratedAndroidCodeAndResources) {
         into("${project.buildDir}/moz.build/src/${variant.name}")
         from("${topobjdir}/mobile/android/base/AndroidManifest.xml")
         filter { it.replaceFirst(/package=".*?"/, 'package="org.mozilla.gecko"') }
         exclude('**/*.mkdir.done')
     }
 
     // Every configuration needs the stub manifest at
     // src/main/AndroidManifest.xml and the generated manifest.  We can't use
@@ -354,19 +354,19 @@ android.applicationVariants.all { varian
 
 
     // Local (read, not 'official') builds want to reflect developer changes to
     // AndroidManifest.xml.in, strings.xml, and preprocessed Java code.  To do
     // this, the Gradle build calls out to the moz.build system, which can be
     // re-entrant.  Official builds are driven by the moz.build system and
     // should never be re-entrant in this way.
     if (!mozconfig.substs.MOZILLA_OFFICIAL) {
-        syncPreprocessedJava.dependsOn rootProject.generateCodeAndResources
-        syncPreprocessedRes.dependsOn rootProject.generateCodeAndResources
-        rewriteManifestPackage.dependsOn rootProject.generateCodeAndResources
+        syncPreprocessedJava.dependsOn rootProject.machBuildGeneratedAndroidCodeAndResources
+        syncPreprocessedRes.dependsOn rootProject.machBuildGeneratedAndroidCodeAndResources
+        rewriteManifestPackage.dependsOn rootProject.machBuildGeneratedAndroidCodeAndResources
     }
 
     // When driven from moz.build via |mach build|, Gradle does not require or
     // use Gecko binaries.  It's only |mach package| that packs the Gecko
     // binaries into the resulting APK.  The "withoutGeckoBinaries" variants
     // handle this.  When driven from Android Studio or Gradle, the
     // "withGeckoBinaries" variants handle packing the Gecko binaries into the
     // resulting APK (for on-device deployment).  They also update the Omnijars
--- a/mobile/android/base/Makefile.in
+++ b/mobile/android/base/Makefile.in
@@ -1,29 +1,16 @@
 # 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/.
 
 # Developer builds call mach -> Make -> gradle -> mach, which races to
 # find and create .mozconfig files and to generate targets.
 .NOTPARALLEL:
 
-generated_resources := \
-  AndroidManifest.xml \
-  res/raw/suggestedsites.json \
-  res/values/strings.xml \
-  $(NULL)
-
-generated_files := \
-  AndroidManifest.xml \
-  generated/preprocessed/org/mozilla/gecko/AdjustConstants.java \
-  generated/preprocessed/org/mozilla/gecko/AppConstants.java \
-  generated/preprocessed/org/mozilla/gecko/MmaConstants.java \
-  $(NULL)
-
 include $(topsrcdir)/config/AB_rCD.mk
 
 chrome-%:: AB_CD=$*
 chrome-%::
 	$(MAKE) \
 	  res/values$(AB_rCD)/strings.xml \
 	  res/raw$(AB_rCD)/suggestedsites.json \
 	  AB_CD=$*
@@ -39,25 +26,17 @@ include $(topsrcdir)/config/rules.mk
 	$(REPORT_BUILD)
 	$(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, 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:
-
-gradle-targets:
 endif
 
-.PHONY: gradle-targets gradle-omnijar
+.PHONY: gradle-omnijar
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -178,31 +178,41 @@ for f in ['res/values/strings.xml',
     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',
     ]
 
+generated_inputs = [
+    '!AndroidManifest.xml',
+    '!generated/preprocessed/org/mozilla/gecko/AdjustConstants.java',
+    '!generated/preprocessed/org/mozilla/gecko/AppConstants.java',
+    '!generated/preprocessed/org/mozilla/gecko/CrashReporterConstants.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/suggestedsites.json',
+    '!res/values/strings.xml',
+]
+
 # 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/CrashReporterConstants.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/suggestedsites.json',
-    '!res/values/strings.xml',
-]
+GENERATED_FILES[t].inputs += generated_inputs
+
+# This is just a convenient way to refer to all of the generated inputs; this
+# allows the Gradle build to invoke mach to ensure that they are fresh.
+t = 'generated_android_code_and_resources'
+GENERATED_FILES += [t]
+GENERATED_FILES[t].force = True
+GENERATED_FILES[t].script = '/mobile/android/gradle.py:generate_android_code_and_resources'
+GENERATED_FILES[t].inputs += generated_inputs
--- a/mobile/android/gradle.py
+++ b/mobile/android/gradle.py
@@ -44,8 +44,13 @@ def generate_sdk_bindings(dummy_output_f
 
 
 def generate_generated_jni_wrappers(dummy_output_file, *args):
     return android('generate-generated-jni-wrappers', *args)
 
 
 def generate_fennec_jni_wrappers(dummy_output_file, *args):
     return android('generate-fennec-jni-wrappers', *args)
+
+
+def generate_android_code_and_resources(*args):
+    """No-op used to ensure inputs are fresh."""
+    return 0
--- a/mobile/android/gradle/with_gecko_binaries.gradle
+++ b/mobile/android/gradle/with_gecko_binaries.gradle
@@ -7,17 +7,17 @@
 // That arrangement labels them nicely in IntelliJ.  See the comment in the
 // :omnijar project for more context.
 evaluationDependsOn(':omnijar')
 
 // The JNI wrapper generation tasks depend on the JAR creation task of the :annotations project.
 evaluationDependsOn(':annotations')
 
 task buildOmnijars(type:Exec) {
-    dependsOn rootProject.generateCodeAndResources
+    dependsOn rootProject.machBuildGeneratedAndroidCodeAndResources
 
     // See comment in :omnijar project regarding interface mismatches here.
     inputs.file(project(':omnijar').sourceSets.main.resources.srcDirs).skipWhenEmpty() 
 
     // Produce both the Fennec and the GeckoView omnijars.
     outputs.file "${topobjdir}/dist/fennec/assets/omni.ja"
     outputs.file "${topobjdir}/dist/geckoview/assets/omni.ja"