[nongnu] elpa/projectile 78be7717cd: [Fix #1755] Cache failure to find project root (#1779)

[ELPA Syncer]

branch: elpa/projectile
commit 78be7717cd7e81f039c7206de9ebef20f845d29f
Author: Edmund Jorgensen <tomheon@gmail.com>
Commit: GitHub <noreply@github.com>

    [Fix #1755] Cache failure to find project root (#1779)
    
    When `projectile-project-root` was unable to find a project root for a
    directory, it would not cache the negative result, so future invocations 
would
    repeat the (often expensive) search for a project root every time.
    
    With this change, `projectile-project-root` caches failure to find a project
    root, when that failure is expected to be permanent, and consults that cache
    for speed on subsequent invocations.  "Expected to be permanent" means that
    either we're trying to find the project root of a local directory, or we're
    successfully connected to a remote directory via TRAMP.  If the directory 
isn't
    local, but we can't connect to it, we consider that a transient failure and
    don't cache it.
    
    Under the hood, this change uses the same `projectile-project-root-cache`
    that's used to cache successful attempts to find a project root, but with a 
new
    key `projectilerootless-{dir}`.  This should allow cache invalidation to 
work
    as expected.
---
 CHANGELOG.md            |  1 +
 projectile.el           | 69 ++++++++++++++++++++++++++++---------------------
 test/projectile-test.el | 38 ++++++++++++++++++++++++++-
 3 files changed, 78 insertions(+), 30 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c6f5b55e7a..041055a396 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,7 @@
 * [#1765](https://github.com/bbatsov/projectile/issues/1765): Fix 
`src-dir`/`test-dir` not defaulting to `"src/"` and `"test/"` with 
`projectile-toggle-between-implementation-and-test`.
 * Fix version extraction logic.
 * [1654](https://github.com/bbatsov/projectile/issues/1654) Fix consecutive 
duplicates appearing in command history
+* [#1755](https://github.com/bbatsov/projectile/issues/1755) Cache failure to 
find project root
 
 ### Changes
 
diff --git a/projectile.el b/projectile.el
index d1a71bf251..aaa968d72f 100644
--- a/projectile.el
+++ b/projectile.el
@@ -1189,42 +1189,53 @@ topmost sequence of matched directories.  Nil 
otherwise."
 (defun projectile-project-root (&optional dir)
   "Retrieves the root directory of a project if available.
 If DIR is not supplied its set to the current directory by default."
-  ;; the cached value will be 'none in the case of no project root (this is to
-  ;; ensure it is not reevaluated each time when not inside a project) so use
-  ;; cl-subst to replace this 'none value with nil so a nil value is used
-  ;; instead
   (let ((dir (or dir default-directory)))
     ;; Back out of any archives, the project will live on the outside and
     ;; searching them is slow.
     (when (and (fboundp 'tramp-archive-file-name-archive)
                (tramp-archive-file-name-p dir))
       (setq dir (file-name-directory (tramp-archive-file-name-archive dir))))
+    ;; the cached value will be 'none in the case of no project root (this is 
to
+    ;; ensure it is not reevaluated each time when not inside a project) so use
+    ;; cl-subst to replace this 'none value with nil so a nil value is used
+    ;; instead
     (cl-subst nil 'none
-              ;; The `is-local' and `is-connected' variables are
-              ;; used to fix the behavior where Emacs hangs
-              ;; because of Projectile when you open a file over
-              ;; TRAMP. It basically prevents Projectile from
-              ;; trying to find information about files for which
-              ;; it's not possible to get that information right
-              ;; now.
-              (or (let ((is-local (not (file-remote-p dir)))      ;; `true' if 
the file is local
-                        (is-connected (file-remote-p dir nil t))) ;; `true' if 
the file is remote AND we are connected to the remote
-                    (when (or is-local is-connected)
-                      ;; Here is where all the magic happens.
-                      ;; We run the functions in 
`projectile-project-root-functions' until we find a project dir.
-                      (cl-some
-                       (lambda (func)
-                         (let* ((cache-key (format "%s-%s" func dir))
-                                (cache-value (gethash cache-key 
projectile-project-root-cache)))
-                           (if (and cache-value (file-exists-p cache-value))
-                               cache-value
-                             (let ((value (funcall func (file-truename dir))))
-                               (puthash cache-key value 
projectile-project-root-cache)
-                               value))))
-                       projectile-project-root-functions)))
-                  ;; set cached to none so is non-nil so we don't try
-                  ;; and look it up again
-                  'none))))
+      (or
+       ;; if we've already failed to find a project dir for this
+       ;; dir, and cached that failure, don't recompute
+       (let* ((cache-key (format "projectilerootless-%s" dir))
+              (cache-value (gethash cache-key projectile-project-root-cache)))
+         cache-value)
+       ;; if the file isn't local, and we're not connected, don't try to
+       ;; find a root now now, but don't cache failure, as we might
+       ;; re-connect.  The `is-local' and `is-connected' variables are
+       ;; used to fix the behavior where Emacs hangs because of
+       ;; Projectile when you open a file over TRAMP. It basically
+       ;; prevents Projectile from trying to find information about
+       ;; files for which it's not possible to get that information
+       ;; right now.
+       (let ((is-local (not (file-remote-p dir)))      ;; `true' if the file 
is local
+             (is-connected (file-remote-p dir nil t))) ;; `true' if the file 
is remote AND we are connected to the remote
+         (unless (or is-local is-connected)
+           'none))
+       ;; if the file is local or we're connected to it via TRAMP, run
+       ;; through the project root functions until we find a project dir
+       (cl-some
+        (lambda (func)
+          (let* ((cache-key (format "%s-%s" func dir))
+                 (cache-value (gethash cache-key 
projectile-project-root-cache)))
+            (if (and cache-value (file-exists-p cache-value))
+                cache-value
+              (let ((value (funcall func (file-truename dir))))
+                (puthash cache-key value projectile-project-root-cache)
+                value))))
+        projectile-project-root-functions)
+       ;; if we get here, we have failed to find a root by all
+       ;; conventional means, and we assume the failure isn't transient
+       ;; / network related, so cache the failure
+       (let ((cache-key (format "projectilerootless-%s" dir)))
+         (puthash cache-key 'none projectile-project-root-cache)
+         'none)))))
 
 (defun projectile-ensure-project (dir)
   "Ensure that DIR is non-nil.
diff --git a/test/projectile-test.el b/test/projectile-test.el
index a83268ccfa..9760e78d1c 100644
--- a/test/projectile-test.el
+++ b/test/projectile-test.el
@@ -691,7 +691,43 @@ Just delegates OPERATION and ARGS for all operations 
except for`shell-command`'.
                                                  projectile-root-top-down
                                                  
projectile-root-top-down-recurring)))
         (projectile-test-should-root-in "projectA/src/" "projectA/src/")
-        (projectile-test-should-root-in "projectA/src/" 
"projectA/src/html"))))))
+        (projectile-test-should-root-in "projectA/src/" 
"projectA/src/html")))))
+
+  (it "caches permanent failure to find a project root"
+    (projectile-test-with-sandbox
+     (projectile-test-with-files
+      ("projectA/src/")
+      (let* ((projectile-project-root-functions '())
+             (dir "projectA/src")
+             (cache-key (format "projectilerootless-%s" dir))
+             (projectile-project-root-cache (make-hash-table :test 'equal)))
+        (expect (gethash cache-key projectile-project-root-cache) :to-be nil)
+        (expect (projectile-project-root dir) :to-be nil)
+        ;; now that this has run once, the cache should be populated with 'none
+        (expect (gethash cache-key projectile-project-root-cache) :to-be 'none)
+        ;; but projectile-project-root should still return nil
+        (expect (projectile-project-root dir) :to-be nil)))))
+
+  (it "does not cache transitory failure to find a project root"
+    (projectile-test-with-sandbox
+     (projectile-test-with-files
+      ("projectA/src/")
+      ;; hackish, but override file-remote-p for a moment, which is called in
+      ;; projectile-project-root with 1 argument to test if the file is remote,
+      ;; and 3 arguments to see if the file is connected.  We want to return t
+      ;; when checking if remote, and nil when checking if connected.
+      (cl-letf (((symbol-function 'file-remote-p)
+                 (lambda (&rest args) (eql 1 (length args)))))
+        (let* ((projectile-project-root-functions '())
+               (dir "projectA/src")
+               (cache-key (format "projectilerootless-%s" dir))
+               (projectile-project-root-cache (make-hash-table :test 'equal)))
+          (expect (gethash cache-key projectile-project-root-cache) :to-be nil)
+          (expect (projectile-project-root dir) :to-be nil)
+          ;; since the failure was transitory, there should be nothing cached
+          (expect (gethash cache-key projectile-project-root-cache) :to-be nil)
+          ;; and projectile-project-root should still return nil
+          (expect (projectile-project-root dir) :to-be nil)))))))
 
 (describe "projectile-file-exists-p"
   (it "returns t if file exists"