Last Comment Bug 661636 - [10.7 SDK] QuickDraw APIs undefined when doing 32-bit builds using the OS X 10.7 SDK
: [10.7 SDK] QuickDraw APIs undefined when doing 32-bit builds using the OS X 1...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Steven Michaud [:smichaud] (Retired)
:
Mentors:
Depends on:
Blocks: 661638
  Show dependency treegraph
 
Reported: 2011-06-02 13:03 PDT by Steven Michaud [:smichaud] (Retired)
Modified: 2011-06-11 06:51 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (3.20 KB, patch)
2011-06-02 14:31 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Review
Fix rev1 (support weak linking) (10.48 KB, patch)
2011-06-10 08:32 PDT, Steven Michaud [:smichaud] (Retired)
bgirard: review+
Details | Diff | Review
Copy of rev1 patch for someone else to land (10.70 KB, patch)
2011-06-10 14:08 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Review

Description Steven Michaud [:smichaud] (Retired) 2011-06-02 13:03:38 PDT
Most QuickDraw header files have disappeared from OS X 10.7, and from
the OS X 10.7 SDK.  This means that 32-bit builds fail on OS X 10.7
when you use the 10.7 SDK, or don't specify any SDK.

The QuickDraw APIs have long been obsolete (they've mostly been
deprecated since OS X 10.4).  But we still use them in a few places
(in 32-bit code) for backwards compatibility.  (QuickDraw APIs are
unavailable in 64-bit code.)

For as long as we still use these APIs, we'll need to supply the
definitions ourselves, when the OS no longer does so.  In a later
comment I'll post a patch that does this.
Comment 1 Steven Michaud [:smichaud] (Retired) 2011-06-02 13:33:25 PDT
Bug 659817 is another bug caused by changes to the QuickDraw header
files on OS X 10.7.  But that bug effects both 32-bit and 64-bit
builds.
Comment 2 Steven Michaud [:smichaud] (Retired) 2011-06-02 14:31:54 PDT
Created attachment 536978 [details] [diff] [review]
Fix

Benoit:  I'd normally ask Josh to review this patch, but I know he's
very busy with his new responsibilities.

If you don't feel comfortable reviewing this, I can still pass it to
Josh -- this bug isn't particularly urgent, so it doesn't matter if
the review takes a while.
Comment 3 Benoit Girard (:BenWa) 2011-06-02 15:21:16 PDT
I don't mind looking at it. Are the symbols still defined in the libraries even if they are not declared in the headers? They must otherwise this wouldn't link. Does this means they could be removed at any time and cause bustage?

Wouldn't it be better to exclude these symbols with #ifdef when we are building with the 10.7 SDK? I was under the impression that NP_NO_CARBON/MAC_CARBON_PLUGINS should solve this.
Comment 4 Steven Michaud [:smichaud] (Retired) 2011-06-02 15:42:05 PDT
> Are the symbols still defined in the libraries even if they are not
> declared in the headers?

Yes, and they do get linked successfully.

> Does this means they could be removed at any time and cause bustage?

Judging by Apple's past practice, the earliest these APIs could be
removed entirely (including from libraries) is with OS X 10.8 -- this
kind of change generally only takes place in a major release.  And
even after they're removed, they should still be available as weak
links.  But trying to follow a weak link would (presumably) cause
runtime bustage.  So I should probably revise my patch to check
whether or not any QuickDraw API that we try to call is a weak link.

Another way to fix the problem is to stop using QuickDraw, even in
32-bit mode.  But this may take a while.

> Wouldn't it be better to exclude these symbols with #ifdef when we
> are building with the 10.7 SDK?

We still need them in a few places in 32-bit mode.
Comment 5 Steven Michaud [:smichaud] (Retired) 2011-06-02 15:43:04 PDT
Comment on attachment 536978 [details] [diff] [review]
Fix

I'll post another patch later, probably tomorrow.
Comment 6 Benoit Girard (:BenWa) 2011-06-02 15:57:08 PDT
What kind of weak link are you referring to? 
gcc's __attribute__ ((weak))?

