Open Bug 1254353 (gradle-automation) Opened 4 years ago Updated 4 months ago

[meta] Use Gradle to build Firefox for Android in automation

Categories

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

defect

Tracking

(Not tracked)

People

(Reporter: nalexander, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: meta)

Attachments

(4 files, 4 obsolete files)

58 bytes, text/x-review-board-request
Details
58 bytes, text/x-review-board-request
Details
58 bytes, text/x-review-board-request
Details
58 bytes, text/x-review-board-request
Details
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.
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/
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/
Attachment #8731421 - Flags: review?(s.kaspari)
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 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.
Attachment #8731420 - Flags: review?(cmanchester)
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
Attachment #8731421 - Flags: review?(s.kaspari) → review+
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 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/
Attachment #8731421 - Attachment description: MozReview Request: Bug 1254353 - Pre: Explicitly request READ_EXTERNAL_MANIFEST permission. r?sebastian → MozReview Request: Bug 1254353 - Pre: Explicitly request READ_EXTERNAL_STORAGE permission. r=sebastian
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/
Attachment #8731424 - Attachment description: MozReview Request: Bug 1254353 - Allow to resign an already signed APK. → MozReview Request: Bug 1254353 - Pre: Allow re-signing already signed APK files. r?gps
Attachment #8731424 - Flags: review?(gps)
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/
Attachment #8731423 - Attachment description: MozReview Request: Bug 1254353 - Include Robocop support files. → MozReview Request: Bug 1254353 - Include Robocop support files. r?gps
Attachment #8731423 - Flags: review?(gps)
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/
Attachment #8731419 - Attachment description: MozReview Request: Bug 1254353 - Build with Gradle in automation. → MozReview Request: Bug 1254353 - Build with Gradle in automation. r?gps
Attachment #8731419 - Flags: review?(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.
Attachment #8731424 - Flags: review?(gps) → review+
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.
Attachment #8731423 - Flags: review?(gps)
Attachment #8731419 - Flags: review?(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.
Attachment #8731424 - Attachment description: MozReview Request: Bug 1254353 - Pre: Allow re-signing already signed APK files. r?gps → MozReview Request: Bug 1254353 - Pre: Allow re-signing already signed APK files. r=gps
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 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/
Attachment #8731423 - Flags: review?(gps)
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/
Attachment #8731419 - Flags: review?(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.
Attachment #8733187 - Flags: review?(gps) → review+
Comment on attachment 8731423 [details]
MozReview Request: Bug 1254353 - Include Robocop support files. r?gps

https://reviewboard.mozilla.org/r/40583/#review38057
Attachment #8731423 - Flags: review?(gps) → review+
Comment on attachment 8731419 [details]
MozReview Request: Bug 1254353 - Build with Gradle in automation. r?gps

https://reviewboard.mozilla.org/r/40575/#review38059
Attachment #8731419 - Flags: review?(gps) → review+
Blocks: 1338673
No longer blocks: 1338673
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?).
(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.
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.
Flags: needinfo?(whuang)
Flags: needinfo?(s.kaspari)
Flags: needinfo?(bbermes)
(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.
Flags: needinfo?(whuang)
Yes, Photon Mobile *must* ship with Photon on Desktop.
Flags: needinfo?(bbermes)
(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.)
Flags: needinfo?(bbermes)
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.
Flags: needinfo?(s.kaspari)
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.
Depends on: 1367768
Depends on: 1370156
No longer depends on: 1370156
Depends on: 1370158
Blocks: 1365505
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.
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: -- → P5
Flags: needinfo?(bbermes)
Product: Firefox for Android → Firefox Build System
You need to log in before you can comment on or make changes to this bug.