859984 - deploy android ndk r8e to all android build slaves

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Trevor Saunders (:tbsaunde)]

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.

Comment 1

[Trevor Saunders (:tbsaunde)]

bug 859341 is android on mac and bug 854535 is b2g.

Comment 2

[Adrian Tamas (:AdrianT)]

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"

Comment 3

[Mike Hommey [:glandium]]

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 :(

Comment 4

[Trevor Saunders (:tbsaunde)]

stripped down ndk tar balls are here sdk tar ball will be coming in a bit.
http://people.mozilla.org/~tsaunders/android-ndk-r8e-arm-gcc4.6.tar.bz2
http://people.mozilla.org/~tsaunders/android-ndk-r8e-x86-gcc4.6.tar.bz2

Comment 5

[Trevor Saunders (:tbsaunde)]

Attachment: script to build stripped down ndk

this repacks the ndk without tests docs samples and tooll chains we don't use.

Comment 6

[Trevor Saunders (:tbsaunde)]

android sdk tarball
people.mozilla.org/~tsaunders/android-sdk-16.tar.bz2

Comment 7

[Chris Cooper [:coop] (he/him)]

Attachment: Add the Android ndk r8e to the mobile tooltool manifests

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?)

Comment 8

[Trevor Saunders (:tbsaunde)]

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?

Comment 9

[Chris Cooper [:coop] (he/him)]

(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.

Comment 10

[Trevor Saunders (:tbsaunde)]

Attachment: bug 859984 - tooltool manifests for android ndk / sdk

Comment 11

[Trevor Saunders (:tbsaunde)]

Attachment: bug 859984 - switch to x86_64 gcc 4.6 from ndk r8e in tooltool and sdk in tooltool

Comment 12

[Mike Hommey [:glandium]]

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.

Comment 13

[Trevor Saunders (:tbsaunde)]

(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.

Comment 14

[Mike Hommey [:glandium]]

(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.

Comment 15

[Trevor Saunders (:tbsaunde)]

(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.

Comment 16

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 17

[Trevor Saunders (:tbsaunde)]

(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 18

[Chris Cooper [:coop] (he/him)]

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.

Comment 19

[Chris Cooper [:coop] (he/him)]

Both setup.sh files are in the tooltool repo now.

Comment 20

[Trevor Saunders (:tbsaunde)]

(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?

Comment 21

[Trevor Saunders (:tbsaunde)]

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

Comment 22

[Nick Thomas [:nthomas] (UTC+12)]

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

Comment 23

[Trevor Saunders (:tbsaunde)]

use the setup.shs
http://hg.mozilla.org/projects/date/rev/9393674a643e

Comment 24

[Trevor Saunders (:tbsaunde)]

Attachment: fix location of android x86 manifest and add one for android-armv6

Comment 25

[Mike Hommey [:glandium]]

(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 26

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/projects/date/rev/8b41c370bc3a

Comment 27

[Chris Cooper [:coop] (he/him)]

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).

Comment 28

[Trevor Saunders (:tbsaunde)]

Attachment: bug 859984 - allow to use tooltool ndk / sdk when available

Comment 29

[Mike Hommey [:glandium]]

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

Comment 30

[Trevor Saunders (:tbsaunde)]

> 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 :(

Comment 31

[Trevor Saunders (:tbsaunde)]

Attachment: bug 859984 - use tooltool ndk / sdk when on a x86_64 host

Comment 32

[Mike Hommey [:glandium]]

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.

Comment 33

[Mike Hommey [:glandium]]

Attachment: use tooltool ndk / sdk when on a x86_64 host.

Refreshed version for landing on inbound.

Comment 34

[Mike Hommey [:glandium]]

(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 35

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ddc934f971ec

Comment 36

[Mike Hommey [:glandium]]

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.

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ddc934f971ec

Comment 38

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 39

[Trevor Saunders (:tbsaunde)]

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?

Comment 40

[Trevor Saunders (:tbsaunde)]

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/adb3cef97557

Comment 41

[Trevor Saunders (:tbsaunde)]

backed out of relbranch remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/08ee3a708e73

Comment 42

[Trevor Saunders (:tbsaunde)]

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/7e6b318f0947

Comment 43

[Aki Sasaki (not active)]

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

Comment 44

[Mike Hommey [:glandium]]

(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).

Comment 45

[Trevor Saunders (:tbsaunde)]

relanded
https://hg.mozilla.org/releases/mozilla-beta/rev/dfdffee03d15