revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-26 00:32:41 +01:00
Code Issues Releases Wiki Activity

Disable lint cache by default

Browse source 
Summary:
Fixes T2266. Motivation:

  - The lint cache does not always invalidate correctly. Because of the nature of the cache, this is a hard problem (right after "naming things").
    - We already have a fair amount of complexity in trying to invalidate it, and are still discovering new places where it doesn't work (e.g., Windows with "/" vs "\" paths).
    - One invalidation failure is when linter code changes, which seems unresolvable in the general case (e.g., changes to external linters).
  - It's not obvious what's happening when the lint cache causes some kind of issue.
    - Particularly while developing or debugging linters, your changes often won't be reflected in the lint output. Some of this is theoretically tractable but the external linter case probably isn't.
  - When someone reports a problem with the lint cache in IRC or elsewhere, there is essentially never a way for me to fix it. The lint cache can't be debugged effectively without access to a working copy where the problem reproduces.
  - The cache provides limited benefit outside of Facebook's install.

To remedy these issues:

  - Introduce configuration which controls cache usage.
  - Default it off.
  - Print a message when the cache is in use.

(I'd tentatively support removing the cache entirely, but I don't know how @vrana and Facebook feel about that.)

Test Plan: Ran `arc set-config --show`, `arc lint --cache 0`, `arc lint --cache 1`.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, mbishopim3, nh, edward, wez

Maniphest Tasks: T2266

Differential Revision: https://secure.phabricator.com/D5766
This commit is contained in:
epriestley 2013-04-23 11:26:28 -07:00
parent 410ea5dc32
commit 957249f56c
2 changed files with 27 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/configuration/ArcanistSettings.php
View file

@ -69,6 +69,16 @@ final class ArcanistSettings {
'branch. Supports \'rebase\' and \'merge\' strategies.',
'example' => '"rebase"',
),
'arc.lint.cache' => array(
'type' => 'bool',
'help' =>
"Enable the lint cache by default. When enabled, 'arc lint' ".
"attempts to use cached results if possible. Currently, the cache ".
"is not always invalidated correctly and may cause 'arc lint' to ".
"report incorrect results, particularly while developing linters. ".
"This is probably worth enabling only if your linters are very slow.",
'example' => 'false',
),
'history.immutable' => array(
'type' => 'bool',
'legacy' => 'immutable_history',

21
src/workflow/ArcanistLintWorkflow.php
View file

@ -129,7 +129,10 @@ EOTEXT
),
'cache' => array(
'param' => 'bool',
'help' => "0 to disable cache, 1 to enable (default).",
'help' =>
"0 to disable cache, 1 to enable. The default value is ".
"determined by 'arc.lint.cache' in configuration, which defaults ".
"to off. See notes in 'arc.lint.cache'.",
),
'*' => 'paths',
);
@ -156,6 +159,7 @@ EOTEXT
}
public function run() {
$console = PhutilConsole::getConsole();
$working_copy = $this->getWorkingCopy();
$engine = $this->getArgument('engine');
@ -170,7 +174,12 @@ EOTEXT
$rev = $this->getArgument('rev');
$paths = $this->getArgument('paths');
$use_cache = $this->getArgument('cache', true);
$use_cache = $this->getArgument('cache', null);
if ($use_cache === null) {
$use_cache = (bool)$working_copy->getConfigFromAnySource(
'arc.lint.cache',
false);
}
if ($rev && $paths) {
throw new ArcanistUsageException("Specify either --rev or paths.");
@ -221,6 +230,12 @@ EOTEXT
$cached[$path] = $messages;
}
}
if ($cached) {
$console->writeErr(
pht("Using lint cache, use '--cache 0' to disable it.")."\n");
}
$engine->setCachedResults($cached);
}
@ -415,8 +430,6 @@ EOTEXT
$all_autofix = true;
$console = PhutilConsole::getConsole();
foreach ($results as $result) {
$result_all_autofix = $result->isAllAutofix();