Build trunk@136087 on Ubuntu 12.10 64 bit, $./Tools/Scripts/build-webkit --efl --cmakearg="-DENABLE_WEBGL=ON" [...] ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXTransportSurface::destroy(): error: undefined reference to 'XDestroyWindow' ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXPBuffer::configuration(): error: undefined reference to 'XFree' ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXPBuffer::configuration(): error: undefined reference to 'XOpenDisplay' ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXTransportSurface::configuration(): error: undefined reference to 'XRenderQueryExtension' ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXTransportSurface::configuration(): error: undefined reference to 'XRenderFindVisualFormat' [...] ../../../lib/libwebcore_efl.a(GraphicsSurfaceGLX.cpp.o):GraphicsSurfaceGLX.cpp:function WebCore::GraphicsSurface::platformGetTextureID(): error: undefined reference to 'XDefaultScreen' ../../../lib/libwebcore_efl.a(GraphicsSurfaceGLX.cpp.o):GraphicsSurfaceGLX.cpp:function WebCore::GraphicsSurface::platformGetTextureID(): error: undefined reference to 'XCompositeNameWindowPixmap' [...] It shows me X11, Xrender and XSomposite libs are not linked with libwebcore, will submit a patch later.
Created attachment 176670 [details] Patch
Comment on attachment 176670 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176670&action=review > Source/WebCore/ChangeLog:10 > + bin/DumpRenderTree What is this ? > Source/WebCore/PlatformEfl.cmake:302 > + "-lXcomposite -lXrender" IMO, it would be good if you link this libraries from FindXXX.cmake file or macro.
(In reply to comment #2) > (From update of attachment 176670 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=176670&action=review > > > Source/WebCore/ChangeLog:10 > > + bin/DumpRenderTree > > What is this ? This link error happens when link libwebcore_efl to DumpRenderTree, I just check whether this binary generated and can be run with my patch. I'll update the comment with more specific word. > > > Source/WebCore/PlatformEfl.cmake:302 > > + "-lXcomposite -lXrender" > > IMO, it would be good if you link this libraries from FindXXX.cmake file or macro. I do this because WebCore.pri add those two libs directly. 226 use?(GRAPHICS_SURFACE) { 227 mac: LIBS += -framework IOSurface -framework CoreFoundation 228 linux-*: { 229 LIBS += -lXcomposite -lXrender 230 CONFIG *= x11 231 } 232 } After going through the FindX11.cmake, I found X11_Xcomposite_LIB and X11_Xrandr_LIB are defined, I'll use them and update the patch.
Created attachment 176683 [details] Patch
Comment on attachment 176683 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176683&action=review > Source/WebCore/ChangeLog:6 > + Reviewed by Gyuyoung Kim. I don't set r+ yet. So, remove my name here.
Created attachment 176688 [details] Patch
Comment on attachment 176688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176688&action=review > Source/WebCore/ChangeLog:8 > + libwebcore_efl.a should link to X11, Xcomposite and Xrender libraries when WTF_USE_3D_GRAPHICS=1 It is ENABLE_WEBGL, not WTF_USE_3D_GRAPHICS
*** Bug 103107 has been marked as a duplicate of this bug. ***
Created attachment 176887 [details] Patch
Comment on attachment 176887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176887&action=review > Source/WebCore/ChangeLog:10 > + Test: bin/DumpRenderTree are generated and can be run. It looks this comment is meaningless. Generally, we have been written test case place in LayoutTest.
(In reply to comment #10) > > Source/WebCore/ChangeLog:10 > > + Test: bin/DumpRenderTree are generated and can be run. > > It looks this comment is meaningless. Generally, we have been written test case place in LayoutTest. Is that acceptable that I just remove the "Test:" line?
Comment on attachment 176887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176887&action=review >> Source/WebCore/ChangeLog:10 >> + Test: bin/DumpRenderTree are generated and can be run. > > It looks this comment is meaningless. Generally, we have been written test case place in LayoutTest. Replace with something like "No new tests, no behavior change for layout tests." This is common practice.
Created attachment 176909 [details] Patch
You already got r+ from Gyuyoung and only did a minor change. Do not request r? flag again, just cq?
(In reply to comment #14) > You already got r+ from Gyuyoung and only did a minor change. Do not request r? flag again, just cq? Thanks for let me know. In this case should I set the review to null or +?
(In reply to comment #15) > (In reply to comment #14) > > You already got r+ from Gyuyoung and only did a minor change. Do not request r? flag again, just cq? > Thanks for let me know. In this case should I set the review to null or +? You should not set r+. null is correct.
(In reply to comment #15) > (In reply to comment #14) > > You already got r+ from Gyuyoung and only did a minor change. Do not request r? flag again, just cq? > Thanks for let me know. In this case should I set the review to null or +? Don't change anything here, the patch is on its way to land. But next time, just let leave r flag empty and set only cq? (provided that you already got r+ on a previous patch that you did minimal changes to and provided that you updated the reviewer name in the Changelog).
Comment on attachment 176909 [details] Patch Clearing flags on attachment: 176909 Committed r136212: <http://trac.webkit.org/changeset/136212>
All reviewed patches have been landed. Closing bug.