If built without OpenGL, libwebkit2gtk fails to link because it is missing '-lX11' and friends. CMakeFiles/WebKit2.dir/PluginProcess/unix/PluginProcessMainUnix.cpp.o: In function `WebKit::webkitXError(_XDisplay*, XErrorEvent*)': PluginProcessMainUnix.cpp:(.text+0x11): undefined reference to `XGetErrorText' CMakeFiles/WebKit2.dir/PluginProcess/unix/PluginProcessMainUnix.cpp.o: In function `WebKit::PluginProcessMain::parseCommandLine(int, char**)': PluginProcessMainUnix.cpp:(.text._ZN6WebKit17PluginProcessMain16parseCommandLineEiPPc[_ZN6WebKit17PluginProcessMain16parseCommandLineEiPPc]+0xe2): undefined reference to `XSetErrorHandler' <snip> ...and a lot more errors along those lines As far as I can tell this is because OptionsGTK.cmake doesn't explicitly call find_package(X11), but relies on FindOpenGL doing so implicitly. ($X11_*_LIB are referenced in WebCore/PlatformGTK.cmake) However, OpenGL is an optional dependency and as such can be disabled using cmakes CMAKE_DISABLE_FIND_PACKAGE_FOO. (see [1]). I think the correct fix would be to explicitly call find_package(X11), probably inside if (ENABLE_X11_TARGET) [1] http://www.cmake.org/cmake/help/v3.0/variable/CMAKE_DISABLE_FIND_PACKAGE_PackageName.html
Any specific version of WebKitGTK+, or are you using tip-of-tree?
Tested with 2.6.2 and 2.7.1, but as far as I can tell it's still the same in current tip-of-tree.
It's not just libX11, there's also many other OpenGL symbols in platforms/graphics/*, is OpenGL really optional for us?
I think you at least need the headers while building.
(In reply to comment #4) > I think you at least need the headers while building. I actually made it work, following these steps: - Disabling WebGL as well. - Defining WTF_USE_TEXTURE_MAPPER=1 (but not TEXTURE_MAPPER_GL) - Building GraphicsLayerTextureMapper.cpp I can prepare a patch, but do we want this?
(In reply to comment #5) > (In reply to comment #4) > > I think you at least need the headers while building. > > I actually made it work, following these steps: > > - Disabling WebGL as well. > - Defining WTF_USE_TEXTURE_MAPPER=1 (but not TEXTURE_MAPPER_GL) > - Building GraphicsLayerTextureMapper.cpp > > I can prepare a patch, but do we want this? I don't think we want to support the fallback TextureMapper configuration (if it even exists any longer).
Created attachment 242768 [details] Patch Here's a quick & dirty patch to make it work, but if we don't want to support this we should close this and bug 138347 as well.
*** Bug 138347 has been marked as a duplicate of this bug. ***
I was building 2.6.4 without X11 on OSX (using the Quartz backend of GTK) and ran into a related issue while linking: Linking CXX shared library ../../lib/libwebkit2gtk-4.0.dylib Undefined symbols for architecture x86_64: "WebCore::GraphicsLayer::create(WebCore::GraphicsLayerFactory*, WebCore::GraphicsLayerClient&)", referenced from: WebKit::WebInspectorClient::showPaintRect(WebCore::FloatRect const&) in WebInspectorClient.cpp.o WebKit::PageOverlayController::initialize() in PageOverlayController.cpp.o WebKit::PageOverlayController::installPageOverlay(WTF::PassRefPtr<WebKit::PageOverlay>, WebKit::PageOverlay::FadeMode) in PageOverlayController.cpp.o WebCore::RenderLayerBacking::createGraphicsLayer(WTF::String const&) in libWebCoreGTK.a(RenderLayerBacking.cpp.o) WebCore::RenderLayerCompositor::ensureRootLayer() in libWebCoreGTK.a(RenderLayerCompositor.cpp.o) WebCore::RenderLayerCompositor::updateOverflowControlsLayers() in libWebCoreGTK.a(RenderLayerCompositor.cpp.o) ld: symbol(s) not found for architecture x86_64 I think the issue here is that without X11, GraphicsLayerTextureMapper.cpp is the only available implementation of WebCore::GraphicsLayer::create(). It's mentioned in comment 6 that the texture mapper is a fallback; I guess the question is fallback from what? What should provide GraphicsLayer::create() in the case of the Quartz backend WebKitGTK port?
I also had to add "${CMAKE_SOURCE_DIR}/Source/ThirdParty/ANGLE/include/GLSLANG" and "${CMAKE_SOURCE_DIR}/Source/ThirdParty/ANGLE/include/KHR" to the include directories in WebCore/PlatformGTK.cmake, by the way, for this fix to compile. I'll provide an updated patch.
Created attachment 243920 [details] Updated patch Updated patch.
Created attachment 252286 [details] Updated patch Updated patch for 2.8.0.
Attachment 252286 [details] did not pass style-queue: Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style.
I think this patch is out of date after the latest changes to the CMake build system.
The comment says the patch is for 2.8.0. :) It'd be great to have a patch that can apply on master as well. Also, the ANGLE directories are already appended to WebCore_INCLUDE_DIRECTORIES in WebCore/CMakeLists.txt, so they should be removed from there.
(In reply to comment #15) > The comment says the patch is for 2.8.0. :) It'd be great to have a patch > that can apply on master as well. > > Also, the ANGLE directories are already appended to > WebCore_INCLUDE_DIRECTORIES in WebCore/CMakeLists.txt, so they should be > removed from there. Oops. Sorry about the noise.
$ Tools/Scripts/svn-apply attachment.cgi\?id=252286 Parsed 0 diffs from patch file(s). It seems it isn't valid patch for svn-apply, similar to bug126433
Created attachment 252685 [details] Patch for 2.8.0 Here's a patch with the requested changes and with a changelog. If this doesn't apply to master, then I'm not sure when I'll get a chance to prepare a patch that does. Due to the esoteric platform (GTK on OSX) I'm compiling on, it's a rare occurrence that I can compile WebKit to a working state at all. For example, the entire 2.6.x series segfaulted on startup for me, and I never figured out why. On master, it tends to be a losing battle. Now that I have a working state on 2.8.0, I am trying to give you all the patches that I can, before they become stale :-( Any help would be appreciated. In any case I'll propose this for inclusion on https://trac.webkit.org/wiki/WebKitGTK/2.8.x
Created attachment 252818 [details] Patch for master Here's a patch that applies to master, as well. I've tested it on 2.9.1.
Thanks for these patches. In WebCore/CMakeLists.txt, I'd prefer to append the two ANGLE include directories once, outside the conditional. Why is one added in PlatformGTK.cmake? Is that needed only when GRAPHICS_CONTEXT_3D is off? I think that should still be added in WebCore/CMakeLists.txt.
Created attachment 252826 [details] Patch for master
You're welcome. I initially kept the GLSLANG directory out of CMakeLists.txt because it wasn't added either in the GRAPHICS_CONTEXT_3D case. But it surely can't be harmful to add it anyway, so I just moved it there as you suggested.
EFL bot is having problems: c++: internal compiler error: Killed (program cc1plus) Please submit a full bug report, with preprocessed source if appropriate. See <file:///usr/share/doc/gcc-4.8/README.Bugs> for instructions. Probably time for them to upgrade GCC, anyway.
(In reply to comment #23) > EFL bot is having problems: > > c++: internal compiler error: Killed (program cc1plus) > Please submit a full bug report, > with preprocessed source if appropriate. > See <file:///usr/share/doc/gcc-4.8/README.Bugs> for instructions. > > Probably time for them to upgrade GCC, anyway. Sorry for inconvenient. EFL EWS was sick yesterday. This patch is fine when building locally. Please land it !
(In reply to comment #24) > Sorry for inconvenient. EFL EWS was sick yesterday. This patch is fine when > building locally. Please land it ! Do you have time to review it? :) It looks good to me.
Comment on attachment 252826 [details] Patch for master View in context: https://bugs.webkit.org/attachment.cgi?id=252826&action=review > Source/cmake/OptionsGTK.cmake:281 > +SET_AND_EXPOSE_TO_BUILD(USE_TEXTURE_MAPPER TRUE) > + FWIW, we are going to be removing the non-OpenGL version of the texture mapper.
Comment on attachment 252826 [details] Patch for master LGTM, however it looks GTK folks might need to have final review.
Comment on attachment 252826 [details] Patch for master View in context: https://bugs.webkit.org/attachment.cgi?id=252826&action=review > Source/WebCore/CMakeLists.txt:3120 > + "${THIRDPARTY_DIR}/ANGLE/include/GLSLANG" I don't get why you added GLSLANG, if it wasn't needed before, why is it needed now? There's some conflict with the in-tree headers and the installed ones, so I prefer not to change this if it's not needed
Created attachment 253122 [details] Updated patch for master
(In reply to comment #28) > I don't get why you added GLSLANG, if it wasn't needed before, why is it > needed now? There's some conflict with the in-tree headers and the installed > ones, so I prefer not to change this if it's not needed I tested and indeed it wasn't needed. I think it was needed when I originally wrote this patch on 2.6.x and I just carried it over. I've updated the patch to remove it.
Attachment 253122 [details] did not pass style-queue: Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 253122 [details] Updated patch for master Rejecting attachment 253122 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 253122, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: fs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 184339 = 76760c75eaef3255b23d23a9f0f0aa94103e47dc r184340 = 23c2df7821affb0236a545d888809229f80b5dda r184341 = 5615cb361c49318e67ede4d0f1ecf598beaf7a74 r184344 = fbf429221728313e13303bdcd358c9a4543987e2 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.appspot.com/results/5510072457232384
Copying what seems to be the relevant output here: Processing patch 253122 from bug 138332. Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html The patch definitely does have a ChangeLog entry, so I'm not sure what's going on here.
The changed files in the diff have incorrect paths: a/Source/WebCore/ChangeLog a/Source/WebCore/CMakeLists.txt a/Source/WebCore/platform/graphics/texmap/BitmapTexturePool.cpp a/Source/cmake/OptionsGTK.cmake a/ChangeLog I don't think the diff was generated correctly.
Created attachment 255294 [details] Patch
I regenerated the patch; not sure what wasn't working before. Since the previous patch was already r+, I'm hoping it won't take too long to review this one.
Comment on attachment 255294 [details] Patch Clearing flags on attachment: 255294 Committed r185805: <http://trac.webkit.org/changeset/185805>
Comment on attachment 252685 [details] Patch for 2.8.0 View in context: https://bugs.webkit.org/attachment.cgi?id=252685&action=review > Source/WebCore/PlatformGTK.cmake:26 > + "${CMAKE_SOURCE_DIR}/Source/ThirdParty/ANGLE/include/GLSLANG" As in trunk patch I don't get why we need to add GLSLANG. > Source/WebCore/PlatformGTK.cmake:216 > + platform/graphics/texmap/GraphicsLayerTextureMapper.cpp > + Why do you need to move this file? This file doesn't belong to WebCorePlatformGTK since it doens't use GTK+ at all
Created attachment 260116 [details] Patch for 2.8.5
Here's an updated patch that applies to 2.8.5. Indeed, I had mistakenly put the file in WebCoreGTK when it should have gone in WebCore.
Comment on attachment 260116 [details] Patch for 2.8.5 View in context: https://bugs.webkit.org/attachment.cgi?id=260116&action=review It looks like this patch does more than fix the build when OpenGL is off. Some of these changes look like they are making the TextureMapper unconditional. > Source/WebCore/CMakeLists.txt:-2985 > - "${THIRDPARTY_DIR}/ANGLE/" > - "${THIRDPARTY_DIR}/ANGLE/include/KHR" This file is shared between all ports, so I don't think you should remove them from here. > Source/WebCore/PlatformGTK.cmake:128 > + platform/graphics/texmap/GraphicsLayerTextureMapper.cpp > + > platform/gtk/ErrorsGtk.cpp What is the purpose of compiling the TextureMapper unconditionally? > Source/cmake/OptionsGTK.cmake:355 > +add_definitions(-DWTF_USE_TEXTURE_MAPPER=1) > + Ditto.
(In reply to comment #41) > Comment on attachment 260116 [details] > Patch for 2.8.5 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=260116&action=review > > It looks like this patch does more than fix the build when OpenGL is off. > Some of these changes look like they are making the TextureMapper > unconditional. This is the same as what was committed to master in http://trac.webkit.org/changeset/185805. The TextureMapper (not the TextureMapperGL, though) was already unconditional in practice, because without it there was no implementation of WebCore::GraphicsLayer::create(). I wondered if there was any other way to fix this in comment #9 but I guess there wasn't. > > Source/WebCore/CMakeLists.txt:-2985 > > - "${THIRDPARTY_DIR}/ANGLE/" > > - "${THIRDPARTY_DIR}/ANGLE/include/KHR" > > This file is shared between all ports, so I don't think you should remove > them from here. True, in that case I'll leave them in place here but still add them to PlatformGTK.
Created attachment 260670 [details] Patch for 2.8 branch
Comment on attachment 260670 [details] Patch for 2.8 branch OK, seems like this is already fixed on master, and the patch is only needed for 2.8. To avoid confusion, I will close this bug as fixed. It's already linked to from [1] which is where we keep the list of backport requests. Please also note that commit-queue is patches for master. [1] https://trac.webkit.org/wiki/WebKitGTK/2.8.x
Comment on attachment 260670 [details] Patch for 2.8 branch Cleared review? from attachment 260670 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).