Add --sdk flag to jsDriver.pl to allow running in the iOS simulator

[David Farler]

In order to run jsc in the simulator without altering the environment, add an --sdk flag to prefix the jsc command with xcrun -sdk $sdk sim.

[David Farler]

Created attachment 202577 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 202577 [details]
Patch

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

r=me

> Source/JavaScriptCore/tests/mozilla/jsDriver.pl:68
> +my $opt_sim_sdk = '';

I think our style is double quoted strings in perl. Here and else where. I'm not sure how strict that is, I don't mind seeing either.

[David Farler]

Created attachment 202634 [details]
Patch

Needs a chomp around the shell call because Perl.

[David Farler]

Created attachment 202635 [details]
Patch

sim needs --adopt-pid to have the correct return code from the forked process

[David Kilzer (:ddkilzer)]

Comment on attachment 202635 [details]
Patch

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

r- to fix the --adopt-pid issue.

> Source/JavaScriptCore/ChangeLog:3
> +        --sdk option to jsDriver.pl to run with iOS Simulator

What happened to "Add" in the title here?  :)

> Source/JavaScriptCore/tests/mozilla/jsDriver.pl:180
> +            chomp($shell_command = `xcrun -sdk $opt_sim_sdk -find sim`) if $opt_sim_sdk;
> +            $shell_command .= " --adopt-pid ";

I think both of these need to be inside an if ($opt_sim_sdk) check since --adopt-pid only makes sense when using the sim tool:

    if ($opt_sim_sdk) {
        chomp($shell_command = `xcrun -sdk $opt_sim_sdk -find sim`);
        $shell_command .= " --adopt-pid ";
    }

[David Farler]

(In reply to comment #5)
> (From update of attachment 202635 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=202635&action=review
> 
> r- to fix the --adopt-pid issue.
> 
> > Source/JavaScriptCore/ChangeLog:3
> > +        --sdk option to jsDriver.pl to run with iOS Simulator
> 
> What happened to "Add" in the title here?  :)

Fixed!

> 
> > Source/JavaScriptCore/tests/mozilla/jsDriver.pl:180
> > +            chomp($shell_command = `xcrun -sdk $opt_sim_sdk -find sim`) if $opt_sim_sdk;
> > +            $shell_command .= " --adopt-pid ";
> 
> I think both of these need to be inside an if ($opt_sim_sdk) check since --adopt-pid only makes sense when using the sim tool:
> 
>     if ($opt_sim_sdk) {
>         chomp($shell_command = `xcrun -sdk $opt_sim_sdk -find sim`);
>         $shell_command .= " --adopt-pid ";
>     }

D’oh. Dumb mistake - that’s what I meant, I’m sure. :)

[David Farler]

Created attachment 202743 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 202743 [details]
Patch

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

r=me with some minor fixes to avoid warnings and unnecessary checks.

> Source/JavaScriptCore/tests/mozilla/jsDriver.pl:180
> +                chomp($shell_command = `xcrun -sdk $opt_sim_sdk -find sim`) if $opt_sim_sdk;

You don't need the " if $opt_sim_sdk;" here now that it's inside the if block.

> Source/JavaScriptCore/tests/mozilla/jsDriver.pl:184
> +            $shell_command .= " $opt_arch ";

If $opt_sim_sdk is not set, we now have a space at the beginning of $shell_command, and I think $shell_command will be undef here (so we'll get a warning about appending a string to undef).

You might consider just adding an else block to the if ($opt_sim_sdk) above to initialize $shell_command to the empty string:

    } else {
        $shell_command = "";
    }

Then you can change this line to:

    $shell_command .= "$opt_arch ";

[David Farler]

Committed r150994: <http://trac.webkit.org/changeset/150994>

[Gyuyoung Kim]

jscore test has been broken r150994.

http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK1/builds/31

[David Farler]

Fix committed: r150998

Reviewed by Joe Pecoraro.