WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
53253
Move wince/mt19937ar.c to ThirdParty and make it a policy choice
https://bugs.webkit.org/show_bug.cgi?id=53253
Summary
Move wince/mt19937ar.c to ThirdParty and make it a policy choice
Daniel Bates
Reported
2011-01-27 12:51:44 PST
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.
Attachments
Patch
(15.62 KB, patch)
2011-01-27 13:35 PST
,
Daniel Bates
paroga
: review-
Details
Formatted Diff
Diff
Patch
(17.88 KB, patch)
2011-01-27 14:43 PST
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Patch
(17.92 KB, patch)
2011-01-28 07:09 PST
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2011-01-27 13:35:26 PST
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.
Patrick R. Gansterer
Comment 2
2011-01-27 13:45:39 PST
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"
Daniel Bates
Comment 3
2011-01-27 14:00:06 PST
(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.
Early Warning System Bot
Comment 4
2011-01-27 14:05:35 PST
Attachment 80361
[details]
did not build on qt: Build output:
http://queues.webkit.org/results/7584370
Patrick R. Gansterer
Comment 5
2011-01-27 14:12:01 PST
(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.
Daniel Bates
Comment 6
2011-01-27 14:43:05 PST
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.
Early Warning System Bot
Comment 7
2011-01-27 15:04:25 PST
Attachment 80365
[details]
did not build on qt: Build output:
http://queues.webkit.org/results/7521351
Csaba Osztrogonác
Comment 8
2011-01-27 15:12:29 PST
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 \
Daniel Bates
Comment 9
2011-01-27 16:25:00 PST
(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!
Daniel Bates
Comment 10
2011-01-28 07:09:27 PST
Created
attachment 80448
[details]
Patch
Eric Seidel (no email)
Comment 11
2011-01-28 12:14:04 PST
Comment on
attachment 80448
[details]
Patch OK.
Daniel Bates
Comment 12
2011-01-29 14:17:51 PST
Comment on
attachment 80448
[details]
Patch Clearing flags on attachment: 80448 Committed
r77070
: <
http://trac.webkit.org/changeset/77070
>
Daniel Bates
Comment 13
2011-01-29 14:17:58 PST
All reviewed patches have been landed. Closing bug.
Daniel Bates
Comment 14
2011-01-29 14:40:46 PST
(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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug