Allow "phage" to print execution status on SIGINT

Summary: Ref T13490. The new Arcanist runtime supports workflow signal handling, but Phage isn't quite able to make use of it. Clean up the last few pieces so it can work.

Test Plan: Ran "phage", hit ^C, got status information.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21060

src/__phutil_library_map__.php

@@ -537,6 +537,7 @@ phutil_register_library_map(array(
     '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 @@ phutil_register_library_map(array(
     'PhageAgentAction' => 'PhageAction',
     'PhageAgentBootloader' => 'Phobject',
     'PhageAgentTestCase' => 'PhutilTestCase',
+    'PhageExecWorkflow' => 'PhageWorkflow',
     'PhageExecuteAction' => 'PhageAction',
     'PhageLocalAction' => 'PhageAgentAction',
     'PhagePHPAgent' => 'Phobject',

src/future/exec/ExecFuture.php

@@ -752,20 +752,29 @@ final class ExecFuture extends PhutilExecutableFuture {
     }
 
     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);

src/phage/action/PhageLocalAction.php

@@ -4,7 +4,17 @@ final class PhageLocalAction
   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;
   }
 
 }

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);
+  }
+
+}

src/workflow/ArcanistWorkflow.php

@@ -126,6 +126,20 @@ abstract class ArcanistWorkflow extends Phobject {
       ->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 @@ abstract class ArcanistWorkflow extends Phobject {
     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())