bug#58525: 28.1: `vc-dir' (key sequence: C-x v d) fails when used with a CVS repository

[Mark Harig]

> > From: Dmitry Gutov <dgutov@yandex.ru>
> >
> > On 17.10.2022 09:06, Eli Zaretskii wrote:
> > > AFAIK, the VC's support for CVS is based on detecting the CVS
> > > subdirectory of a directory where you invoke vc-dir.  If that
> > > subdirectory is not found, VC will assume the backend is not
> > > CVS.  In which case your assumptions seem to be mistaken.
> > >
> > > But I will let VC expert to chime in here, because I may be
> > > wrong or confused.
> >
> > Here's the basic logic:
> >
> > (defun vc-cvs-registered (f)
> >    "Return non-nil if file F is registered with CVS."
> >    (when (file-readable-p (expand-file-name
> >               "CVS/Entries" (file-name-directory f)))
> 
> >      ...
> 
> Thanks.  So after performing the steps in the original report, I do
> have ~/tmp3/project1/CVS/Entries here.

The `vc-cvs-registered' function is not called before the error
is triggered.

`vc-dir' calls `vc-responsible-backend'.

`vc-responsible-backend' contains the following expression:

> (let ((dirs (delq nil
>                   (mapcar
>                    (lambda (backend)
>                      (when-let ((dir (vc-call-backend
>                                       backend 'responsible-p file)))
>                        (cons backend dir)))
>                    vc-handled-backends))))
>   ;; Just a single response (or none); use it.
>   (if (< (length dirs) 2)
>       (caar dirs)
>     ;; Several roots; we seem to have one vc inside another's
>     ;; directory.  Choose the most specific.
>     (caar (sort dirs (lambda (d1 d2)
>                        (< (length (cdr d2)) (length (cdr d1))))))))

The value of `dirs' is set by the following expression:

> (dirs (delq nil
>             (mapcar
>              (lambda (backend)
>                (when-let ((dir (vc-call-backend
>                                 backend 'responsible-p file)))
>                  (cons backend dir)))
>              vc-handled-backends)))

On my system, the `mapcar' expression is returning the following
value:

> (nil (CVS . t) nil nil nil nil (Git . "~/") nil)

which is surprising and incorrect.

The `delq' expression then reduces this to:

> ((CVS . t) (Git . "~/"))

So, for some reason, the VC elisp code thinks that in
~/tmp3/project1, there is both a CVS and a Git VC backend
controlling the files that were checked out of the CVS
repository.

Because the Git cons contains "~/", I checked my home directory
for files named ".git" and found a directory created two years
ago.  I renamed that directory and re-ran `C-x v d'.  The
original error reported disappeared and `vc-dir' listed the CVS
status and files as expected.  I restored the directory in ~/ to
its original name (~/.git) and the error returned.

The problem appears to be a result of the function `vc-find-root'
finding the "~/.git" directory.  As its doc string says:

>   "Find the root of a checked out project.
> The function walks up the directory tree from FILE looking for
> WITNESS.  If WITNESS if not found, return nil, otherwise return
> the root."

So, after finding the CVS backend, the `mapcar' expression, above,
continues and checks for a Git backend, which it finds in the directory
that contains tmp3/project1, namely, ~, the home directory.  Because
it found ~/.git/, it sets the variable `dirs' to an erroneous value and
later logic fails because of this.

What is the solution to this problem?  What should the VC
functions (not just `vc-dir') do when they find more than one VC
backend indicator in the directory tree?  Should it issue an
error indicating more than one VC backend detected, (since files
cannot be under the control of multiple VC backends)?  Or, should
it stop after the "most local" VC backend is found and attempt to
use that?

The current behavior (issuing an obscure error message that gives
the user no clue as the what is causing the problem) should be
corrected.

(End.)