From 957249f56c35f23e3305e2b07492181fd2a03c78 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 23 Apr 2013 11:26:28 -0700
Subject: [PATCH] Disable lint cache by default

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
---
 src/configuration/ArcanistSettings.php | 10 ++++++++++
 src/workflow/ArcanistLintWorkflow.php  | 21 +++++++++++++++++----
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/src/configuration/ArcanistSettings.php b/src/configuration/ArcanistSettings.php
index 58a9642a..f50f1cbf 100644
--- a/src/configuration/ArcanistSettings.php
+++ b/src/configuration/ArcanistSettings.php
@@ -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',
diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php
index 02909515..218d89be 100644
--- a/src/workflow/ArcanistLintWorkflow.php
+++ b/src/workflow/ArcanistLintWorkflow.php
@@ -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();