Last Comment Bug 673799 - Improve auto-discovering of Android toolchain directory
: Improve auto-discovering of Android toolchain directory
[needs NSPR update before fixing]
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Mounir Lamouri (:mounir)
: Gregory Szorc [:gps]
Depends on:
  Show dependency treegraph
Reported: 2011-07-24 11:34 PDT by Mounir Lamouri (:mounir)
Modified: 2011-08-19 09:57 PDT (History)
1 user (show)
mounir: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (1.47 KB, patch)
2011-07-24 11:34 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Patch v2 (58.51 KB, patch)
2011-07-24 16:10 PDT, Mounir Lamouri (:mounir)
blassey.bugs: review+
Details | Diff | Splinter Review
NSPR patch (1.45 KB, patch)
2011-07-25 10:56 PDT, Mounir Lamouri (:mounir)
ted: review+
Details | Diff | Splinter Review
NSPR Patch (1.52 KB, patch)
2011-07-29 12:02 PDT, Mounir Lamouri (:mounir)
bugspam.Callek: checkin+
Details | Diff | Splinter Review
m-c patch (2.96 KB, patch)
2011-07-29 12:03 PDT, Mounir Lamouri (:mounir)
mounir: checkin+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2011-07-24 11:34:30 PDT
Created attachment 548034 [details] [diff] [review]
Patch v1
Comment 1 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-24 11:57:18 PDT
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) 2011-07-24 16:10:04 PDT
Created attachment 548055 [details] [diff] [review]
Patch v2
Comment 3 Brad Lassey [:blassey] (use needinfo?) 2011-07-25 08:10:42 PDT
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) 2011-07-25 10:23:58 PDT
Ted, note that I've open a bug to backport the NSPR part to NSPR trunk.
Comment 5 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-25 10:27:05 PDT
We don't generally backport NSPR changes ... we land them upstream and pull a tag.
Comment 6 Mounir Lamouri (:mounir) 2011-07-25 10:56:26 PDT
Created attachment 548221 [details] [diff] [review]
NSPR patch
Comment 7 Ted Mielczarek [:ted.mielczarek] 2011-07-26 04:04:43 PDT
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] 2011-07-26 08:00:09 PDT
Comment on attachment 548055 [details] [diff] [review]
Patch v2

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

@@ +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 Ted Mielczarek [:ted.mielczarek] 2011-07-29 11:30:33 PDT
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/
@@ +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) 2011-07-29 12:02:36 PDT
Created attachment 549433 [details] [diff] [review]
NSPR Patch

Ted, could you push this patch to the NSPR trunk?
Comment 11 Mounir Lamouri (:mounir) 2011-07-29 12:03:15 PDT
Created attachment 549434 [details] [diff] [review]
m-c patch

This patch will have to be pushed whet the NSPR patch will be in m-c.
Comment 12 Ted Mielczarek [:ted.mielczarek] 2011-08-08 09:12:09 PDT
Landed the NSPR patch:
Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.309; previous revision: 1.308
Checking in;
/cvsroot/mozilla/nsprpub/,v  <--
new revision: 1.311; previous revision: 1.310
Comment 13 Justin Wood (:Callek) 2011-08-10 00:51:48 PDT
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 Ted Mielczarek [:ted.mielczarek] 2011-08-10 05:09:43 PDT
We don't land NSPR patches directly in m-c.
Comment 15 Mounir Lamouri (:mounir) 2011-08-10 05:15:18 PDT
Ted, is there a bug for the next NSPR snapshot landing in m-c?
Comment 16 Ted Mielczarek [:ted.mielczarek] 2011-08-10 05:17:52 PDT
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) 2011-08-10 05:26:16 PDT
m-c patch has been pushed to inbound. This will not work until NSPR is updated in the tree.
Comment 18 Mounir Lamouri (:mounir) 2011-08-11 04:31:26 PDT

Have to wait for NSPR merge now.
Comment 19 Ted Mielczarek [:ted.mielczarek] 2011-08-19 09:57:36 PDT
The NSPR update landed:

Note You need to log in before you can comment on or make changes to this bug.