1398848 - Proposal: Stop naming Fennec ARMv7 builds "android-api-XX"

Status: RESOLVED WONTFIX

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

Description

[Johan Lorenzo [:jlorenzo]]

HISTORY

Dropping API level 15 (bug 1316462) was unnecessarily painful, because many parameters are named after "android-api-XX" (XX being a number):
* Bug 1384482 renamed treeherder, and most of the release infrastructure, which includes file paths on archive.mozilla.org
* Bug 1394960 updated Autophone after Bug 1384482
* Same for https://github.com/marco-c/missing_builds/pull/1/files 
* People got confused by try-chooser while it was being deployed in bug 1395226.
* https://www.mozilla.org/firefox/android/nightly/all/ was pointing at an outdated build (still after Bug 1384482).

The next API upgrade may feel the same way.



RATIONALE

I think names with "android-api-XX" create more problems than they solve. I.e.: 
* Finding every dependency outside of mozilla-central is not an easy task, because projects keep evolving. For instance, on the releng side, the last API upgrade was documented in bug 1219094. However, most of the infrastructure changed, so the patch set was very different.
* It creates a false sense of knowing what API level is supported. E.g.: API-16 had been shipped to Nightly and Beta users for more than a month, under the name "android-api-15".
* It may lead people to think API level is ARM-specific. However, API level is an OS concept. It applies to x86[1] and ARM-64[2]. None of them references the API level in their file name.




POTENTIAL DRAWBACKS

I see 1: We won't be able to ship several API levels anymore. I know we used to support 2 API levels at once. I wonder if it's going to be the case, in the future. For instance, today we ship API level 16, which translates Android 4.1 & 4.1.1[3]. Android 4.1 was released 5 years ago (on July 13, 2012[4]) and is officially not supported anymore. If we wanted to catch up with the latest Android Oreo, we'd have to bump to API level 26. I may be wrong, but upgrading to non-outdated Android version seems unlikely.
Would you all see other cons? 



SOLUTION?
Rename "android-api-XX" into "android-arm" or "android-armv7". What do you all think? (Requesting feedback from people involved in aforementioned bugs, feel free to redirect/add more people).



[1] https://dxr.mozilla.org/mozilla-central/rev/fd87bb184e299fec695f69bd2977276c25719b98/mobile/android/config/mozconfigs/android-x86/nightly#9
[2] https://dxr.mozilla.org/mozilla-central/rev/fd87bb184e299fec695f69bd2977276c25719b98/mobile/android/config/mozconfigs/android-aarch64/nightly#6
[3] https://developer.android.com/guide/topics/manifest/uses-sdk-element.html#ApiLevels
[4] https://en.wikipedia.org/wiki/Android_Jelly_Bean

Comment 1

[Sylvestre Ledru [:Sylvestre]]

+Sebastian and Stefan as CC

Comment 2

[Jan Henning [:JanH]]

(In reply to Johan Lorenzo [:jlorenzo] from comment #0)
> Dropping API level 15 (bug 1316462) was unnecessarily painful, because many
> parameters are named after "android-api-XX" (XX being a number):
> * Bug 1384482 renamed treeherder, and most of the release infrastructure,
> which includes file paths on archive.mozilla.org
> * Bug 1394960 updated Autophone after Bug 1384482
> * Same for https://github.com/marco-c/missing_builds/pull/1/files 
> * People got confused by try-chooser while it was being deployed in bug
> 1395226.
> * https://www.mozilla.org/firefox/android/nightly/all/ was pointing at an
> outdated build (still after Bug 1384482).

Mozregression, too: https://github.com/mozilla/mozregression/pull/460

Comment 3

[Richard Newman [:rnewman]]

Two things to think about:

1. Will we ever want to deliver a split APK again? E.g., one for Android Auto, one for XXXXLarge tablets, and one for other devices?

2. Why do we include the API level in the name, when it's an incidental attribute of the split?


We used to have three APKs: one that was suitable for ARM Gingerbread devices, one for x86 devices, and one for everything else.

Now we have two APKs: one for x86 devices and one for everything else.

It _just so happens_ that the constraints we apply to the APK, both in the manifest and when uploaded to Play, restrict the CPU and Android API version. But those can change over time, as we are all well aware!

My counter-proposal is: accept that we have different build targets (currently Firefox for Android on x86 and Firefox for Android on all other Android devices), and rename all of our configurations to match _intent_, not _parameters_.

My first stab for this APK would be 'fennec-main' (and 'fennec-x86' alongside). 'android' isn't sufficiently precise, and the API version is both incidental and too precise.

Comment 4

[Geoff Brown [:gbrown] (pto Apr 11-18)]

I find the treeherder names and try names for non-autophone Android unnecessarily and annoyingly complex. Inclusion of the api level seems inappropriate and troublesome. I think "android" is a good parallel to traditional desktop names like "linux", "osx", etc.

For try, I'd suggest "-p android,android-gradle,android-x86" instead of "-p android-api-16,android-api-16-gradle,android-x86".

For treeherder labels:

Android 4.0 API16+ opt       -> Android opt 	
Android 4.0 API16+ debug     -> Android debug
Android 4.2 x86 opt          -> Android x86 opt
Android 4.3 API16+ opt       -> Android opt (no need to distinguish from Android 4.0 API16+ opt builds)
Android 4.3 API16+ debug     -> Android debug
Android API16+ Gradle opt    -> Android Gradle opt

(Android-arm ... or Android-main ... also seem reasonable, but my personal preference is to keep the default/most-used case as short and simple as possible. I wouldn't want to see "fennec" used for these labels; I'm not sure that "fennec" is very well recognized outside of the mobile project.)

Comment 5

[Nick Alexander :nalexander [he/him]]

There are two significant levels at which we might want to split the delivered artifacts: at the Gecko level (C/C++/Rust code) and at the Android/Java level (Java code).  The existing splits by API and CPU split at both levels; that is, different C++ code shipped depending on the API and the CPU.  I doubt we'll ever stop shipping different C++ code depending on CPU, since that's not how JITs work :)

