Page MenuHomePhabricator
Files F15415784

D21047.id50147.diff
No OneTemporary
Actions

D21047.id50147.diff
View Options

diff --git a/src/future/exec/ExecFuture.php b/src/future/exec/ExecFuture.php
--- a/src/future/exec/ExecFuture.php
+++ b/src/future/exec/ExecFuture.php
@@ -605,6 +605,9 @@
$trap->destroy();
} else {
$err = error_get_last();
+ if ($err) {
+ $err = $err['message'];
+ }
}
if ($is_windows) {
@@ -618,18 +621,21 @@
// 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
--- a/src/future/exec/PhutilExecPassthru.php
+++ b/src/future/exec/PhutilExecPassthru.php
@@ -68,15 +68,22 @@
$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
--- a/src/workflow/ArcanistWorkflow.php
+++ b/src/workflow/ArcanistWorkflow.php
@@ -2069,13 +2069,19 @@
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 @@
}
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;
}
}

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 21, 7:51 AM (1 w, 4 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7649426
Default Alt Text
D21047.id50147.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility