Bug 25998 - [GTK] Build failure: conflicting declaration 'typedef XID Cursor'
Summary: [GTK] Build failure: conflicting declaration 'typedef XID Cursor'
Status: RESOLVED LATER
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-24 18:27 PDT by Vincent Lefevre
Modified: 2012-02-03 15:45 PST (History)
5 users (show)

See Also:


Attachments
Patch to fix QuickDraw/X11 namespace conflict (702 bytes, patch)
2009-07-13 15:27 PDT, David Evans
no flags Details | Formatted Diff | Diff
Revised patch with ChangeLog (1.69 KB, patch)
2009-07-14 10:20 PDT, David Evans
jmalonzo: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Lefevre 2009-05-24 18:27:51 PDT
When building webkit r44078 under Mac OS X 10.4.11 (with GTK+ using X11 rendering, installed by MacPorts), I get the following error:

libtool: compile:  /usr/bin/g++-4.0 -DHAVE_CONFIG_H -I. -I./WebKitTools/DumpRenderTree -I./WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/ForwardingHeaders -I./WebCore -I./WebCore/bridge -I./WebCore/plugins -I./WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj -DBUILDING_CAIRO__=1 -DBUILDING_GTK__=1 -DWTF_CHANGES -DWTF_USE_ICU_UNICODE=1 -DNDEBUG -I./JavaScriptCore/ForwardingHeaders -I./JavaScriptCore/parser -I./JavaScriptCore/wtf -I./DerivedSources -I./JavaScriptCore -I./JavaScriptCore/API -I./JavaScriptCore/ForwardingHeaders -I./JavaScriptCore/interpreter -I./JavaScriptCore/bytecode -I./JavaScriptCore/bytecompiler -I./JavaScriptCore/debugger -I./JavaScriptCore/jit -I./JavaScriptCore/pcre -I./JavaScriptCore/profiler -I./JavaScriptCore/runtime -I./JavaScriptCore/wrec -I./JavaScriptCore/jit -I./JavaScriptCore/assembler -I./JavaScriptCore/wtf/unicode -I./JavaScriptCore/yarr -I./JavaScriptCore/pcre -I./JavaScriptCore/parser -I./JavaScriptCore/runtime -I/opt/local/include -O2 -O2 -MT WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/TestNetscapePlugin_libtestnetscapeplugin_la-TestNetscapePlugin.lo -MD -MP -MF WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/.deps/TestNetscapePlugin_libtestnetscapeplugin_la-TestNetscapePlugin.Tpo -c WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/TestNetscapePlugin.cpp  -fno-common -DPIC -o WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/.libs/TestNetscapePlugin_libtestnetscapeplugin_la-TestNetscapePlugin.o
/opt/local/include/X11/X.h:108: error: conflicting declaration 'typedef XID Cursor'
/System/Library/Frameworks/ApplicationServices.framework/Frameworks/QD.framework/Headers/Quickdraw.h:278: error: 'Cursor' has a previous declaration as 'typedef struct Cursor Cursor'
make[1]: *** [WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/TestNetscapePlugin_libtestnetscapeplugin_la-TestNetscapePlugin.lo] Error 1
make: *** [all] Error 2

It seems that Quickdraw.h (that is the cause of the error) is included via

    #include <ApplicationServices/ApplicationServices.h>

in WebCore/bridge/npapi.h.
Comment 1 David Evans 2009-07-13 15:22:33 PDT
This bug applies to X11 builds Mac OS X systems and stems from the fact that
QuickDraw and X11 use the same names for a number of key types, namely,

Cursor
WindowPtr
Picture
Picture
BOOL
EventType

The problem is restricted to the file

WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/TestNetscapePlugin.cpp

in release versions version         1.1.9 onward


Attached is a patch against release 1.1.11 which solves the problem
by renaming the QuickTime types locally so as to not conflict with
their X11 counterparts.

This patch has been in use successfully with the MacPorts port webkit-gtk
since version 1.1.10.
Comment 2 David Evans 2009-07-13 15:27:03 PDT
Created attachment 32683 [details]
Patch to fix QuickDraw/X11 namespace conflict
Comment 3 Jan Alonzo 2009-07-14 04:24:58 PDT
(In reply to comment #2)
> Created an attachment (id=32683) [details]
> Patch to fix QuickTime/X11 namespace conflict

Hi Vincent. Your patch needs a ChangeLog. Please use WebKitTools/Scripts/prepare-ChangeLog to do it.
Comment 4 David Evans 2009-07-14 10:20:46 PDT
Created attachment 32719 [details]
Revised patch with ChangeLog

Dave here, actually.  I'm one of the MacPorts maintainers for port webkit-gtk.

Attached is a revised patch with ChangeLog as requested.
Comment 5 Holger Freyther 2009-07-15 06:16:07 PDT
Where is the QuickDraw dependency coming from? from the npapi.h? npruntime.h?
Comment 6 Jan Alonzo 2009-07-15 06:29:18 PDT
(In reply to comment #5)
> Where is the QuickDraw dependency coming from? from the npapi.h? npruntime.h?

From npapi.h apparently (see comment #0)
Comment 7 David Evans 2009-07-15 08:30:16 PDT
(In reply to comment #5)
> Where is the QuickDraw dependency coming from? from the npapi.h? npruntime.h?

in WebCore/bridge/npapi.h
included from WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/ForwardingHeaders/WebKit/npapi.h

#if defined(__APPLE_CC__) && !defined(__MACOS_CLASSIC__) && !defined(XP_UNIX)
#   define XP_MACOSX
#endif

#ifdef XP_MAC
    #include <Quickdraw.h>
    #include <Events.h>
#endif

#if defined(XP_MACOSX) && defined(__LP64__)
#define NP_NO_QUICKDRAW
#define NP_NO_CARBON
#endif

#ifdef XP_MACOSX
    #include <ApplicationServices/ApplicationServices.h>
    #include <OpenGL/OpenGL.h>
#ifndef NP_NO_CARBON
    #include <Carbon/Carbon.h>
#endif
#endif
Comment 8 Holger Freyther 2009-07-15 08:41:28 PDT
(In reply to comment #7)
> (In reply to comment #5)
> > Where is the QuickDraw dependency coming from? from the npapi.h? npruntime.h?
> 
> in WebCore/bridge/npapi.h
> included from
> WebKitTools/DumpRenderTree/gtk/TestNetscapePlugin/ForwardingHeaders/WebKit/npapi.h
> 
> #if defined(__APPLE_CC__) && !defined(__MACOS_CLASSIC__) && !defined(XP_UNIX)
> #   define XP_MACOSX
> #endif
> 
> #ifdef XP_MAC
>     #include <Quickdraw.h>
>     #include <Events.h>
> #endif
> 
> #if defined(XP_MACOSX) && defined(__LP64__)
> #define NP_NO_QUICKDRAW
> #define NP_NO_CARBON
> #endif
> 
> #ifdef XP_MACOSX
>     #include <ApplicationServices/ApplicationServices.h>
>     #include <OpenGL/OpenGL.h>
> #ifndef NP_NO_CARBON
>     #include <Carbon/Carbon.h>
> #endif
> #endif


Could we define XP_UNIX when building the plugin? Any idea how to not get XP_MAC defined? I would pretty much prefer a solution that does not include Quickdraw.h and such in the first place.
Comment 9 David Evans 2009-07-15 10:11:02 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #5)
> 
> 
> Could we define XP_UNIX when building the plugin? Any idea how to not get
> XP_MAC defined? I would pretty much prefer a solution that does not include
> Quickdraw.h and such in the first place.

That would seem to need a wider code review to see what the impact might be.

Currently, the only build problem in this one file.

Note that it is possible to build with GTK+ on the Mac using either the
X11 or native quartz backends, so there may be legitimate needs for QuickDraw
that are not immediately apparent.  

In addition, I believe the Carbon and Applications Services frameworks probably
include QuickDraw indirectly although I haven't verified that as yet.

The namespace conflicts between QuickDraw and X11 are well known in the Apple
world and this fix is not original but is borrowed from usage in the XQuartz
project. As mentioned in the ChangeLog, jeremyhu from that group recommended the fix in the first place.

See 

http://cgit.freedesktop.org/xorg/xserver/tree/hw/xquartz/sanitizedCarbon.h

http://cgit.freedesktop.org/xorg/xserver/tree/hw/xquartz/sanitizedCocoa.h
Comment 10 Holger Freyther 2009-07-15 17:55:34 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #5)
> > 
> > 
> > Could we define XP_UNIX when building the plugin? Any idea how to not get
> > XP_MAC defined? I would pretty much prefer a solution that does not include
> > Quickdraw.h and such in the first place.
> 
> That would seem to need a wider code review to see what the impact might be.

Right, but it is a lot better than attempting undoing damage later.


> 
> Currently, the only build problem in this one file.
> 
> Note that it is possible to build with GTK+ on the Mac using either the
> X11 or native quartz backends, so there may be legitimate needs for QuickDraw
> that are not immediately apparent.

The plugin we are talking about is purely written for unix/X11 and Gtk+ does not support plugins on OSX yet, and once we do we will not use the Gtk+ plugin but the mac one (as it already supports the three different drawing models).
Comment 11 David Evans 2009-07-15 21:31:10 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (In reply to comment #7)
> > > > (In reply to comment #5)
> > > 
> > > 
> > > Could we define XP_UNIX when building the plugin? Any idea how to not get
> > > XP_MAC defined? I would pretty much prefer a solution that does not include
> > > Quickdraw.h and such in the first place.
> > 
> > That would seem to need a wider code review to see what the impact might be.
> 
> Right, but it is a lot better than attempting undoing damage later.

Sorry, I didn't mean to get into an argument here. 

What specific damage do you see here?  Do you have an alternative patch
that allows webkit-gtk to build on Mac platforms.  Last that built
as is, was 1.7.

Perhaps a better solution is to not include npapi.h here which seems
to include much more than is necessary (mac native includes for instance) and only include what is necessary to make this code work.

> 
> 
> The plugin we are talking about is purely written for unix/X11 and Gtk+ does
> not support plugins on OSX yet, and once we do we will not use the Gtk+ plugin
> but the mac one (as it already supports the three different drawing models).

Is this available in 1.11 or just in trunk and if so where?  How is it
configured Ideally on a Mac platform configure should detect whether gtk-x11 is available or not and enable the correct plugin (gtk-x11 or mac native
if I understand correctly).

Sorry for sounding dense but I'm new to the webkit code base and am trying to
understand.
Comment 12 Holger Freyther 2009-07-16 02:27:22 PDT
(In reply to comment #11)

> > Right, but it is a lot better than attempting undoing damage later.
> 
> Sorry, I didn't mean to get into an argument here. 
> 
> What specific damage do you see here?

At least some part of npapi.h believes we are supposed to use QuickDraw. This could mean that structs like  NPWindow, NPEvent, NPRegion... could have wrong sizes (e.g. missing a pointer we need on X11). We might be lucky and things just work... One proper fix is to make sure that our copy of npapi.h respects that we want to build for unix (Netscape's equivalent for using X11).





> Do you have an alternative patch

No, other patch... whenever I boot into OSX again I can come up with one, if you are willing to wait. The patch will be along the lines of defining XP_UNIX in the buildsystem and making sure that XP_MAC is not defined...


> > 
> > 
> > The plugin we are talking about is purely written for unix/X11 and Gtk+ does
> > not support plugins on OSX yet, and once we do we will not use the Gtk+ plugin
> > but the mac one (as it already supports the three different drawing models).
> 
> Is this available in 1.11 or just in trunk and if so where?  How is it
> configured Ideally on a Mac platform configure should detect whether gtk-x11 is
> available or not and enable the correct plugin (gtk-x11 or mac native
> if I understand correctly).
> 
> Sorry for sounding dense but I'm new to the webkit code base and am trying to
> understand.

There is no code for Gtk+ yet. If you look into WebCore/plugins/mac/ you will see the code used by Qt to support plugins on the mac. We would use the same code...and add #elif PLATFORM(GTK) to the nativeWindowFor and cgHandleFor method (and others were Qt specific code is) in the PluginViewMac.cpp.
Comment 13 Eric Seidel (no email) 2009-07-23 23:28:26 PDT
Comment on attachment 32719 [details]
Revised patch with ChangeLog

Why do these need to be separate blocks?  Are both #ifdef blocks needed?
Comment 14 Jan Alonzo 2009-07-31 21:30:38 PDT
Comment on attachment 32719 [details]
Revised patch with ChangeLog

r- for now per Holger's feedback on the ideal approach on this.
Comment 15 Martin Robinson 2012-02-03 15:45:35 PST
I'm sure there are many things to fix for OS X support. This patch has certainly bitrotted by now. I'm going to close this bug, but if anyone is interested in fixing trunk to build on OS X, open a new bug and CC me.