693329 - Update Android/x86 port

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Makoto Kato [:m_kato]]

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.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: fix

Comment 2

[Makoto Kato [:m_kato]]

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

Comment 4

[Wan-Teh Chang]

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.

Comment 5

[Makoto Kato [:m_kato]]

ted, could you review this?   Also, current arm build option is still broken. (see comment #2)

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 581554 [details] [diff] [review]
fix

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

Sorry for the delay. This looks fine.

Comment 7

[(not currently active) Ted Mielczarek]

This patch doesn't apply cleanly to NSPR CVS for me. The config.guess hunk fails.

Comment 8

[Makoto Kato [:m_kato]]

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

Comment 9

[Makoto Kato [:m_kato]]

Attachment: fix v2

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 598828 [details] [diff] [review]
fix v2

don't need update config.* since bug 722125 is landed.

Comment 11

[(not currently active) Ted Mielczarek]

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

Comment 13

[Wan-Teh Chang]

Attachment: 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.

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

Comment 14

[Makoto Kato [:m_kato]]

(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.

Comment 15

[Makoto Kato [:m_kato]]

Attachment: fix for latest cvs

Comment 16

[Wan-Teh Chang]

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