More work in progress.
Created attachment 249901 [details] Patch
I am under the impression that I did not change any bmalloc behavior for non-Windows systems. Why is the efl bot red?
Comment on attachment 249901 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249901&action=review > Source/CMakeLists.txt:5 > +if (!WIN32) It looks this causes EFL build break. This seems not to work properly. CMake Error at Source/cmake/WebKitHelpers.cmake:28 (set_target_properties): set_target_properties Can not find target to add properties to: bmalloc Call Stack (most recent call first): Source/CMakeLists.txt:34 (WEBKIT_SET_EXTRA_COMPILER_FLAGS) > Source/cmake/OptionsWindows.cmake:8 > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_VIEW_MODE_CSS_MEDIA OFF) Duplicated definition with 6 line ?
Comment on attachment 249901 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249901&action=review >> Source/CMakeLists.txt:5 >> +if (!WIN32) > > It looks this causes EFL build break. This seems not to work properly. > > CMake Error at Source/cmake/WebKitHelpers.cmake:28 (set_target_properties): > set_target_properties Can not find target to add properties to: bmalloc > Call Stack (most recent call first): > Source/CMakeLists.txt:34 (WEBKIT_SET_EXTRA_COMPILER_FLAGS) ditto. > Source/WTF/wtf/CMakeLists.txt:226 > +if (!WIN32) Please use "if (NOT WIN32)".
(In reply to comment #4) > > Source/WTF/wtf/CMakeLists.txt:226 > > +if (!WIN32) > > Please use "if (NOT WIN32)". Aha! Thanks! I'm still getting used to CMake. I write too much C++ code :)
Created attachment 249923 [details] Patch
Created attachment 249931 [details] Patch
Comment on attachment 249931 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249931&action=review R=me. Consider making your FIXME's cite bugs, but it's no biggie. > Source/CMakeLists.txt:4 > +# FIXME: Port bmalloc to Windows. I've always liked to have FIXME's cite the bug. There's gotta be a bug for porting bmalloc to Windows. You could cite it here. > Source/ThirdParty/ANGLE/include/GLES2/gl2.h:473 > +// FIXME: Get WebGL working on Windows with CMake. Similar comment.
Comment on attachment 249931 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249931&action=review > Source/WTF/wtf/CMakeLists.txt:-182 > - WorkQueue.cpp Why don't we use this one on Windows? It is listed in Source/WTF/WTF.vcxproj/WTF.vcxproj. > Source/WebCore/PlatformMac.cmake:-86 > - platform/cf/SharedTimerCF.mm It is SharedTimerCF.cpp now
(In reply to comment #9) > Comment on attachment 249931 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=249931&action=review > > > Source/WTF/wtf/CMakeLists.txt:-182 > > - WorkQueue.cpp > > Why don't we use this one on Windows? It is listed in > Source/WTF/WTF.vcxproj/WTF.vcxproj. I'm not sure. It was causing compiling errors and I was under the impression it wasn't used on Windows, but looking into it I'm pretty sure that was a false assumption. I'll land it without this change. > > > Source/WebCore/PlatformMac.cmake:-86 > > - platform/cf/SharedTimerCF.mm > > It is SharedTimerCF.cpp now Thanks!
http://trac.webkit.org/changeset/182243
Comment on attachment 249931 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249931&action=review This has broken GTK+ build, bots didn't notice it because it only affects clean builds. > Source/JavaScriptCore/CMakeLists.txt:1091 > - ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/Protocol/InspectorBackendCommands.js > + ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/inspector/InspectorBackendCommands.js Why did you change this path? GTK+ build still looks for inspector UI files in ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/ > Source/JavaScriptCore/CMakeLists.txt:1095 > - COMMAND ${PYTHON_EXECUTABLE} ${JavaScriptCore_INSPECTOR_SCRIPTS_DIR}/generate-inspector-protocol-bindings.py --outputDir "${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}" --framework JavaScriptCore ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/CombinedDomains.json && mkdir -p ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector && cp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorBackendDispatchers.h ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorBackendDispatchers.cpp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorFrontendDispatchers.h ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorFrontendDispatchers.cpp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorProtocolObjects.h ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorProtocolObjects.cpp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector && mkdir -p ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/Protocol && cp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorBackendCommands.js ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/Protocol > + COMMAND ${PYTHON_EXECUTABLE} ${JavaScriptCore_INSPECTOR_SCRIPTS_DIR}/generate-inspector-protocol-bindings.py --outputDir "${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector" --framework JavaScriptCore ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/CombinedDomains.json > VERBATIM) InspectorBackendCommands.js is no longer copied to the expected directory ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/Protocol.
(In reply to comment #12) > Comment on attachment 249931 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=249931&action=review > > This has broken GTK+ build, bots didn't notice it because it only affects > clean builds. > > > Source/JavaScriptCore/CMakeLists.txt:1091 > > - ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/Protocol/InspectorBackendCommands.js > > + ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/inspector/InspectorBackendCommands.js > > Why did you change this path? GTK+ build still looks for inspector UI files > in ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/ > > > Source/JavaScriptCore/CMakeLists.txt:1095 > > - COMMAND ${PYTHON_EXECUTABLE} ${JavaScriptCore_INSPECTOR_SCRIPTS_DIR}/generate-inspector-protocol-bindings.py --outputDir "${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}" --framework JavaScriptCore ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/CombinedDomains.json && mkdir -p ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector && cp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorBackendDispatchers.h ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorBackendDispatchers.cpp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorFrontendDispatchers.h ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorFrontendDispatchers.cpp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorProtocolObjects.h ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorProtocolObjects.cpp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector && mkdir -p ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/Protocol && cp ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InspectorBackendCommands.js ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/Protocol > > + COMMAND ${PYTHON_EXECUTABLE} ${JavaScriptCore_INSPECTOR_SCRIPTS_DIR}/generate-inspector-protocol-bindings.py --outputDir "${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector" --framework JavaScriptCore ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/CombinedDomains.json > > VERBATIM) > > InspectorBackendCommands.js is no longer copied to the expected directory > ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/Protocol. Is the copying required? Can't we just use the file from DerivedSources/inspector/?
It's required for the scripts used by GTK+, of course we can change that, but I think it's a different issue. The patch changed the output to be ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/inspector/InspectorBackendCommands.js but the file is actually written to ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector/. Our script generate-inspector-gresource-manifest.py expects all passed files to have a WebInspectorUI/ dir at some point, to generate the xml file with paths relative to that. Then we can pass Source/WebInspectorUI and DerivedSources/WebInspectorUI as source dirs to glib-compile-resources
(In reply to comment #12) > Why did you change this path? GTK+ build still looks for inspector UI files > in ${DERIVED_SOURCES_WEBINSPECTORUI_DIR}/UserInterface/ Sorry about that. The point of that change was to get rid of unix commands like cp and mkdir so we can use CMake on Windows. The ews bots were all green, and I thought these files were only used for building. If I broke something, we should change it back, or add a copy command somewhere. (In reply to comment #14) > It's required for the scripts used by GTK+ Are these scripts in WebKit or another repository?
(In reply to comment #15) > (In reply to comment #14) > > It's required for the scripts used by GTK+ > Are these scripts in WebKit or another repository? They are in WebKit, but only clean builds were affected. Carlos and I agreed that the current situation is more elegant, and that port-specific rules should be added if that port requires a file from DerivedSources/JavaScriptCore/inspector/ in some specific location. That's what I'll do tomorrow, in a separate bug.
New bug report to fix the build: bug143361