Closed
Bug 1485045
Opened 6 years ago
Closed 6 years ago
Allow building app with multiple GeckoView architectures included
Categories
(GeckoView :: General, enhancement, P2)
Tracking
(geckoview64 wontfix, firefox64 wontfix, firefox65 fixed)
RESOLVED
FIXED
mozilla65
People
(Reporter: sebastian, Assigned: snorp)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Currently we build multiple GeckoView AARs based on CPU architecture. Inside the apps we have configured gradle to build custom build variants where every variant only uses one GeckoView artifact.
This has multiple drawbacks for app developers:
* The build variant setup is complicated and error-prone
* The build variant setup is slowing down builds because a lot of variants have to be build (some of our apps have multiple flavor dimensions)
* The Android Gradle plugin already supports APK splits out of the box and can split by things like ABI (but right now we can only include one GeckoView):
https://developer.android.com/studio/build/configure-apk-splits#configure-abi-split
* With the new publishing format "Android App Bundles" we do not need to build multiple APKs anymore. The Play Store will dynamically compose a minimal APK for the device that wants to install the app. One of the used dimensions for that is the CPU architecture. However we can only include one GeckoView in our build..
https://developer.android.com/platform/technology/app-bundle/
Those reasons make it desirable to include all GeckoView architectures into the build and let Gradle (ABI split) or Google Play (Android App Bundle) decide what to include for a specific setup.
Is this something we could support? What would be needed for that?
Reporter | ||
Comment 1•6 years ago
|
||
I assume there are two (theoretically) options:
A) Make it possible to add multiple GeckoView builds into an application
B) Build an "universal" GeckoView artifact that includes everything for all CPU architectures (assuming this would work with APK split / App bundles)
Using (A) currently causes the build to fail because of duplicated Java classes (the first one it complains about is Exoplayer). Would it be an option to move those classes to a "geckoview-base" artifact that the other artifacts depend on? This would allow gradle to not include those multiple times.
Reporter | ||
Comment 2•6 years ago
|
||
> (the first one it complains about is Exoplayer)
Technically at least Exoplayer is already a separate AAR:
https://github.com/google/ExoPlayer
However we do not list it as dependency in the POM file and instead have it in the tree[1] and package it into our own AAR and then it will be duplicated once we add two GeckoView AARs to an app. I assume the same error will show up for our own Java/Kotlin code once the exoplayer issue is resolved.
[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/geckoview/src/thirdparty/java/com/google/android/exoplayer2
Reporter | ||
Comment 3•6 years ago
|
||
An additional reason why I would like to add multiple CPU architectures at once: We do the same for our Rust-based components. We build just one component that contains the code for all CPU architectures. Apps can then use the APK split feature or Android App Bundles to pick what they need. Together with GeckoView and Rust-based components in the same app things get very complicated.
Reporter | ||
Comment 4•6 years ago
|
||
NI: :snorp and :nalexander (doesn't accept NI requests) may know if this is possible and what would be required to get there.
Flags: needinfo?(snorp)
Assignee | ||
Comment 5•6 years ago
|
||
Building a fat AAR should be possible. I think the easiest way may be to simply add a new task that merges the native parts of the individual AARs. Right now the Java build would be unnecessarily duplicated, but that might be solvable if care enough. There will be some other fallout due to things like AppConstants.MOZ_APP_ABI being unusable.
The big wrench in the gears may be the fact that libxul.so and friends are in assets/ instead of lib/ because of the custom xz compression. Downstream tools won't understand that, so we'd have to stop that. This would be a significant APK size increase; I'm guessing maybe 20%.
Flags: needinfo?(snorp)
Reporter | ||
Comment 6•6 years ago
|
||
> The big wrench in the gears may be the fact that libxul.so and friends are in assets/ instead of lib/ because of the custom xz compression. Downstream tools won't understand that, so we'd have to stop that. This would be a significant APK size increase; I'm guessing maybe 20%.
It would be interesting to measure that and see how much Google Play can save with app bundles. Also as I understand it not doing our own compression would allow Google Play to create smaller app update patches resulting in smaller update sizes [1]?
[1] https://github.com/googlesamples/apk-patch-size-estimator
Comment 7•6 years ago
|
||
Sebastian, can you help us assign a priority here? Do we need this soon?
Flags: needinfo?(s.kaspari)
Reporter | ||
Comment 8•6 years ago
|
||
> Sebastian, can you help us assign a priority here? Do we need this soon?
"Soon" is vague here. It's not a Focus blocker.
I think there are two driving factors here:
* Eventually we want to use Android App Bundles because it is the most promising way to ship small APKs to our users.
* We want to stop maintaining this custom split APK solution that will get even more complex once we have more libraries with native (Rust) code that ship as a "multiarch" AAR.
So, yeah it's not a P1 but I would be good if this could find a place on the roadmap.
Flags: needinfo?(s.kaspari)
Comment 9•6 years ago
|
||
Thanks, it makes sense for us to choose the right path forward as early as we can. Going with P2.
Priority: -- → P2
Comment 10•6 years ago
|
||
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #5)
> Building a fat AAR should be possible. I think the easiest way may be to
> simply add a new task that merges the native parts of the individual AARs.
> Right now the Java build would be unnecessarily duplicated, but that might
> be solvable if care enough. There will be some other fallout due to things
> like AppConstants.MOZ_APP_ABI being unusable.
Good point. Probably a lot of the "don't load on bad ABIs" would need to become more dynamic.
The way that I would try to achieve this is for the GeckoView universal AAR to depend on N arch-specific AARs and have the arch-specific AARs only include libraries.
> The big wrench in the gears may be the fact that libxul.so and friends are
> in assets/ instead of lib/ because of the custom xz compression. Downstream
> tools won't understand that, so we'd have to stop that. This would be a
> significant APK size increase; I'm guessing maybe 20%.
We can prefix the assets/ path -- so assets/{armeabi-v7a,arm-v8a,x86}. This is just good hygiene across the board. Of course we'd need to make the choice of assets/ path depend on the device more, so there's another point of failure... but it feels do-able.
Assignee | ||
Comment 11•6 years ago
|
||
This allows us to use the same Java code for any native platform,
enabling a "fat" AAR.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → snorp
Comment 12•6 years ago
|
||
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7174015fbbd5
Make Java parts of GeckoView independent from build ABI r=jchen
Comment 13•6 years ago
|
||
Backed out for failing android robocop
Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7174015fbbd584f9afa8783c1ac119ab3ac8bbf4
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=211739121&repo=autoland&lineNumber=1359
Backout: https://hg.mozilla.org/integration/autoland/rev/debe7b5d5bbc9d10205517f42d93a5a1d5e72bb2
Flags: needinfo?(snorp)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(snorp)
Comment 14•6 years ago
|
||
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bae43dfd3700
Make Java parts of GeckoView independent from build ABI r=jchen
Comment 15•6 years ago
|
||
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3bd6b94b23e5
Backed out changeset bae43dfd3700 for toolchains gradle bustages. CLOSED TREE
Comment 16•6 years ago
|
||
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0bb898d00ed7
Make Java parts of GeckoView independent from build ABI r=jchen
Comment 17•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Comment 18•6 years ago
|
||
64=wontfix because we don't need to uplift to Beta.
status-firefox64:
--- → wontfix
Updated•6 years ago
|
Product: Firefox for Android → GeckoView
Updated•6 years ago
|
Target Milestone: Firefox 65 → mozilla65
You need to log in
before you can comment on or make changes to this bug.
Description
•