bug#43464: 28.0.50; vc: Error calling vc-revert for repo root

[Dmitry Gutov <dgutov@yandex.ru>]

[-- Attachment #1: Type: text/plain, Size: 1986 bytes --]

On 07.10.2020 16:16, Andrii Kolomoiets wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 05.10.2020 09:02, Andrii Kolomoiets wrote:
>>> Dmitry Gutov <dgutov@yandex.ru> writes:
>>>
>>>> Where is that vc-backend called from, in our scenario?
>>> It's called from 'vc-call'.  The 'vc-revert-file' used it twice
>>> to call the 'make-version-backups-p' and the 'revert' backend functions.
>>
>> Then a change in vc-revert-file could be sufficient.
> 
> Can you please advice me what this change should look like?  Get rid of
> calling 'vc-call'?

Yes. How about the attached patch?

> In this case the function 'vc-version-backup-file'
> must be changed as well.

Does it actually make sense to use it on a directory?

>>>> Could we make do with changing that code to use vc-responsible-backend
>>>> instead of vc-backend instead? If it's not a function called
>>>> frequently.
>>> I went a little different way and made the 'vc-backend' return
>>> correct
>>> backend for directories.  In case you missed it somehow here is the link
>>> to the message:
>>> https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-09/msg02508.html
>>
>> Like I explained, it will create a cache entry that is never invalidated.
> 
> In what way VC backend for directory could be changed? Like 'rm -rf .hg
> && git init'?  We can make the 'vc-backend' function to ignore cached
> backend for directories.  Though I think it's not efficient.

Something like that. Or 'git init' inside a subdirectory. Not a frequent 
occurrence, but if we start using directories and files interchangeably 
in more places, we are likely to start caching other properties on them, 
too. So it's better to use a different function to detect which backend 
a directory belongs to.

Also, your patch makes vc-registered work on directories. What does it 
mean to have a directory "registered"? It's not a well-defined notion, 
given that most of contemporary VC systems don't track directories, only 
the files inside them.

[-- Attachment #2: vc-revert-dirs.diff --]
[-- Type: text/x-patch, Size: 1129 bytes --]

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 3852a64550..8efd9fd560 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2788,14 +2788,21 @@ vc-version-backup-file
 	  backup-file)))))
 
 (defun vc-revert-file (file)
-  "Revert FILE back to the repository working revision it was based on."
+  "Revert FILE back to the repository working revision it was based on.
+FILE can be a plain file or a directory, and in the latter case
+all files inside that directory will be reverted."
   (with-vc-properties
    (list file)
-   (let ((backup-file (vc-version-backup-file file)))
+   (let* ((dir (file-directory-p file))
+          (backup-file (unless dir (vc-version-backup-file file))))
      (when backup-file
        (copy-file backup-file file 'ok-if-already-exists)
        (vc-delete-automatic-version-backups file))
-     (vc-call revert file backup-file))
+     (vc-call-backend
+      (if dir
+          (vc-backend file)
+        (vc-responsible-backend file))
+      'revert file backup-file))
    `((vc-state . up-to-date)
      (vc-checkout-time . ,(file-attribute-modification-time
 			   (file-attributes file)))))