Format "arc land" passthru commands more nicely, and execute them from CWD

Summary:
Fixes T13380. Ref T13546. Use slightly nicer command formatting for passthru commands, to make it more obvious what's going on and which pieces of output are from "arc" vs various subcommands.

Also, execute repository API passthru commands from the working copy root. All other commands already did this, the older API just didn't support it.

Test Plan: Ran "arc land" in Git and Mercurial repositories, saw nicer output formatting.

Maniphest Tasks: T13546, T13380

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

src/land/engine/ArcanistGitLandEngine.php

@@ -174,10 +174,9 @@ final class ArcanistGitLandEngine
           'Synchronizing "%s" from Perforce...',
           $target->getRef()));
 
-      $err = $api->execPassthru(
+      $err = $this->newPassthru(
         'p4 sync --silent --branch %s --',
         $target->getRemote().'/'.$target->getRef());
-
       if ($err) {
         throw new ArcanistUsageException(
           pht(
@@ -416,12 +415,11 @@ final class ArcanistGitLandEngine
       // fix conflicts and run "arc land" again.
       $flags_argv[] = '--conflict=quit';
 
-      $err = $api->execPassthru(
+      $err = $this->newPassthru(
         '%LR p4 submit %LR --commit %R --',
         $config_argv,
         $flags_argv,
         $into_commit);
-
       if ($err) {
         throw new ArcanistUsageException(
           pht(
@@ -435,7 +433,7 @@ final class ArcanistGitLandEngine
       pht('PUSHING'),
       pht('Pushing changes to "%s".', $this->getOntoRemote()));
 
-    $err = $api->execPassthru(
+    $err = $this->newPassthru(
       'push -- %s %Ls',
       $this->getOntoRemote(),
       $this->newOntoRefArguments($into_commit));
@@ -1298,9 +1296,7 @@ final class ArcanistGitLandEngine
     $ignore_failure = false) {
     $api = $this->getRepositoryAPI();
 
-    // TODO: Format this fetch nicely as a workflow command.
+    $err = $this->newPassthru(
-
-    $err = $api->execPassthru(
       'fetch --no-tags --quiet -- %s %s',
       $target->getRemote(),
       $target->getRef());

src/land/engine/ArcanistLandEngine.php

@@ -1548,4 +1548,20 @@ abstract class ArcanistLandEngine extends Phobject {
     return $sets;
   }
 
+  final protected function newPassthru($pattern /* , ... */) {
+    $workflow = $this->getWorkflow();
+    $argv = func_get_args();
+
+    $api = $this->getRepositoryAPI();
+
+    $passthru = call_user_func_array(
+      array($api, 'newPassthru'),
+      $argv);
+
+    $command = $workflow->newCommand($passthru)
+      ->setResolveOnError(true);
+
+    return $command->execute();
+  }
+
 }

src/land/engine/ArcanistMercurialLandEngine.php

@@ -360,14 +360,14 @@ final class ArcanistMercurialLandEngine
 
     // TODO: Support bookmarks.
     // TODO: Deal with bookmark save/restore behavior.
-    // TODO: Format this nicely with passthru.
     // TODO: Raise a good error message when the ref does not exist.
 
-    $api->execPassthru(
+    $err = $this->newPassthru(
       'pull -b %s -- %s',
       $target->getRef(),
       $target->getRemote());
 
+    // TODO: Deal with errors.
     // TODO: Deal with multiple branch heads.
 
     list($stdout) = $api->execxLocal(
@@ -507,7 +507,7 @@ final class ArcanistMercurialLandEngine
 
     // TODO: This does not respect "--into" or "--onto" properly.
 
-    $api->execxLocal(
+    $this->newPassthru(
       'push --rev %s -- %s',
       hgsprintf('%s', $into_commit),
       $this->getOntoRemote());

src/repository/api/ArcanistGitAPI.php

@@ -20,12 +20,11 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
   protected function buildLocalFuture(array $argv) {
     $argv[0] = 'git '.$argv[0];
 
-    $future = newv('ExecFuture', $argv);
+    return newv('ExecFuture', $argv)
-    $future->setCWD($this->getPath());
+      ->setCWD($this->getPath());
-    return $future;
   }
 
-  public function execPassthru($pattern /* , ... */) {
+  public function newPassthru($pattern /* , ... */) {
     $args = func_get_args();
 
     static $git = null;
@@ -43,10 +42,10 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
 
     $args[0] = $git.' '.$args[0];
 
-    return call_user_func_array('phutil_passthru', $args);
+    return newv('PhutilExecPassthru', $args)
+      ->setCWD($this->getPath());
   }
 
-
   public function getSourceControlSystemName() {
     return 'git';
   }

src/repository/api/ArcanistMercurialAPI.php

@@ -27,18 +27,16 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     return $future;
   }
 
-  public function execPassthru($pattern /* , ... */) {
+  public function newPassthru($pattern /* , ... */) {
     $args = func_get_args();
 
     $env = $this->getMercurialEnvironmentVariables();
 
     $args[0] = 'hg '.$args[0];
 
-    $passthru = newv('PhutilExecPassthru', $args)
+    return newv('PhutilExecPassthru', $args)
       ->setEnv($env)
       ->setCWD($this->getPath());
-
-    return $passthru->resolve();
   }
 
   public function getSourceControlSystemName() {

src/repository/api/ArcanistRepositoryAPI.php

@@ -675,6 +675,20 @@ abstract class ArcanistRepositoryAPI extends Phobject {
       ->setResolveOnError(false);
   }
 
+  public function newPassthru($pattern /* , ... */) {
+    throw new PhutilMethodNotImplementedException();
+  }
+
+  final public function execPassthru($pattern /* , ... */) {
+    $args = func_get_args();
+
+    $future = call_user_func_array(
+      array($this, 'newPassthru'),
+      $args);
+
+    return $future->resolve();
+  }
+
   final public function setRuntime(ArcanistRuntime $runtime) {
     $this->runtime = $runtime;
     return $this;

src/toolset/command/ArcanistCommand.php

@@ -5,6 +5,7 @@ final class ArcanistCommand
 
   private $logEngine;
   private $executableFuture;
+  private $resolveOnError = false;
 
   public function setExecutableFuture(PhutilExecutableFuture $future) {
     $this->executableFuture = $future;
@@ -24,6 +25,15 @@ final class ArcanistCommand
     return $this->logEngine;
   }
 
+  public function setResolveOnError($resolve_on_error) {
+    $this->resolveOnError = $resolve_on_error;
+    return $this;
+  }
+
+  public function getResolveOnError() {
+    return $this->resolveOnError;
+  }
+
   public function execute() {
     $log = $this->getLogEngine();
     $future = $this->getExecutableFuture();
@@ -41,7 +51,7 @@ final class ArcanistCommand
 
     $log->writeNewline();
 
-    if ($err) {
+    if ($err && !$this->getResolveOnError()) {
       $log->writeError(
         pht('ERROR'),
         pht(
@@ -55,5 +65,7 @@ final class ArcanistCommand
         '',
         '');
     }
+
+    return $err;
   }
 }