1254353 - (gradle-automation) [meta] Use Gradle to build Firefox for Android in automation

Status: NEW

(Firefox Build System :: Android Studio and Gradle Integration, defect, P5)

Description

[Nick Alexander :nalexander [he/him]]

The groundwork is in place from Bug 1119520 and dependents.  This ticket tracks trying to flip --with-gradle for Nightly builds, and eventually removing https://dxr.mozilla.org/mozilla-central/source/config/makefiles/java-build.mk and everything that consumes it entirely.

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1254353 - Pre: Assemble app and tests when collecting dependencies. r=me

In the wild, the Robotium Maven dependency is not being fetched.
Let's see if this forces it to be downloaded.

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

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1254353 - Build with Gradle in automation. r?gps

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

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1254353 - Pre: Add GRADLE_FLAGS to control Gradle in automation. r?chmanchester

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

Comment 4

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1254353 - Pre: Explicitly request READ_EXTERNAL_STORAGE permission. r=sebastian

This permission was added in API 16, and is only enforced in API 19+.
(It's benign to add it to APKs installed on API 15.)

We want to declare it explicitly so that the bouncer APK and the main
APK have the same permission set.  There appears to be some fanciness
with Gradle's implied permission system where the bouncer APK does not
request READ implicitly where-as the main APK does request READ
implicitly.  This just makes things explicit (and uniform).

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

Comment 5

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1254353 - Don't build GeckoView.

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

Comment 6

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1254353 - Include Robocop support files. r?gps

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

Comment 7

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1254353 - Pre: Allow re-signing already signed APK files. r=gps

Gradle produces signed, unaligned APK files.  We expect unsigned,
unaligned APK files.  This change discards any existing signature,
turning a signed, unaligned APK into an unsigned, unaligned APK.

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

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8731420 [details]
MozReview Request: Bug 1254353 - Pre: Add GRADLE_FLAGS to control Gradle in automation. r?chmanchester

https://reviewboard.mozilla.org/r/40577/#review37327

::: old-configure.in:4703
(Diff revision 1)
>      if test -z "$GRADLE" -o ! -x "$GRADLE" ; then
>          AC_MSG_ERROR([The program gradlew/gradle was not found.  Use --with-gradle=/path/to/bin/gradle}])
>      fi
>  fi
>  AC_SUBST(GRADLE)
> +AC_SUBST(GRADLE_FLAGS)

Surely Python configure can handle this. We have a mobile/android/moz.configure.

If that's giving you trouble let me know and I'll come back with a patch.

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8731421 [details]
MozReview Request: Bug 1254353 - Pre: Explicitly request READ_EXTERNAL_STORAGE permission. r=sebastian

https://reviewboard.mozilla.org/r/40579/#review37583

The commit message says "_MANIFEST": READ_EXTERNAL_MANIFEST

Comment 10

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8731418 [details]
MozReview Request: Bug 1254353 - Pre: Assemble app and tests when collecting dependencies. r=me

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40573/diff/1-2/

Comment 11

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8731421 [details]
MozReview Request: Bug 1254353 - Pre: Explicitly request READ_EXTERNAL_STORAGE permission. r=sebastian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40579/diff/1-2/

Comment 12

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8731424 [details]
MozReview Request: Bug 1254353 - Pre: Allow re-signing already signed APK files. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40585/diff/1-2/

Comment 13

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8731423 [details]
MozReview Request: Bug 1254353 - Include Robocop support files. r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40583/diff/1-2/

Comment 14

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8731419 [details]
MozReview Request: Bug 1254353 - Build with Gradle in automation. r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40575/diff/1-2/

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8731424 [details]
MozReview Request: Bug 1254353 - Pre: Allow re-signing already signed APK files. r=gps

https://reviewboard.mozilla.org/r/40585/#review37973

Please add an inline comment basically stating what you said in the commit message because the source code is not obvious.

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8731423 [details]
MozReview Request: Bug 1254353 - Include Robocop support files. r?gps

https://reviewboard.mozilla.org/r/40583/#review37975

::: mobile/android/tests/browser/moz.build:12
(Diff revision 2)
> -    'robocop/roboextender',
> +        'robocop/roboextender',
> -]
> +    ]
> +else:
> +    TEST_DIRS += [
> +        'robocop/roboextender',
> +    ]

Having robocop/roboextender listed twice is weird. It should be pulled outside of a conditional.

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8731419 [details]
MozReview Request: Bug 1254353 - Build with Gradle in automation. r?gps

https://reviewboard.mozilla.org/r/40575/#review37977

How were the archives uploaded to tooltool produced? What happens if you get hit by a bus? How can we reproduce those archives? Are the archives produced deterministically?

