146740 2015-07-08 15:18:09 -0700 [GTK] Build race with -DENABLE_WAYLAND_TARGET=ON 2015-07-13 12:12:59 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC Linux RESOLVED FIXED http://www.cmake.org/Bug/view.php?id=15454 P2 Normal --- 146057 1 mcatanzaro clopez cgarcia clopez commit-queue gyuyoung.kim mcatanzaro mrobinson svillar zan oldest_to_newest 1107898 0 mcatanzaro 2015-07-08 15:18:09 -0700 In file included from ../../Source/WebCore/platform/graphics/PlatformDisplay.cpp:37: ../../Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.h:32:10: fatal error: 'WebKitGtkWaylandClientProtocol.h' file not found #include "WebKitGtkWaylandClientProtocol.h" ^ 1 error generated. PlatformDisplayWayland.h needs to be built after WebKitGtkWaylandClientProtocol.h. The problem is, it seems like it should be: # Wayland protocol extension. add_custom_command( OUTPUT ${DERIVED_SOURCES_WEBCORE_DIR}/WebKitGtkWaylandClientProtocol.c DEPENDS ${WEBCORE_DIR}/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml COMMAND wayland-scanner server-header < ${WEBCORE_DIR}/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml > ${DERIVED_SOURCES_WEBCORE_DIR}/WebKitGtkWaylandServerProtocol.h COMMAND wayland-scanner client-header < ${WEBCORE_DIR}/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml > ${DERIVED_SOURCES_WEBCORE_DIR}/WebKitGtkWaylandClientProtocol.h COMMAND wayland-scanner code < ${WEBCORE_DIR}/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml > ${DERIVED_SOURCES_WEBCORE_DIR}/WebKitGtkWaylandClientProtocol.c ) if (ENABLE_WAYLAND_TARGET) list(APPEND WebCorePlatformGTK_SOURCES platform/graphics/wayland/PlatformDisplayWayland.cpp platform/graphics/wayland/WaylandEventSource.cpp platform/graphics/wayland/WaylandSurface.cpp ${DERIVED_SOURCES_WEBCORE_DIR}/WebKitGtkWaylandClientProtocol.c ) list(APPEND WebCore_SYSTEM_INCLUDE_DIRECTORIES ${WAYLAND_INCLUDE_DIRS} ) list(APPEND WebCore_LIBRARIES ${WAYLAND_LIBRARIES} ) endif () You can see that ${DERIVED_SOURCES_WEBCORE_DIR}/WebKitGtkWaylandClientProtocol.c is listed as the OUTPUT for a command and also a required source for compiling WebCore. I messed around with adding an explicit dependency, but it didn't help. I'm not sure what's wrong. This might be a CMake bug. 1107899 1 clopez 2015-07-08 15:26:04 -0700 I have been bitten by this. Re-running the build makes it continue, but that is not a proper solution. The build should work on the first try. 1107912 2 mcatanzaro 2015-07-08 15:50:23 -0700 In the generated build.ninja file, I see: ############################################# # Order-only phony target for WebCorePlatformGTK build cmake_order_depends_target_WebCorePlatformGTK: phony || lib/libbmalloc.a lib/libWTFGTK.a lib/libjavascriptcoregtk-4.0.so lib/libWebCoreGTK.a DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c Which looks sane. But I also see: # Custom command for DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c build DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c: CUSTOM_COMMAND ../../Source/WebCore/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml || lib/libWTFGTK.a lib/libWebCoreGTK.a lib/libbmalloc.a lib/libjavascriptcoregtk-4.0.so COMMAND = cd /home/mcatanzaro/WebKit/WebKitBuild/GNOME/Source/WebCore && wayland-scanner server-header < /home/mcatanzaro/WebKit/Source/WebCore/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml > /home/mcatanzaro/WebKit/WebKitBuild/GNOME/DerivedSources/WebCore/WebKitGtkWaylandServerProtocol.h && wayland-scanner client-header < /home/mcatanzaro/WebKit/Source/WebCore/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml > /home/mcatanzaro/WebKit/WebKitBuild/GNOME/DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.h && wayland-scanner code < /home/mcatanzaro/WebKit/Source/WebCore/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml > /home/mcatanzaro/WebKit/WebKitBuild/GNOME/DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c DESC = Generating ../../DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c Which I guess means DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c somehow has a bogus dependency on lib/libWebCoreGTK.a. I confirmed this by running 'ninja -t browse libWebCoreGTK.a' in my build directory, which showed libWebCoreGTK.a depends on lib/libWebCoreGTK.a depends on DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c depends on lib/libWebCoreGTK.a. So if I am reading this right, that is a cyclic dependency; the question is, why does it exist? DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c target is built using rule CUSTOM_COMMAND of ../../Source/WebCore/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml lib/libWTFGTK.a (order-only) lib/libWebCoreGTK.a (order-only) lib/libbmalloc.a (order-only) lib/libjavascriptcoregtk-4.0.so (order-only) dependent edges build: Source/WebCore/CMakeFiles/WebCorePlatformGTK.dir/__/__/DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c.o cmake_order_depends_target_WebCorePlatformGTK 1107929 3 mcatanzaro 2015-07-08 16:14:34 -0700 Copypasting from [1]: """The special rule name phony can be used to create aliases for other targets. For example: build foo: phony some/file/in/a/faraway/subdir/foo This makes ninja foo build the longer path. Semantically, the phony rule is equivalent to a plain rule where the command does nothing, but phony rules are handled specially in that they aren’t printed when run, logged (see below), nor do they contribute to the command count printed as part of the build process. phony can also be used to create dummy targets for files which may not exist at build time. If a phony build statement is written without any dependencies, the target will be considered out of date if it does not exist. Without a phony build statement, Ninja will report an error if the file does not exist and is required by the build.""" This rule: build DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c: CUSTOM_COMMAND ../../Source/WebCore/platform/graphics/wayland/WebKitGtkWaylandClientProtocol.xml || lib/libWTFGTK.a lib/libWebCoreGTK.a lib/libbmalloc.a lib/libjavascriptcoregtk-4.0.so Means that lib/libWebCoreGTK.a must be built before DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c if DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c does not already exist. Everything after the || is an order-only dependency, meaning that DerivedSources/WebCore/WebKitGtkWaylandClientProtocol.c does not need to be rebuilt when lib/libWebCoreGTK.a changes. [2] I can find no justification for why those dependencies exist. I wonder if it's a CMake bug. [1] https://martine.github.io/ninja/manual.html#_the_literal_phony_literal_rule [2] https://martine.github.io/ninja/manual.html#ref_dependencies 1107943 4 mcatanzaro 2015-07-08 16:32:51 -0700 But it looks like it's normal that these dependencies exist for all WebCore derived sources. E.g. # Custom command for DerivedSources/WebCore/JSTypeConversions.cpp build DerivedSources/WebCore/JSTypeConversions.cpp DerivedSources/WebCore/JSTypeConversions.h: CUSTOM_COMMAND ../../Source/WebCore/bindings/scripts/generate-bindings.pl ../../Source/WebCore/bindings/scripts/CodeGenerator.pm ../../Source/WebCore/bindings/scripts/CodeGenerator.pm ../../Source/WebCore/bindings/scripts/CodeGeneratorJS.pm ../../Source/WebCore/bindings/scripts/IDLParser.pm ../../Source/WebCore/bindings/scripts/InFilesParser.pm ../../Source/WebCore/bindings/scripts/preprocessor.pm DerivedSources/WebCore/supplemental_dependency.tmp ../../Source/WebCore/bindings/scripts/IDLAttributes.txt DerivedSources/WebCore/DOMWindowConstructors.idl ../../Source/WebCore/bindings/scripts/CodeGeneratorJS.pm ../../Source/WebCore/testing/TypeConversions.idl || lib/libANGLESupport.a lib/libWTFGTK.a lib/libWebCoreGTK.a lib/libbmalloc.a lib/libjavascriptcoregtk-4.0.so OK, I don't understand this at all. 1107966 5 mcatanzaro 2015-07-08 16:53:19 -0700 My guess is, this is http://www.cmake.org/Bug/view.php?id=15454 Which is fixed in CMake 3.3. So not a WebKit bug, and the workaround is to use make. 1107995 6 clopez 2015-07-08 17:17:52 -0700 Thanks for the investigation. This information is very useful. Unfortunately, not only the bots and EWS machines use ninja, but also many developers. CMake 3.3 isn't even released, so depending on it is not an option. Neither is an option telling developers to not use ninja (or to run the build twice) if they want to use the default build options. I will try to find a workaround, because this is blocker for bug 146057 1108030 7 mcatanzaro 2015-07-08 18:03:44 -0700 (In reply to comment #5) > My guess is, this is http://www.cmake.org/Bug/view.php?id=15454 > > Which is fixed in CMake 3.3. So not a WebKit bug, and the workaround is to > use make. That can't be right, because in comment #4 I observed that the dependencies looked just like all our other DerivedSources deps, none of which have this problem. I think Carlos Lopez has found the real problem; I'll let him post about it. 1108122 8 zan 2015-07-08 22:53:46 -0700 I hit this error too, but the PlatformDisplayWayland.h header (which includes WebKitGtkWaylandClientProtocol.h) wasn't included from PlatformDisplay.cpp, but from some other file in a library other than libWebCorePlatformGTK. The simplest solution is to move the WebKitGtkWaylandClientProtocol.h header inclusion into the PlatformDisplayWayland.cpp file, and use a forward declaration for the wl_webkitgtk struct in the header. On a side note, PlatformDisplayX11.cpp is built into libWebCore, while PlatformDisplay.cpp and PlatformDisplayWayland.cpp are built into libWebCorePlatformGTK. Would be nice to build these into the same library if possible. 1108183 9 clopez 2015-07-09 03:38:45 -0700 (In reply to comment #8) > On a side note, PlatformDisplayX11.cpp is built into libWebCore, while > PlatformDisplay.cpp and PlatformDisplayWayland.cpp are built into > libWebCorePlatformGTK. Would be nice to build these into the same library if > possible. Yes, this is the issue here I think. Including PlatformDisplayWayland.cpp in libWebCore instead of libWebCorePlatformGTK fixes this bug. I will upload a patch. 1108184 10 256479 clopez 2015-07-09 03:47:14 -0700 Created attachment 256479 Patch 1109136 11 clopez 2015-07-13 10:52:52 -0700 reviewers, any comments on this patch? Thanks 1109137 12 256479 mrobinson 2015-07-13 10:55:14 -0700 Comment on attachment 256479 Patch Sensible. 1109162 13 256479 commit-queue 2015-07-13 12:12:53 -0700 Comment on attachment 256479 Patch Clearing flags on attachment: 256479 Committed r186768: <http://trac.webkit.org/changeset/186768> 1109163 14 commit-queue 2015-07-13 12:12:59 -0700 All reviewed patches have been landed. Closing bug. 256479 2015-07-09 03:47:14 -0700 2015-07-13 12:12:53 -0700 Patch bug-146740-20150709124613.patch text/plain 3163 clopez U3VidmVyc2lvbiBSZXZpc2lvbjogMTg2NTE1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNWJjOThmMWE2OTA2NzZk ZjhhN2U2NmEyMWY5ZDFiMjg1OGRmNjA2OS4uYjBmYjM5OTgzZWRiZjliNzNkMTY1YzliNmIyMjdh Y2Q0M2RjYWVjNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE1LTA3LTA5ICBDYXJs b3MgQWxiZXJ0byBMb3BleiBQZXJleiAgPGNsb3BlekBpZ2FsaWEuY29tPgorCisgICAgICAgIEdU S10gQnVpbGQgcmFjZSB3aXRoIC1ERU5BQkxFX1dBWUxBTkRfVEFSR0VUPU9OCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDY3NDAKKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVzdHMsIG5vIGJl aGF2aW9yIGNoYW5nZXMuCisKKyAgICAgICAgKiBQbGF0Zm9ybUdUSy5jbWFrZTogcGxhdGZvcm0v Z3JhcGhpY3Mvd2F5bGFuZCBzb3VyY2VzIHNob3VsZCBiZQorICAgICAgICBpbmNsdWRlZCBpbnRv IFdlYkNvcmUgc291cmNlIGxpc3QgaW5zdGVhZCBvZiBXZWJDb3JlUGxhdGZvcm1HVEsuCisgICAg ICAgIEFsc28gbW92ZSB0aGUgYWRkX2N1c3RvbV9jb21tYW5kKCkgYmxvY2sgaW50byBpZihFTkFC TEVfV0FZTEFORF9UQVJHRVQpLAorICAgICAgICB0aGlzIGlzIG5vdCBzdHJpY3RseSBuZWVkZWQg dG8gZml4IHRoaXMgYnVnLCBidXQgaXQgc2VlbXMgY2xlYW5lci4KKwogMjAxNS0wNy0wOCAgWmFs YW4gQnVqdGFzICA8emFsYW5AYXBwbGUuY29tPgogCiAgICAgICAgIERvIG5vdCB3YXN0ZSB3aW5k b3cgc2VydmVyIG1lbW9yeSBmb3IgcGxhY2Vob2xkZXIgd2luZG93cyBpbml0aWF0ZWQgYnkgVGhl bWVNYWMuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9QbGF0Zm9ybUdUSy5jbWFrZSBiL1Nv dXJjZS9XZWJDb3JlL1BsYXRmb3JtR1RLLmNtYWtlCmluZGV4IGViZTkyOTA4MTM0MzQzY2U4NGM3 NTBhN2VlY2ExZGZhNDBiMjY3NjQuLjFmYzY4NTNkN2E3NzExZDk1NDU0NTNkNmRiN2NjOWNmOGJj NTZmM2MgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL1BsYXRmb3JtR1RLLmNtYWtlCisrKyBi L1NvdXJjZS9XZWJDb3JlL1BsYXRmb3JtR1RLLmNtYWtlCkBAIC00NjcsMTcgKzQ2NywxNyBAQCBp ZiAoRU5BQkxFX1BMVUdJTl9QUk9DRVNTX0dUSzIpCiAgICAgKQogZW5kaWYgKCkKIAotIyBXYXls YW5kIHByb3RvY29sIGV4dGVuc2lvbi4KLWFkZF9jdXN0b21fY29tbWFuZCgKLSAgICBPVVRQVVQg JHtERVJJVkVEX1NPVVJDRVNfV0VCQ09SRV9ESVJ9L1dlYktpdEd0a1dheWxhbmRDbGllbnRQcm90 b2NvbC5jCi0gICAgREVQRU5EUyAke1dFQkNPUkVfRElSfS9wbGF0Zm9ybS9ncmFwaGljcy93YXls YW5kL1dlYktpdEd0a1dheWxhbmRDbGllbnRQcm90b2NvbC54bWwKLSAgICBDT01NQU5EIHdheWxh bmQtc2Nhbm5lciBzZXJ2ZXItaGVhZGVyIDwgJHtXRUJDT1JFX0RJUn0vcGxhdGZvcm0vZ3JhcGhp Y3Mvd2F5bGFuZC9XZWJLaXRHdGtXYXlsYW5kQ2xpZW50UHJvdG9jb2wueG1sID4gJHtERVJJVkVE X1NPVVJDRVNfV0VCQ09SRV9ESVJ9L1dlYktpdEd0a1dheWxhbmRTZXJ2ZXJQcm90b2NvbC5oCi0g ICAgQ09NTUFORCB3YXlsYW5kLXNjYW5uZXIgY2xpZW50LWhlYWRlciA8ICR7V0VCQ09SRV9ESVJ9 L3BsYXRmb3JtL2dyYXBoaWNzL3dheWxhbmQvV2ViS2l0R3RrV2F5bGFuZENsaWVudFByb3RvY29s LnhtbCA+ICR7REVSSVZFRF9TT1VSQ0VTX1dFQkNPUkVfRElSfS9XZWJLaXRHdGtXYXlsYW5kQ2xp ZW50UHJvdG9jb2wuaAotICAgIENPTU1BTkQgd2F5bGFuZC1zY2FubmVyIGNvZGUgPCAke1dFQkNP UkVfRElSfS9wbGF0Zm9ybS9ncmFwaGljcy93YXlsYW5kL1dlYktpdEd0a1dheWxhbmRDbGllbnRQ cm90b2NvbC54bWwgPiAke0RFUklWRURfU09VUkNFU19XRUJDT1JFX0RJUn0vV2ViS2l0R3RrV2F5 bGFuZENsaWVudFByb3RvY29sLmMKLSkKLQogaWYgKEVOQUJMRV9XQVlMQU5EX1RBUkdFVCkKLSAg ICBsaXN0KEFQUEVORCBXZWJDb3JlUGxhdGZvcm1HVEtfU09VUkNFUworICAgICMgV2F5bGFuZCBw cm90b2NvbCBleHRlbnNpb24uCisgICAgYWRkX2N1c3RvbV9jb21tYW5kKAorICAgICAgICBPVVRQ VVQgJHtERVJJVkVEX1NPVVJDRVNfV0VCQ09SRV9ESVJ9L1dlYktpdEd0a1dheWxhbmRDbGllbnRQ cm90b2NvbC5jCisgICAgICAgIERFUEVORFMgJHtXRUJDT1JFX0RJUn0vcGxhdGZvcm0vZ3JhcGhp Y3Mvd2F5bGFuZC9XZWJLaXRHdGtXYXlsYW5kQ2xpZW50UHJvdG9jb2wueG1sCisgICAgICAgIENP TU1BTkQgd2F5bGFuZC1zY2FubmVyIHNlcnZlci1oZWFkZXIgPCAke1dFQkNPUkVfRElSfS9wbGF0 Zm9ybS9ncmFwaGljcy93YXlsYW5kL1dlYktpdEd0a1dheWxhbmRDbGllbnRQcm90b2NvbC54bWwg PiAke0RFUklWRURfU09VUkNFU19XRUJDT1JFX0RJUn0vV2ViS2l0R3RrV2F5bGFuZFNlcnZlclBy b3RvY29sLmgKKyAgICAgICAgQ09NTUFORCB3YXlsYW5kLXNjYW5uZXIgY2xpZW50LWhlYWRlciA8 ICR7V0VCQ09SRV9ESVJ9L3BsYXRmb3JtL2dyYXBoaWNzL3dheWxhbmQvV2ViS2l0R3RrV2F5bGFu ZENsaWVudFByb3RvY29sLnhtbCA+ICR7REVSSVZFRF9TT1VSQ0VTX1dFQkNPUkVfRElSfS9XZWJL aXRHdGtXYXlsYW5kQ2xpZW50UHJvdG9jb2wuaAorICAgICAgICBDT01NQU5EIHdheWxhbmQtc2Nh bm5lciBjb2RlIDwgJHtXRUJDT1JFX0RJUn0vcGxhdGZvcm0vZ3JhcGhpY3Mvd2F5bGFuZC9XZWJL aXRHdGtXYXlsYW5kQ2xpZW50UHJvdG9jb2wueG1sID4gJHtERVJJVkVEX1NPVVJDRVNfV0VCQ09S RV9ESVJ9L1dlYktpdEd0a1dheWxhbmRDbGllbnRQcm90b2NvbC5jCisgICAgKQorCisgICAgbGlz dChBUFBFTkQgV2ViQ29yZV9TT1VSQ0VTCiAgICAgICAgIHBsYXRmb3JtL2dyYXBoaWNzL3dheWxh bmQvUGxhdGZvcm1EaXNwbGF5V2F5bGFuZC5jcHAKICAgICAgICAgcGxhdGZvcm0vZ3JhcGhpY3Mv d2F5bGFuZC9XYXlsYW5kRXZlbnRTb3VyY2UuY3BwCiAgICAgICAgIHBsYXRmb3JtL2dyYXBoaWNz L3dheWxhbmQvV2F5bGFuZFN1cmZhY2UuY3BwCg==