Update Android/x86 port

RESOLVED FIXED in 4.9.1

Status

NSPR
NSPR
RESOLVED FIXED
6 years ago
6 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)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 581554 [details] [diff] [review]
fix
Attachment #581554 - Flags: review?(wtc)
(Assignee)

Comment 2

6 years ago
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
(Assignee)

Updated

6 years ago
Duplicate of this bug: 708723

Comment 4

6 years ago
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)
(Assignee)

Comment 5

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

Updated

6 years ago
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.
(Assignee)

Comment 8

6 years ago
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
(Assignee)

Comment 9

6 years ago
Created attachment 598828 [details] [diff] [review]
fix v2
(Assignee)

Comment 10

6 years ago
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)
(Assignee)

Updated

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.9.1

Updated

6 years ago
Duplicate of this bug: 746778

Comment 13

6 years ago
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.

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
(Assignee)

Comment 14

6 years ago
(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.
(Assignee)

Comment 15

6 years ago
Created attachment 621537 [details] [diff] [review]
fix for latest cvs
Attachment #621537 - Flags: review?(wtc)

Comment 16

6 years ago
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.