What we might do is change the level that we do Android API splits, making the C++ code API level agnostic (likely by having the C++ take the current Android API level as a runtime parameter) while having the Android/Java code specialize more finely.  The entire Gradle build system and ecosystem is designed to produce the split APKs that really matter in Android land: those that ship only the Android resources that the consuming device will use.  These split APKs are most naturally modeled as a *single* build step that produces a huge number of APKs; or, if we can manage it, as artifact-like builds that consume the single set of C++ built artifacts and produce a single split APK.

This approach trades some size (we might ship multiple implementations of a feature implemented in C++) while dramatically simplifying the splits (by moving the split logic mostly out of Mozilla's automation files, as documented in https://bugzilla.mozilla.org/show_bug.cgi?id=1398848#c0).

I think that argues in favour of the approach proposed by gbrown in https://bugzilla.mozilla.org/show_bug.cgi?id=1398848#c4, while  potentially addressing the issues raised by rnewman in https://bugzilla.mozilla.org/show_bug.cgi?id=1398848#c3.  That is, we:

1) drop the API levels from automation, instead focusing on the C++-specific configurations (CPU, opt/debug)
2) prepare for a world where any one automation build job will produce a huge number of APKs

Comment 6

[Bob Clary [:bc] (inactive)]

I don't like the fennec-main vs fennec-x86 naming. 'main' has no inherent meaning and is unrelated to cpu. I prefer that we keep things symmetric in that if we are using the cpu for x86 we should also do so for arm. Thus fennec-arm and fennec-x86 would be preferable in the apk filenames.

As for the Treeherder display, using the default Android to mean Android on arm while Android x86 would mean what its name explicitly means would be fine.

In terms of Treeherder labels. I definitely can not afford to lose the Android version number. Emulator jobs all use the same version but our differing devices in Autophone currently include 4.2, 4.4, 6.0, 7.1 with 8.0 coming soon. Autophone bugs are frequently version specific though that may also be an artifact that we currently have a fixed device to android version mapping.

If the future includes a large number of apks differentiated by various attributes which may or may not include minimum api, or form factor or other attributes, I would only ask that we specify a consistent naming pattern where the meta data describing a particular build is available from the filename.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Nick Alexander :nalexander [parental leave until September 15th] from comment #5)
> 
> 1) drop the API levels from automation, instead focusing on the C++-specific
> configurations (CPU, opt/debug)

So android-armv7-opt, android-aarch64-opt, android-x86-debug, etc? Sounds fine to me.

> 2) prepare for a world where any one automation build job will produce a
> huge number of APKs

A lot of different APKs does seem inevitable, but I don't think we will need i.e. mochitest jobs for all of them. Ideally, the current set of automated tests (modulo robocop) would be for "gecko", would run against a GeckoView app crafted specifically for running tests, and would not really care too much about the SDK split. The frontend tests could then run whatever they want  on various APKs and probably not in the mozilla automation system.

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Nick Alexander :nalexander [parental leave until September 15th] from comment #5)
> The entire Gradle build system and ecosystem is
> designed to produce the split APKs that really matter in Android land: those
> that ship only the Android resources that the consuming device will use. 
> These split APKs are most naturally modeled as a *single* build step that
> produces a huge number of APKs; or, if we can manage it, as artifact-like
> builds that consume the single set of C++ built artifacts and produce a
> single split APK.

Agree here: If we ever get back to split APKs then this should be on top of Gradle and not require multiple build tasks.

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Bob Clary [:bc:] from comment #6)
> I don't like the fennec-main vs fennec-x86 naming. 'main' has no inherent
> meaning and is unrelated to cpu. I prefer that we keep things symmetric in
> that if we are using the cpu for x86 we should also do so for arm. Thus
> fennec-arm and fennec-x86 would be preferable in the apk filenames.
> 
> As for the Treeherder display, using the default Android to mean Android on
> arm while Android x86 would mean what its name explicitly means would be
> fine.
> 
> In terms of Treeherder labels. I definitely can not afford to lose the
> Android version number. Emulator jobs all use the same version but our
> differing devices in Autophone currently include 4.2, 4.4, 6.0, 7.1 with 8.0
> coming soon. Autophone bugs are frequently version specific though that may
> also be an artifact that we currently have a fixed device to android version
> mapping.

I agree: Android test jobs need to include the details of the test environment in the names.  I think this is similar to Mac OS X build (and test?) jobs, and Windows build (and test?) jobs, which historically have included the SDK/target OS version in the name.

> If the future includes a large number of apks differentiated by various
> attributes which may or may not include minimum api, or form factor or other
> attributes, I would only ask that we specify a consistent naming pattern
> where the meta data describing a particular build is available from the
> filename.

I think this is well supported by Gradle: all of the "attribute dimensions" are baked into the resulting APK file names.  We can keep this in mind, certainly.

Comment 10

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

Comment 11

[Johan Lorenzo [:jlorenzo]]

Fennec doesn't exist anymore! Closing bug.