Closed Bug 1360587 Opened 3 years ago Closed 2 years ago
Remove the bouncer APK
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".
I'm not aware of any users, but certainly :mkaply is the expert here.
Nope, we have no users. We can kill it. It didn't work like we thought.
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.
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.
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?
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.
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
Pushed by firstname.lastname@example.org: 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.