1352015 - (build-android-o) [meta] Build with Android O SDK (API 26)

Status: RESOLVED FIXED

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

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

We haven't switched to the M SDK (API 23/24) because of some issues (bug 1259098). Switching to Android O might be easier. This is the meta bug tracking the work needed for that.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sebastian Kaspari (:sebastian) from comment #0)
> We haven't switched to the M SDK (API 23/24) because of some issues

I didn't have enough coffee when I filed the bug: I meant the N SDK and API level 24/25.

Comment 2

[Nick Alexander :nalexander [he/him]]

In https://developer.android.com/preview/migration.html#bfa, I see:

"Writable and Executable Segments Enforced - For native libraries, Android O enforces the rule that data shouldn’t be executable, and code shouldn’t be writable"

Such restrictions tend to target JITs.  snorp, jchen, others: can you try to figure out if Gecko will run afoul of this?  If we will, then there's a seriously important hurdle coming down the pipe that we need to prepare for.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1352015 - DO NOT LAND - Hack together a working build targeting Android O. f?sebastian

With these changes, I have an x86 artifact build installing, running,
and rendering on the Android O x86 emulator image (identified as
"system-images;android-O;google_apis;x86").

My intention with posting this is to merely show what I needed to do
to get this working locally; I'll file follow-up tickets tracking the
work needed to make this happen for real.  No try builds yet 'cuz I
don't feel like building tooltool packages right now.

There are several areas of incompatibility that will need to be
audited or manually tested to see if we're Android O compatible, but
that's future work for other people.

Review commit: https://reviewboard.mozilla.org/r/125648/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/125648/

Comment 4

[Nick Alexander :nalexander [he/him]]

sebastian: my WIP patch, if you want to play with this.

snorp, jchen: doesn't look like our JIT is hitting the issues I flag above.  Still worth one of you investigating, I think.

Comment 5

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

The native library changes should not affect us. It sounds like they are basically targeting busted libraries there. Also we still use our own linker to load libxul. We should just push the SDK bump to Try and see what breaks.

Comment 6

[Jan Henning [:JanH]]

According to https://android-developers.googleblog.com/2017/12/improving-app-security-and-performance.html, we *have* to get the target API change to 26 onto the Release channel by November 2018, i.e. Firefox 63 [1] at the latest.

If we want to make life easier for Orfox, we'd actually have to get this into the ESR as well, which would mean Firefox 60.

[1] Planned release 2018-10-23, but a) best to leave some room for potential slippage and b) potential dot releases in November!

Comment 7

[Nevin Chen(Not active on Bugzilla)]

Also ni Max

Comment 8

[Nick Alexander :nalexander [he/him]]

Just FYI, this should be trivial to experiment with locally and in automation: "just" update the various versions scattered throughout the tree, in particular:

https://searchfox.org/mozilla-central/rev/b24e6342d744c5a83fab5c15972e11eeb69d68e6/old-configure.in#2147
https://searchfox.org/mozilla-central/source/python/mozboot/mozboot/android-packages.txt#2

Comment 9

[Wesly Huang (EPM)]

Per the update in Austin the 2018 mobile planning is still on-going, so far I'm not sure if this is still a priority for fennec, keep monitoring.

Comment 10

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

I think the main blocker to this is that audio will be broken, bug 1410456.

Comment 11

[Andrei Lazar]

Attachment: Bug 1352015 - changed the targetSdkVersion to 26. 1352015 - changed the targetSdkVersion to 26. 1352015 - changed the targetSdkVersion to 26. 1352015 - changed the targetSdkVersion to 26. 1352015 - changed the targetSdkVersion to 26.

Review commit: https://reviewboard.mozilla.org/r/236058/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/236058/

Comment 12

[Andrei Lazar]

Attachment: Bug 1352015 - changed the targetSdkVersion to 26.

Review commit: https://reviewboard.mozilla.org/r/236060/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/236060/

Comment 13

[Jan Henning [:JanH]]

Comment on attachment 8967370 [details]
Bug 1352015 - changed the targetSdkVersion to 26.

https://reviewboard.mozilla.org/r/236060/#review241802

nalexander might be the best person for a final go-ahead, but in the meantime I have a number of objections here:

::: old-configure.in:2078
(Diff revision 1)
>  dnl mobile target.
>  dnl ========================================================
>  
>  case "$MOZ_BUILD_APP" in
>  mobile/android)
> -    MOZ_ANDROID_SDK(23, 23, 26.0.2, 26.0.0 26.0.0-dev 25.3.2 25.3.1)
> +    MOZ_ANDROID_SDK(23, 26, 26.0.2, 26.0.0 26.0.0-dev 25.3.2 25.3.1)

1. Are you sure that upgrading the Leanplum SDK required bumping the **target** SDK version, instead of just the **compile** SDK version?
2. I don't think you can actually target a higher version than the version you're compiling for, i.e. you'd have to raise the compile SDK version to at least 26 as well.
3. Most importantly, raising the target SDK version isn't something that should be done lightly, as this will turn off a number of compatibility behaviours that we benefit from when running under Android N or O. So simply raising the target SDK version without any further work will leave Firefox users on Android N and higher in a broken state. I therefore very strongly suggest only raising the target SDK version after **all** [issues blocking bug 1450450](https://bugzilla.mozilla.org/showdependencytree.cgi?id=1450450&hide_resolved=1) have been resolved.

Comment 14

[Andrei Lazar]

(In reply to Jan Henning [:JanH] from comment #13)
> Comment on attachment 8967370 [details]
> Bug 1352015 - changed the targetSdkVersion to 26.
> 
> https://reviewboard.mozilla.org/r/236060/#review241802
> 
> nalexander might be the best person for a final go-ahead, but in the
> meantime I have a number of objections here:
> 
> ::: old-configure.in:2078
> (Diff revision 1)
> >  dnl mobile target.
> >  dnl ========================================================
> >  
> >  case "$MOZ_BUILD_APP" in
> >  mobile/android)
> > -    MOZ_ANDROID_SDK(23, 23, 26.0.2, 26.0.0 26.0.0-dev 25.3.2 25.3.1)
> > +    MOZ_ANDROID_SDK(23, 26, 26.0.2, 26.0.0 26.0.0-dev 25.3.2 25.3.1)
> 
> 1. Are you sure that upgrading the Leanplum SDK required bumping the
> **target** SDK version, instead of just the **compile** SDK version?
> 2. I don't think you can actually target a higher version than the version
> you're compiling for, i.e. you'd have to raise the compile SDK version to at
> least 26 as well.
> 3. Most importantly, raising the target SDK version isn't something that
> should be done lightly, as this will turn off a number of compatibility
> behaviours that we benefit from when running under Android N or O. So simply
> raising the target SDK version without any further work will leave Firefox
> users on Android N and higher in a broken state. I therefore very strongly
> suggest only raising the target SDK version after **all** [issues blocking
> bug
> 1450450](https://bugzilla.mozilla.org/showdependencytree.
> cgi?id=1450450&hide_resolved=1) have been resolved.

