In order to transition to --with-gradle builds, we need to support generating JNI wrappers. This is tricky. The _right_ way to do this is to complete Bug 1067217 by making the existing annotation processing Java program a real Java Annotation Processor, and then to hook it into the Android-Gradle annotation processor support. Unfortunately, this has the following dependency chain (see https://developer.android.com/studio/releases/gradle-plugin.html): - Android-Gradle plugin version 2.3 or later - Gradle 3.3 or later - Java 1.8 - Android build-tools 25.0.0 or later Now, the new build-tools require - glibc 2.14 or later and the existing build images (centos6-build-upd, android-gradle-build) are pinned to glibc 2.12. That's a lot of upgrading in order to get annotation processing! So we need to investigate a way to get the right JAR files and classpaths to the existing annotation processing program. That's this ticket.

The transition to Gradle is finally going to happen \o/ I'm going to split this into v1 (now) and v2 (future) meta tickets, and move them out of Core :: Build Config, since they're really Firefox for Android-specific.

(In reply to Nick Alexander :nalexander from comment #2) 2> The transition to Gradle is finally going to happen \o/ I'm going to split > this into v1 (now) and v2 (future) meta tickets, and move them out of Core > :: Build Config, since they're really Firefox for Android-specific. Gah! Wrong ticket.

We're going to do this correctly -- using the `annotationProcessor` capability built into Android-Gradle plugin 2.3+ -- for our initial landing. This is mostly 'cuz we need to do almost all of the upgrade work to target Android O, but also because figuring out the location of the relevant .class and .jar files for the existing annotation processing program (see #c0) turned out to be challenging.

Comment on attachment 8917602 [details] Bug 1384312 - Support generating JNI wrappers under --with-gradle, https://reviewboard.mozilla.org/r/188558/#review194104 This is looking pretty good! Just one substantial comment about finding the JAR tasks. ::: mobile/android/base/Makefile.in:188 (Diff revision 1) > FENNEC_JARS += ../stumbler/stumbler.jar > endif > > +else # MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE > + > +$(gradle_dir)/geckoview/intermediates/bundles/debug/classes.jar: .gradle.deps Push these two lines below the `*_JARS` declarations, and then make these lines: ``` $(GECKOVIEW_JARS): .gradle.deps $(FENNEC_JARS): .gradle.deps ``` That'll save us repeating ourselves once. ::: mobile/android/base/Makefile.in:260 (Diff revision 1) > cp $(gradle_dir)/app/intermediates/transforms/dex/officialPhoton/debug/folders/1000/1f/main/classes.dex $@ > + > +GeneratedJNIWrappers.cpp: .gradle.deps > + $(REPORT_BUILD) > + > +# This annotation processing step also generates Rather than commenting, let's be explicit: ``` FennecJNIWrappers.cpp FennecJNIWrappers.h FennecJNINatives.h: .gradle.deps ... ``` (Here and above.) ::: mobile/android/base/Makefile.in:645 (Diff revision 1) > FENNEC_AIDLS = \ > $(NULL) > > fennec_aidl_src_path := $(srcdir)/aidl > fennec_aidl_target_path := generated > -fennec_aidl_targets := $(addprefix $(fennec_aidl_target_path)/,$(patsubst %.aidl,%.java,$(FENNEC_AIDLS:.java=))) > +fennec_aidl_targets := $(addprefix $(fennec_aidql_target_path)/,$(patsubst %.aidl,%.java,$(FENNEC_AIDLS:.java=))) This looks like a little typo -- probably me :) ::: mobile/android/gradle/with_gecko_binaries.gradle:129 (Diff revision 1) > android.sourceSets."${sourceSet}".assets.srcDir syncOmnijarFromDistDir.destinationDir > android.sourceSets."${sourceSet}".assets.srcDir syncAssetsFromDistDir.destinationDir > android.sourceSets."${sourceSet}".jniLibs.srcDir syncLibsFromDistDir.destinationDir > } > + > +ext.configureJNIWithGeckoBinaries = { variant, module -> This name doesn't make much sense, since there's nothing about the Gecko binaries involved. How about `configureVariantWithJNIWrappers`? ::: mobile/android/gradle/with_gecko_binaries.gradle:132 (Diff revision 1) > } > + > +ext.configureJNIWithGeckoBinaries = { variant, module -> > + > + def jarTask = tasks.findByName("jarOfficialPhotonDebugClasses") > + if(jarTask == null){ nit: space between `if` and `(`. (Here and below.) Also, this isn't doing the right thing for the _variant_ -- it's always using "OfficialPhotonDebug" or whatever. Can you figure out how to get the JAR task for the given variant? You'll need to do something different for application and library variants. ::: mobile/android/gradle/with_gecko_binaries.gradle:144 (Diff revision 1) > + throw new GradleException("Jar task output multiple files other than one single jar") > + } > + > + task("generateJNIWrappersFor${module}${variant.name.capitalize()}", type: JavaExec) { > + classpath = variant.javaCompile.classpath > + // throw new GradleException(variant.javaCompile.options.bootClasspath) nit: kill this. ::: mobile/android/gradle/with_gecko_binaries.gradle:156 (Diff revision 1) > + > + workingDir "${topobjdir}/mobile/android/base" > + > + dependsOn jarTask > + } > + nit: kill empty line.

Comment on attachment 8917602 [details] Bug 1384312 - Support generating JNI wrappers under --with-gradle, https://reviewboard.mozilla.org/r/188558/#review194222 ::: mobile/android/gradle/with_gecko_binaries.gradle:131 (Diff revisions 1 - 2) > android.sourceSets."${sourceSet}".jniLibs.srcDir syncLibsFromDistDir.destinationDir > } > > -ext.configureJNIWithGeckoBinaries = { variant, module -> > +ext.configureVariantWithJNIWrappers = { variant, module -> > > def jarTask = tasks.findByName("jarOfficialPhotonDebugClasses") This still doesn't address the fact that the variant might be `local` or something else (so not `official`) and the configuration might not be `debug`.

Comment on attachment 8914572 [details] Bug 1384312 - Part 1: Move omg.annotation; add stub Gradle annotationProcessor. We're taking a different approach, so this doesn't need review.

Comment on attachment 8917602 [details] Bug 1384312 - Support generating JNI wrappers under --with-gradle, https://reviewboard.mozilla.org/r/188558/#review194622 Looks good! Add something like: ``` - task("generateJNIWrappersFor${module}${variant.name.capitalize()}", type: JavaExec) { + def wrapperTask = task("generateJNIWrappersFor${module}${variant.name.capitalize()}", type: JavaExec) { classpath = variant.javaCompile.classpath // Include android.jar. classpath variant.javaCompile.options.bootClasspath classpath "${topobjdir}/build/annotationProcessors/annotationProcessors.jar" ... workingDir "${topobjdir}/mobile/android/base" dependsOn jarTask } + + variant.assemble.dependsOn wrapperTask ``` so this happens every build, and let's land it!

Pushed by max@mxli.us: https://hg.mozilla.org/integration/autoland/rev/9a3c4f7cd85e Support generating JNI wrappers under --with-gradle, r=nalexander

Backed out in https://hg.mozilla.org/integration/autoland/rev/ca9e5d0ade02 for https://treeherder.mozilla.org/logviewer.html#?job_id=136942768&repo=autoland

Comment on attachment 8919076 [details] Bug 1384312 - Support generating JNI wrappers under --with-gradle, https://reviewboard.mozilla.org/r/189984/#review195176 Max and I worked through these issues together, and jointly reviewed. Try looks green -- bombs away!

Pushed by nalexander@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fab13cda2355 Support generating JNI wrappers under --with-gradle, r=nalexander