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)
Firefox Build System
Android Studio and Gradle Integration
All
Android
Tracking
(fennec+, firefox60 wontfix, firefox61 wontfix, firefox62 fixed)
RESOLVED
FIXED
mozilla62
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.
Reporter | ||
Comment 1•8 years ago
|
||
(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•8 years ago
|
||
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)
Comment hidden (mozreview-request) |
Comment 4•8 years ago
|
||
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)
Updated•8 years ago
|
Flags: needinfo?(nchen)
Priority: -- → P1
Depends on: 1316934
Assignee | ||
Comment 6•7 years ago
|
||
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)
Comment 8•7 years ago
|
||
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•7 years ago
|
||
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: ? → +
Updated•7 years ago
|
Flags: needinfo?(max)
Assignee | ||
Updated•7 years ago
|
Blocks: target-android-o
Updated•7 years ago
|
Assignee: nobody → andrei.a.lazar
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8967364 -
Attachment is obsolete: true
Attachment #8967364 -
Flags: review?(sdaswani)
Assignee | ||
Comment 13•7 years ago
|
||
mozreview-review |
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-
Comment 14•7 years ago
|
||
(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•7 years ago
|
||
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)
Comment 16•7 years ago
|
||
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)
Comment 17•7 years ago
|
||
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)
Assignee | ||
Comment 18•7 years ago
|
||
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
Comment 19•7 years ago
|
||
(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)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 22•7 years ago
|
||
(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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 25•7 years ago
|
||
mozreview-review |
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 26•7 years ago
|
||
mozreview-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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 29•7 years ago
|
||
Comment 30•7 years ago
|
||
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
Comment 31•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/60e83456adb9
https://hg.mozilla.org/mozilla-central/rev/2e9ace719b06
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Updated•7 years ago
|
status-firefox60:
--- → wontfix
status-firefox61:
--- → wontfix
Updated•7 years ago
|
Flags: needinfo?(andrei.a.lazar)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(jcheng)
Updated•5 years ago
|
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.
Description
•