298529 - Core support for Mac 10.4u ("universal") SDK

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mark Mentovai]

The 10.4u ("universal") SDK will be used to build products for MacOSX86.  It
contains minor API changes.  Although we can't build MacOSX86 binaries yet, we
can target the 10.4u SDK on PPC and get the API changes out of the way early.

Also, the fact that there's a "u" in a critical part of the SDK version number
breaks some parts of the build.  Fallback code handles it gracefully and
properly in most areas, but we should switch from looking for a dot to looking
for a non-digit.

Comment 1

[Mark Mentovai]

Attachment: Configure fix for SDK version detection +

perl interprets 4u as 4 in a numeric context, but I'm so used to "use warnings"
that I'm not comfortable leaving it as-is.  I'm fixing the regexp.  While I'm
at it, I'm transitioning off of the hardcoded powerpc-apple-darwin for obvious
reasons.  I didn't use ${target_os} because it's in a block where any value
other than darwin would be preposterous, and using ${target_os} would require
more heinous regexp work to strip the version out.

Comment 2

[Mark Mentovai]

Attachment: API fix

This is the only API change needed to build the Fox under the 10.4u SDK.

http://developer.apple.com/documentation/MacOSX/Conceptual/universal_binary/universal_binary_tips/chapter_5_section_2.html

Comment 3

[Mark Mentovai]

Attachment: API fix

Sorry, this one's better.  Avoids two calls when only one would do.  The
dereference followed by address-of (via macro) for >= 10.4 is ugly, but the
code makes the most sense conceptually this way.  Alternatives would be to send
the pointer to the macro, then we'd dereference twice on < 10.4, or to just
have two separate code paths.  Not worth it, the compiler will optimize this
out anyway.

Comment 4

[Simon Fraser [no longer active]]

Comment on attachment 187094 [details] [diff] [review]
API fix

>Index: mozilla/xpcom/io/nsLocalFileOSX.cpp
>===================================================================

> // Mac Includes
> #include <Aliases.h>
> #include <Gestalt.h>
> #include <AppleEvents.h>
> #include <AEDataModel.h>
> #include <Processes.h>
> #include <Carbon/Carbon.h>
>+#include <AvailabilityMacros.h>

Can't we just do away with all the includes other than <Carbon/Carbon.h> ?

Rest looks good.

Comment 5

[Mark Mentovai]

Attachment: API fix

> Can't we just do away with all the includes other than <Carbon/Carbon.h> ?

Yup.

Comment 6

[Mark Mentovai]

Attachment: API fix, v4

-+#if !defined(MAC_OS_X_VERSION_10_4) || MAC_OS_X_VERSION_MIN_REQUIRED <
MAC_OS_X_VERSION_10_4
++#if !defined(MAC_OS_X_VERSION_10_4) || MAC_OS_X_VERSION_MAX_ALLOWED <
MAC_OS_X_VERSION_10_4

Ugh.

MIN_REQUIRED (platform to exec on) corresponds to MACOSX_DEPLOYMENT_TARGET.
MAX_ALLOWED (APIs to expose) corresponds to the SDK.

We want the latter.

Comment 7

[Mark Mentovai]

Attachment: API fix, v5

I've thought about this some more.  This is a sticky situation.  API patch v4
is definitely wrong.

There are two key factors here, the deployment target (specifies the minimum
runtime) and the SDK (build time).  DT <= SDK, always.

With SDK >= 10.4u, AliasRecord is an opaque type, so we're forced to use
GetAliasSizeFromPtr.

With DT < 10.4, we're forced into accessing the structure directly, because
GetAliasSizeFromPtr doesn't exist.

The problem is that you can specify, for example, DT 10.2 with SDK 10.4u.  See?
 Ouch.

So I'd rather test for the DT with MAC_OS_X_VERSION_MIN_REQUIRED as in API
patch v3.  If SDK >= 10.4u, the build breaks, which sucks.  The reason I prefer
this is that if testing for the SDK as v4 does, with a DT < 10.4, running the
app on a runtime < 10.4, the app will crash at runtime because there's no
GetAliasSizeFromPtr.  I'd rather break the build than break the app.

The other option is this patch, v5, which is the same as v3 except it handles
the possibility of build breakage by defining the structure when all else has
failed.  It's dirty, but it will work - even though Apple's made the structure
opaque, they can't change its layout.  I probably explained this better in the
patch comments.  This is the way I think we should go.

Comment 8

[Simon Fraser [no longer active]]

The standard way to deal with this situation ("call new routine at runtime if it
exists, otherwise do things the old way") is to test for "GetAliasSizeFromPtr"
at runtime, and call it if it exists (so that it can do byteswapping magic),
otherwise do things the old way.

Until we build using the 10.4SDK, we aren't subject to opaque alias records.

Comment 9

[Mark Mentovai]

Comment on attachment 187099 [details] [diff] [review]
API fix

Simon and I had a pow-wow on IRC and we came to the following conclusions:

- The run-time check won't help us, because there still needs to be something
to fall back on if the accessor isn't available.
- The DT < 10.4 with SDK >= 10.4 case may never apply in production, so maybe
it's better to not worry about it now.

So I'm de-obsoleting v3, which is the next-best alternative, with the warning
that it will break casual Mozilla ppc builds when no SDK or DT are set by the
user.

Comment 10

[Benjamin Smedberg]

Comment on attachment 187091 [details] [diff] [review]
Configure fix for SDK version detection +

please don't request approvals until reviews are complete

Comment 11

[Benjamin Smedberg]

Comment on attachment 187091 [details] [diff] [review]
Configure fix for SDK version detection +

Please land quickly (before Thursday morning PDT)

Comment 12

[Mark Mentovai]

Landed configure fix - still need to handle the API stuff with either v3 or v5.

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1478; previous revision: 1.1477
done

Comment 13

[Mark Mentovai]

Comment on attachment 187099 [details] [diff] [review]
API fix

I think the best way to go is to take v3 for now and then follow up with a new
"can't build without an SDK or DT set" bug.

Comment 14

[Mark Mentovai]

Checking in xpcom/io/nsLocalFileOSX.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileOSX.cpp,v  <--  nsLocalFileOSX.cpp
new revision: 1.29; previous revision: 1.28
done

The remaining API issue is bug 299213.

Comment 15

[Benjamin Smedberg]

Comment on attachment 187099 [details] [diff] [review]
API fix

a=bsmedberg for checkin on 6/30 only