(stable) Promote 2015 Week 43

src/lint/linter/ArcanistExternalLinter.php

@@ -13,6 +13,7 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
   private $bin;
   private $interpreter;
   private $flags;
+  private $versionRequirement;
 
 
 /* -(  Interpreters, Binaries and Flags  )----------------------------------- */
@@ -42,6 +43,16 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
    */
   abstract public function getInstallInstructions();
 
+  /**
+   * Return a human-readable string describing how to upgrade the linter.
+   *
+   * @return string Human readable upgrade instructions
+   * @task bin
+   */
+  public function getUpgradeInstructions() {
+      return null;
+  }
+
   /**
    * Return true to continue when the external linter exits with an error code.
    * By default, linters which exit with an error code are assumed to have
@@ -102,6 +113,18 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
     return $this;
   }
 
+  /**
+   * Set the binary's version requirement.
+   *
+   * @param string Version requirement.
+   * @return this
+   * @task bin
+   */
+  final public function setVersionRequirement($version) {
+    $this->versionRequirement = trim($version);
+    return $this;
+  }
+
   /**
    * Return the binary or script to execute. This method synthesizes defaults
    * and configuration. You can override the binary with @{method:setBinary}.
@@ -258,6 +281,64 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
     }
   }
 
+  /**
+   * If a binary version requirement has been specified, compare the version
+   * of the configured binary to the required version, and if the binary's
+   * version is not supported, throw an exception.
+   *
+   * @param  string   Version string to check.
+   * @return void
+   */
+  final protected function checkBinaryVersion($version) {
+    if (!$this->versionRequirement) {
+      return;
+    }
+
+    if (!$version) {
+      $message = pht(
+        'Linter %s requires %s version %s. Unable to determine the version '.
+        'that you have installed.',
+         get_class($this),
+         $this->getBinary(),
+         $this->versionRequirement);
+
+      $instructions = $this->getUpgradeInstructions();
+      if ($instructions) {
+        $message .= "\n".pht('TO UPGRADE: %s', $instructions);
+      }
+
+      throw new ArcanistMissingLinterException($message);
+    }
+
+    $operator = '==';
+    $compare_to = $this->versionRequirement;
+
+    $matches = null;
+    if (preg_match('/^([<>]=?|=)\s*(.*)$/', $compare_to, $matches)) {
+      $operator = $matches[1];
+      $compare_to = $matches[2];
+      if ($operator === '=') {
+        $operator = '==';
+      }
+    }
+
+    if (!version_compare($version, $compare_to, $operator)) {
+      $message = pht(
+        'Linter %s requires %s version %s. You have version %s.',
+        get_class($this),
+        $this->getBinary(),
+        $this->versionRequirement,
+        $version);
+
+      $instructions = $this->getUpgradeInstructions();
+      if ($instructions) {
+        $message .= "\n".pht('TO UPGRADE: %s', $instructions);
+      }
+
+      throw new ArcanistMissingLinterException($message);
+    }
+  }
+
   /**
    * Get the composed executable command, including the interpreter and binary
    * but without flags or paths. This can be used to execute `--version`
@@ -308,6 +389,7 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
     $version = $this->getVersion();
 
     if ($version) {
+      $this->checkBinaryVersion($version);
       return $version.'-'.json_encode($this->getCommandFlags());
     } else {
       // Either we failed to parse the version number or the `getVersion`
@@ -395,6 +477,13 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
           'Provide a list of additional flags to pass to the linter on the '.
           'command line.'),
       ),
+      'version' => array(
+        'type' => 'optional string',
+        'help' => pht(
+          'Specify a version requirement for the binary. The version number '.
+          'may be prefixed with <, <=, >, >=, or = to specify the version '.
+          'comparison operator (default: =).'),
+      ),
     );
 
     if ($this->shouldUseInterpreter()) {
@@ -456,6 +545,9 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
       case 'flags':
         $this->setFlags($value);
         return;
+      case 'version':
+        $this->setVersionRequirement($value);
+        return;
     }
 
     return parent::setLinterConfigurationValue($key, $value);

src/lint/linter/ArcanistScriptAndRegexLinter.php

@@ -324,7 +324,7 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
    * Get the line and character of the message from the regex match.
    *
    * @param dict Captured groups from regex.
-   * @return pair<int,int> Line and character of the message.
+   * @return pair<int,int|null> Line and character of the message.
    *
    * @task parse
    */
@@ -336,8 +336,19 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
       return array($line + 1, $char + 1);
     }
 
-    $line = idx($match, 'line', 1);
+    $line = idx($match, 'line');
+    if ($line) {
+      $line = (int)$line;
+    } else {
+      $line = 1;
+    }
+
     $char = idx($match, 'char');
+    if ($char) {
+      $char = (int)$char;
+    } else {
+      $char = null;
+    }
 
     return array($line, $char);
   }

src/workflow/ArcanistUpgradeWorkflow.php

@@ -53,7 +53,27 @@ EOTEXT
 
       $this->setRepositoryAPI($repository);
 
-      $this->requireCleanWorkingCopy();
+      // 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();
+      if ($uncommitted) {
+        $message = pht(
+          'You have uncommitted changes in the working copy for this '.
+          'library:');
+
+        $list = id(new PhutilConsoleList())
+          ->setWrap(false)
+          ->addItems(array_keys($uncommitted));
+
+        id(new PhutilConsoleBlock())
+          ->addParagraph($message)
+          ->addList($list)
+          ->draw();
+
+        throw new ArcanistUsageException(
+          pht('`arc upgrade` can only upgrade clean working copies.'));
+      }
 
       $branch_name = $repository->getBranchName();
       if ($branch_name != 'master' && $branch_name != 'stable') {
@@ -71,10 +91,13 @@ EOTEXT
       }
 
       chdir($root);
+
       try {
-        phutil_passthru('git pull --rebase');
+        execx('git pull --rebase');
       } catch (Exception $ex) {
-        phutil_passthru('git rebase --abort');
+        // If we failed, try to go back to the old state, then throw the
+        // original exception.
+        exec_manual('git rebase --abort');
         throw $ex;
       }
     }