Closed Bug 645459 Opened 9 years ago Closed 8 years ago

Default to 32-bit NSPR builds on OS X 10.6 (Snow Leopard) and later

Categories

(NSPR :: NSPR, defect, P2)

x86_64
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mbugz, Assigned: mbugz)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch Proposed patch (obsolete) — Splinter Review
According to https://developer.mozilla.org/en/NSPR_build_instructions:

> On a dual 32-bit/64-bit platform, NSPR build generates a 32-bit build
> by default.

With OS X 10.6 (Snow Leopard), Apple changed the default for gcc to x86_64.

The attached patch (to configure.in) makes sure that the OS X build continues to default to 32 bit, by adding the "-arch i386" option. For the sake of consistency, I suggest applying a similar option for ppc, while we're at it.
Attachment #522179 - Flags: review?(wtc)
Blocks: 645460
(In reply to comment #0)
> The attached patch (to configure.in) makes sure that the OS X build continues
> to default to 32 bit

Why?
Comment on attachment 522179 [details] [diff] [review]
Proposed patch

r=wtc.

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

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.297; previous revision: 1.296
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.300; previous revision: 1.299
done
Attachment #522179 - Flags: review?(wtc) → review+
Priority: -- → P2
Target Milestone: --- → 4.8.8
(In reply to bug 654468 comment #2)
> I can improve the patch in NSPR bug 645459 to handle this properly,
> but for NSPR 4.8.8 it's simpler to just exclude that patch.

Here is one idea:

- default to i386 (instead of ppc)

- only append a "-arch" option if $CC doesn't have one already (so things like CC="gcc -arch x86_64 -arch i386" will still work and compile universal binaries in one step)
Attachment #529934 - Flags: review?(wtc)
I backed out the original change from NSPR trunk, as approved by Wan-Teh in bug 654468 comment 2 (end of that comment).

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.298; previous revision: 1.297
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.302; previous revision: 1.301
done
Would you mind to target this for 4.8.9 ?
Target Milestone: 4.8.8 → 4.8.9
Comment on attachment 529934 [details] [diff] [review]
Patch 2: only add "-arch" when needed

Thank you for the patch.

case "${target_cpu}" should also check for x86_64 (implies USE_64=1)
and i*86* or i?86.  The default case *) should not assume any CPU
architecture.  (Does autoconf treat iOS as arm-apple-darwin*?)
Attachment #529934 - Flags: review?(wtc) → review-
Attachment #522179 - Attachment is obsolete: true
(In reply to comment #6)
> Comment on attachment 529934 [details] [diff] [review] [review]
> case "${target_cpu}" should also check for x86_64 (implies USE_64=1)
> and i*86* or i?86.

I deliberately left out the check for x86_64, because I was assuming that --enable-64bit should be the only method to "force" configure to a 64-bit build (the original reason for this bug).

> The default case *) should not assume any CPU
> architecture.  (Does autoconf treat iOS as arm-apple-darwin*?)

Don't know about iOS, but so far, there was already a default case in the configure script... my patch just changes that from ppc to i386.

I'm not opposed to implementing a different approach, but then we should first agree on the new expected behavior (and perhaps change the scope of the bug).
I haven't had time to study how Mozilla builds NSPR
(I suspect some sort of cross-compilation setup is
necessary).  So I created a small patch that I think
should work for both standalone NSPR builds and
Mozilla-driven NSPR builds.

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

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.306; previous revision: 1.305
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.308; previous revision: 1.307
done
Attachment #529934 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Assignee: wtc → mozbugzilla
Summary: Default to 32-bit NSPR builds on OS X 10.6 (Snow Leopard) and later, too → Default to 32-bit NSPR builds on OS X 10.6 (Snow Leopard) and later
You need to log in before you can comment on or make changes to this bug.