Page MenuHomePhabricator
Files F15336173

D21060.diff
No OneTemporary
Actions

D21060.diff
View Options

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(
+ "<Process was terminated by signal %s (%d).>\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 @@
+<?php
+
+final class PhageExecWorkflow
+ extends PhageWorkflow {
+
+ public function getWorkflowName() {
+ return 'exec';
+ }
+
+ public function getWorkflowArguments() {
+ return array(
+ $this->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())

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 9, 6:20 PM (2 w, 22 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7390519
Default Alt Text
D21060.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility