Bug 51948

Summary: [Qt] REGRESSION(r74776) TestNetscapePlugin doesn't link on OS X
Product: WebKit Reporter: Jan Erik Hanssen <jhanssen>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, commit-queue
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

Jan Erik Hanssen
Reported 2011-01-05 13:18:55 PST
TestNetscapePlugin fails to link on OS X, with the following error: g++ -headerpad_max_install_names -arch x86_64 -single_module -dynamiclib -Xarch_x86_64 -mmacosx-version-min=10.5 -o TestNetscapePlugIn PluginObjectMac.o PluginObject.o PluginTest.o TestObject.o DocumentOpenInDestroyStream.o EvaluateJSAfterRemovingPluginElement.o GetUserAgentWithNullNPPFromNPPNew.o NPRuntimeObjectFromDestroyedPlugin.o NPRuntimeRemoveProperty.o NullNPPGetValuePointer.o PassDifferentNPPStruct.o PluginScriptableNPObjectInvokeDefault.o main.o -F/Users/jhanssen/dev/WebKit/WebKitBuild-qt/Debug/lib -F/Users/jhanssen/dev/qt/lib -L/Users/jhanssen/dev/WebKit/WebKitBuild-qt/Debug/lib -L/Users/jhanssen/dev/qt/lib -framework QtWebKit -framework Carbon -framework Cocoa -framework QuartzCore -lX11 -framework QtGui -L/Users/jhanssen/dev/qt/lib -F/Users/jhanssen/dev/qt/lib -framework QtCore ld: library not found for -lX11 collect2: ld returned 1 exit status The problem is that r74776 (from bug 51700) introduced a dependency on libX11 for all Unix platforms, and Qt considers OS X a unix platform. I'll upload a patch shortly that addresses this problem.
Attachments
Patch (1.13 KB, patch)
2011-01-05 13:24 PST, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
Patch (1.16 KB, patch)
2011-01-05 16:18 PST, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jan Erik Hanssen
Comment 1 2011-01-05 13:24:05 PST
Created attachment 78033 [details] Patch
Ariya Hidayat
Comment 2 2011-01-05 16:13:04 PST
Comment on attachment 78033 [details] Patch re=me
Ariya Hidayat
Comment 3 2011-01-05 16:14:21 PST
Comment on attachment 78033 [details] Patch Apparently it needs more exclusion. r- meanwhile
Jan Erik Hanssen
Comment 4 2011-01-05 16:18:31 PST
Created attachment 78060 [details] Patch
Ariya Hidayat
Comment 5 2011-01-05 16:21:52 PST
Comment on attachment 78060 [details] Patch re=me
WebKit Commit Bot
Comment 6 2011-01-05 20:56:29 PST
Comment on attachment 78060 [details] Patch Clearing flags on attachment: 78060 Committed r75134: <http://trac.webkit.org/changeset/75134>
WebKit Commit Bot
Comment 7 2011-01-05 20:56:33 PST
All reviewed patches have been landed. Closing bug.
Konstantin Tokarev
Comment 8 2011-01-06 01:48:28 PST
I don't agree with this patch. If platform does not match win32, embedded, mac, and symbian, it does not mean that this platform uses X11 (e.g., it will break Haiku) I think that the most correct expression is unix:!mac It it was CMake, I'd used Q_WS_X11, but I don't know if it's available in QMake
Jan Erik Hanssen
Comment 9 2011-01-06 05:04:20 PST
(In reply to comment #8) > I don't agree with this patch. If platform does not match win32, embedded, mac, and symbian, it does not mean that this platform uses X11 (e.g., it will break Haiku) > > I think that the most correct expression is > > unix:!mac That won't work for Qt Embedded, which is still Unix but doesn't use X11. The expression from my last patch is already used by Qt (see $QTDIR/src/gui/gui.pro). If there is a CONFIG option for haiku then we might have to add that to the list.
Konstantin Tokarev
Comment 10 2011-01-06 05:28:18 PST
(In reply to comment #9) I don't know anything about Haiku, I only thought that it's erroneous to assume that everything "unknown" uses X11. Inside Tools/DumpRenderTree/qt/main.cpp Q_WS_X11 macro is used. For example it will be defined if Qt on Mac is compiled with X11 support. "Exotic" platforms may add patches to make Q_WS_X11 and related macros work properly. Unfortunately I don't know how to check for Q_WS_X11 in qmake (in CMake it's trivial). Let it be:)
Note You need to log in before you can comment on or make changes to this bug.