1317089 - [geckoview] Use Android's manifest-merger.jar to merge manifest fragments

Status: RESOLVED WORKSFORME

(GeckoView :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

Right now, we have a cobbled together system that:
* uses the preprocessor to produce AndroidManifest.xml;
* uses the preprocessor to product AppConstants.java;
* uses build flags to include or exclude source files.

Gradle (really, the Android build toolchain) has defined how to do this "for real".  One declares build types, product flavors, and flavor dimensions, and uses an overlay system to produce a final AndroidManifest.xml, defines build config variables in Gradle files, and combines sets of Java, resource, and asset sources.  It's nice!

Gradle does some of this work internally (build config variables, source sets) but exposes a standalone manifest merger.  This ticket tracks using that merger -- technically, the Android Manifest Merger (version 2) -- to assemble AndroidManifest.xml.

It sucks to do this work rather than "just" use Gradle, but I think we'll need to keep both systems functional while we continue to try to switch over.  Wish we could have made Aurora "just use" Gradle and figure out what's broken there.

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1317089 - Pre: Pass --debug-mode to aapt.

Gradle always sets android:debuggable from build.gradle switches (and
warns about having it in AndroidManifest.xml).  Future patches in this
series unify how we produce the Android manifest, and this makes it
easier to compare the resulting productions.

Review commit: https://reviewboard.mozilla.org/r/98736/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98736/

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1317089 - Pre: Make MOZ_ANDROID_SHARED_ID mandatory.

It already is mandatory; the build system will set the ID (based on
the package name) if it's not given.

Review commit: https://reviewboard.mozilla.org/r/98738/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98738/

Comment 3

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8818783 [details]
Bug 1317089 - Pre: Pass --debug-mode to aapt.

https://reviewboard.mozilla.org/r/98736/#review99024

::: mobile/android/base/Makefile.in:448
(Diff revision 1)
>  # We touch the target file before invoking aapt so that aapt's outputs
>  # are fresher than the target, preventing a subsequent invocation from
>  # thinking aapt's outputs are stale.  This is safe because Make
>  # removes the target file if any recipe command fails.
>  
> +# The --debug-mode flag below is intended to be:

gps: any suggestion on how to express this cleanly appreciated.

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8818783 [details]
Bug 1317089 - Pre: Pass --debug-mode to aapt.

https://reviewboard.mozilla.org/r/98736/#review99380

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8818784 [details]
Bug 1317089 - Pre: Make MOZ_ANDROID_SHARED_ID mandatory.

https://reviewboard.mozilla.org/r/98738/#review99382

It's so sad that we can't get rid of the shared user id anymore.

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8818783 [details]
Bug 1317089 - Pre: Pass --debug-mode to aapt.

https://reviewboard.mozilla.org/r/98736/#review99024

> gps: any suggestion on how to express this cleanly appreciated.

GNU make doesn't have boolean logic support. The closest you can do is eval something via a shell. But that's just as ugly. What you've done here is acceptable.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8818783 [details]
Bug 1317089 - Pre: Pass --debug-mode to aapt.

https://reviewboard.mozilla.org/r/98736/#review100746

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

We use gradle now so this Just Works (mostly)