673799 - Improve auto-discovering of Android toolchain directory

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 548034 [details] [diff] [review]
Patch v1

/me delegates.

blassey is more qualified than me to review Android build stuff anyways.

Comment 2

[Mounir Lamouri (:mounir)]

Attachment: Patch v2

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 548055 [details] [diff] [review]
Patch v2

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

r=blassey for the non-nspr bits. Ted will need to review the nspr changes (and he may want them as a separate patch)

Comment 4

[Mounir Lamouri (:mounir)]

Ted, note that I've open a bug to backport the NSPR part to NSPR trunk.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We don't generally backport NSPR changes ... we land them upstream and pull a tag.

Comment 6

[Mounir Lamouri (:mounir)]

Attachment: NSPR patch

Comment 7

[(not currently active) Ted Mielczarek]

Can you move the NSPR patch over to bug 673818? It makes it easier to track NSPR changes when they're in their own bug.

Comment 8

[Mike Hommey [:glandium] OoO until Dec 30]

Comment on attachment 548055 [details] [diff] [review]
Patch v2

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

::: configure.in
@@ +300,5 @@
> +        AC_MSG_CHECKING([for android toolchain directory])
> +
> +        kernel_name=`uname -s | tr "[[:upper:]]" "[[:lower:]]"`
> +
> +        android_toolchain="$android_ndk"/build/prebuilt/$kernel_name-x86/arm-eabi-4.4.0

NDK 6 removed the arm-eabi-4.4.0 toolchain (and the toolchain directory changed), so effectively, without the --with-android-toolchain option, you can't build with NDK 6.

Also note that at least in theory, it should be possible to have a x86-64 toolchain (and considering I'm building a new NDK with a newer binutils and gcc, I might be doing that), so the hardcoded -x86 would get in the way.

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 548221 [details] [diff] [review]
NSPR patch

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

Note that this needs to land in NSPR CVS, not directly in mozilla-central.

::: nsprpub/configure.in
@@ +183,5 @@
> +
> +        if test -d "$android_toolchain" ; then
> +            AC_MSG_RESULT([$android_toolchain])
> +        else
> +            AC_MSG_ERROR([not found])

Should this error message instruct you to use --with-android-toolchain?

Comment 10

[Mounir Lamouri (:mounir)]

Attachment: NSPR Patch

Ted, could you push this patch to the NSPR trunk?

Comment 11

[Mounir Lamouri (:mounir)]

Attachment: m-c patch

This patch will have to be pushed whet the NSPR patch will be in m-c.

Comment 12

[(not currently active) Ted Mielczarek]

Landed the NSPR patch:
Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.309; previous revision: 1.308
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.311; previous revision: 1.310
done

Comment 13

[Justin Wood (:Callek)]

If either patch needs to/should land in m-c directly, please request checkin? or checkin-needed keyword again, and someone will push them. From the looks of it we are already done here.

Comment 14

[(not currently active) Ted Mielczarek]

We don't land NSPR patches directly in m-c.

Comment 15

[Mounir Lamouri (:mounir)]

Ted, is there a bug for the next NSPR snapshot landing in m-c?

Comment 16

[(not currently active) Ted Mielczarek]

No, there isn't. We should probably wait until after the next Aurora uplift, so that we don't have to worry about having unreleased NSPR changes in Aurora.

Comment 17

[Mounir Lamouri (:mounir)]

m-c patch has been pushed to inbound. This will not work until NSPR is updated in the tree.

Comment 18

[Mounir Lamouri (:mounir)]

http://hg.mozilla.org/mozilla-central/rev/06d657efa434

Have to wait for NSPR merge now.

Comment 19

[(not currently active) Ted Mielczarek]

The NSPR update landed:
http://hg.mozilla.org/mozilla-central/rev/4ab18c9e38b1