We should ideally have tools in tree to reproduce the archives (if they aren't already). See build/windows_toolchain.py for what I recently wrote for VS2015. I also added docs under build/docs describing how to run it. We need all this stuff documented and as reproducible as possible.

Comment 18

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8731424 [details]
MozReview Request: Bug 1254353 - Pre: Allow re-signing already signed APK files. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40585/diff/2-3/

Comment 19

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1254353 - Pre: Document how to produce Android Gradle dependencies. r?gps

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

Comment 20

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8731423 [details]
MozReview Request: Bug 1254353 - Include Robocop support files. r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40583/diff/2-3/

Comment 21

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8731419 [details]
MozReview Request: Bug 1254353 - Build with Gradle in automation. r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40575/diff/2-3/

Comment 22

[Gregory Szorc [:gps]]

Comment on attachment 8733187 [details]
MozReview Request: Bug 1254353 - Pre: Document how to produce Android Gradle dependencies. r?gps

https://reviewboard.mozilla.org/r/41627/#review38053

This is awesome. Thank you.

Comment 23

[Gregory Szorc [:gps]]

Comment on attachment 8731423 [details]
MozReview Request: Bug 1254353 - Include Robocop support files. r?gps

https://reviewboard.mozilla.org/r/40583/#review38057

Comment 24

[Gregory Szorc [:gps]]

Comment on attachment 8731419 [details]
MozReview Request: Bug 1254353 - Build with Gradle in automation. r?gps

https://reviewboard.mozilla.org/r/40575/#review38059

Comment 25

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=132b07bcd0f1

Comment 26

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1444a959b5b0

Comment 27

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d7469aced22

Comment 28

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=192cce69467b

Comment 29

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bef85f77e931

Comment 30

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb9484636dbe

Comment 31

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fa684807f17

Comment 32

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ff93f4139ac

Comment 33

[Chenxia Liu [:liuche]]

During eng meeting, we were discussing how having an all-gradle build could make Photon work easier by allowing a different Gradle config for Photon work (slated to ship in 57?).

Comment 34

[Nick Alexander :nalexander [he/him]]

(In reply to Chenxia Liu [:liuche] from comment #33)
> During eng meeting, we were discussing how having an all-gradle build could
> make Photon work easier by allowing a different Gradle config for Photon
> work (slated to ship in 57?).

My expectation is that you could develop Photon entirely --with-gradle using a different Gradle flavour, and then "flip the switch" and push all the new things into the existing moz.build.  That would achieve the separation you want without requiring us to solve the outstanding problems blocking building Nightly and Release --with-gradle.

Consulting https://wiki.mozilla.org/RapidRelease/Calendar, I see that Fennec 57 opens in June -- 2 months out.  Given the tight timeline, if the Fennec team wants to push for --with-gradle builds for Photon, we need to talk now about my availability.  I happen to have picked up bits of this last week just for fun, and I am happy to help others actually make this happen, but I think that I will have to be heavily involved for this to succeed.

Comment 35

[Chenxia Liu [:liuche]]

Let me redirect this question to Barbara/Wesley:

Is Android Photon work slated to ship in 57 along with the Desktop ship of Photon? It wasn't clear during our Android eng meeting if that was the plan, although it looks like there is a Photon work week in YVR the week of May 8.

Comment 36

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

(In reply to Chenxia Liu [:liuche] from comment #35)
> Let me redirect this question to Barbara/Wesley:
> 
> Is Android Photon work slated to ship in 57 along with the Desktop ship of
> Photon? It wasn't clear during our Android eng meeting if that was the plan,
> although it looks like there is a Photon work week in YVR the week of May 8.

Yes. We are in the progress of scoping Photon on Fennec, targeting 57.
Likely will focus one visual refresh and animation changes those potentially improve perceived performance.

Comment 37

[Barbara Bermes [:barbara] - NI please!]

Yes, Photon Mobile *must* ship with Photon on Desktop.

Comment 38

[Nick Alexander :nalexander [he/him]]

(In reply to Barbara Bermes [:barbara] - NI please! from comment #37)
> Yes, Photon Mobile *must* ship with Photon on Desktop.

OK, let's take discussion of what's needed technically for Photon Mobile to a ticket.  However, I can't find a candidate ticket!  barbara, sebastian: where can we discuss what's actually needed from the build system to support Photon Mobile?  (This ticket is a meta ticket and tracks a different project, so it's not a good place for that discussion.)

Comment 39

[Sebastian Kaspari (:sebastian; :pocmo)]

I need to talk to antlam once he's back next week to know about the scope of Photon to answer that. Right now I was merely brainstorming ideas about how to land disabled bits of Photon in 55/56 - without blowing up the APK size. Gradle module overlays seemed to be one promising approach. I'll make sure to file tickets once I know about the scope.

Comment 40

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

I just filed Bug 1355774 as a meta to track Photon Mobile on Firefox for Android.
For the time being (before we have bugs for each specific bugs) I think it's fine to have all kinds of discussions going on that meta.

Comment 41

[Nick Alexander :nalexander [he/him]]

Making a note of this for myself: per https://dxr.mozilla.org/mozilla-central/rev/c697e756f738ce37abc56f31bfbc48f55625d617/mobile/android/tests/browser/junit3/src/org/mozilla/tests/browser/junit3/TestJarReader.java#77, we need to stuff package-name.txt into Gradle-produced APK files.

Comment 42

[Nick Alexander :nalexander [he/him]]

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.

Comment 43

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.