Bug 60146 – [CMAKE] Refactoring wtf related code.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Ryuan Choi]

There are wtf related code in cmake/OptionsCommon.cmake, cmake/OptionsEfl.cmake, JavaScriptCore/CMakeLists.txt
IMO, It should be moved to JavaScriptCore/wtf/CMakeLists.txt

[Ryuan Choi]

Created attachment 92208 [details]
Patch

[Patrick R. Gansterer]

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.

[Ryuan Choi]

(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.

[Ryuan Choi]

Created attachment 92369 [details]
Patch

[Ryuan Choi]

(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.

[Patrick R. Gansterer]

(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.

[Eric Seidel (no email)]

Comment on attachment 92369 [details]
Patch

I'm happy to rubberstamp this... but I know little about the cmake build.

[Ryuan Choi]

Created attachment 93257 [details]
Patch

[Leandro Pereira]

(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?

[Patrick R. Gansterer]

(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%.

[WebKit Commit Bot]

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

[Ryuan Choi]

Created attachment 94370 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 94370 [details]
Patch

Clearing flags on attachment: 94370

Committed r87061: <http://trac.webkit.org/changeset/87061>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.