Bug 154337 - [cmake] Build system cleanup
Summary: [cmake] Build system cleanup
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-17 06:26 PST by Csaba Osztrogonác
Modified: 2016-02-18 02:34 PST (History)
6 users (show)

See Also:


Attachments
Patch (6.89 KB, patch)
2016-02-17 06:30 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (6.41 KB, patch)
2016-02-18 00:52 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (5.94 KB, patch)
2016-02-18 01:38 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2016-02-17 06:26:46 PST
Now runtime/* sources and llint/* sources are added to JavaScriptCore_SOURCES
unconditionally, but in a different place. Let's move them to the main list.

And there is no need to add ftl/* sources conditionally to JavaScriptCore_SOURCES.
FTL JIT is enabled almost everywhere and these files are guarded properly,
no need to have a build system condition too.
Comment 1 Csaba Osztrogonác 2016-02-17 06:30:51 PST
Created attachment 271551 [details]
Patch
Comment 2 Gyuyoung Kim 2016-02-17 17:27:50 PST
Comment on attachment 271551 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=271551&action=review

> Source/JavaScriptCore/CMakeLists.txt:-995
> -if (ENABLE_FTL_JIT)

Personally I don't like to use guard in *.cmake though, I know win port doesn't enable FTL JIT yet. It looks there are still people to use guard in cmake files.
Comment 3 Csaba Osztrogonác 2016-02-17 23:43:34 PST
(In reply to comment #2)
> Comment on attachment 271551 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=271551&action=review
> 
> > Source/JavaScriptCore/CMakeLists.txt:-995
> > -if (ENABLE_FTL_JIT)
> 
> Personally I don't like to use guard in *.cmake though, I know win port
> doesn't enable FTL JIT yet. It looks there are still people to use guard in
> cmake files.

As I said, FTL JIT files are guarded properly, there is 
no reason to duplicate these guards in the build system too.

It is a common design in WebKit, that we add source files 
unconditionally in the build system. FTL JIT shouldn't be an exception.
Comment 4 Csaba Osztrogonác 2016-02-18 00:52:00 PST
Created attachment 271643 [details]
Patch

updated to ToT
Comment 5 Csaba Osztrogonác 2016-02-18 01:38:44 PST
Created attachment 271644 [details]
Patch

really updated to ToT
Comment 6 Zan Dobersek 2016-02-18 01:45:04 PST
Comment on attachment 271644 [details]
Patch

The GTK+ CQ is a bit overflown, I'll ping the appropriate people. I don't think this should cause build problems as it is, though.
Comment 7 WebKit Commit Bot 2016-02-18 02:34:46 PST
Comment on attachment 271644 [details]
Patch

Clearing flags on attachment: 271644

Committed r196752: <http://trac.webkit.org/changeset/196752>
Comment 8 WebKit Commit Bot 2016-02-18 02:34:52 PST
All reviewed patches have been landed.  Closing bug.