Closed Bug 543089 Opened 11 years ago Closed 11 years ago

XP_MACOSX should be defined when cross-compiling using toolwhip

Categories

(NSPR :: NSPR, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file)

Attached patch Proposed fixSplinter Review
Right now, the NSPR configure doesn't set XP_MACOSX unless it finds /System/Library/Frameworks/Carbon.framework/Carbon which means that trying to cross-compile Gecko using toolwhip fails to produce a properly working binary.  In particular, prlink ends up unable to load dylibs, which means NSS doesn't work correctly.

The proposed patch leaves the behavior as it is now by default, but if MACOS_SDK_DIR is explicitly set looks for Carbon in that SDK instead of looking for it in the "system default sdk".  This lets one use --with-macos-sdk for victory.
Attachment #424303 - Flags: review?(ted.mielczarek)
bz: your patch should be fine.  But NSPR should be able
to load dylibs when XP_MACOSX is not defined.  Only the
ability to load CFM & CFBundle plugins should be affected.
We should find out why...
I see.  I was confused between loading "loadable bundles"
and loading dylibs.  The loading of dylibs is only enabled
if XP_MACOSX is defined, which I believe is overly strict.

In any case bz's patch is fine by me, but let's have Ted
review it authoritatively.
> I was confused between loading "loadable bundles" and loading dylibs.  The
> loading of dylibs is only enabled if XP_MACOSX is defined

Yep, that's exactly what I was seeing.
Attachment #424303 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 424303 [details] [diff] [review]
Proposed fix

This looks fine. Would be easier to review if you hadn't included configure in the diff. :)

I'll land this for you on NSPR HEAD.
Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.266; previous revision: 1.265
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.270; previous revision: 1.269
done
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.8.4
You need to log in before you can comment on or make changes to this bug.