bug#73320: [PATCH] project--vc-list-files: use Git's sparse-index

[Sean Allred <allred.sean@gmail.com>]

Dmitry Gutov <dmitry@gutov.dev> writes:
>>> Yeah, I expect project-find-regexp, project-search,
>>> project-query-replace-regexp might start misbehaving without
>>> additional filtering -- either throwing up errors or, best case,
>>> continuing to search through the "hidden" directories.
>> Not sure how best to track that we should come back to this, but
>> yeah.
>> It seems like the right place to add some sort of switch would be in the
>> `project-files` defmethod. From here, it looks like all the functions
>> you mention could choose the behavior right for them. (Based on the
>> function names alone -- it seems they would /also/ be interested in
>> operating on only those files which exist on disk.)
>
> I think we can just remove the names ending with '/'. The built-in
> commands don't seem to error out on them right now - probably because
> there is some protection against nonexistent files - but those files
> are (were) still shown as completions for project-find-file. Try out
> this addition please. The performance here seems about the same even
> with a large list (something I was worried about):
>
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index b29d5ed5404..a2e3f3f52e6 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -663,7 +663,7 @@ project--vc-list-files
>    (pcase backend
>      (`Git
>       (let* ((default-directory (expand-file-name
>       (file-name-as-directory dir)))
> -            (args '("-z"))
> +            (args '("-z" "--sparse"))
>              (vc-git-use-literal-pathspecs nil)
>              (include-untracked (project--value-in-dir
>                                  'project-vc-include-untracked
> @@ -703,7 +703,8 @@ project--vc-list-files
>               (delq nil
>                     (mapcar
>                      (lambda (file)
> -                      (unless (member file submodules)
> +                      (unless (or (member file submodules)
> +                                  (eq ?/ (aref file (1- (length file)))))
>                          (if project-files-relative-names
>                              file
>                            (concat default-directory file))))

Works fine for me :-) Though I've added an additional version check
inlined below.

>> Incidentally looking at the version check within `project-files`, it's
>> worthwhile to point out that `--sparse` is likely /not/ compatible with
>> ancient versions of Git. [...]
>
> [...]
>
> We can call vc-git--program-version the same way it's used in
> vc-git-state. Which version should we make the minimum?

The `--sparse` option was introduced in 2.35. The following seems to
work well for me:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index b29d5ed5404..873bc92729d 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -663,7 +663,8 @@ project--vc-list-files
   (pcase backend
     (`Git
      (let* ((default-directory (expand-file-name (file-name-as-directory dir)))
-            (args '("-z"))
+            (args `("-z" ,@(when (version<= "2.35" (vc-git--program-version))
+                             '("--sparse"))))
             (vc-git-use-literal-pathspecs nil)
             (include-untracked (project--value-in-dir
                                 'project-vc-include-untracked
@@ -703,7 +704,8 @@ project--vc-list-files
              (delq nil
                    (mapcar
                     (lambda (file)
-                      (unless (member file submodules)
+                      (unless (or (member file submodules)
+                                  (eq ?/ (aref file (1- (length file)))))
                         (if project-files-relative-names
                             file
                           (concat default-directory file))))

Since we're getting a bit busy with our conditions, though, it might be
better to start using `cond`:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 873bc92729d..b42415154e3 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -704,11 +704,11 @@ project--vc-list-files
              (delq nil
                    (mapcar
                     (lambda (file)
-                      (unless (or (member file submodules)
-                                  (eq ?/ (aref file (1- (length file)))))
-                        (if project-files-relative-names
-                            file
-                          (concat default-directory file))))
+                      (cond
+                       ((member file submodules) nil)
+                       ((eq ?/ (aref file (1- (length file)))) nil)
+                       (project-files-relative-names file)
+                       (t (concat default-directory file))))
                     (split-string
                      (with-output-to-string
                        (apply #'vc-git-command standard-output 0 nil "ls-files" args))

This seems to help readability -- at least to me. There's probably also
a nominal performance benefit since `cond` is a special form.

I've pushed this as branch `sa/sparse-index-2` to my repository. (This
is in addition to the `sa/sparse-index` branch, which contains the
`file-exists-p` check mentioned below plus what might be, I take it, an
ultimately unneeded opt-out parameter in `project-files`.)

It's worth noting that actually performing a `file-exists-p` check here
would have the added benefit of handling the awkward state between Git
2.25 (where sparse-checkout was introduced) and 2.35 (where git-ls-files
learned --sparse) where ls-files could still report things that _look_
like files but are not present. This would be fixed by just replacing
the (eq ..) form with (not (file-exists-p file)).

-- 
Sean Allred