976699 - Place architecture-specific assets in a different subdirectory

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Nathan Toone [:toonetown]]

Attachment: arch-assets.diff

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_1) AppleWebKit/537.73.11 (KHTML, like Gecko) Version/7.0.1 Safari/537.73.11

Steps to reproduce:

It would be nice to have architecture-specific assets placed within the apk at assets/{architecture} - thus facilitating the ability to create a single apk for multiple platforms.



Expected results:

The attached patch allows you to specify ANDROID_ASSET_ARCH in a mozconfig in order to place the asset libraries in that subdirectory.  For example, after applying the patch, you can add:
export ANDROID_ASSET_ARCH=armeabi-v7a
and asset libraries will be placed within (and loaded from) the apk at assets/armeabi-v7a.  The location of the omni.ja file is unchanged.

Comment 1

[Aaron Train [:aaronmt]]

Comment on attachment 8381592 [details] [diff] [review]
arch-assets.diff

Not sure if you wanted this looked at or actually reviewed, feel free to flip flag on the patch to review.

Comment 2

[Nathan Toone [:toonetown]]

This is just a change that I implemented after a (quick) discussion on the #mobile IRC channel.  I submitted the bug in the spirit of helping out...

I don't know if it will be valuable or helpful to others - it's more of a "feature request" than a "bug".  I just wasn't sure what the process for submitting patches back upstream was, so I figured I'd log it here.  Feel free to review/give feedback/close/delete as makes sense for the overall project.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8381592 [details] [diff] [review]
arch-assets.diff

Review of attachment 8381592 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the long delay, I was heads down on something.
Seems to me ANDROID_ASSET_ARCH should always have the same value as ANDROID_CPU_ARCH, and that there is no value in having this being conditional, but I'd rather leave this for nalexander to tell.

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8381592 [details] [diff] [review]
arch-assets.diff

Review of attachment 8381592 [details] [diff] [review]:
-----------------------------------------------------------------

I like this idea, and think we should do something like it.  This could be valuable for GeckoView consumers; it's possible they'll want to ship multi-arch APKs.  But I would like to see a slightly different approach.

1) Let's drop ANDROID_ASSET_ARCH in favour of ANDROID_CPU_ARCH, and always use it.  So we get

assets/omni.ja
assets/armeabi/libnss.so
...

The fewer conditionals in the packager, the better.

2) Rather than bake the arch into APKOpen, let's put the assets/$ARCH computation into the APKOpen consumer.  One less path assumption in the C++ layer.  And I have toyed with the idea of loading these libraries from other locations (/data/data, a different APK entirely): see Bug 974462.

Comment 5

[Nick Alexander :nalexander [he/him]]

> 2) Rather than bake the arch into APKOpen, let's put the assets/$ARCH
> computation into the APKOpen consumer.  One less path assumption in the C++
> layer.  And I have toyed with the idea of loading these libraries from other
> locations (/data/data, a different APK entirely): see Bug 974462.

Further to this -- this computation should be dynamic, and the Java code has easy arch inspection.  (I imagine the C++ layer does too.)

Comment 6

[Nick Alexander :nalexander [he/him]]

Attachment: Place architecture specific assets in assets/$(ANDROID_CPU_ARCH). r=glandium

Works locally; I'll push to try in a moment.

Comment 7

[Nick Alexander :nalexander [he/him]]

https://tbpl.mozilla.org/?tree=Try&rev=2cacdd6d9c0d

Comment 8

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/25449ca505b4

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/25449ca505b4