Port "arc upgrade" to Toolsets

Summary: Ref T13490. Allows "arc upgrade" to run through the new Toolsets infrastructure.

Test Plan: Ran "arc upgrade", although you can't upgrade feature branches so this can't be entirely tested until it hits "master".

Maniphest Tasks: T13490

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

src/__phutil_library_map__.php

@@ -95,6 +95,7 @@ phutil_register_library_map(array(
     'ArcanistClosureLinterTestCase' => 'lint/linter/__tests__/ArcanistClosureLinterTestCase.php',
     'ArcanistCoffeeLintLinter' => 'lint/linter/ArcanistCoffeeLintLinter.php',
     'ArcanistCoffeeLintLinterTestCase' => 'lint/linter/__tests__/ArcanistCoffeeLintLinterTestCase.php',
+    'ArcanistCommand' => 'toolset/command/ArcanistCommand.php',
     'ArcanistCommentRemover' => 'parser/ArcanistCommentRemover.php',
     'ArcanistCommentRemoverTestCase' => 'parser/__tests__/ArcanistCommentRemoverTestCase.php',
     'ArcanistCommentSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistCommentSpacingXHPASTLinterRule.php',
@@ -1036,6 +1037,7 @@ phutil_register_library_map(array(
     'ArcanistClosureLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistCoffeeLintLinter' => 'ArcanistExternalLinter',
     'ArcanistCoffeeLintLinterTestCase' => 'ArcanistExternalLinterTestCase',
+    'ArcanistCommand' => 'Phobject',
     'ArcanistCommentRemover' => 'Phobject',
     'ArcanistCommentRemoverTestCase' => 'PhutilTestCase',
     'ArcanistCommentSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
@@ -1399,7 +1401,7 @@ phutil_register_library_map(array(
     'ArcanistUnnecessarySymbolAliasXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistUnsafeDynamicStringXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistUnsafeDynamicStringXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
-    'ArcanistUpgradeWorkflow' => 'ArcanistWorkflow',
+    'ArcanistUpgradeWorkflow' => 'ArcanistArcWorkflow',
     'ArcanistUploadWorkflow' => 'ArcanistWorkflow',
     'ArcanistUsageException' => 'Exception',
     'ArcanistUseStatementNamespacePrefixXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',

src/future/exec/ExecFuture.php

@@ -33,7 +33,6 @@ final class ExecFuture extends PhutilExecutableFuture {
 
   private $stdoutPos    = 0;
   private $stderrPos    = 0;
-  private $command      = null;
 
   private $readBufferSize;
   private $stdoutSizeLimit = PHP_INT_MAX;
@@ -55,42 +54,13 @@ final class ExecFuture extends PhutilExecutableFuture {
     2 => array('pipe', 'w'),  // stderr
   );
 
-
-/* -(  Creating ExecFutures  )----------------------------------------------- */
-
-
-  /**
-   * Create a new ExecFuture.
-   *
-   *   $future = new ExecFuture('wc -l %s', $file_path);
-   *
-   * @param string  `sprintf()`-style command string which will be passed
-   *                through @{function:csprintf} with the rest of the arguments.
-   * @param ...     Zero or more additional arguments for @{function:csprintf}.
-   * @return ExecFuture ExecFuture for running the specified command.
-   * @task create
-   */
-  public function __construct($command) {
-    $argv = func_get_args();
-    $this->command = call_user_func_array('csprintf', $argv);
+  protected function didConstruct() {
     $this->stdin = new PhutilRope();
   }
 
-
 /* -(  Command Information  )------------------------------------------------ */
 
 
-  /**
-   * Retrieve the raw command to be executed.
-   *
-   * @return string Raw command.
-   * @task info
-   */
-  public function getCommand() {
-    return $this->command;
-  }
-
-
   /**
    * Retrieve the byte limit for the stderr buffer.
    *
@@ -349,7 +319,7 @@ final class ExecFuture extends PhutilExecutableFuture {
   public function resolvex($timeout = null) {
     list($err, $stdout, $stderr) = $this->resolve($timeout);
     if ($err) {
-      $cmd = $this->command;
+      $cmd = $this->getCommand();
 
       if ($this->getWasKilledByTimeout()) {
         // NOTE: The timeout can be a float and PhutilNumber only handles
@@ -385,7 +355,7 @@ final class ExecFuture extends PhutilExecutableFuture {
   public function resolveJSON($timeout = null) {
     list($stdout, $stderr) = $this->resolvex($timeout);
     if (strlen($stderr)) {
-      $cmd = $this->command;
+      $cmd = $this->getCommand();
       throw new CommandException(
         pht(
           "JSON command '%s' emitted text to stderr when none was expected: %d",
@@ -399,7 +369,7 @@ final class ExecFuture extends PhutilExecutableFuture {
     try {
       return phutil_json_decode($stdout);
     } catch (PhutilJSONParserException $ex) {
-      $cmd = $this->command;
+      $cmd = $this->getCommand();
       throw new CommandException(
         pht(
           "JSON command '%s' did not produce a valid JSON object on stdout: %s",
@@ -579,7 +549,7 @@ final class ExecFuture extends PhutilExecutableFuture {
         $this->profilerCallID = $profiler->beginServiceCall(
           array(
             'type'    => 'exec',
-            'command' => (string)$this->command,
+            'command' => phutil_string_cast($this->getCommand()),
           ));
       }
 
@@ -588,10 +558,8 @@ final class ExecFuture extends PhutilExecutableFuture {
         $this->start = microtime(true);
       }
 
-      $unmasked_command = $this->command;
-      if ($unmasked_command instanceof PhutilCommandString) {
-        $unmasked_command = $unmasked_command->getUnmaskedString();
-      }
+      $unmasked_command = $this->getCommand();
+      $unmasked_command = $unmasked_command->getUnmaskedString();
 
       $pipes = array();
 
@@ -674,7 +642,7 @@ final class ExecFuture extends PhutilExecutableFuture {
           pht(
             'Call to "proc_open()" to open a subprocess failed: %s',
             $err),
-          $this->command,
+          $this->getCommand(),
           1,
           '',
           '');

src/future/exec/PhutilExecPassthru.php

@@ -20,30 +20,12 @@
  */
 final class PhutilExecPassthru extends PhutilExecutableFuture {
 
-
-  private $command;
   private $passthruResult;
 
 
 /* -(  Executing Passthru Commands  )---------------------------------------- */
 
 
-  /**
-   * Build a new passthru command.
-   *
-   *   $exec = new PhutilExecPassthru('ls %s', $dir);
-   *
-   * @param string Command pattern. See @{function:csprintf}.
-   * @param ...    Pattern arguments.
-   *
-   * @task command
-   */
-  public function __construct($pattern /* , ... */) {
-    $args = func_get_args();
-    $this->command = call_user_func_array('csprintf', $args);
-  }
-
-
   /**
    * Execute this command.
    *
@@ -52,7 +34,7 @@ final class PhutilExecPassthru extends PhutilExecutableFuture {
    * @task command
    */
   public function execute() {
-    $command = $this->command;
+    $command = $this->getCommand();
 
     $profiler = PhutilServiceProfiler::getInstance();
     $call_id = $profiler->beginServiceCall(
@@ -65,11 +47,7 @@ final class PhutilExecPassthru extends PhutilExecutableFuture {
     $spec  = array(STDIN, STDOUT, STDERR);
     $pipes = array();
 
-    if ($command instanceof PhutilCommandString) {
-      $unmasked_command = $command->getUnmaskedString();
-    } else {
-      $unmasked_command = $command;
-    }
+    $unmasked_command = $command->getUnmaskedString();
 
     if ($this->hasEnv()) {
       $env = $this->getEnv();

src/future/exec/PhutilExecutableFuture.php

@@ -5,10 +5,38 @@
  */
 abstract class PhutilExecutableFuture extends Future {
 
-
+  private $command;
   private $env;
   private $cwd;
 
+  final public function __construct($pattern /* , ... */) {
+    $args = func_get_args();
+
+    if ($pattern instanceof PhutilCommandString) {
+      if (count($args) !== 1) {
+        throw new Exception(
+          pht(
+            'Command (of class "%s") was constructed with a '.
+            '"PhutilCommandString", but also passed arguments. '.
+            'When using a preprebuilt command, you must not pass '.
+            'arguments.',
+            get_class($this)));
+      }
+      $this->command = $pattern;
+    } else {
+      $this->command = call_user_func_array('csprintf', $args);
+    }
+
+    $this->didConstruct();
+  }
+
+  protected function didConstruct() {
+    return;
+  }
+
+  final public function getCommand() {
+    return $this->command;
+  }
 
   /**
    * Set environmental variables for the command.

src/toolset/command/ArcanistCommand.php

@@ -0,0 +1,59 @@
+<?php
+
+final class ArcanistCommand
+  extends Phobject {
+
+  private $logEngine;
+  private $executableFuture;
+
+  public function setExecutableFuture(PhutilExecutableFuture $future) {
+    $this->executableFuture = $future;
+    return $this;
+  }
+
+  public function getExecutableFuture() {
+    return $this->executableFuture;
+  }
+
+  public function setLogEngine(ArcanistLogEngine $log_engine) {
+    $this->logEngine = $log_engine;
+    return $this;
+  }
+
+  public function getLogEngine() {
+    return $this->logEngine;
+  }
+
+  public function execute() {
+    $log = $this->getLogEngine();
+    $future = $this->getExecutableFuture();
+    $command = $future->getCommand();
+
+    $log->writeNewline();
+
+    $log->writeStatus(
+      ' $ ',
+      tsprintf('**%s**', phutil_string_cast($command)));
+
+    $log->writeNewline();
+
+    $err = $future->resolve();
+
+    $log->writeNewline();
+
+    if ($err) {
+      $log->writeError(
+        pht('ERROR'),
+        pht(
+          'Command exited with error code %d.',
+          $err));
+
+      throw new CommandException(
+        pht('Command exited with nonzero error code.'),
+        $command,
+        $err,
+        '',
+        '');
+    }
+  }
+}

src/workflow/ArcanistUpgradeWorkflow.php

@@ -1,30 +1,31 @@
 <?php
 
-/**
- * Upgrade arcanist itself.
- */
-final class ArcanistUpgradeWorkflow extends ArcanistWorkflow {
+final class ArcanistUpgradeWorkflow
+  extends ArcanistArcWorkflow {
 
   public function getWorkflowName() {
     return 'upgrade';
   }
 
-  public function getCommandSynopses() {
-    return phutil_console_format(<<<EOTEXT
-      **upgrade**
+  public function getWorkflowInformation() {
+    $help = pht(<<<EOTEXT
+Upgrade Arcanist to the latest version.
 EOTEXT
-      );
+);
+
+    return $this->newWorkflowInformation()
+      ->setSynopsis(pht('Upgrade Arcanist to the latest version.'))
+      ->addExample(pht('**upgrade**'))
+      ->setHelp($help);
   }
 
-  public function getCommandHelp() {
-    return phutil_console_format(<<<EOTEXT
-          Supports: cli
-          Upgrade arcanist and libphutil to the latest versions.
-EOTEXT
-      );
+  public function getWorkflowArguments() {
+    return array();
   }
 
-  public function run() {
+  public function runWorkflow() {
+    $log = $this->getLogEngine();
+
     $roots = array(
       'arcanist' => dirname(phutil_get_library_root('arcanist')),
     );
@@ -32,41 +33,46 @@ EOTEXT
     $supported_branches = array(
       'master',
       'stable',
-      'experimental',
     );
     $supported_branches = array_fuse($supported_branches);
 
-    foreach ($roots as $lib => $root) {
-      echo phutil_console_format(
-        "%s\n",
-        pht('Upgrading %s...', $lib));
+    foreach ($roots as $library => $root) {
+      $log->writeStatus(
+        pht('PREPARING'),
+        pht(
+          'Preparing to upgrade "%s"...',
+          $library));
 
-      $working_copy = ArcanistWorkingCopyIdentity::newFromPath($root);
-      $configuration_manager = clone $this->getConfigurationManager();
-      $configuration_manager->setWorkingCopyIdentity($working_copy);
-      $repository = ArcanistRepositoryAPI::newAPIFromConfigurationManager(
-        $configuration_manager);
+      $is_git = false;
 
-      if (!Filesystem::pathExists($repository->getMetadataPath())) {
-        throw new ArcanistUsageException(
-          pht(
-            "%s must be in its git working copy to be automatically upgraded. ".
-            "This copy of %s (in '%s') is not in a git working copy.",
-            $lib,
-            $lib,
-            $root));
+      $working_copy = ArcanistWorkingCopy::newFromWorkingDirectory($root);
+      if ($working_copy) {
+        $repository_api = $working_copy->newRepositoryAPI();
+        if ($repository_api instanceof ArcanistGitAPI) {
+          $is_git = true;
+        }
       }
 
-      $this->setRepositoryAPI($repository);
+      if (!$is_git) {
+        throw new PhutilArgumentUsageException(
+          pht(
+            'The "arc upgrade" workflow uses "git pull" to upgrade '.
+            'Arcanist, but the "arcanist/" directory  (in "%s") is not a Git '.
+            'working copy. You must leave "arcanist/" as a Git '.
+            'working copy to use "arc upgrade".',
+            $root));
+      }
 
       // NOTE: Don't use requireCleanWorkingCopy() here because it tries to
       // amend changes and generally move the workflow forward. We just want to
       // abort if there are local changes and make the user sort things out.
-      $uncommitted = $repository->getUncommittedStatus();
+      $uncommitted = $repository_api->getUncommittedStatus();
       if ($uncommitted) {
         $message = pht(
-          'You have uncommitted changes in the working copy for this '.
-          'library:');
+          'You have uncommitted changes in the working copy ("%s") for this '.
+          'library ("%s"):',
+          $root,
+          $library);
 
         $list = id(new PhutilConsoleList())
           ->setWrap(false)
@@ -75,29 +81,45 @@ EOTEXT
         id(new PhutilConsoleBlock())
           ->addParagraph($message)
           ->addList($list)
+          ->addParagraph(
+            pht(
+              'Discard these changes before running "arc upgrade".'))
           ->draw();
 
-        throw new ArcanistUsageException(
-          pht('`arc upgrade` can only upgrade clean working copies.'));
+        throw new PhutilArgumentUsageException(
+          pht('"arc upgrade" can only upgrade clean working copies.'));
       }
 
-      $branch_name = $repository->getBranchName();
+      $branch_name = $repository_api->getBranchName();
       if (!isset($supported_branches[$branch_name])) {
-        throw new ArcanistUsageException(
+        throw new PhutilArgumentUsageException(
           pht(
             'Library "%s" (in "%s") is on branch "%s", but this branch is '.
             'not supported for automatic upgrades. Supported branches are: '.
             '%s.',
-            $lib,
+            $library,
             $root,
             $branch_name,
             implode(', ', array_keys($supported_branches))));
       }
 
-      chdir($root);
+      $log->writeStatus(
+        pht('UPGRADING'),
+        pht(
+          'Upgrading "%s" (on branch "%s").',
+          $library,
+          $branch_name));
+
+      $command = csprintf(
+        'git pull --rebase origin -- %R',
+        $branch_name);
+
+      $future = (new PhutilExecPassthru($command))
+        ->setCWD($root);
 
       try {
-        execx('git pull --rebase');
+        $this->newCommand($future)
+          ->execute();
       } catch (Exception $ex) {
         // If we failed, try to go back to the old state, then throw the
         // original exception.
@@ -106,10 +128,10 @@ EOTEXT
       }
     }
 
-    echo phutil_console_format(
-      "**%s** %s\n",
-      pht('Updated!'),
-      pht('Your copy of arc is now up to date.'));
+    $log->writeSuccess(
+      pht('UPGRADED'),
+      pht('Your copy of Arcanist is now up to date.'));
+
     return 0;
   }
 

src/workflow/ArcanistWorkflow.php

@@ -2243,4 +2243,10 @@ abstract class ArcanistWorkflow extends Phobject {
     return false;
   }
 
+  final public function newCommand(PhutilExecutableFuture $future) {
+    return id(new ArcanistCommand())
+      ->setLogEngine($this->getLogEngine())
+      ->setExecutableFuture($future);
+  }
+
 }