Summary: | [CMAKE] Refactoring wtf related code. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryuan Choi <ryuan.choi> | ||||||||||
Component: | WebKit EFL | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, gyuyoung.kim, leandro, lucas.de.marchi, paroga | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 60244 | ||||||||||||
Attachments: |
|
Description
Ryuan Choi
2011-05-04 02:18:04 PDT
Created attachment 92208 [details]
Patch
Comment on attachment 92208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92208&action=review Expect for the wtf-files, LGTM > Source/JavaScriptCore/CMakeLists.txt:-174 > - > - wtf/DateMath.cpp > - wtf/PageAllocationAligned.cpp > - wtf/PageBlock.cpp I'm not sure if that's correct. Some other ports add this to JSC and not to WTF too. (I don't know why they do that and if i remenber corretly I had some problem with this files in in the WinCE port) e.g. http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj?rev=83506#L641 At lest the move of the files can be put into an addtional patch. (In reply to comment #2) > (From update of attachment 92208 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=92208&action=review > > Expect for the wtf-files, LGTM > > > Source/JavaScriptCore/CMakeLists.txt:-174 > > - > > - wtf/DateMath.cpp > > - wtf/PageAllocationAligned.cpp > > - wtf/PageBlock.cpp > > I'm not sure if that's correct. Some other ports add this to JSC and not to WTF too. (I don't know why they do that and if i remenber corretly I had some problem with this files in in the WinCE port) > e.g. http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj?rev=83506#L641 > > At lest the move of the files can be put into an addtional patch. Thank you for information. Yes, I got an errors while moving those and just add "${JavaScriptCore_INCLUDE_DIRECTORIES}" in WTF_INCLUDE_DIRECTORIES to avoid. I'll investigate it more. Created attachment 92369 [details]
Patch
(In reply to comment #2) > (From update of attachment 92208 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=92208&action=review > > Expect for the wtf-files, LGTM > > > Source/JavaScriptCore/CMakeLists.txt:-174 > > - > > - wtf/DateMath.cpp > > - wtf/PageAllocationAligned.cpp > > - wtf/PageBlock.cpp > > I'm not sure if that's correct. Some other ports add this to JSC and not to WTF too. (I don't know why they do that and if i remenber corretly I had some problem with this files in in the WinCE port) > e.g. http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj?rev=83506#L641 > > At lest the move of the files can be put into an addtional patch. I updated my patch. I checked it using v8 option (in our local branch). V8 build requires DateMath.cpp (with wtf). and I hope to seperate wtf from JavaScriptCore to support v8 option. So, IMO, we'd better to move it into wtf. I tested on EFL port, but I can't on WinCE port. Could you give address on WinCE port for this patch, if any issues? Thanks. (In reply to comment #5) > I tested on EFL port, but I can't on WinCE port. > Could you give address on WinCE port for this patch, if any issues? I'll try to build it on the weekend. Comment on attachment 92369 [details]
Patch
I'm happy to rubberstamp this... but I know little about the cmake build.
Created attachment 93257 [details]
Patch
(In reply to comment #8) > Created an attachment (id=93257) [details] > Patch Patrick, could you please set cq+ on this patch after you verify that WinCE port still builds? (In reply to comment #9) > (In reply to comment #8) > > Created an attachment (id=93257) [details] [details] > > Patch > > Patrick, could you please set cq+ on this patch after you verify that WinCE port still builds? Ok, I'll set it to cq after i tested locally. But I don't see a problem if you want to land it now. So feel free to cq+ it. It will work 99.99%. Comment on attachment 93257 [details] Patch Rejecting attachment 93257 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'land-a..." exit_code: 1 Last 500 characters of output: From git://git.webkit.org/WebKit 2fe4b3b..5519cde master -> origin/master M Source/JavaScriptCore/wtf/PassOwnPtr.h M Source/JavaScriptCore/ChangeLog r87048 = 5519cde95c92f1c48cef0ee48d4235a4d1fce39a (refs/remotes/trunk) M Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp M Source/WebKit/gtk/ChangeLog r87049 = 5b0954d8922f1161c67860c2b7081eb7dc52c8f5 (refs/remotes/trunk) First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/trunk. Full output: http://queues.webkit.org/results/8726301 Created attachment 94370 [details]
Patch
Comment on attachment 94370 [details] Patch Clearing flags on attachment: 94370 Committed r87061: <http://trac.webkit.org/changeset/87061> All reviewed patches have been landed. Closing bug. |