Bug 153778 - [CMake] Remove meaningless conditional statements in CMakeLists.txt
Summary: [CMake] Remove meaningless conditional statements in CMakeLists.txt
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joonghun Park
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-01 16:22 PST by Joonghun Park
Modified: 2016-02-18 00:28 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.98 KB, patch)
2016-02-01 16:28 PST, Joonghun Park
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joonghun Park 2016-02-01 16:22:55 PST
Use CMAKE_FOO_OUTPUT_DIRECTORY located in CMakeLists.txt as default value.
And remove conditional statements which has enclosed these one because they don't have meaning anymore.
Comment 1 Joonghun Park 2016-02-01 16:28:40 PST
Created attachment 270449 [details]
Patch
Comment 2 Alex Christensen 2016-02-01 16:31:28 PST
Comment on attachment 270449 [details]
Patch

This needs to be like this.
See http://trac.webkit.org/changeset/195478
Comment 3 Joonghun Park 2016-02-01 16:38:55 PST
(In reply to comment #2)
> Comment on attachment 270449 [details]
> Patch
> 
> This needs to be like this.
> See http://trac.webkit.org/changeset/195478

Hmm, doesn't those things be overwritten by OptionsWin.cmake?
Because now those things are precede to include(OptionsCommon) by https://bugs.webkit.org/show_bug.cgi?id=153373.

They don't follow the include(OptionsCommon) in order, so it seems that they don't need any conditionals there.

But if I missed something, please let me know.
Comment 4 Alex Christensen 2016-02-01 16:40:23 PST
The internal Windows build builds each directory (WTF, JavaScriptCore, WebCore, and WebKit) without the other directories where they are.  This causes problems with that setup.
Comment 5 Joonghun Park 2016-02-01 16:48:01 PST
(In reply to comment #4)
> The internal Windows build builds each directory (WTF, JavaScriptCore,
> WebCore, and WebKit) without the other directories where they are.  This
> causes problems with that setup.

Ah, I see. Thank you for letting me know about it
Comment 6 Michael Catanzaro 2016-02-01 18:09:18 PST
Alex, now that include(WebKitCommon) has been moved, surely those conditionals will never be hit anymore? I think Joonghun's patch is correct.
Comment 7 Alex Christensen 2016-02-02 12:43:13 PST
I wasn't aware they had moved.  We're still in the process of verifying the internal AppleWin build works, so I'd rather not touch this right now unless this is blocking something.
Comment 8 Michael Catanzaro 2016-02-17 17:28:45 PST
(In reply to comment #7)
> I wasn't aware they had moved.  We're still in the process of verifying the
> internal AppleWin build works, so I'd rather not touch this right now unless
> this is blocking something.

Any progress on this?

I'd still like to r+ this, but don't mind delaying if that's convenient for you.
Comment 9 Csaba Osztrogonác 2016-02-17 23:38:48 PST
Comment on attachment 270449 [details]
Patch

> The internal Windows build builds each directory (WTF, JavaScriptCore, WebCore, and WebKit) without the other directories where they are.  This causes problems with that setup.

So it is a dead code, out-of-tree use case isn't a good reason to keep dead code.
But I'd be happy to add it back once there is a public use-case in trunk.
(I know this strict policy since bug154323 from Filip Pizlo.)

Let's go ahead and remove it, r=me.
Comment 10 WebKit Commit Bot 2016-02-18 00:28:44 PST
Comment on attachment 270449 [details]
Patch

Clearing flags on attachment: 270449

Committed r196748: <http://trac.webkit.org/changeset/196748>
Comment 11 WebKit Commit Bot 2016-02-18 00:28:48 PST
All reviewed patches have been landed.  Closing bug.