> 
> We still need them in a few places in 32-bit mode.

That's for Quickdraw support for plugins that still need them?
Comment 7 Steven Michaud [:smichaud] (Retired) 2011-06-02 16:34:22 PDT
> What kind of weak link are you referring to?
> gcc's __attribute__ ((weak))?

Actually I'm not entirely sure.  I need to find out.

>> We still need them in a few places in 32-bit mode.
>
> That's for Quickdraw support for plugins that still need them?

Yes.  We currently use it to support plugin printing (see
nsObjectFrame::PrintPlugin()) and both QuickDraw and CoreGraphics in
32-bit mode (see nsChildView::StartDrawPlugin()).  But I hope that
we'll eventually be able to find alternatives that don't require
QuickDraw.

> Wouldn't it be better to exclude these symbols with #ifdef when we
> are building with the 10.7 SDK?

I'm beginning to wonder if this isn't the best solution, after all.
We're not going to be using the 10.7 SDK to do official builds anytime
soon (if only to keep supporting 10.6 and earlier).  It's quite
possible (and I hope likely) that we'll no longer be using any
QuickTime APIs by the time we need to start using the 10.7 SDK
officially.  So it may be a mistake to go to a lot of trouble to keep
using QuickTime APIs with the 10.7 SDK.

I'll sleep on it, and see what I think tomorrow :-)
Comment 8 Steven Michaud [:smichaud] (Retired) 2011-06-02 16:39:23 PDT
> It's quite possible (and I hope likely) that we'll no longer be
> using any QuickTime APIs by the time we need to start using the 10.7
> SDK officially.  So it may be a mistake to go to a lot of trouble to
> keep using QuickTime APIs with the 10.7 SDK.

QuickTime -> QuickDraw
Comment 9 Benoit Girard (:BenWa) 2011-06-02 18:09:31 PDT
(In reply to comment #7)
> I'm beginning to wonder if this isn't the best solution, after all.
> We're not going to be using the 10.7 SDK to do official builds anytime
> soon (if only to keep supporting 10.6 and earlier).  It's quite
> possible (and I hope likely) that we'll no longer be using any
> QuickTime APIs by the time we need to start using the 10.7 SDK
> officially.  So it may be a mistake to go to a lot of trouble to keep
> using QuickTime APIs with the 10.7 SDK.
> 
> I'll sleep on it, and see what I think tomorrow :-)

If we're inrested in using odd features from 10.7 we can dlopen() like we do for Core Animation.
Comment 10 Steven Michaud [:smichaud] (Retired) 2011-06-10 08:32:45 PDT
Created attachment 538521 [details] [diff] [review]
Fix rev1 (support weak linking)

I've changed my mind yet again:  I think it's best to weak-link to all
the QuickDraw APIs we still use, and check whether or not they're
present before we use them.

It helps that Apple has its own support for weak linking, which makes
this quite easy to do.

As I mentioned above, the actual calls are still present on OS X 10.7
(though they're missing from the relevant header files), and will
probably stay available at least until OS X 10.8.  But I did test a
version of my patch that assumed they're absent now, and I didn't see
any terrible problems (the worst was that plugins no longer print).

Let's hope we can stop using any of these QuickDraw APIs before they
actually disappear.  But with this patch we can deal gracefully with
their disappearance -- whenever that happens.

(I'll open a bug on rewriting our Mac plugin printing code to no
longer use QuickDraw APIs.)
Comment 11 Benoit Girard (:BenWa) 2011-06-10 10:01:20 PDT
Comment on attachment 538521 [details] [diff] [review]
Fix rev1 (support weak linking)

I think it's a good solution. Ideally this code will be gone by the time these are competently removed.
Comment 12 Steven Michaud [:smichaud] (Retired) 2011-06-10 14:08:44 PDT
Created attachment 538604 [details] [diff] [review]
Copy of rev1 patch for someone else to land
Comment 13 Dão Gottwald [:dao] 2011-06-11 01:22:19 PDT
http://hg.mozilla.org/mozilla-central/rev/ee4ae99764ec

Note You need to log in before you can comment on or make changes to this bug.