We use almost identical code in webkitdirs::run{MiniBrowser, Safari, TestWebKitAPI, WebKitTestRunner}() to launch the respective application on Mac OS X. We should extract the common code into a function, say runMacWebKitApp(), so that we can remove duplicate code.
Created attachment 108416 [details] Patch
Comment on attachment 108416 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108416&action=review It would be nice if run-webkit-app used this function, too. > Tools/Scripts/webkitdirs.pm:2013 > + return system $appPath, @ARGV; While you're at it, it would be nice to switch this to the direct object form of system() so that it will work correctly even if $appPath contains spaces and @ARGV is empty.
(In reply to comment #2) > (From update of attachment 108416 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=108416&action=review > > It would be nice if run-webkit-app used this function, too. Will do. > > > Tools/Scripts/webkitdirs.pm:2013 > > + return system $appPath, @ARGV; > > While you're at it, it would be nice to switch this to the direct object form of system() so that it will work correctly even if $appPath contains spaces and @ARGV is empty. Will change to use direct object form of system().
Created attachment 108504 [details] Patch Updated patch based on Adam Roben's suggestions. I added an optional argument $useOpenCommand to runMacWebKitApp() that when equal to USE_OPEN_COMMAND will use the open(1) to launch the application.
Comment on attachment 108504 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108504&action=review > Tools/Scripts/webkitdirs.pm:2028 > + if ($useOpenCommand == USE_OPEN_COMMAND) { > + exec("open", "-a", $appPath, "--args", @ARGV); > + } It's a little surprising that USE_OPEN_COMMAND also implies using exec() instead of system(). Maybe we should have a separate parameter for that?
(In reply to comment #5) > (From update of attachment 108504 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=108504&action=review > > > Tools/Scripts/webkitdirs.pm:2028 > > + if ($useOpenCommand == USE_OPEN_COMMAND) { > > + exec("open", "-a", $appPath, "--args", @ARGV); > > + } > > It's a little surprising that USE_OPEN_COMMAND also implies using exec() instead of system(). Maybe we should have a separate parameter for that? For now, I'll convert this to system() and pass -W to open(1) so that it blocks until the application it opened exits. This makes the behavior of run-webkit-app consistent with the behavior of {run, debug}-safari. Originally I thought to just use exec() so that we don't fork(2) a process for open(1), which also fork(2)s a process for the application $appPath. That is, I was looking to minimizing fork(2)ing since it seemed excessive to call fork(2) twice.
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 108504 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=108504&action=review > > > > > Tools/Scripts/webkitdirs.pm:2028 > > > + if ($useOpenCommand == USE_OPEN_COMMAND) { > > > + exec("open", "-a", $appPath, "--args", @ARGV); > > > + } > > > > It's a little surprising that USE_OPEN_COMMAND also implies using exec() instead of system(). Maybe we should have a separate parameter for that? > > For now, I'll convert this to system() and pass -W to open(1) so that it blocks until the application it opened exits. This makes the behavior of run-webkit-app consistent with the behavior of {run, debug}-safari. > More importantly, by using system() we can makes the usage of runMacWebKitApp() consistent. It will always return an exit status to its caller. Although the exit status of open(1) is probably not as valuable as the exit status of the application :-/.
Committed r96448: <http://trac.webkit.org/changeset/96448>
I'm now getting an error from run-safari on Lion: Use of uninitialized value $useOpenCommand in numeric eq (==) at /Users/ap/Safari/OpenSource/Tools/Scripts/webkitdirs.pm line 2026.
(In reply to comment #9) > I'm now getting an error from run-safari on Lion: > > Use of uninitialized value $useOpenCommand in numeric eq (==) at /Users/ap/Safari/OpenSource/Tools/Scripts/webkitdirs.pm line 2026. Substituted "defined($useOpenCommand) && $useOpenCommand == USE_OPEN_COMMAND" for "$useOpenCommand == USE_OPEN_COMMAND" in the if-statement condition in runMacWebKitApp() (http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitdirs.pm?rev=96448#L2026). Committed this change in changeset 96664 <http://trac.webkit.org/changeset/96664>.