Honor the VCS ignores in the watch-driven cache updates

CHANGELOG.md

@@ -77,6 +77,7 @@
 
 ### Bugs fixed
 
+- [#2139](https://github.com/bbatsov/projectile/pull/2139): The file-notification cache updates now honor the VCS's ignore rules too, so a watched project no longer gains files a re-index would never have listed - the last place `.gitignore` was going unread, after [#2126](https://github.com/bbatsov/projectile/pull/2126) fixed it for files opened by hand. One `git check-ignore` covers a whole batch of events, since a batch can be an entire directory moved into the project.
 - [#1927](https://github.com/bbatsov/projectile/issues/1927): A known projects file Projectile can't read is now moved aside with a `.corrupt` suffix and reported, instead of being read as an empty list - which made it look like another Emacs had removed every project, so the merge dropped the session's projects too and overwrote the file. Projectile also strips text properties when saving, since a propertized string whose properties don't read back is how the file gets corrupted in the first place.
 
 - [#2118](https://github.com/bbatsov/projectile/issues/2118): Fix `projectile-find-file` showing "Projectile is indexing" forever with `projectile-async-indexing` enabled: Projectile waited for the indexing process's sentinel, which Emacs doesn't guarantee to run while a command sits waiting on the process, and now collects the finished command's output itself instead (`projectile-async-index-sentinel-timeout`).

doc/modules/ROOT/pages/indexing.adoc

@@ -515,13 +515,11 @@ Be aware of the trade-offs before enabling it:
 * Watched directories are derived from the cached file list, so a
   directory that contains no cached files (e.g. an empty directory) isn't
   watched until the next full re-index.
-* VCS-level ignores (e.g. `.gitignore`) are not consulted when adding
-  newly created files, only Projectile's own ignore settings and your
-  `.projectile` file. Under `alien` indexing a watched project may thus
-  temporarily pick up files your VCS would have excluded. (Opening such a
-  file doesn't cache it - that path does consult the VCS - but a watch
-  batch can cover hundreds of files at once, which is too many processes
-  to ask git about one at a time.)
+* Under `alien` and `hybrid` indexing the VCS is consulted too, so a
+  watched project doesn't gain files your `.gitignore` excludes. That's one
+  `git check-ignore` per batch of events rather than one per file, since a
+  batch can cover a whole directory that was moved in. Other version control
+  systems aren't asked, and `native` indexing lists such files anyway.
 
 Watches are dropped automatically when a project's cache is invalidated
 (and re-armed when it's next filled), when the option is disabled via

projectile.el

@@ -2038,6 +2038,54 @@ project was unwatched."
                                #'projectile--process-watch-events project)
                projectile--watch-debounce-timers))))
 
+(defvar projectile--watch-added-files nil
+  "Paths the watch batch in progress has added to the cache.
+Bound by `projectile--process-watch-events\\=' so the whole batch can be
+put to the VCS in one go; nil outside one.")
+
+(defun projectile--vcs-ignored-subset (root relatives)
+  "Return the members of RELATIVES that the VCS at ROOT ignores.
+
+One `git check-ignore\\=' answers for the whole list, which is what makes
+this usable from the watch path - a process per file would not be.
+Returns nil for anything but git, and for a git that fails; treating the
+answer as \"nothing is ignored\" is conservative, since the only cost is
+the drift this exists to remove.
+
+`call-process-region\\=' rather than a TRAMP-aware call because watches are
+never armed for a remote project in the first place."
+  (when (and relatives (eq (projectile-project-vcs root) 'git))
+    (let ((default-directory root))
+      (with-temp-buffer
+        ;; Exit status 1 means "none of them are ignored", which is an
+        ;; answer rather than a failure; only 0 produces output.
+        (when (eq 0 (ignore-errors
+                      (call-process-region
+                       (mapconcat #'identity relatives "\0") nil
+                       "git" nil t nil
+                       "check-ignore" "-z" "--stdin")))
+          (split-string (buffer-string) "\0" t))))))
+
+(defun projectile--watch-drop-vcs-ignored (project added)
+  "Remove from PROJECT\\='s cache the ADDED paths its VCS ignores.
+
+The watch path applies Projectile\\='s own ignore rules, which know nothing
+about a `.gitignore\\='; under `alien\\=' and `hybrid\\=' indexing the VCS is
+what produced the file list, so without this a watched project slowly
+gains files a re-index would never have listed (see issue #1075, which
+fixed the same hole for files opened by hand).
+
+Returns non-nil when the cache was changed."
+  (when (memq projectile-indexing-method '(alien hybrid))
+    (when-let* ((ignored (projectile--vcs-ignored-subset project added)))
+      (let ((set (make-hash-table :test 'equal :size (length ignored))))
+        (dolist (file ignored) (puthash file t set))
+        (puthash project
+                 (seq-remove (lambda (file) (gethash file set))
+                             (gethash project projectile-projects-cache))
+                 projectile-projects-cache)
+        t))))
+
 (defun projectile--process-watch-events (project)
   "Apply PROJECT's queued file-notify events to its cached file list.
 Runs from the debounce timer.  If any event can't be applied
@@ -2060,12 +2108,19 @@ scheduled via `projectile--schedule-cache-flush'."
               'projectile--none)
           (projectile--unwatch-project project)
         (let* ((mutated nil)
+               (projectile--watch-added-files nil)
                (fallback
                 (catch 'projectile--watch-fallback
                   (dolist (event events)
                     (when (projectile--watch-apply-event project event)
                       (setq mutated t)))
                   nil)))
+          ;; One question to the VCS for everything the batch added, rather
+          ;; than one per file as the opened-file path can afford.
+          (when (and (not fallback)
+                     (projectile--watch-drop-vcs-ignored
+                      project projectile--watch-added-files))
+            (setq mutated t))
           (cond
            (fallback
             (when projectile-verbose
@@ -2143,6 +2198,7 @@ Returns non-nil when the cached list was mutated."
       (when (and (not (member relative
                               (gethash project projectile-projects-cache)))
                  (projectile--watch-keep-file-p project relative))
+        (push relative projectile--watch-added-files)
         (puthash project
                  (cons relative (gethash project projectile-projects-cache))
                  projectile-projects-cache)

test/projectile-watch-test.el

@@ -585,6 +585,110 @@ watch bookkeeping into each other, caching is on (transient) and
                     :to-have-same-items-as '("src/a.el" "src/b.el"))
             (projectile--unwatch-project root)))))))
 
+(describe "VCS-ignored files under a watch"
+  (defun projectile-watch-test--git (&rest args)
+    (apply #'call-process "git" nil nil nil args))
+
+  (describe "projectile--vcs-ignored-subset"
+    (it "answers for a whole list in one call"
+      (projectile-test-with-sandbox
+        (projectile-test-with-files ("project/" "project/build/" "project/main.el")
+          (let* ((root (projectile-test-project-root))
+                 (default-directory root))
+            (write-region "build/\n*.log\n" nil ".gitignore")
+            (projectile-watch-test--git "init" "-q")
+            (spy-on 'projectile-project-vcs :and-return-value 'git)
+            (expect (projectile--vcs-ignored-subset
+                     root '("main.el" "build/out.o" "debug.log" ".gitignore"))
+                    :to-have-same-items-as '("build/out.o" "debug.log"))))))
+
+    (it "reads an all-clear as an answer, not a failure"
+      ;; `git check-ignore' exits 1 when nothing matches.
+      (projectile-test-with-sandbox
+        (projectile-test-with-files ("project/" "project/main.el")
+          (let* ((root (projectile-test-project-root))
+                 (default-directory root))
+            (projectile-watch-test--git "init" "-q")
+            (spy-on 'projectile-project-vcs :and-return-value 'git)
+            (expect (projectile--vcs-ignored-subset root '("main.el")) :to-be nil)))))
+
+    (it "declines for a VCS it cannot ask"
+      (spy-on 'projectile-project-vcs :and-return-value 'hg)
+      (spy-on 'call-process-region)
+      (expect (projectile--vcs-ignored-subset "/proj/" '("a.el")) :to-be nil)
+      (expect 'call-process-region :not :to-have-been-called))
+
+    (it "asks nothing when the batch added nothing"
+      (spy-on 'call-process-region)
+      (expect (projectile--vcs-ignored-subset "/proj/" nil) :to-be nil)
+      (expect 'call-process-region :not :to-have-been-called)))
+
+  (describe "projectile--watch-drop-vcs-ignored"
+    (it "removes the ignored additions from the cache"
+      (let ((projectile-indexing-method 'alien)
+            (projectile-projects-cache (make-hash-table :test 'equal)))
+        (spy-on 'projectile--vcs-ignored-subset
+                :and-return-value '("build/out.o"))
+        (puthash "/proj/" '("main.el" "build/out.o") projectile-projects-cache)
+        (expect (projectile--watch-drop-vcs-ignored "/proj/" '("build/out.o"))
+                :to-be-truthy)
+        (expect (gethash "/proj/" projectile-projects-cache)
+                :to-equal '("main.el"))))
+
+    (it "leaves native indexing alone, which lists ignored files anyway"
+      (let ((projectile-indexing-method 'native)
+            (projectile-projects-cache (make-hash-table :test 'equal)))
+        (spy-on 'projectile--vcs-ignored-subset)
+        (puthash "/proj/" '("build/out.o") projectile-projects-cache)
+        (expect (projectile--watch-drop-vcs-ignored "/proj/" '("build/out.o"))
+                :to-be nil)
+        (expect 'projectile--vcs-ignored-subset :not :to-have-been-called)
+        (expect (gethash "/proj/" projectile-projects-cache)
+                :to-equal '("build/out.o"))))
+
+    (it "reports no change when nothing was ignored"
+      (let ((projectile-indexing-method 'alien)
+            (projectile-projects-cache (make-hash-table :test 'equal)))
+        (spy-on 'projectile--vcs-ignored-subset :and-return-value nil)
+        (puthash "/proj/" '("main.el") projectile-projects-cache)
+        (expect (projectile--watch-drop-vcs-ignored "/proj/" '("main.el"))
+                :to-be nil)))))
+
+(describe "file watch integration with a gitignore"
+  (it "does not let a watch cache a file the VCS ignores"
+    (assume (bound-and-true-p file-notify--library)
+            "no file notification backend available")
+    (projectile-test-with-sandbox
+      (projectile-test-with-files ("project/src/a.el")
+        (projectile-watch-test-env
+          (let* ((root (file-name-as-directory
+                        (file-truename (expand-file-name "project"))))
+                 (default-directory root)
+                 (projectile-indexing-method 'alien))
+            ;; Both new files land in `src/', the one directory the initial
+            ;; cache causes to be watched - creating one somewhere unwatched
+            ;; would prove nothing, since no event would fire for it.
+            (write-region "*.log\n" nil (expand-file-name ".gitignore" root))
+            (call-process "git" nil nil nil "init" "-q")
+            (spy-on 'projectile-project-root :and-return-value root)
+            (spy-on 'projectile-project-vcs :and-return-value 'git)
+            (projectile-cache-project root '("src/a.el"))
+            (with-temp-file (expand-file-name "src/debug.log" root))
+            (with-temp-file (expand-file-name "src/b.el" root))
+            (let ((deadline (+ (float-time) 10)))
+              (while (and (not (member "src/b.el"
+                                       (gethash root projectile-projects-cache)))
+                          (< (float-time) deadline))
+                (read-event nil nil 0.1)))
+            (let ((cached (gethash root projectile-projects-cache)))
+              ;; The watch may have been dropped for reasons of its own (the
+              ;; backend refusing, the limit) - that invalidates the cache and
+              ;; says so, and there is nothing for this spec to assert then.
+              (when cached
+                (expect cached :to-contain "src/b.el")
+                (expect cached :not :to-contain "src/debug.log")))
+            (projectile--unwatch-project root)))))))
+
 (provide 'projectile-watch-test)
 
 ;;; projectile-watch-test.el ends here