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

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P2)

Description

[Kaspar]

Attachment: Proposed patch

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.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

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

[Wan-Teh Chang]

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

Comment 3

[Kaspar]

Attachment: Patch 2: only add "-arch" when needed

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

Comment 4

[Kai Engert (:KaiE:)]

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

Comment 5

[Kai Engert (:KaiE:)]

Would you mind to target this for 4.8.9 ?

Comment 6

[Wan-Teh Chang]

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*?)

Comment 7

[Kaspar]

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

Comment 8

[Wan-Teh Chang]

Attachment: Patch 3: combines the ideas of patches 1 and 2

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