Status

defect
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

other
4.9.1
x86
Android

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

By bug 681558, nsprpub can build for android/x86, but...

- I forgot adding *_x86.s for Android.
- -msoft-float may cause missing symbols because Bionic x86 have no soft-float functions.
Posted patch fix (obsolete) — Splinter Review
Attachment #581554 - Flags: review?(wtc)
also, this has arm build fix.

- Although CPU_ARCH isn't set, we try to use CPU_ARCH.  we should use OS_TEST instead of. 
- OS_TARGET and OS_ARCH is invalid on current target.  We need sync config.* with mozilla-central version.

So although android arm builder wants to use -mthumb etc, these options isn't set now.
See https://tbpl.mozilla.org/php/getParsedLog.php?id=7921247&tree=Mozilla-Inbound&full=1
Duplicate of this bug: 708723
Comment on attachment 581554 [details] [diff] [review]
fix

Ted: could you also review this patch?  You are more familiar with
the build system code for Android.  Thanks.
Attachment #581554 - Flags: review?(ted.mielczarek)
ted, could you review this?   Also, current arm build option is still broken. (see comment #2)
Depends on: 722125
Blocks: 723713
Comment on attachment 581554 [details] [diff] [review]
fix

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

Sorry for the delay. This looks fine.
Attachment #581554 - Flags: review?(ted.mielczarek) → review+
This patch doesn't apply cleanly to NSPR CVS for me. The config.guess hunk fails.
ted, could you land bug 722125 before? I have no CVS access.

After landing it, I will rebase and send review request to you again
Posted patch fix v2Splinter Review
Comment on attachment 598828 [details] [diff] [review]
fix v2

don't need update config.* since bug 722125 is landed.
Attachment #598828 - Flags: review?(ted.mielczarek)
Attachment #581554 - Attachment is obsolete: true
Attachment #581554 - Flags: review?(wtc)
Attachment #598828 - Flags: review?(ted.mielczarek) → review+
Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.323; previous revision: 1.322
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.325; previous revision: 1.324
done
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.9.1
Duplicate of this bug: 746778
Share Android and Linux definitions in configure.in, to avoid anything
unexpectedly missing for Android.

The patch was originally attached to bug 746778 by Mike Hommey in
attachment 617399 [details] [diff] [review].  It is a more future-proof solution.

Makoto, could you please test it?  Thanks.

Patch checked in on the NSPR trunk (NSPR 4.9.1).

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.325; previous revision: 1.324
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.328; previous revision: 1.327
done
(In reply to Wan-Teh Chang from comment #13)
> Created attachment 621183 [details] [diff] [review]
> Patch by Mike Hommey
> 
> Share Android and Linux definitions in configure.in, to avoid anything
> unexpectedly missing for Android.
> 
> The patch was originally attached to bug 746778 by Mike Hommey in
> attachment 617399 [details] [diff] [review].  It is a more future-proof
> solution.
> 
> Makoto, could you please test it?  Thanks.

Humm, target_os of configure into nspr isn't android if using --target=i686-android-linux.  (google's toolchain name is i686-android-linux...).  So this code doesn't work correctly.
Attachment #621537 - Flags: review?(wtc)
Comment on attachment 621537 [details] [diff] [review]
fix for latest cvs

r=wtc.

Makoto: Thank you very much for testing the patch and fixing
the problem.  (I actually wondered about the proper value of
${target_os} for Android when I reviewed the patch.)  Your
use of ${target} makes it easier to see it is correct.

I checked in your patch on the NSPR trunk (NSPR 4.9.1).

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.326; previous revision: 1.325
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.329; previous revision: 1.328
done
Attachment #621537 - Flags: review?(wtc) → review+
Duplicate of this bug: 753254
No longer blocks: 723713
No longer depends on: 722125
You need to log in before you can comment on or make changes to this bug.