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

[Dmitry Gutov]

On 19/09/2024 07:25, Sean Allred wrote:

> > 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.

Great, thanks!

> > > 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))))

Like Eli suggested, we can use directory-name-p instead of the second 
condition.

> 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 think I'd rather keep the 'unless' for now: it groups the first two 
cases a bit more clearly.

> 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`.)

Yeah, it doesn't seem to be useful to return files that don't exist in 
the current work directory - no idea what a caller would do with them.

> 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)).

'file-exists-p' does I/O, so it would make processing an order of a 
magnitude slower. Here's a benchmark you can try:

  (benchmark-run 1 (project-files (project-current)))