[webkit-reviews] review granted: [Bug 127073] [GTK][CMake] build-webkit should try harder to avoid re-running cmake : [Attachment 221315] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 127073: [GTK][CMake] build-webkit should try harder to avoid re-running
cmake
https://bugs.webkit.org/show_bug.cgi?id=127073

Attachment 221315: Patch
https://bugs.webkit.org/attachment.cgi?id=221315&action=review

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=221315&action=review


> Tools/Scripts/build-webkit:303
> +    if (isNix()) {
> +	   if ($noWebKit2) {
> +	       print STDERR "Nix is a WebKit2 port, you can't disable WebKit2
on it!\n";
> +	       exit 1;
> +	   }
>  
> -    buildCMakeProjectOrExit($clean, "Nix", $prefixPath, $makeArgs,
(cmakeBasedPortArguments(), cMakeArgsFromFeatures()), $cmakeArgs);
> -}
> +	   $cmakeArgs = "-DWTF_USE_CURL=ON " . $cmakeArgs if $curl;
> +	   $cmakeArgs = "-DWTF_USE_OPENGL_ES_2=ON " . $cmakeArgs if $opengles2;


This code isn't necessary as the Nix port is no longer applicable.

> Tools/Scripts/webkitdirs.pm:1719
> +sub cachedArgumentFileOutOfDate($@)

Maybe a better name for this function would be isCachedArgumentFileOutOfDate
since this function returns a boolean result.

> Tools/Scripts/webkitdirs.pm:1896
> +    my $optionsCache = File::Spec->catdir(baseProductDir(), configuration(),
'build-webkit-options.txt');

Nit: ' (single quote) => " (double quote)

> Tools/Scripts/webkitdirs.pm:1912
> +    my $platformConfiguration = File::Spec->catdir(sourceDir(), 'Source',
'cmake', 'Options' . cmakeBasedPortName() . '.cmake');

Ditto.

> Tools/Scripts/webkitdirs.pm:1917
> +    my $globalConfiguration = File::Spec->catdir(sourceDir(), 'Source',
'cmake', 'OptionsCommon.cmake');

Ditto.

> Tools/Scripts/webkitdirs.pm:2047
> +    return isEfl() || isWinCE() || isNix() || isGtkCMake();

Remove isNix() as it's no longer applicable.