mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-22 23:02:41 +01:00
Allow caching lint results
Summary: There is one big decision: How to get linters version. I've created `getCacheVersion()` which is supposed to be bumped every time engine or any linter is changed. This is not very nice but the other alternative (detect this automatically) seems worse: - The engine may be outside repo and may or may not be under version control so getting its version through something like `git log` may not be even possible. - If it is in the same repo then every rebase will obsolete the whole cache. Even though bumping the version manually is PITA I still think it's a better solution. Test Plan: $ time arc lint --cache 1 # verified file $ arc arc lint --cache 1 # added some debug output to see the cached results # also observed better time (.57 s instead of 2.19 s) Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2036 Differential Revision: https://secure.phabricator.com/D4006
This commit is contained in:
parent
0cf8ef02d8
commit
3cfd58c29c
4 changed files with 85 additions and 18 deletions
|
@ -48,6 +48,8 @@ final class ArcanistLintMessage {
|
||||||
'severity' => $this->getSeverity(),
|
'severity' => $this->getSeverity(),
|
||||||
'name' => $this->getName(),
|
'name' => $this->getName(),
|
||||||
'description' => $this->getDescription(),
|
'description' => $this->getDescription(),
|
||||||
|
'original' => $this->getOriginalText(),
|
||||||
|
'replacement' => $this->getReplacementText(),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -165,22 +165,6 @@ abstract class ArcanistLintEngine {
|
||||||
$stopped = array();
|
$stopped = array();
|
||||||
$linters = $this->buildLinters();
|
$linters = $this->buildLinters();
|
||||||
|
|
||||||
if (!$linters) {
|
|
||||||
throw new ArcanistNoEffectException("No linters to run.");
|
|
||||||
}
|
|
||||||
|
|
||||||
$have_paths = false;
|
|
||||||
foreach ($linters as $linter) {
|
|
||||||
if ($linter->getPaths()) {
|
|
||||||
$have_paths = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!$have_paths) {
|
|
||||||
throw new ArcanistNoEffectException("No paths are lintable.");
|
|
||||||
}
|
|
||||||
|
|
||||||
$exceptions = array();
|
$exceptions = array();
|
||||||
foreach ($linters as $linter_name => $linter) {
|
foreach ($linters as $linter_name => $linter) {
|
||||||
try {
|
try {
|
||||||
|
@ -251,6 +235,22 @@ abstract class ArcanistLintEngine {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!$linters) {
|
||||||
|
throw new ArcanistNoEffectException("No linters to run.");
|
||||||
|
}
|
||||||
|
|
||||||
|
$have_paths = false;
|
||||||
|
foreach ($linters as $linter) {
|
||||||
|
if ($linter->getPaths()) {
|
||||||
|
$have_paths = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!$have_paths) {
|
||||||
|
throw new ArcanistNoEffectException("No paths are lintable.");
|
||||||
|
}
|
||||||
|
|
||||||
if ($exceptions) {
|
if ($exceptions) {
|
||||||
throw new PhutilAggregateException('Some linters failed:', $exceptions);
|
throw new PhutilAggregateException('Some linters failed:', $exceptions);
|
||||||
}
|
}
|
||||||
|
@ -291,7 +291,7 @@ abstract class ArcanistLintEngine {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
private function getResultForPath($path) {
|
public function getResultForPath($path) {
|
||||||
if (empty($this->results[$path])) {
|
if (empty($this->results[$path])) {
|
||||||
$result = new ArcanistLintResult();
|
$result = new ArcanistLintResult();
|
||||||
$result->setPath($path);
|
$result->setPath($path);
|
||||||
|
@ -336,6 +336,10 @@ abstract class ArcanistLintEngine {
|
||||||
return $this;
|
return $this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getCacheVersion() {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
protected function getPEP8WithTextOptions() {
|
protected function getPEP8WithTextOptions() {
|
||||||
// E101 is subset of TXT2 (Tab Literal).
|
// E101 is subset of TXT2 (Tab Literal).
|
||||||
// E501 is same as TXT3 (Line Too Long).
|
// E501 is same as TXT3 (Line Too Long).
|
||||||
|
|
|
@ -354,6 +354,13 @@ EOTEXT
|
||||||
'Run lint and unit tests on background. '.
|
'Run lint and unit tests on background. '.
|
||||||
'"0" to disable, "1" to enable (default).',
|
'"0" to disable, "1" to enable (default).',
|
||||||
),
|
),
|
||||||
|
'cache' => array(
|
||||||
|
'param' => 'bool',
|
||||||
|
'help' => "0 to disable lint cache (default), 1 to enable.",
|
||||||
|
'passthru' => array(
|
||||||
|
'lint' => true,
|
||||||
|
),
|
||||||
|
),
|
||||||
'*' => 'paths',
|
'*' => 'paths',
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|
|
@ -120,6 +120,10 @@ EOTEXT
|
||||||
array_keys(ArcanistLintSeverity::getLintSeverities())).
|
array_keys(ArcanistLintSeverity::getLintSeverities())).
|
||||||
"'. Defaults to '".self::DEFAULT_SEVERITY."'.",
|
"'. Defaults to '".self::DEFAULT_SEVERITY."'.",
|
||||||
),
|
),
|
||||||
|
'cache' => array(
|
||||||
|
'param' => 'bool',
|
||||||
|
'help' => "0 to disable cache (default), 1 to enable.",
|
||||||
|
),
|
||||||
'*' => 'paths',
|
'*' => 'paths',
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
@ -132,6 +136,15 @@ EOTEXT
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function getCacheKey() {
|
||||||
|
return implode("\n", array(
|
||||||
|
get_class($this->engine),
|
||||||
|
$this->getArgument('severity', self::DEFAULT_SEVERITY),
|
||||||
|
$this->getArgument('lintall'),
|
||||||
|
$this->engine->getCacheVersion(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
public function run() {
|
public function run() {
|
||||||
$working_copy = $this->getWorkingCopy();
|
$working_copy = $this->getWorkingCopy();
|
||||||
|
|
||||||
|
@ -176,6 +189,25 @@ EOTEXT
|
||||||
$engine->setMinimumSeverity(
|
$engine->setMinimumSeverity(
|
||||||
$this->getArgument('severity', self::DEFAULT_SEVERITY));
|
$this->getArgument('severity', self::DEFAULT_SEVERITY));
|
||||||
|
|
||||||
|
$cached = false;
|
||||||
|
if ($this->getArgument('cache')) {
|
||||||
|
$paths = array_combine($paths, $paths);
|
||||||
|
$cache = $this->readScratchJSONFile('lint-cache.json');
|
||||||
|
$cache = idx($cache, $this->getCacheKey(), array());
|
||||||
|
foreach ($cache as $path => $messages) {
|
||||||
|
$messages = idx($messages, md5_file($engine->getFilePathOnDisk($path)));
|
||||||
|
// TODO: Some linters work with the whole directory.
|
||||||
|
if ($messages !== null) {
|
||||||
|
foreach ($messages as $message) {
|
||||||
|
$engine->getResultForPath($path)->addMessage(
|
||||||
|
ArcanistLintMessage::newFromDictionary($message));
|
||||||
|
}
|
||||||
|
$cached = true;
|
||||||
|
unset($paths[$path]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Propagate information about which lines changed to the lint engine.
|
// Propagate information about which lines changed to the lint engine.
|
||||||
// This is used so that the lint engine can drop warning messages
|
// This is used so that the lint engine can drop warning messages
|
||||||
// concerning lines that weren't in the change.
|
// concerning lines that weren't in the change.
|
||||||
|
@ -201,8 +233,12 @@ EOTEXT
|
||||||
try {
|
try {
|
||||||
$engine->run();
|
$engine->run();
|
||||||
} catch (Exception $ex) {
|
} catch (Exception $ex) {
|
||||||
|
if ($ex instanceof ArcanistNoEffectException && $cached) {
|
||||||
|
// Swallow.
|
||||||
|
} else {
|
||||||
$failed = $ex;
|
$failed = $ex;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
$results = $engine->getResults();
|
$results = $engine->getResults();
|
||||||
|
|
||||||
|
@ -365,6 +401,24 @@ EOTEXT
|
||||||
}
|
}
|
||||||
$this->unresolvedMessages = $unresolved;
|
$this->unresolvedMessages = $unresolved;
|
||||||
|
|
||||||
|
if ($this->getArgument('cache')) {
|
||||||
|
$cached = array();
|
||||||
|
foreach ($results as $result) {
|
||||||
|
$path = $result->getPath();
|
||||||
|
$hash = md5_file($engine->getFilePathOnDisk($path));
|
||||||
|
$cached[$path] = array($hash => array());
|
||||||
|
foreach ($result->getMessages() as $message) {
|
||||||
|
if (!$message->isPatchApplied()) {
|
||||||
|
$cached[$path][$hash][] = $message->toDictionary();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
$cache = $this->readScratchJSONFile('lint-cache.json');
|
||||||
|
$cache[$this->getCacheKey()] = $cached;
|
||||||
|
// TODO: Garbage collection.
|
||||||
|
$this->writeScratchJSONFile('lint-cache.json', $cache);
|
||||||
|
}
|
||||||
|
|
||||||
// Take the most severe lint message severity and use that
|
// Take the most severe lint message severity and use that
|
||||||
// as the result code.
|
// as the result code.
|
||||||
if ($has_errors) {
|
if ($has_errors) {
|
||||||
|
|
Loading…
Reference in a new issue