diff --git a/src/future/exec/ExecFuture.php b/src/future/exec/ExecFuture.php index afb0fd14..a49d5062 100644 --- a/src/future/exec/ExecFuture.php +++ b/src/future/exec/ExecFuture.php @@ -605,6 +605,9 @@ final class ExecFuture extends PhutilExecutableFuture { $trap->destroy(); } else { $err = error_get_last(); + if ($err) { + $err = $err['message']; + } } if ($is_windows) { @@ -618,18 +621,21 @@ final class ExecFuture extends PhutilExecutableFuture { // it fails to resolve the command. // When you run an invalid command on a Windows system, we bypass the - // shell and the "proc_open()" itself fails. Throw a "CommandException" - // here for consistency with the Linux behavior in this common failure - // case. + // shell and the "proc_open()" itself fails. See also T13504. Fail the + // future immediately, acting as though it exited with an error code + // for consistency with Linux. - throw new CommandException( + $result = array( + 1, + '', pht( 'Call to "proc_open()" to open a subprocess failed: %s', $err), - $this->getCommand(), - 1, - '', - ''); + ); + + $this->setResult($result); + + return true; } if ($is_windows) { diff --git a/src/future/exec/PhutilExecPassthru.php b/src/future/exec/PhutilExecPassthru.php index 44eef97e..6dec0f2a 100644 --- a/src/future/exec/PhutilExecPassthru.php +++ b/src/future/exec/PhutilExecPassthru.php @@ -68,15 +68,22 @@ final class PhutilExecPassthru extends PhutilExecutableFuture { $trap->destroy(); if (!is_resource($proc)) { - throw new Exception( - pht( - 'Failed to passthru %s: %s', - 'proc_open()', - $errors)); + // See T13504. When "proc_open()" is given a command with a binary + // that isn't available on Windows with "bypass_shell", the function + // fails entirely. + if (phutil_is_windows()) { + $err = 1; + } else { + throw new Exception( + pht( + 'Failed to passthru %s: %s', + 'proc_open()', + $errors)); + } + } else { + $err = proc_close($proc); } - $err = proc_close($proc); - return $err; } diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index 7fc3db1a..94f5b66d 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -2069,13 +2069,19 @@ abstract class ArcanistWorkflow extends Phobject { protected function openURIsInBrowser(array $uris) { $browser = $this->getBrowserCommand(); + + // The "browser" may actually be a list of arguments. + if (!is_array($browser)) { + $browser = array($browser); + } + foreach ($uris as $uri) { - $err = phutil_passthru('%s %s', $browser, $uri); + $err = phutil_passthru('%LR %R', $browser, $uri); if ($err) { throw new ArcanistUsageException( pht( - "Failed to open '%s' in browser ('%s'). ". - "Check your 'browser' config option.", + 'Failed to open URI "%s" in browser ("%s"). '. + 'Check your "browser" config option.', $uri, $browser)); } @@ -2089,19 +2095,29 @@ abstract class ArcanistWorkflow extends Phobject { } if (phutil_is_windows()) { - return 'start'; + // See T13504. We now use "bypass_shell", so "start" alone is no longer + // a valid binary to invoke directly. + return array( + 'cmd', + '/c', + 'start', + ); } - $candidates = array('sensible-browser', 'xdg-open', 'open'); + $candidates = array( + 'sensible-browser' => array('sensible-browser'), + 'xdg-open' => array('xdg-open'), + 'open' => array('open', '--'), + ); // NOTE: The "open" command works well on OS X, but on many Linuxes "open" // exists and is not a browser. For now, we're just looking for other // commands first, but we might want to be smarter about selecting "open" // only on OS X. - foreach ($candidates as $cmd) { + foreach ($candidates as $cmd => $argv) { if (Filesystem::binaryExists($cmd)) { - return $cmd; + return $argv; } }