Closed
Bug 681558
Opened 12 years ago
Closed 12 years ago
NSPR supports Android-x86
Categories
(NSPR :: NSPR, enhancement, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
4.9
People
(Reporter: m_kato, Assigned: m_kato)
References
Details
Attachments
(1 file)
1.00 KB,
patch
|
wtc
:
review+
ted
:
review+
|
Details | Diff | Splinter Review |
buildconfig of NSPR should support Android-x86.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #555335 -
Flags: review?(wtc)
Comment 2•12 years ago
|
||
m_kato: thank you for the patch. Ted: who's the best person to review build system patches for Android? You, Brad Lassey, or Mike Hommey?
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → 4.9
Comment 3•12 years ago
|
||
Comment on attachment 555335 [details] [diff] [review] fix v1 In pr/src/Makefile.in: > ifeq ($(OS_ARCH),Linux) > ifeq ($(USE_PTHREADS), 1) >+ifeq ($(OS_TARGET),Android) >+# Android has no libpthread.so in NDK >+OS_LIBS = -ldl >+else > OS_LIBS = -lpthread -ldl >+endif > else > OS_LIBS = -ldl > endif m_kato: why doesn't Android-arm need this change?
Comment 4•12 years ago
|
||
I can review these patches.
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Wan-Teh Chang from comment #3) > Comment on attachment 555335 [details] [diff] [review] > fix v1 > > In pr/src/Makefile.in: > > > ifeq ($(OS_ARCH),Linux) > > ifeq ($(USE_PTHREADS), 1) > >+ifeq ($(OS_TARGET),Android) > >+# Android has no libpthread.so in NDK > >+OS_LIBS = -ldl > >+else > > OS_LIBS = -lpthread -ldl > >+endif > > else > > OS_LIBS = -ldl > > endif > > m_kato: why doesn't Android-arm need this change? There is no libpthread.so in NDK. On arm toolchain, -lpthread doesn't cause error even if there is no libpthread.so. But, on x86 toolchain, it causes error due to missing library. So we should remove -lpthread if target is Android.
Comment 6•12 years ago
|
||
Comment on attachment 555335 [details] [diff] [review] fix v1 r=wtc. Ted, when you review this patch, please verify that the Android NDK does not have libpthread.so.
Attachment #555335 -
Flags: review?(wtc)
Attachment #555335 -
Flags: review?(ted.mielczarek)
Attachment #555335 -
Flags: review+
Comment 7•12 years ago
|
||
Comment on attachment 555335 [details] [diff] [review] fix v1 Review of attachment 555335 [details] [diff] [review]: ----------------------------------------------------------------- I checked, the NDK does not include a pthread library, just the headers. I'm not sure why this didn't break on the ARM build. This patch looks fine.
Attachment #555335 -
Flags: review?(ted.mielczarek) → review+
Comment 8•12 years ago
|
||
Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 1.314; previous revision: 1.313 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 1.316; previous revision: 1.315 done Checking in pr/src/Makefile.in; /cvsroot/mozilla/nsprpub/pr/src/Makefile.in,v <-- Makefile.in new revision: 1.61; previous revision: 1.60 done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 9•12 years ago
|
||
Pushed to mozilla-central in the latest NSPR update: https://hg.mozilla.org/mozilla-central/rev/7e25caedd620
Comment 10•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #9) > Pushed to mozilla-central in the latest NSPR update: > https://hg.mozilla.org/mozilla-central/rev/7e25caedd620 The change you linked to isn't the same as the patch posted here...and that patch doesn't appear in current m-c.
Comment 11•12 years ago
|
||
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #10) > (In reply to Ehsan Akhgari [:ehsan] from comment #9) > > Pushed to mozilla-central in the latest NSPR update: > > https://hg.mozilla.org/mozilla-central/rev/7e25caedd620 > > The change you linked to isn't the same as the patch posted here...and that > patch doesn't appear in current m-c. You're right. Sorry, the confusion was on my part. Ted, was this excluded from the NSPR tag you created? If yes, I think we need to create another one...
Comment 12•12 years ago
|
||
I tagged that beta during the week of September 12th, and I landed this the 22nd, so yeah, this wouldn't have made the tag.
You need to log in
before you can comment on or make changes to this bug.
Description
•