We should make the decision to use the Merseene Twister pseudorandom number generator a policy choice and move the implementation of the Merseene Twister algorithm, in file wince/mt19937ar.c, to common location, say ThirdParty. Currently, the choice to use this pseudorandom number generator is dependent on whether we are building JavaScriptCore on Windows CE. Instead, we should make this a policy choice and enable this by default in the Windows CE port.
Created attachment 80361 [details] Patch I thought to move the file wince/mt19937ar.c to Source/ThirdParty since it is a third party library. I am open to suggestions on where to place this file as well as the macro define to use. I am not very familiar with CMake and the Windows CE port so I am unsure of the correctness of the Cmake change. I used WebCore/CMakeLists.txt and JavaScriptCore/CMakeLists.txt as references.
Comment on attachment 80361 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80361&action=review > Source/JavaScriptCore/CMakeListsWinCE.txt:3 > +LIST(APPEND JavaScriptCore_INCLUDE_DIRECTORIES > + "${CMAKE_SOURCE_DIR}/Source/ThirdParty" > +) ${CMAKE_SOURCE_DIR} is already <root>/Source, so it should read "${CMAKE_SOURCE_DIR}/ThirdParty" I'll try this change on my local build, because I'm not sure if it's the correct "*_INCLUDE_DIRECTORIES"
(In reply to comment #2) > (From update of attachment 80361 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80361&action=review > > > Source/JavaScriptCore/CMakeListsWinCE.txt:3 > > +LIST(APPEND JavaScriptCore_INCLUDE_DIRECTORIES > > + "${CMAKE_SOURCE_DIR}/Source/ThirdParty" > > +) > > ${CMAKE_SOURCE_DIR} is already <root>/Source, so it should read "${CMAKE_SOURCE_DIR}/ThirdParty" Will change. Based on your response then we should also change <http://trac.webkit.org/browser/trunk/Source/WebCore/CMakeLists.txt?rev=76792#L83>. > > I'll try this change on my local build, because I'm not sure if it's the correct "*_INCLUDE_DIRECTORIES" Actually, we should probably do this in change in file JavaScriptCore/wtf/CmakeListsWinCE.txt.
Attachment 80361 [details] did not build on qt: Build output: http://queues.webkit.org/results/7584370
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 80361 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=80361&action=review > > > > > Source/JavaScriptCore/CMakeListsWinCE.txt:3 > > > +LIST(APPEND JavaScriptCore_INCLUDE_DIRECTORIES > > > + "${CMAKE_SOURCE_DIR}/Source/ThirdParty" > > > +) > > > > ${CMAKE_SOURCE_DIR} is already <root>/Source, so it should read "${CMAKE_SOURCE_DIR}/ThirdParty" > > Will change. FYI, IMHO the "correct" change: diff --git a/Source/JavaScriptCore/wtf/CMakeLists.txt b/Source/JavaScriptCore/wtf/CMakeLists.txt index b1931d7..9406f5d 100644 --- a/Source/JavaScriptCore/wtf/CMakeLists.txt +++ b/Source/JavaScriptCore/wtf/CMakeLists.txt @@ -136,6 +136,7 @@ INCLUDE_IF_EXISTS(${JAVASCRIPTCORE_DIR}/wtf/CMakeLists${PORT}.txt) LIST(APPEND WTF_INCLUDE_DIRECTORIES "${CMAKE_BINARY_DIR}" + "${CMAKE_SOURCE_DIR}/ThirdParty" ) WEBKIT_WRAP_SOURCELIST(${WTF_SOURCES}) OR diff --git a/Source/JavaScriptCore/wtf/CMakeListsWinCE.txt b/Source/JavaScriptCore/wtf/CMakeListsWinCE.txt index 9c558eb..19e9ceb 100644 --- a/Source/JavaScriptCore/wtf/CMakeListsWinCE.txt +++ b/Source/JavaScriptCore/wtf/CMakeListsWinCE.txt @@ -22,6 +22,10 @@ LIST(APPEND WTF_SOURCES ${3RDPARTY_DIR}/ce-compat/ce_unicode.cpp ) +LIST(APPEND WTF_INCLUDE_DIRECTORIES + "${CMAKE_SOURCE_DIR}/ThirdParty" +) + LIST(APPEND WTF_LIBRARIES mmtimer ) I like the first more, since you changed OS(WINCE) to USE(MERSENNE_TWISTER_19937). > Based on your response then we should also change > <http://trac.webkit.org/browser/trunk/Source/WebCore/CMakeLists.txt?rev=76792#L83>. Unfortunately CMake build system need some cleanup :-(. There are many "wrong" includes from the early "EFL only" days too.
Created attachment 80365 [details] Patch Updated patch based on Patrick Gansterer's suggestions. Appended Source/ThirdParty to the end of the list of include directories in JavaScriptCore/JavaScriptCore.pri in an attempt to fix the Qt Windows EWS bot.
Attachment 80365 [details] did not build on qt: Build output: http://queues.webkit.org/results/7521351
Comment on attachment 80365 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80365&action=review > Source/JavaScriptCore/JavaScriptCore.pri:26 > + $$PWD/../ThirdParty $$PWD/../ThirdParty \
(In reply to comment #8) > (From update of attachment 80365 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80365&action=review > > > Source/JavaScriptCore/JavaScriptCore.pri:26 > > + $$PWD/../ThirdParty > > $$PWD/../ThirdParty \ Thanks!
Created attachment 80448 [details] Patch
Comment on attachment 80448 [details] Patch OK.
Comment on attachment 80448 [details] Patch Clearing flags on attachment: 80448 Committed r77070: <http://trac.webkit.org/changeset/77070>
All reviewed patches have been landed. Closing bug.
(In reply to comment #5) > > Based on your response then we should also change > > <http://trac.webkit.org/browser/trunk/Source/WebCore/CMakeLists.txt?rev=76792#L83>. > Unfortunately CMake build system need some cleanup :-(. > There are many "wrong" includes from the early "EFL only" days too. Filed bug #53382 to remove references to ${CMAKE_SOURCE_DIR}/Source in CMake files.