This is similar to https://bugs.webkit.org/show_bug.cgi?id=72816, except the focus is to build WinCairo using CMake. Unifying build systems is still important to many people; the 5-6 build systems (mac, apple win / wincairo, gtk, qt, and cmake) we have now continues to generate extra work. By adopting CMake, we're eliminating and improving build systems. GTK is working on adopting CMake now (EFL, Blackberry, WinCE are already there). At the contributor's meeting this year, we discussed unifying build systems. (Notes taken http://trac.webkit.org/wiki/May%202013%20Meeting%20-%20Unifying%20Build%20Systems%20-%20Notes). The consensus at the meeting was this - even if we can't have all build systems switch to CMake now, any time a build system switches to CMake it's a win.
Created attachment 201439 [details] Proposed fix From build directory, execute: cmake -G "Visual Studio 8 2005" -DPORT=Win -DPORT_FLAVOR=WinCairo <repo src path> - Patch is based on the tree from April 15th; I'll rebase it soon. - DumpRenderTree needs to be included. - Patch is the same as the first commit off tree at https://github.com/masali-hp/webkit/commits/master After this patch, I'd like to submit patches: 1) To allow building without CF/CFLite on windows 2) To build the same CMake Win port for WinCE Because of this, there are a few places where you may notice something like this: list(APPEND WebKit_Source_Files Common file 1 Common file 2 ..... ) list(APPEND WebKit_Source_Files CF specific file 1 CF specific file 2 ....... )
Comment on attachment 201439 [details] Proposed fix Please reuse the WinCE files too: See https://github.com/paroga/webkit/commit/0b6c0bafbb02d7ef50914f8895de5d85265598e3 Also rebase your patch (e.g. pthread isn't required anymore) and make additional patches for the changes in the CPP files.
(In reply to comment #2) > (From update of attachment 201439 [details]) > Please reuse the WinCE files too: See https://github.com/paroga/webkit/commit/0b6c0bafbb02d7ef50914f8895de5d85265598e3 Patrick - can you explain what you mean by "reuse the WinCE files too"? I looked at your private branch and believe your design is: - make each windows port a separate port (WinCairo, WinCE, eventually WinApple) - a port can define what the parent port (fallback port) is, the fallback port is included first then the port file is included I could modify my patch to go this route. Currently with my patch you set PORT=Win, PORT_FLAVOR=WinCairo. I could adopt your approach with minimal turmoil; the common build files would know to include an Options{fallback}.cmake and the files I've set up should work. I want to add support for WinCE in these steps (ultimately my vision is to build WinCairo for WinCE with the same - perhaps a few exceptions - set of source files used to build for Windows XP): 1) Add a CMake port for WinCairo. 2) Make it possible to build WinCairo without CoreFoundation. 3) Upstream cairo changes to support WinCE. 4) Make changes to build WinCairo for WinCE. > Also rebase your patch (e.g. pthread isn't required anymore) and make additional patches for the changes in the CPP files. FastMalloc.cpp still requires pthreads when not USE_SYSTEM_MALLOC. I just rebased, I'll attach that patch.
Created attachment 202624 [details] Proposed fix -- rebased off 150501 No changelogs modified yet, I think there is probably more feedback before this can be integrated.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 201439 [details] [details]) > > Please reuse the WinCE files too: See https://github.com/paroga/webkit/commit/0b6c0bafbb02d7ef50914f8895de5d85265598e3 > > Patrick - can you explain what you mean by "reuse the WinCE files too"? Many files in PlatformWin.cmake and PlatformWinCE.cmake are similar, so "reuse the list in the WinCE files" to avoid duplication- > I looked at your private branch and believe your design is: > - make each windows port a separate port (WinCairo, WinCE, eventually WinApple) > - a port can define what the parent port (fallback port) is, the fallback port is included first then the port file is included > > I could modify my patch to go this route. Currently with my patch you set PORT=Win, PORT_FLAVOR=WinCairo. I could adopt your approach with minimal turmoil; the common build files would know to include an Options{fallback}.cmake and the files I've set up should work. > > I want to add support for WinCE in these steps (ultimately my vision is to build WinCairo for WinCE with the same - perhaps a few exceptions - set of source files used to build for Windows XP): > > 1) Add a CMake port for WinCairo. > 2) Make it possible to build WinCairo without CoreFoundation. > 3) Upstream cairo changes to support WinCE. > 4) Make changes to build WinCairo for WinCE. I'm also working on the CMake changes, but I want to commit a clean version in the first step. So I'd like to merge FilesSystemWin(CE).cpp first, which is blocked with bug 116205. Would it be ok for you to wait with your CMake files until the split of the CMake files (as in my github branch) is in trunk too? IMHO you can just start with your second point, since they do not depend on each other. > > Also rebase your patch (e.g. pthread isn't required anymore) and make additional patches for the changes in the CPP files. > > FastMalloc.cpp still requires pthreads when not USE_SYSTEM_MALLOC. Not for OS(WINDOWS). All other Windows ports removed pthread dependency.
Created attachment 208175 [details] Patch This is a first version to show the direction, but I'd like to hear some feedback from the current WinCairo maintainer...
Attachment 208175 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'CMakeLists.txt', u'ChangeLog', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/PlatformWin.cmake', u'Source/JavaScriptCore/PlatformWinCairo.cmake', u'Source/WebCore/ChangeLog', u'Source/WebCore/PlatformWinCairo.cmake', u'Source/WebKit/ChangeLog', u'Source/WebKit/PlatformWinCairo.cmake', u'Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in', u'Source/WebKit/cf/ChangeLog', u'Source/WebKit/cf/WebCoreSupport/WebInspectorClientCF.cpp', u'Source/cmake/OptionsWinCairo.cmake', u'Source/cmake/OptionsWindows.cmake', u'Tools/CMakeLists.txt', u'Tools/ChangeLog', u'Tools/DumpRenderTree/win/CMakeLists.txt', u'Tools/WinLauncher/CMakeLists.txt', u'Tools/win/DLLLauncher/CMakeLists.txt']" exit_code: 1 Source/cmake/OptionsWinCairo.cmake:50: Use lowercase command "file" [command/lowercase] [5] Source/cmake/OptionsWinCairo.cmake:54: Use lowercase command "file" [command/lowercase] [5] Source/cmake/OptionsWinCairo.cmake:58: Use lowercase command "file" [command/lowercase] [5] Source/cmake/OptionsWinCairo.cmake:62: Use lowercase command "file" [command/lowercase] [5] Total errors found: 4 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 208175 [details] Patch Attachment 208175 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1451705
Created attachment 212265 [details] Patch (based on r156181) Depending on the used perl version https://bugs.webkit.org/attachment.cgi?id=212261&action=review must be applied first. To build: - All requrements for the build tool of trac.webkit.org/wiki/WinCE apply (bison, flex, gperf, perl, python) - Download and extract the WinCairoRequrements.zip - Run the following command in an empty directory: cmake -G "Visual Studio 10" -DPORT=WinCairo path/to/source - Open the generated sln file and build or run "cmake --build ." in that directory
I've updated most of this and I'm writing this with a WinCairo WinLauncher browser made using CMake.
Created attachment 250047 [details] Patch
Attachment 250047 [details] did not pass style-queue: ERROR: Tools/WinLauncher/stdafx.h:73: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 250047 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250047&action=review > Source/WebCore/PlatformWinCairo.cmake:106 > + platform/image-decoders/bmp/BMPImageReader.cpp Wrong alphabet order. > Source/cmake/OptionsWindows.cmake:8 > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_GEOLOCATION ON) nit: wrong alphabet order. > Tools/CMakeLists.txt:30 > +if (WIN32) To be compatible with existing code, how about using below condition ? elseif ("${PORT}" STREQUAL "WIN32") add_subdirectory(DumpRenderTree/win) add_subdirectory(WinLauncher) endif () > Tools/DumpRenderTree/win/CMakeLists.txt:23 > + ../AccessibilityController.cpp Doesn't these files already included in DumpRenderTree/CMakeLists.txt ? > Tools/DumpRenderTree/win/CMakeLists.txt:50 > +set(DumpRenderTree_LIBRARIES ${DumpRenderTree_LIBRARIES}) Why do we need to set DumpRenderTree_LIBRARIES again ? > Tools/WinLauncher/stdafx.h:73 > +#include <wtf/ExportMacros.h> nit: wrong alphabet order.
Comment on attachment 250047 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250047&action=review > Source/WebCore/CMakeLists.txt:-280 > - Modules/streams/ReadableStream.idl > - Modules/streams/ReadableStreamReader.idl > - why? We shouldn't switch off this feature everywhere. If WinCairo has problems with STREAM_API, define it to 0. > Source/WebCore/CMakeLists.txt:-931 > - Modules/streams/ReadableStream.cpp > - Modules/streams/ReadableStreamReader.cpp why? > Source/WebCore/CMakeLists.txt:-1163 > - bindings/js/JSReadableStreamCustom.cpp > - bindings/js/JSReadableStreamReaderCustom.cpp > - bindings/js/ReadableStreamJSSource.cpp why?
(In reply to comment #13) > > Tools/CMakeLists.txt:30 > > +if (WIN32) > > To be compatible with existing code, how about using below condition ? > > elseif ("${PORT}" STREQUAL "WIN32") > add_subdirectory(DumpRenderTree/win) > add_subdirectory(WinLauncher) > endif () The other things obviously need to be fixed in a new patch, but I think WIN32 is nicer than this: "${PORT}" STREQUAL "WinCairo" OR "${PORT}" STREQUAL "AppleWin" Would you be opposed if I started using WIN32 and APPLE CMake variables in the CMake code for OS-specific things?
(In reply to comment #15) > (In reply to comment #13) > > > Tools/CMakeLists.txt:30 > > > +if (WIN32) > > > > To be compatible with existing code, how about using below condition ? > > > > elseif ("${PORT}" STREQUAL "WIN32") > > add_subdirectory(DumpRenderTree/win) > > add_subdirectory(WinLauncher) > > endif () > The other things obviously need to be fixed in a new patch, but I think > WIN32 is nicer than this: > "${PORT}" STREQUAL "WinCairo" OR "${PORT}" STREQUAL "AppleWin" > > Would you be opposed if I started using WIN32 and APPLE CMake variables in > the CMake code for OS-specific things? I don't have big opposition. Just wanted to be aligned with existing one. So, if someone else doesn't mention about this use, I see. Let's use it you want.
Created attachment 250268 [details] Patch
I removed the questionable parts of this patch. I'll land them in another patch getting everything working the right way.
Comment on attachment 250268 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250268&action=review Looks reasonable. r=me. > Source/WebCore/PlatformWinCairo.cmake:105 > + platform/image-decoders/bmp/BMPImageDecoder.cpp alphabetical sorting. > Source/WebCore/PlatformWinCairo.cmake:106 > + platform/image-decoders/bmp/BMPImageReader.cpp ditto.
http://trac.webkit.org/changeset/182545
*** Bug 121832 has been marked as a duplicate of this bug. ***