Closed Bug 1360587 Opened 3 years ago Closed 2 years ago

Remove the bouncer APK

Categories

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

enhancement
Not set

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: nalexander, Assigned: maliu)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 3 obsolete files)

This is follow-up to Bug 1320310.  This ticket tracks ensuring that the bouncer and main Fennec APKs agree on permissions and badging.  We should probably compare the output of `aapt dump badging` to implement this.
mkaply: snorp: I'm evaluating whether this work is required in order to "flip the switch" to use --with-gradle in automation.  Are there any consumers of the bouncer APK at this time?  Do we foresee any consumers in a reasonable time frame?  If we do not, are you aware of any reason why we should keep the bouncer APK?  It adds a slight burden to the build system and is untested -- read, "probably broken".
Flags: needinfo?(snorp)
Flags: needinfo?(mozilla)
I'm not aware of any users, but certainly :mkaply is the expert here.
Flags: needinfo?(snorp)
Nope, we have no users. We can kill it. It didn't work like we thought.
Flags: needinfo?(mozilla)
Thanks for the prompt response, folks.  Morphing this ticket to be about removing entirely the bouncer APK introduced by Bug 1234629, which will simplify the build system a little.

Since the bouncer is unused, this doesn't block https://bugzilla.mozilla.org/show_bug.cgi?id=gradle-automation-v1.  It's a future task, so it does block https://bugzilla.mozilla.org/show_bug.cgi?id=gradle-automation-v2.
Depends on: bouncerapk
Summary: Ensure that Gradle-produced bouncer APK agrees with Gradle-produced app APK → Remove the bouncer APK
maliu: this is perhaps a good task for you, although it's not critical that we do it immediately.  The bouncer reaches into a bunch of places in the build system; see if you can uncover them and figure out how to roll back the logic.
Flags: needinfo?(max)
Max: this is perhaps more interesting, because I have green oak Nightly's (without bouncer) that fail because the beetmover expects the bouncer.  See the bm-S tasks at:

https://hg.mozilla.org/projects/oak/rev/c965d3adbcbf35f417dbf0832b44de6c52b89535

Have you started looking at this?
jlorenzo: I want to _remove_ bouncer.apk completely.  Would killing the in-tree references in

http://searchfox.org/mozilla-central/search?q=bouncer.apk&path=

and killing the blocks in

https://github.com/mozilla-releng/beetmoverscript/search?utf8=%E2%9C%93&q=bouncer.apk&type=

be sufficient?  Can you review both of these things?  How do I get a new beetmoverscript deployed?
Flags: needinfo?(jlorenzo)
I recommend killing the in-tree blocks, to remove bouncer.apk from the beetmover task's upstreamArtifacts.
Aiui, beetmoverscript's templates are a superset of files, and affect all trees, so I recommend keeping the bouncer.apk entries in those templates until bouncer.apk is removed from all gecko trains (i.e., merged to mozilla-release)
I agree with Aki, removing the in-tree references is sufficient for mozilla-central. If a file is missing from the task definition,  beetmover won't bail out, if I remember correct. 

I can review both changes, and deploy a new beetmover once every train stopped building bouncer.apk.
Flags: needinfo?(jlorenzo)
Assignee: nobody → max
Flags: needinfo?(max)
Comment on attachment 8921929 [details]
Bug 1360587 - Part 1. Remove apk upload task,

https://reviewboard.mozilla.org/r/192946/#review198230
Attachment #8921929 - Flags: review?(aki) → review+
Comment on attachment 8921930 [details]
Bug 1360587 - Part 2. Remove bouncer apk build config; source; docs,

https://reviewboard.mozilla.org/r/192948/#review198334
Attachment #8921930 - Flags: review?(nalexander) → review+
Comment on attachment 8921931 [details]
Bug 1360587 - Part 3. Remove bouncer source,

https://reviewboard.mozilla.org/r/192950/#review198338
Attachment #8921931 - Flags: review?(nalexander) → review+
Comment on attachment 8921932 [details]
Bug 1360587 - Part 4. Remove AndroidManifest refer to bouncer,

https://reviewboard.mozilla.org/r/192952/#review198342

::: mobile/android/base/AndroidManifest.xml.in:18
(Diff revision 1)
> -<!-- The bouncer APK and the main APK should define the same set of
> -     <permission>, <uses-permission>, and <uses-feature> elements.  This reduces
> -     the likelihood of permission-related surprises when installing the main APK
> -     on top of a pre-installed bouncer APK.  Add such shared elements in the
> -     fileincluded here, so that they can be referenced by both APKs. -->
>  #include FennecManifest_permissions.xml.in

There's no need to separate the permissions now, so please file a [good first bug] ticket tracking inlining these permissions, and add a comment referencing it here.
Attachment #8921932 - Flags: review?(nalexander) → review+
Comment on attachment 8921933 [details]
Bug 1360587 - Part 5. Remove bouncer related docs,

https://reviewboard.mozilla.org/r/192954/#review198346

This all looks great, Max!  Most of these can't be built without the entire sequence, so I think it makes the most sense to squash them all down.  Or maybe number them 2a-2d, suggesting that the 2* bits all go together.
Attachment #8921933 - Flags: review?(nalexander) → review+
hg error in cmd: hg pull gecko -r fc88b1097704d74fb88251dc494a3904315cd97a: pulling from https://reviewboard-hg.mozilla.org/gecko
abort: HTTP Error 504: Gateway Timeout
See Also: → 1411809
Attachment #8921931 - Attachment is obsolete: true
Attachment #8921931 - Flags: review?(s.kaspari)
Attachment #8921932 - Attachment is obsolete: true
Attachment #8921932 - Flags: review?(s.kaspari)
Attachment #8921933 - Attachment is obsolete: true
Attachment #8921933 - Flags: review?(s.kaspari)
Pushed by max@mxli.us:
https://hg.mozilla.org/integration/autoland/rev/d72b09c9228c
Part 1. Remove apk upload task, r=aki
https://hg.mozilla.org/integration/autoland/rev/31f87080283b
Part 2. Remove bouncer apk build config; source; docs, r=nalexander
Comment on attachment 8921929 [details]
Bug 1360587 - Part 1. Remove apk upload task,

Late r+. LGTM.
Attachment #8921929 - Flags: review?(jlorenzo) → review+
Comment on attachment 8921930 [details]
Bug 1360587 - Part 2. Remove bouncer apk build config; source; docs,

..
Attachment #8921930 - Flags: review?(s.kaspari)
Removing this from templates in beetmoverscript as well since we're far along the way with 63.0
https://github.com/mozilla-releng/beetmoverscript/pull/188
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 58 → mozilla58
You need to log in before you can comment on or make changes to this bug.