Update "arc upload" for Toolsets

Summary: Ref T13490. This largely makes "arc upload" work, although the Future stuff is still a bit wonky. See T11968.

Test Plan: Ran "arc upload README.md", got an upload.

Maniphest Tasks: T13490

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

src/__phutil_library_map__.php

@@ -1401,7 +1401,7 @@ phutil_register_library_map(array(
     'ArcanistUnsafeDynamicStringXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistUnsafeDynamicStringXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistUpgradeWorkflow' => 'ArcanistArcWorkflow',
-    'ArcanistUploadWorkflow' => 'ArcanistWorkflow',
+    'ArcanistUploadWorkflow' => 'ArcanistArcWorkflow',
     'ArcanistUsageException' => 'Exception',
     'ArcanistUseStatementNamespacePrefixXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistUseStatementNamespacePrefixXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',

src/conduit/ArcanistConduitEngine.php

@@ -7,8 +7,6 @@ final class ArcanistConduitEngine
   private $conduitToken;
 
   private $conduitTimeout;
-  private $basicAuthUser;
-  private $basicAuthPass;
 
   private $client;
   private $callKey = 0;
@@ -46,24 +44,6 @@ final class ArcanistConduitEngine
     return $this->conduitTimeout;
   }
 
-  public function setBasicAuthUser($basic_auth_user) {
-    $this->basicAuthUser = $basic_auth_user;
-    return $this;
-  }
-
-  public function getBasicAuthUser() {
-    return $this->basicAuthUser;
-  }
-
-  public function setBasicAuthPass($basic_auth_pass) {
-    $this->basicAuthPass = $basic_auth_pass;
-    return $this;
-  }
-
-  public function getBasicAuthPass() {
-    return $this->basicAuthPass;
-  }
-
   public function newCall($method, array $parameters) {
     if ($this->conduitURI == null) {
       $this->raiseURIException();
@@ -102,12 +82,6 @@ final class ArcanistConduitEngine
         $client->setTimeout($timeout);
       }
 
-      $basic_user = $this->getBasicAuthUser();
-      $basic_pass = $this->getBasicAuthPass();
-      if ($basic_user !== null || $basic_pass !== null) {
-        $client->setBasicAuthCredentials($basic_user, $basic_pass);
-      }
-
       $token = $this->getConduitToken();
       if ($token) {
         $client->setConduitToken($this->getConduitToken());
@@ -148,10 +122,10 @@ final class ArcanistConduitEngine
           'Run in a working copy with "phabricator.uri" set in ".arcconfig".'))
       ->addItem(
         pht(
-          'Set a default URI with `arc set-config default <uri>`.'))
+          'Set a default URI with `arc set-config phabricator.uri <uri>`.'))
       ->addItem(
         pht(
-          'Specify a URI explicitly with `--conduit-uri=<uri>`.'));
+          'Specify a URI explicitly with `--config phabricator.uri=<uri>`.'));
 
     $block = id(new PhutilConsoleBlock())
       ->addParagraph(

src/runtime/ArcanistRuntime.php

@@ -107,6 +107,8 @@ final class ArcanistRuntime {
     $workflows = $this->newWorkflows($toolset);
     $this->workflows = $workflows;
 
+    $conduit_engine = $this->newConduitEngine($config);
+
     $phutil_workflows = array();
     foreach ($workflows as $key => $workflow) {
       $phutil_workflows[$key] = $workflow->newPhutilWorkflow();
@@ -114,7 +116,8 @@ final class ArcanistRuntime {
       $workflow
         ->setRuntime($this)
         ->setConfigurationEngine($config_engine)
-        ->setConfigurationSourceList($config);
+        ->setConfigurationSourceList($config)
+        ->setConduitEngine($conduit_engine);
     }
 
 
@@ -673,4 +676,49 @@ final class ArcanistRuntime {
     return $this->stack;
   }
 
+  private function newConduitEngine(ArcanistConfigurationSourceList $config) {
+
+    $conduit_uri = $config->getConfig('phabricator.uri');
+    if ($conduit_uri === null) {
+      $conduit_uri = $config->getConfig('default');
+    }
+
+    if ($conduit_uri) {
+      // Set the URI path to '/api/'. TODO: Originally, I contemplated letting
+      // you deploy Phabricator somewhere other than the domain root, but ended
+      // up never pursuing that. We should get rid of all "/api/" silliness
+      // in things users are expected to configure. This is already happening
+      // to some degree, e.g. "arc install-certificate" does it for you.
+      $conduit_uri = new PhutilURI($conduit_uri);
+      $conduit_uri->setPath('/api/');
+      $conduit_uri = phutil_string_cast($conduit_uri);
+    }
+
+    $engine = new ArcanistConduitEngine();
+
+    if ($conduit_uri !== null) {
+      $engine->setConduitURI($conduit_uri);
+    }
+
+    // TODO: This isn't using "getConfig()" because we aren't defining a
+    // real config entry for the moment.
+
+    $hosts = array();
+
+    $hosts_list = $config->getStorageValueList('hosts');
+    foreach ($hosts_list as $hosts_config) {
+      $hosts += $hosts_config->getValue();
+    }
+
+    $host_config = idx($hosts, $conduit_uri, array());
+    $user_name = idx($host_config, 'user');
+    $conduit_token = idx($host_config, 'token');
+
+    if ($conduit_token !== null) {
+      $engine->setConduitToken($conduit_token);
+    }
+
+    return $engine;
+  }
+
 }

src/upload/ArcanistFileUploader.php

@@ -25,22 +25,14 @@
  */
 final class ArcanistFileUploader extends Phobject {
 
-  private $conduit;
+  private $conduitEngine;
   private $files = array();
 
 
 /* -(  Configuring the Uploader  )------------------------------------------- */
 
-
+  public function setConduitEngine(ArcanistConduitEngine $engine) {
-  /**
+    $this->conduitEngine = $engine;
-   * Provide a Conduit client to choose which server to upload files to.
-   *
-   * @param ConduitClient Configured client.
-   * @return this
-   * @task config
-   */
-  public function setConduitClient(ConduitClient $conduit) {
-    $this->conduit = $conduit;
     return $this;
   }
 
@@ -99,8 +91,8 @@ final class ArcanistFileUploader extends Phobject {
    * @task upload
    */
   public function uploadFiles() {
-    if (!$this->conduit) {
+    if (!$this->conduitEngine) {
-      throw new PhutilInvalidStateException('setConduitClient');
+      throw new PhutilInvalidStateException('setConduitEngine');
     }
 
     $files = $this->files;
@@ -113,7 +105,7 @@ final class ArcanistFileUploader extends Phobject {
       }
     }
 
-    $conduit = $this->conduit;
+    $conduit = $this->conduitEngine;
     $futures = array();
     foreach ($files as $key => $file) {
       $params = $this->getUploadParameters($file) + array(
@@ -126,7 +118,11 @@ final class ArcanistFileUploader extends Phobject {
         $params['deleteAfterEpoch'] = $delete_after;
       }
 
-      $futures[$key] = $conduit->callMethod('file.allocate', $params);
+      // TOOLSETS: This should be a real future, but ConduitEngine and
+      // ConduitCall are currently built oddly and return pretend futures.
+
+      $futures[$key] = new ImmediateFuture(
+        $conduit->resolveCall('file.allocate', $params));
     }
 
     $iterator = id(new FutureIterator($futures))->limit(4);
@@ -219,9 +215,9 @@ final class ArcanistFileUploader extends Phobject {
    * @task internal
    */
   private function uploadChunks(ArcanistFileDataRef $file, $file_phid) {
-    $conduit = $this->conduit;
+    $conduit = $this->conduitEngine;
 
-    $chunks = $conduit->callMethodSynchronous(
+    $chunks = $conduit->resolveCall(
       'file.querychunks',
       array(
         'filePHID' => $file_phid,
@@ -262,7 +258,7 @@ final class ArcanistFileUploader extends Phobject {
     foreach ($remaining as $chunk) {
       $data = $file->readBytes($chunk['byteStart'], $chunk['byteEnd']);
 
-      $conduit->callMethodSynchronous(
+      $conduit->resolveCall(
         'file.uploadchunk',
         array(
           'filePHID' => $file_phid,
@@ -282,11 +278,11 @@ final class ArcanistFileUploader extends Phobject {
    * @task internal
    */
   private function uploadData(ArcanistFileDataRef $file) {
-    $conduit = $this->conduit;
+    $conduit = $this->conduitEngine;
 
     $data = $file->readBytes(0, $file->getByteSize());
 
-    return $conduit->callMethodSynchronous(
+    return $conduit->resolveCall(
       'file.upload',
       $this->getUploadParameters($file) + array(
         'data_base64' => base64_encode($data),

src/workflow/ArcanistUploadWorkflow.php

@@ -1,70 +1,56 @@
 <?php
 
-/**
+final class ArcanistUploadWorkflow
- * Upload a file to Phabricator.
+  extends ArcanistArcWorkflow {
- */
-final class ArcanistUploadWorkflow extends ArcanistWorkflow {
-
-  private $paths;
-  private $json;
 
   public function getWorkflowName() {
     return 'upload';
   }
 
-  public function getCommandSynopses() {
+  public function getWorkflowInformation() {
-    return phutil_console_format(<<<EOTEXT
+    $help = pht(<<<EOTEXT
-      **upload** __file__ [__file__ ...] [--json]
+Upload one or more files from local disk to Phabricator.
 EOTEXT
-      );
+);
+
+    return $this->newWorkflowInformation()
+      ->setSynopsis(pht('Upload files.'))
+      ->addExample(pht('**upload** [__options__] -- __file__ [__file__ ...]'))
+      ->setHelp($help);
   }
 
-  public function getCommandHelp() {
+  public function getWorkflowArguments() {
-    return phutil_console_format(<<<EOTEXT
-          Supports: filesystems
-          Upload a file from local disk.
-EOTEXT
-      );
-  }
-
-  public function getArguments() {
     return array(
-      'json' => array(
+      $this->newWorkflowArgument('json')
-        'help' => pht('Output upload information in JSON format.'),
+        ->setHelp(pht('Output upload information in JSON format.')),
-      ),
+      $this->newWorkflowArgument('temporary')
-      'temporary' => array(
+        ->setHelp(
-        'help' => pht(
+          pht(
-          'Mark the file as temporary. Temporary files will be deleted '.
+            'Mark the file as temporary. Temporary files will be '.
-          'automatically after 24 hours.'),
+            'deleted after 24 hours.')),
-      ),
+      $this->newWorkflowArgument('paths')
-      '*' => 'paths',
+        ->setWildcard(true)
+        ->setIsPathArgument(true),
     );
   }
 
-  protected function didParseArguments() {
+  public function runWorkflow() {
     if (!$this->getArgument('paths')) {
-      throw new ArcanistUsageException(
+      throw new PhutilArgumentUsageException(
-        pht('Specify one or more files to upload.'));
+        pht('Specify one or more paths to files you want to upload.'));
     }
 
-    $this->paths = $this->getArgument('paths');
-    $this->json = $this->getArgument('json');
-  }
-
-  public function requiresAuthentication() {
-    return true;
-  }
-
-  public function run() {
     $is_temporary = $this->getArgument('temporary');
+    $is_json = $this->getArgument('json');
+    $paths = $this->getArgument('paths');
 
-    $conduit = $this->getConduit();
+    $conduit = $this->getConduitEngine();
     $results = array();
 
     $uploader = id(new ArcanistFileUploader())
-      ->setConduitClient($conduit);
+      ->setConduitEngine($conduit);
 
-    foreach ($this->paths as $key => $path) {
+    foreach ($paths as $key => $path) {
       $file = id(new ArcanistFileDataRef())
         ->setName(basename($path))
         ->setPath($path);
@@ -89,7 +75,7 @@ EOTEXT
       $phid = $file->getPHID();
       $name = $file->getName();
 
-      $info = $conduit->callMethodSynchronous(
+      $info = $conduit->resolveCall(
         'file.info',
         array(
           'phid' => $phid,
@@ -97,14 +83,15 @@ EOTEXT
 
       $results[$path] = $info;
 
-      if (!$this->json) {
+      if (!$is_json) {
         $id = $info['id'];
         echo "  F{$id} {$name}: ".$info['uri']."\n\n";
       }
     }
 
-    if ($this->json) {
+    if ($is_json) {
-      echo json_encode($results)."\n";
+      $output = id(new PhutilJSON())->encodeFormatted($results);
+      echo $output;
     } else {
       $this->writeStatus(pht('Done.'));
     }
@@ -125,7 +112,7 @@ EOTEXT
     }
 
     $this->writeStatus(pht('Beginning chunked upload of large file...'));
-    $chunks = $conduit->callMethodSynchronous(
+    $chunks = $conduit->resolveCall(
       'file.querychunks',
       array(
         'filePHID' => $file_phid,
@@ -182,7 +169,7 @@ EOTEXT
             'fread()'));
       }
 
-      $conduit->callMethodSynchronous(
+      $conduit->resolveCall(
         'file.uploadchunk',
         array(
           'filePHID' => $file_phid,