The default bug view has changed. See this FAQ.

Improve auto-discovering of Android toolchain directory

RESOLVED FIXED

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [needs NSPR update before fixing])

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 548034 [details] [diff] [review]
Patch v1
Attachment #548034 - Flags: review?(khuey)
Attachment #548034 - Flags: feedback?(blassey.bugs)
(Assignee)

Updated

6 years ago
Assignee: nobody → mounir
Status: NEW → ASSIGNED
Whiteboard: [needs review]
Comment on attachment 548034 [details] [diff] [review]
Patch v1

/me delegates.

blassey is more qualified than me to review Android build stuff anyways.
Attachment #548034 - Flags: review?(khuey)
Attachment #548034 - Flags: review?(blassey.bugs)
Attachment #548034 - Flags: feedback?(blassey.bugs)
(Assignee)

Comment 2

6 years ago
Created attachment 548055 [details] [diff] [review]
Patch v2
Attachment #548034 - Attachment is obsolete: true
Attachment #548034 - Flags: review?(blassey.bugs)
Attachment #548055 - Flags: review?(blassey.bugs)
(Assignee)

Updated

6 years ago
Blocks: 673818
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)
Attachment #548055 - Flags: review?(ted.mielczarek)
Attachment #548055 - Flags: review?(blassey.bugs)
Attachment #548055 - Flags: review+
(Assignee)

Comment 4

6 years ago
Ted, note that I've open a bug to backport the NSPR part to NSPR trunk.
We don't generally backport NSPR changes ... we land them upstream and pull a tag.
(Assignee)

Comment 6

6 years ago
Created attachment 548221 [details] [diff] [review]
NSPR patch
Attachment #548221 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

6 years ago
Attachment #548055 - Flags: review?(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 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 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?
Attachment #548221 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 10

6 years ago
Created attachment 549433 [details] [diff] [review]
NSPR Patch

Ted, could you push this patch to the NSPR trunk?
Attachment #548221 - Attachment is obsolete: true
Attachment #549433 - Flags: checkin?
(Assignee)

Comment 11

6 years ago
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.
Attachment #548055 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Whiteboard: [needs review] → [needs landing]
(Assignee)

Updated

6 years ago
No longer blocks: 673818
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

Updated

6 years ago
Attachment #549433 - Flags: checkin? → checkin+
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.
We don't land NSPR patches directly in m-c.
(Assignee)

Comment 15

6 years ago
Ted, is there a bug for the next NSPR snapshot landing in m-c?
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.
(Assignee)

Updated

6 years ago
Attachment #549434 - Flags: checkin+
(Assignee)

Comment 17

6 years ago
m-c patch has been pushed to inbound. This will not work until NSPR is updated in the tree.
Whiteboard: [needs landing] → [needs NSPR update before fixing][inbound]
(Assignee)

Comment 18

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

Have to wait for NSPR merge now.
Flags: in-testsuite-
Whiteboard: [needs NSPR update before fixing][inbound] → [needs NSPR update before fixing]
The NSPR update landed:
http://hg.mozilla.org/mozilla-central/rev/4ab18c9e38b1
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.