You are right, I was actually following the official documentation (https://developer.android.com/about/versions/oreo/android-8.0-changes.html#o-apps) for migrating to Oreo and somehow I've missed this. Thank you for the fast response.

Comment 15

[:sdaswani]

Yes thanks for the response Jan, I wasn't sure who was going to be able to review this. We both appreciate your review.

Comment 16

[Kim Moir [:kmoir] ET]

Andrei, do you intend to have these patches reviewed and run a try run?  Nick Alexander mentioned last week that someone from the build team needed to help with this but it looks like you are well on your way with this work.

Comment 17

[Andrei Lazar]

Kim, I am currently still working on this and the patches that I've uploaded are not "final", as soon as will finish this and the potential issues caused by this upgrade I will ask for help with a need info flag. Thank you very much!

Comment 18

[Jan Henning [:JanH]]

I've got a patch set which seems to be working without issues:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=997597dc2e5fadf87536692aca09819bf266ee8c

The more tricky bit will be updating the support library version as well, since doing that *does* cause some issues, see bug 1385464 comment 3.

Comment 19

[:sdaswani]

(In reply to Jan Henning [:JanH] from comment #18)
> I've got a patch set which seems to be working without issues:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=997597dc2e5fadf87536692aca09819bf266ee8c
> 
> The more tricky bit will be updating the support library version as well,
> since doing that *does* cause some issues, see bug 1385464 comment 3.

Thanks Jan. I think Andrei has been working on this too, so perhaps he can review your code and/or compare it to his work?

Comment 20

[Jan Henning [:JanH]]

Attachment: Bug 1352015 - Part 1: Fix Android SDK configure checks.

It's the *compile*SdkVersion that needs to match the installed Android SDK plat-
form in order to be able to build an app, whereas the *target*SdkVersion is
merely a compatibility flag.

Since the received wisdom is that targetSdkVersion should be <= compileSdk-
Version and Android Studio is also showing a warning to that effect if you
modify the build.gradle of a small sample app accordingly, I've also added a
corresponding configure check of our own to enforce this.

Review commit: https://reviewboard.mozilla.org/r/242114/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242114/

Comment 21

[Jan Henning [:JanH]]

Attachment: Bug 1352015 - Part 2: Build with Android O SDK.

Review commit: https://reviewboard.mozilla.org/r/242116/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242116/

Comment 22

[Jan Henning [:JanH]]

(In reply to :sdaswani from comment #19)
> (In reply to Jan Henning [:JanH] from comment #18)
> > I've got a patch set which seems to be working without issues:
> > https://treeherder.mozilla.org/#/
> > jobs?repo=try&revision=997597dc2e5fadf87536692aca09819bf266ee8c
> > 
> > The more tricky bit will be updating the support library version as well,
> > since doing that *does* cause some issues, see bug 1385464 comment 3.
> 
> Thanks Jan. I think Andrei has been working on this too, so perhaps he can
> review your code and/or compare it to his work?

Just raising the compileSdkVersion is rather unlikely to have any negative side-effects and thanks to the build system work by nalexander actually doing so is quite straightforward.
I'd be happy if someone else could also look at the support library issues, though.

Comment 23

[Jan Henning [:JanH]]

Comment on attachment 8973742 [details]
Bug 1352015 - Part 2: Build with Android O SDK.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242116/diff/1-2/

Comment 24

[Jan Henning [:JanH]]

Comment on attachment 8973742 [details]
Bug 1352015 - Part 2: Build with Android O SDK.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242116/diff/2-3/

Comment 25

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8973741 [details]
Bug 1352015 - Part 1: Fix Android SDK configure checks.

https://reviewboard.mozilla.org/r/242114/#review247992

Huh, I do believe you're correct.  If it's green on try, roll on.

Comment 26

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8973742 [details]
Bug 1352015 - Part 2: Build with Android O SDK.

https://reviewboard.mozilla.org/r/242116/#review247996

Thanks for making this easy for me to stamp!  Again, green try means good to go.

::: old-configure.in:2083
(Diff revision 3)
>  dnl mobile target.
>  dnl ========================================================
>  
>  case "$MOZ_BUILD_APP" in
>  mobile/android)
> -    MOZ_ANDROID_SDK(23, 23, 26.0.2, 26.0.0 26.0.0-dev 25.3.2 25.3.1)
> +    MOZ_ANDROID_SDK(26, 23, 26.0.2, 26.0.0 26.0.0-dev 25.3.2 25.3.1)

This will need rebasing on top of https://bugzilla.mozilla.org/show_bug.cgi?id=1453417.

Comment 27

[Jan Henning [:JanH]]

Comment on attachment 8973741 [details]
Bug 1352015 - Part 1: Fix Android SDK configure checks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242114/diff/1-2/

Comment 28

[Jan Henning [:JanH]]

Comment on attachment 8973742 [details]
Bug 1352015 - Part 2: Build with Android O SDK.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242116/diff/3-4/

Comment 29

[Jan Henning [:JanH]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c8fb4a5108676ee7935f785e5c57d5072d5232d

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60e83456adb9
Part 1: Fix Android SDK configure checks. r=nalexander
https://hg.mozilla.org/integration/autoland/rev/2e9ace719b06
Part 2: Build with Android O SDK. r=nalexander

Comment 31

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/60e83456adb9
https://hg.mozilla.org/mozilla-central/rev/2e9ace719b06