RESOLVED FIXED 116339
Add --sdk flag to jsDriver.pl to allow running in the iOS simulator
https://bugs.webkit.org/show_bug.cgi?id=116339
Summary Add --sdk flag to jsDriver.pl to allow running in the iOS simulator
David Farler
Reported 2013-05-17 12:02:05 PDT
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.
Attachments
Patch (3.16 KB, patch)
2013-05-22 11:09 PDT, David Farler
no flags
Patch (3.16 KB, patch)
2013-05-22 18:07 PDT, David Farler
no flags
Patch (3.21 KB, patch)
2013-05-22 18:24 PDT, David Farler
no flags
Patch (3.27 KB, patch)
2013-05-23 15:09 PDT, David Farler
ddkilzer: review+
ddkilzer: commit-queue-
David Farler
Comment 1 2013-05-22 11:09:35 PDT
Joseph Pecoraro
Comment 2 2013-05-22 12:32:22 PDT
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
Comment 3 2013-05-22 18:07:42 PDT
Created attachment 202634 [details] Patch Needs a chomp around the shell call because Perl.
David Farler
Comment 4 2013-05-22 18:24:53 PDT
Created attachment 202635 [details] Patch sim needs --adopt-pid to have the correct return code from the forked process
David Kilzer (:ddkilzer)
Comment 5 2013-05-23 15:03:40 PDT
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
Comment 6 2013-05-23 15:08:47 PDT
(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
Comment 7 2013-05-23 15:09:14 PDT
David Kilzer (:ddkilzer)
Comment 8 2013-05-28 13:39:30 PDT
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
Comment 9 2013-05-30 17:16:17 PDT
Gyuyoung Kim
Comment 10 2013-05-30 18:28:10 PDT
David Farler
Comment 11 2013-05-30 18:46:22 PDT
Fix committed: r150998 Reviewed by Joe Pecoraro.
Note You need to log in before you can comment on or make changes to this bug.