diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -537,6 +537,7 @@ 'PhageAgentAction' => 'phage/action/PhageAgentAction.php', 'PhageAgentBootloader' => 'phage/bootloader/PhageAgentBootloader.php', 'PhageAgentTestCase' => 'phage/__tests__/PhageAgentTestCase.php', + 'PhageExecWorkflow' => 'phage/workflow/PhageExecWorkflow.php', 'PhageExecuteAction' => 'phage/action/PhageExecuteAction.php', 'PhageLocalAction' => 'phage/action/PhageLocalAction.php', 'PhagePHPAgent' => 'phage/agent/PhagePHPAgent.php', @@ -1484,6 +1485,7 @@ 'PhageAgentAction' => 'PhageAction', 'PhageAgentBootloader' => 'Phobject', 'PhageAgentTestCase' => 'PhutilTestCase', + 'PhageExecWorkflow' => 'PhageWorkflow', 'PhageExecuteAction' => 'PhageAction', 'PhageLocalAction' => 'PhageAgentAction', 'PhagePHPAgent' => 'Phobject', 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 @@ -752,20 +752,29 @@ } if ($is_done) { + $signal_info = null; + // If the subprocess got nuked with `kill -9`, we get a -1 exitcode. // Upgrade this to a slightly more informative value by examining the // terminating signal code. $err = $status['exitcode']; if ($err == -1) { if ($status['signaled']) { - $err = 128 + $status['termsig']; + $signo = $status['termsig']; + + $err = 128 + $signo; + + $signal_info = pht( + "\n\n", + phutil_get_signal_name($signo), + $signo); } } $result = array( $err, $this->stdout, - $this->stderr, + $signal_info.$this->stderr, ); $this->setResult($result); diff --git a/src/phage/action/PhageLocalAction.php b/src/phage/action/PhageLocalAction.php --- a/src/phage/action/PhageLocalAction.php +++ b/src/phage/action/PhageLocalAction.php @@ -4,7 +4,17 @@ extends PhageAgentAction { protected function newAgentFuture(PhutilCommandString $command) { - return new ExecFuture('sh -c %s', $command); + $arcanist_src = phutil_get_library_root('arcanist'); + $bin_dir = Filesystem::concatenatePaths( + array( + dirname($arcanist_src), + 'bin', + )); + + $future = id(new ExecFuture('%s exec -- %C', './phage', $command)) + ->setCWD($bin_dir); + + return $future; } } diff --git a/src/phage/workflow/PhageExecWorkflow.php b/src/phage/workflow/PhageExecWorkflow.php new file mode 100644 --- /dev/null +++ b/src/phage/workflow/PhageExecWorkflow.php @@ -0,0 +1,69 @@ +newWorkflowArgument('argv') + ->setHelp(pht('Command to execute.')) + ->setWildcard(true), + ); + } + + public function getWorkflowInformation() { + return $this->newWorkflowInformation() + ->setSynopsis(pht('Execute a Phage subprocess.')); + } + + protected function runWorkflow() { + $argv = $this->getArgument('argv'); + if (!$argv) { + throw new PhutilArgumentUsageException( + pht( + 'Specify a command to execute using one or more arguments.')); + } + + // This workflow is just a thin wrapper around running a subprocess as + // its own process group leader. + // + // If the Phage parent process executes a subprocess and that subprocess + // does not turn itself into a process group leader, sending "^C" to the + // parent process will also send the signal to the subprocess. Phage + // handles SIGINT as an input and we don't want it to propagate to children + // by default. + // + // Some versions of Linux have a binary named "setsid" which does the same + // thing, but this binary doesn't exist on macOS. + + // NOTE: This calls is documented as ever being able to fail. For now, + // trust the documentation? + + $pid = posix_getpid(); + + $pgid = posix_getpgid($pid); + if ($pgid === false) { + throw new Exception(pht('Call to "posix_getpgid(...)" failed!')); + } + + // If this process was run directly from a shell with "phage exec ...", + // we'll already be the process group leader. In this case, we don't need + // to become the leader of a new session (and the call will fail if we + // try). + + $is_leader = ($pid === $pgid); + if (!$is_leader) { + $sid = posix_setsid(); + if ($sid === -1) { + throw new Exception(pht('Call to "posix_setsid()" failed!')); + } + } + + return phutil_passthru('exec -- %Ls', $argv); + } + +} diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -126,6 +126,20 @@ ->setArguments($specs); $information = $this->getWorkflowInformation(); + + if ($information !== null) { + if (!($information instanceof ArcanistWorkflowInformation)) { + throw new Exception( + pht( + 'Expected workflow ("%s", of class "%s") to return an '. + '"ArcanistWorkflowInformation" object from call to '. + '"getWorkflowInformation()", got %s.', + $this->getWorkflowName(), + get_class($this), + phutil_describe_type($information))); + } + } + if ($information) { $synopsis = $information->getSynopsis(); if (strlen($synopsis)) { @@ -2255,10 +2269,14 @@ return $this->getConfigurationSourceList()->getConfig($key); } - final public function canHandleSignal($signo) { + public function canHandleSignal($signo) { return false; } + public function handleSignal($signo) { + return; + } + final public function newCommand(PhutilExecutableFuture $future) { return id(new ArcanistCommand()) ->setLogEngine($this->getLogEngine())