Closed Bug 859984 Opened 7 years ago Closed 7 years ago

deploy android ndk r8e to all android build slaves

Categories

(Release Engineering :: General, defect)

ARM
Android
defect
Not set

Tracking

(firefox21 wontfix, firefox22+ fixed, firefox23+ fixed)

RESOLVED FIXED
Tracking Status
firefox21 --- wontfix
firefox22 + fixed
firefox23 + fixed

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

Details

Attachments

(6 files, 2 obsolete files)

it looks like we're starting to have trouble with ld OOMing when building libxul for android and b2g, so we should switch to a ndk that has a  x86_64 toolchain.  We can either build our own or use ndk r8e because that is the first one that comes with a prebuilt x86_64 toolchain.  Using ndk r8e seems easier, and provides gcc 4.6 which is the same release series we've been using.

Note this isn't to switch what we build with just to make the newer ndk available to test.
bug 859341 is android on mac and bug 854535 is b2g.
I got passed bug 859341 with NDK r8e but I had to add the toolchain option in the mozconfig because of the new folder structure:
ac_add_options --with-android-toolchain="/home/adriantamas/android-ndk-r8e/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86_64"
So, this is a bit more complicated. We only have interest in the x86_64 toolchain for r8e, but the android build environments are 32-bits, bit 64 :(
Depends on: 860246
this repacks the ndk without tests docs samples and tooll chains we don't use.
android sdk tarball
people.mozilla.org/~tsaunders/android-sdk-16.tar.bz2
The ndk files themselves have been added to the repo, but AFAICT these manifest files aren't actually used by the build system yet. Follow-up work needed (bug 860246 perhaps?)
Attachment #737627 - Flags: review?(trev.saunders)
Comment on attachment 737627 [details] [diff] [review]
Add the Android ndk r8e to the mobile tooltool manifests

I guess I can rubber stamp this, but I have no idea if its actually correct the tar balls sort of seem to line up with the directories but...

I guess we need to write a setup.sh and put it in tooltool too like we seem to do for clang or is there a better way to unpack the tarballs?
(In reply to Trevor Saunders (:tbsaunde) from comment #8)
> I guess we need to write a setup.sh and put it in tooltool too like we seem
> to do for clang or is there a better way to unpack the tarballs?

I don't know how the mobile team wants the contents of those tarballs arranged on disk, e.g. symlinks to avoid hardcoded versions in the code. Yes, you'll probably need a setup.sh, but I don't think I'm equipped to write it.

The gonk setup.sh looks like this for ref:

#!/bin/bash
# This script knows how to set up a gonk toolchain in a given builder's
# directory.
set -xe
rm -rf gonk-toolchain
tar jxf gonk-toolchain-1.tar.bz2
mv gonk-toolchain-1 gonk-toolchain

If you want to use my patch as a starting point, add the setup.sh entry, and then pass the setup.sh file to me to push up to tooltool, we can do that. I'm happy to review the new manifest at that point.
Attachment #737627 - Attachment is obsolete: true
Attachment #737627 - Flags: review?(trev.saunders)
Comment on attachment 737796 [details] [diff] [review]
bug 859984 - switch to x86_64 gcc 4.6 from ndk r8e in tooltool and sdk in tooltool

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

::: mobile/android/config/mozconfigs/android-armv6/debug
@@ +8,5 @@
>  
>  # Android
>  ac_add_options --target=arm-linux-androideabi
>  ac_add_options --with-arch=armv6
> +ac_add_options --with-android-ndk="$topsrcdir/android-ndk-r8e-arm-gcc4.6"

It would probably be more convenient if all NDKs ended up in the same directory under $topsrcdir, so that only the manifest would have to be changed in the future, like the SDK.
Attachment #737796 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #12)
> Comment on attachment 737796 [details] [diff] [review]
> bug 859984 - switch to x86_64 gcc 4.6 from ndk r8e in tooltool and sdk in
> tooltool
> 
> Review of attachment 737796 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/config/mozconfigs/android-armv6/debug
> @@ +8,5 @@
> >  
> >  # Android
> >  ac_add_options --target=arm-linux-androideabi
> >  ac_add_options --with-arch=armv6
> > +ac_add_options --with-android-ndk="$topsrcdir/android-ndk-r8e-arm-gcc4.6"
> 
> It would probably be more convenient if all NDKs ended up in the same
> directory under $topsrcdir, so that only the manifest would have to be
> changed in the future, like the SDK.


I was sort of concerned how the two seperate ndks would interact if we did that.

by the end of this I was thinking of consolodating the ndk / compiler options into a seperate file that could be included which I think would mostly resolve this issue too.
(In reply to Trevor Saunders (:tbsaunde) from comment #13)
> I was sort of concerned how the two seperate ndks would interact if we did
> that.

You wouldn't install several NDKs for a given build.
(In reply to Mike Hommey [:glandium] from comment #14)
> (In reply to Trevor Saunders (:tbsaunde) from comment #13)
> > I was sort of concerned how the two seperate ndks would interact if we did
> > that.
> 
> You wouldn't install several NDKs for a given build.

true, but I've heard conflicting things about if things are  installed for each individual build or only the first time after a clobber.
That sounds like something I said on IRC about how tooltool works but might have missed what your question really was. The build directory isn't shared between different builds, so android arm, armv6 and x86 wouldn't be swapping the ndk at all.
(In reply to Nick Thomas [:nthomas] from comment #16)
> That sounds like something I said on IRC about how tooltool works but might
> have missed what your question really was. The build directory isn't shared
> between different builds, so android arm, armv6 and x86 wouldn't be swapping
> the ndk at all.

I think what you said is correct for what the question was at the time, but for some reason I thought we might share a source tree between multiple types of builds, but I guess not.  In that case yeah just android-ndk/ would probably be a better name.

saddly I believe coop's already uploaded the tarballs that produce android-ndk-r8e-<arch>-gcc4.6 so changing this go around is going to be a little bit painful :/

It seems like the easiest thing to do might just be to fix this when we modify the ndk next.
Comment on attachment 737678 [details] [diff] [review]
bug 859984 - tooltool manifests for android ndk / sdk

Manifests look good. 

Unless you are going to commit these as part of another change, you can land them with DONTBUILD since they aren't used yet and save some unnecessary builds.
Attachment #737678 - Flags: review?(coop) → review+
Both setup.sh files are in the tooltool repo now.
(In reply to Chris Cooper [:coop] from comment #18)
> Comment on attachment 737678 [details] [diff] [review]
> bug 859984 - tooltool manifests for android ndk / sdk
> 
> Manifests look good. 
> 
> Unless you are going to commit these as part of another change, you can land
> them with DONTBUILD since they aren't used yet and save some unnecessary
> builds.

well, I was planning to land them with the other patch in this bug to start using them or do we need to do something else first?
http://hg.mozilla.org/projects/date/167ad164b0bb
http://hg.mozilla.org/projects/date/72c902e52a19
and fix up for manifest
http://hg.mozilla.org/projects/date/46f207ef9593
this doesn't make android-x86 work because I think the manifest should be in android-x86/releng.manifest not android-x86/android/releng.manifest
and android-arm6 seems to need its own copy of android/releng.manifest in android-arm6/releng.manifest
but I want to see if android is green before trying to fix that
Per request on irc, updated setup files:
http://people.mozilla.org/~tsaunders/setup.sh.arm --> 0c21a793d87773b3ed82d141bea33869a6b647e5dddab3ecfee1738751e5c8d3d0b9ca45d7453ef88d335949c73f935c37de3521978b318ca51d8369be894b92

http://people.mozilla.org/~tsaunders/setup.sh.x86 --> 3db7c58c2002e96e6c0c34468f3349ca7de17c7473e07edec054ee3083ae39e6ead0e2a8f9177c1ce5891539073280e1afda1a6518f78e999dddc5d3f02a1e18
Attachment #738415 - Attachment description: fix location of android x86 manifest and add one for android-noion → fix location of android x86 manifest and add one for android-armv6
(In reply to Trevor Saunders (:tbsaunde) from comment #24)
> Created attachment 738415 [details] [diff] [review]
> fix location of android x86 manifest and add one for android-armv6

Pushed on the date branch
https://hg.mozilla.org/projects/date/rev/e69420afaee0
Comment on attachment 738415 [details] [diff] [review]
fix location of android x86 manifest and add one for android-armv6

Don't forget to land with DONTBUILD if at all possible (especially during the b2g workweek).
Attachment #738415 - Flags: review?(coop) → review+
Comment on attachment 741494 [details] [diff] [review]
bug 859984 - allow to use tooltool ndk / sdk when available

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

::: mobile/android/config/mozconfigs/android-armv6/toolchain
@@ +1,2 @@
> +ac_add_options --target=arm-linux-androideabi
> +ac_add_options --with-arch=armv6

I think you can keep these in the individual mozconfigs (if we want to factor further, there's more to factor along, and that's a candidate for a followup).

The toolchain ones below, however, are probably better shared in a single file for all android platforms. mobile/android/config/mozconfigs/common seems like the right place to do so.

@@ +1,4 @@
> +ac_add_options --target=arm-linux-androideabi
> +ac_add_options --with-arch=armv6
> +
> +if test -d "$topsrcdir/mobile/android/config/mozconfigs/android/toolchain"; then

This test is always going to be true. test `uname -m` = "x86_64" sounds better.

@@ +8,5 @@
> +  ac_add_options --with-android-ndk="/tools/android-ndk-r8c"
> +  ac_add_options --with-android-sdk="/tools/android-sdk-r16/platforms/android-16"
> +fi
> +
> +  ac_add_options --with-android-gnu-compiler-version=4.6

indentation
Attachment #741494 - Flags: review?(mh+mozilla) → review-
> The toolchain ones below, however, are probably better shared in a single
> file for all android platforms. mobile/android/config/mozconfigs/common
> seems like the right place to do so.

unfortunately that doesn't quiet work because I was silly and used different paths for arm / x86.


> @@ +1,4 @@
> > +ac_add_options --target=arm-linux-androideabi
> > +ac_add_options --with-arch=armv6
> > +
> > +if test -d "$topsrcdir/mobile/android/config/mozconfigs/android/toolchain"; then
> 
> This test is always going to be true. test `uname -m` = "x86_64" sounds
> better.

erg yeah, and I tested on try with the tooltool manifest patch in my queue, and locally without the tooltool ndk so didn't catch that :(
Attachment #746588 - Flags: review?(mh+mozilla)
Comment on attachment 746588 [details] [diff] [review]
bug 859984 - use tooltool ndk / sdk when on a x86_64 host

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

We'll need to backport this, too, like we did bug 864262. There might be the issue that not all of them are using ndk r8.
Attachment #746588 - Flags: review?(mh+mozilla) → review+
Refreshed version for landing on inbound.
Attachment #746588 - Attachment is obsolete: true
(In reply to Mike Hommey [:glandium] from comment #33)
> Created attachment 747160 [details] [diff] [review]
> use tooltool ndk / sdk when on a x86_64 host.
> 
> Refreshed version for landing on inbound.

This corresponds to the differences between m-i and c33fbaa07f7c on the date branch for the mobile/android/config subdirectory.
Comment on attachment 747160 [details] [diff] [review]
use tooltool ndk / sdk when on a x86_64 host.

[Approval Request Comment]
This is in preparation for switching android builders to a 64-bits environment. This is effectively a no-op when builds use a 32-bits builder, and will allow older branches to be pushed to try with 64-bits builders for android builds. ESR17 is using NDK r5c, so that won't work there, but I doubt anyone is pushing to try to do esr17 android builds.
Attachment #747160 - Flags: approval-mozilla-beta?
Attachment #747160 - Flags: approval-mozilla-aurora?
Blocks: 860246
No longer depends on: 860246
https://hg.mozilla.org/mozilla-central/rev/ddc934f971ec
Assignee: nobody → trev.saunders
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment on attachment 747160 [details] [diff] [review]
use tooltool ndk / sdk when on a x86_64 host.

we can take this on FF22 and it will get on beta on Monday.
Attachment #747160 - Flags: approval-mozilla-beta?
Attachment #747160 - Flags: approval-mozilla-beta-
Attachment #747160 - Flags: approval-mozilla-aurora?
Attachment #747160 - Flags: approval-mozilla-aurora+
Comment on attachment 747160 [details] [diff] [review]
use tooltool ndk / sdk when on a x86_64 host.

it would seem nobody could land this on aurora betwween friday evening and monday morning, any reason it would be a problem to land it on beta now?
Attachment #747160 - Flags: approval-mozilla-beta- → approval-mozilla-beta?
Attachment #747160 - Flags: approval-mozilla-beta?
Attachment #747160 - Flags: approval-mozilla-beta+
Attachment #747160 - Flags: approval-mozilla-aurora+
Backed out of mozilla-beta, since beta 2 died during mozconfig comparisons (mobile/android/config/mozconfigs/android/release was modified, but not mobile/android/config/mozconfigs/android/nightly).

http://hg.mozilla.org/releases/mozilla-beta/rev/adb3cef97557
(In reply to Aki Sasaki [:aki] from comment #43)
> Backed out of mozilla-beta, since beta 2 died during mozconfig comparisons
> (mobile/android/config/mozconfigs/android/release was modified, but not
> mobile/android/config/mozconfigs/android/nightly).
> 
> http://hg.mozilla.org/releases/mozilla-beta/rev/adb3cef97557

Sounds like a bad transplant, the file was supposed to be modified (it was modified when landing on inbound).
Product: mozilla.org → Release Engineering
Attachment #737336 - Attachment mime type: application/x-sh → text/plain
You need to log in before you can comment on or make changes to this bug.