Add "--conduit-timeout" to arc and modernize argument parsing

Summary:
We added "repeat" to PhutilArgumentParser a while ago, so we can modernize the rest of the argument parsing (allow multiple --load-phutil-library flags).

Add a "--conduit-timeout" flag to allow users to set the timeout. See D2602.

Test Plan:
  $ arc diff --conduit-timeout=0.01
  Exception
  [cURL/28] <CURLE_OPERATION_TIMEOUTED> The request took too long to complete.
  (Run with --trace for a full exception trace.)
  $ arc diff --conduit-version 33
  Exception
  ERR-BAD-VERSION: Your 'arc' client version is '33', which is newer than the server version, '5'. Upgrade your Phabricator install.
  (Run with --trace for a full exception trace.)
  $ arc diff --conduit-uri http://derp.derp/
  Usage Exception: YOU NEED TO INSTALL A CERTIFICATE TO LOGIN TO PHABRICATOR

  You are trying to connect to 'http://derp.derp/api/' but do not have a certificate installed for this host. Run:

        $ arc install-certificate

  ...to install one.
  $ arc diff --load-phutil-library src --load-phutil-library ../phabricator/src --trace
  Loading phutil library '0' from '/INSECURE/devtools/arcanist/src'...
  Loading phutil library '1' from '/INSECURE/devtools/phabricator/src'...

Reviewers: phleet, vrana, btrahan

Reviewed By: btrahan

CC: aran

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

scripts/arcanist.php

@@ -26,31 +26,41 @@ ini_set('memory_limit', -1);
 $original_argv = $argv;
 $args = new PhutilArgumentParser($argv);
 $args->parseStandardArguments();
+$args->parsePartial(
+  array(
+    array(
+      'name'    => 'load-phutil-library',
+      'param'   => 'path',
+      'help'    => 'Load a libphutil library.',
+      'repeat'  => true,
+    ),
+    array(
+      'name'    => 'conduit-uri',
+      'param'   => 'uri',
+      'help'    => 'Connect to Phabricator install specified by __uri__.',
+    ),
+    array(
+      'name'    => 'conduit-version',
+      'param'   => 'version',
+      'help'    => '(Developers) Mock client version in protocol handshake.',
+    ),
+    array(
+      'name'    => 'conduit-timeout',
+      'param'   => 'timeout',
+      'help'    => 'Set Conduit timeout (in seconds).',
+    ),
+  ));
 
-$argv = $args->getUnconsumedArgumentVector();
 $config_trace_mode = $args->getArg('trace');
 
-$force_conduit = null;
-$force_conduit_version = null;
-$args = $argv;
-$load = array();
-$matches = null;
-foreach ($args as $key => $arg) {
-  if ($arg == '--') {
-    break;
-  } else if (preg_match('/^--load-phutil-library=(.*)$/', $arg, $matches)) {
-    unset($args[$key]);
-    $load[] = $matches[1];
-  } else if (preg_match('/^--conduit-uri=(.*)$/', $arg, $matches)) {
-    unset($args[$key]);
-    $force_conduit = $matches[1];
-  } else if (preg_match('/^--conduit-version=(.*)$/', $arg, $matches)) {
-    unset($args[$key]);
-    $force_conduit_version = $matches[1];
-  }
-}
+$force_conduit = $args->getArg('conduit-uri');
+$force_conduit_version = $args->getArg('conduit-version');
+$conduit_timeout = $args->getArg('conduit-timeout');
+$load = $args->getArg('load-phutil-library');
+
+$argv = $args->getUnconsumedArgumentVector();
+$args = array_values($argv);
 
-$args = array_values($args);
 $working_directory = getcwd();
 
 try {
@@ -170,6 +180,9 @@ try {
   if ($force_conduit_version) {
     $workflow->forceConduitVersion($force_conduit_version);
   }
+  if ($conduit_timeout) {
+    $workflow->setConduitTimeout($conduit_timeout);
+  }
 
   $need_working_copy    = $workflow->requiresWorkingCopy();
   $need_conduit         = $workflow->requiresConduit();
@@ -289,7 +302,7 @@ try {
   }
 
   echo phutil_console_format(
-    "\n**Exception:**\n%s\n%s\n",
+    "**Exception**\n%s\n%s\n",
     $ex->getMessage(),
     "(Run with --trace for a full exception trace.)");
 

src/workflow/ArcanistBaseWorkflow.php

@@ -57,6 +57,7 @@ abstract class ArcanistBaseWorkflow {
   private $conduitCredentials;
   private $conduitAuthenticated;
   private $forcedConduitVersion;
+  private $conduitTimeout;
 
   private $userPHID;
   private $userName;
@@ -146,6 +147,10 @@ abstract class ArcanistBaseWorkflow {
 
     $this->conduit = new ConduitClient($this->conduitURI);
 
+    if ($this->conduitTimeout) {
+      $this->conduit->setTimeout($this->conduitTimeout);
+    }
+
     return $this;
   }
 
@@ -179,15 +184,54 @@ abstract class ArcanistBaseWorkflow {
     return $this;
   }
 
+
+  /**
+   * Force arc to identify with a specific Conduit version during the
+   * protocol handshake. This is primarily useful for development (especially
+   * for sending diffs which bump the client Conduit version), since the client
+   * still actually speaks the builtin version of the protocol.
+   *
+   * Controlled by the --conduit-version flag.
+   *
+   * @param int Version the client should pretend to be.
+   * @return this
+   * @task conduit
+   */
   public function forceConduitVersion($version) {
     $this->forcedConduitVersion = $version;
     return $this;
   }
 
+
+  /**
+   * Get the protocol version the client should identify with.
+   *
+   * @return int Version the client should claim to be.
+   * @task conduit
+   */
   public function getConduitVersion() {
     return nonempty($this->forcedConduitVersion, 5);
   }
 
+
+  /**
+   * Override the default timeout for Conduit.
+   *
+   * Controlled by the --conduit-timeout flag.
+   *
+   * @param float Timeout, in seconds.
+   * @return this
+   * @task conduit
+   */
+  public function setConduitTimeout($timeout) {
+    $this->conduitTimeout = $timeout;
+    if ($this->conduit) {
+      $this->conduit->setConduitTimeout($timeout);
+    }
+    return $this;
+  }
+
+
   /**
    * Open and authenticate a conduit connection to a Phabricator server using
    * provided credentials. Normally, Arcanist does this for you automatically
@@ -307,7 +351,7 @@ abstract class ArcanistBaseWorkflow {
    * @task conduit
    */
   final protected function isConduitAuthenticated() {
-    return (bool) $this->conduitAuthenticated;
+    return (bool)$this->conduitAuthenticated;
   }
 
 

src/workflow/ArcanistHelpWorkflow.php

@@ -195,15 +195,17 @@ EOTEXT
           Ignore libraries listed in .arcconfig and explicitly load specified
           libraries instead. Mostly useful for Arcanist development.
 
-      __--conduit-uri=...__
+      __--conduit-uri__ __uri__
           Ignore configured Conduit URI and use an explicit one instead. Mostly
           useful for Arcanist development.
 
-      __--conduit-version=...__
+      __--conduit-version__ __version__
           Ignore software version and claim to be running some other version
           instead. Mostly useful for Arcanist development. May cause bad things
           to happen.
 
+      __--conduit-timeout__ __timeout__
+          Override the default Conduit timeout. Specified in seconds.
 
 EOTEXT
     );