Bug 84229 - [CMake] Use jsc target instead of ONLY_BUILD_JAVASCRIPTCORE
Summary: [CMake] Use jsc target instead of ONLY_BUILD_JAVASCRIPTCORE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Thiago Marcos P. Santos
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-18 00:26 PDT by Dominik Röttsches (drott)
Modified: 2012-04-18 09:14 PDT (History)
11 users (show)

See Also:


Attachments
Patch fixing buildsystem. (2.76 KB, patch)
2012-04-18 05:41 PDT, Thiago Marcos P. Santos
no flags Details | Formatted Diff | Diff
Same patch, better changelog. (2.75 KB, patch)
2012-04-18 06:03 PDT, Thiago Marcos P. Santos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dominik Röttsches (drott) 2012-04-18 00:26:42 PDT
Build 203 [1] on the debug and build 35578 on the release buildbot included important changes from bug 84052 and bug 83281. However, they did not trigger rebuilds of the required objects, leading to an increase in failure rates by about 140 cases. So, from svn revision 114416 to 114420, the buildsystem failed to trigger the appropriate rebuild. This needs to be fixed urgently.

[1] http://build.webkit.org/builders/EFL%20Linux%20Debug/builds/203
[2] http://build.webkit.org/builders/EFL%20Linux%20Release/builds/35578
Comment 1 Dominik Röttsches (drott) 2012-04-18 01:00:41 PDT
--cmakearg="-DCMAKE_VERBOSE_MAKEFILE=ON" as an argument to build-webkit --efl might be helpful.
Comment 2 Sudarsana Nagineni (babu) 2012-04-18 01:26:31 PDT
Looks like EFl buildbots are triggering rebuilds of changes only when we force a clean build.
Comment 3 Sudarsana Nagineni (babu) 2012-04-18 05:09:53 PDT
The root cause of this issue is ONLY_BUILD_JAVASCRIPTCORE is always ON after running run-javascriptcore-tests(value is stored in CMakeCache.txt) and the following check in CMakeLists.txt ignores building other modules when this option is ON.

OPTION(ONLY_BUILD_JAVASCRIPTCORE "only build JavaScriptCore")
IF (ONLY_BUILD_JAVASCRIPTCORE)
    SET(ENABLE_WEBCORE OFF)
    SET(ENABLE_WEBKIT OFF)
    SET(ENABLE_WEBKIT2 OFF)
    SET(ENABLE_TOOLS OFF)
ENDIF ()

To fix the issue we should set this option OFF when we run build-webkit.
Comment 4 Thiago Marcos P. Santos 2012-04-18 05:41:13 PDT
Created attachment 137674 [details]
Patch fixing buildsystem.
Comment 5 Thiago Marcos P. Santos 2012-04-18 06:03:45 PDT
Created attachment 137677 [details]
Same patch, better changelog.
Comment 6 Raphael Kubo da Costa (:rakuco) 2012-04-18 06:45:45 PDT
Comment on attachment 137677 [details]
Same patch, better changelog.

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

> Tools/Scripts/build-jsc:73
> +    buildCMakeProjectOrExit(0, cmakeBasedPortName(), undef, "jsc", cmakeBasedPortArguments()); # This call only returns if nothing wrong happened

As we were discussing on IRC, this will break if JSC_EXECUTABLE_NAME is set to something else in Options${PORT}.cmake. Adding a note in a comment before this line should be enough for now, I guess.
Comment 7 Rob Buis 2012-04-18 08:55:28 PDT
Comment on attachment 137677 [details]
Same patch, better changelog.

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

>> Tools/Scripts/build-jsc:73
>> +    buildCMakeProjectOrExit(0, cmakeBasedPortName(), undef, "jsc", cmakeBasedPortArguments()); # This call only returns if nothing wrong happened
> 
> As we were discussing on IRC, this will break if JSC_EXECUTABLE_NAME is set to something else in Options${PORT}.cmake. Adding a note in a comment before this line should be enough for now, I guess.

Why not use JSC_EXECUTABLE_NAME in the first place instead of hardcoding jsc?
Comment 8 Rob Buis 2012-04-18 09:07:54 PDT
Comment on attachment 137677 [details]
Same patch, better changelog.

Looks good.
Comment 9 WebKit Review Bot 2012-04-18 09:14:18 PDT
Comment on attachment 137677 [details]
Same patch, better changelog.

Clearing flags on attachment: 137677

Committed r114508: <http://trac.webkit.org/changeset/114508>
Comment 10 WebKit Review Bot 2012-04-18 09:14:23 PDT
All reviewed patches have been landed.  Closing bug.