[webkit-reviews] review denied: [Bug 29656] Make WebKitTools/Scripits/run-sunspider script work on Windows platform : [Attachment 40304] proposed patch

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has denied Csaba Osztrogonác
<oszi at inf.u-szeged.hu>'s request for review:
Bug 29656: Make WebKitTools/Scripits/run-sunspider script work on Windows
platform
https://bugs.webkit.org/show_bug.cgi?id=29656

Attachment 40304: proposed patch
https://bugs.webkit.org/attachment.cgi?id=40304&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
Since both of these scripts are Perl scripts, they had to be invoked with a
Perl interpreter originally.  We should use the same Perl interpreter when
running subsequent scripts.

I would add a method like this to webkitdirs.pm (see "man perlvar" for
details):

use Config;

[...]

sub currentPerlPath()
{
    my $thisPerl = $^X;
    if ($^O ne 'VMS') {
	$thisPerl .= $Config{_exe} unless $thisPerl =~ m/$Config{_exe}$/i;
    }
    return $thisPerl;
}

Then the following changes can be made:

> diff --git a/WebKitTools/Scripts/run-sunspider
b/WebKitTools/Scripts/run-sunspider
> +my @perl;
> +push @perl, "perl" if isWindows();

These are no longer needed.

> -	   my $buildResult = system "WebKitTools/Scripts/build-jsc", @ARGV;
> +	   my $buildResult = system @perl, "WebKitTools/Scripts/build-jsc",
@ARGV;

Replace @perl with currentPerlPath() here.

> -exec "./sunspider", @args;
> +exec @perl, "./sunspider", @args;

Ditto.

> diff --git a/WebKitTools/Scripts/sunspider-compare-results
b/WebKitTools/Scripts/sunspider-compare-results
> +my @perl;
> +push @perl, "perl" if isWindows();

These are no longer needed.

> -	   my $buildResult = system "WebKitTools/Scripts/build-jsc", "--" .
$configuration;
> +	   my $buildResult = system @perl, "WebKitTools/Scripts/build-jsc",
"--" . $configuration;

Replace @perl with currentPerlPath().

> -exec "./sunspider-compare-results", @args, @ARGV;
> +exec @perl, "./sunspider-compare-results", @args, @ARGV;

Ditto.

r- to make the above changes.