Closed Bug 1352015 (build-android-o) Opened 8 years ago Closed 7 years ago

[meta] Build with Android O SDK (API 26)

Categories

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

All
Android
enhancement

Tracking

(fennec+, firefox60 wontfix, firefox61 wontfix, firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
fennec + ---
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: sebastian, Assigned: JanH)

References

Details

(Keywords: meta)

Attachments

(4 files, 1 obsolete file)

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.
(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.
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.
Flags: needinfo?(snorp)
Flags: needinfo?(nchen)
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.
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.
Flags: needinfo?(snorp)
Flags: needinfo?(nchen)
Blocks: 1385464
Depends on: 1410456
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!
Severity: normal → major
tracking-fennec: --- → ?
Flags: needinfo?(wehuang)
Flags: needinfo?(snorp)
Flags: needinfo?(jcheng)
Also ni Max
Flags: needinfo?(max)
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.
Flags: needinfo?(wehuang)
I think the main blocker to this is that audio will be broken, bug 1410456.
Flags: needinfo?(snorp)
tracking-fennec: ? → +
Flags: needinfo?(max)
Blocks: 1444776
Blocks: 1438716
Assignee: nobody → andrei.a.lazar
Attachment #8967364 - Attachment is obsolete: true
Attachment #8967364 - Flags: review?(sdaswani)
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.
Attachment #8967370 - Flags: review-
(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.
Yes thanks for the response Jan, I wasn't sure who was going to be able to review this. We both appreciate your review.
Attachment #8967370 - Flags: review?(sdaswani)
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.
Flags: needinfo?(andrei.a.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!
Flags: needinfo?(andrei.a.lazar)
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.
Assignee: andrei.a.lazar → jh+bugzilla
(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?
Flags: needinfo?(andrei.a.lazar)
(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 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.
Attachment #8973741 - Flags: review?(nalexander) → review+
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.
Attachment #8973742 - Flags: review?(nalexander) → review+
Pushed by mozilla@buttercookie.de: 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
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
No longer blocks: 1384866
Flags: needinfo?(andrei.a.lazar)
Flags: needinfo?(jcheng)
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 62 → mozilla62
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: