diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php index c10cbfdc..d25c10a2 100644 --- a/src/lint/linter/ArcanistExternalLinter.php +++ b/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); diff --git a/src/lint/linter/ArcanistScriptAndRegexLinter.php b/src/lint/linter/ArcanistScriptAndRegexLinter.php index 70ce944a..ca8883cb 100644 --- a/src/lint/linter/ArcanistScriptAndRegexLinter.php +++ b/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 Line and character of the message. + * @return pair 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); } diff --git a/src/workflow/ArcanistUpgradeWorkflow.php b/src/workflow/ArcanistUpgradeWorkflow.php index c9e0e858..50449ef7 100644 --- a/src/workflow/ArcanistUpgradeWorkflow.php +++ b/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; } }