Bug 91387 - [CMake][EFL] Building jsc causes reconfiguration
Summary: [CMake][EFL] Building jsc causes reconfiguration
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thiago Marcos P. Santos
URL:
Keywords:
Depends on:
Blocks: 88717
  Show dependency treegraph
 
Reported: 2012-07-16 07:05 PDT by Thiago Marcos P. Santos
Modified: 2012-07-18 05:59 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.03 KB, patch)
2012-07-17 04:19 PDT, Thiago Marcos P. Santos
no flags Details | Formatted Diff | Diff
Patch (2.93 KB, patch)
2012-07-18 04:06 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 Thiago Marcos P. Santos 2012-07-16 07:05:32 PDT
One of the steps of our build bot calls ./Tools/Scripts/run-javascriptcore-tests which calls ./Tools/Scripts/build-jsc internally.

This script builds the "jsc" target, which is fine. The problem is: we are removing the CMakeCache.txt every time we build a target using the convenience perl script "buildCMakeProjectOrExit". In this case, the whole project is reconfigured using the default build options.

This leads to 2 side effects:

- jsc is rebuilt on the bots every time (since it takes different configuration).
- We are not building and running the WebKit2 unit tests on the bots.

We should remove the cache only when running ./Tools/Scripts/build-webkit.
Comment 1 Thiago Marcos P. Santos 2012-07-17 04:19:20 PDT
Created attachment 152731 [details]
Patch
Comment 2 Daniel Bates 2012-07-17 11:04:16 PDT
Comment on attachment 152731 [details]
Patch

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

OK

> Tools/ChangeLog:9
> +        other it will cause a reconfiguration every time someone calls

Nit: other => otherwise

> Tools/ChangeLog:11
> +        not running WebKit 2 unit tests on the bots because the project was

Nit: WebKit 2 => WebKit2

> Tools/Scripts/webkitdirs.pm:2129
> +    my $config = configuration();
> +    my $buildPath = File::Spec->catdir(baseProductDir(), $config);
> +    my $cacheFilePath = File::Spec->catdir($buildPath, "CMakeCache.txt");

I don't see much value in defining the variables $config and $buildPath that are only used towards building the filesystem path to the appropriate file CMakeCache.txt. I would write this as:

my $cacheFilePath = File::Spec->catdir(baseProductDir(), configuration(), "CMakeCache.txt");

> Tools/Scripts/webkitdirs.pm:2130
> +

Nit: I don't feel that this empty line improves the readability of this function and I suggest that we remove it. The body of this function is concise and straightforward to follow without this empty line.
Comment 3 Thiago Marcos P. Santos 2012-07-18 04:06:11 PDT
Created attachment 152983 [details]
Patch

Thanks a lot for reviewing.
Comment 4 WebKit Review Bot 2012-07-18 05:59:36 PDT
Comment on attachment 152983 [details]
Patch

Clearing flags on attachment: 152983

Committed r122956: <http://trac.webkit.org/changeset/122956>
Comment 5 WebKit Review Bot 2012-07-18 05:59:42 PDT
All reviewed patches have been landed.  Closing bug.