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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43464 in the body.
You can then email your comments to 43464 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; vc: Error calling vc-revert for repo root
Date: Thu, 17 Sep 2020 10:29:08 +0300

1. Create repo with modified file:
   mkdir gittest
   cd gittest
   git init
   touch foo.txt
   git add .
   git commit -m "foo"
   echo "bar" > foo.txt
2. emacs -Q
3. C-x v d
4. C-x v u
5. Confirm discarding changes

Debugger entered--Lisp error: (file-missing "Cannot open load file" "No such file or directory" "vc-nil")
  require(vc-nil)
  vc-find-backend-function(nil make-version-backups-p)
  vc-call-backend(nil make-version-backups-p "/private/tmp/gittest/")
  vc-version-backup-file("/private/tmp/gittest/")
  vc-revert-file("/private/tmp/gittest/")
  vc-revert()
  funcall-interactively(vc-revert)
  call-interactively(vc-revert nil nil)
  command-execute(vc-revert)

At least for `hg` and `git` backends.  Maybe because `vc-registered` for
repo root is nil.

Same error in Emacs 26 and 27.

Message #8 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Thu, 17 Sep 2020 18:10:57 +0200

Andrii Kolomoiets <andreyk.mad <at> gmail.com> writes:

> 1. Create repo with modified file:
>    mkdir gittest
>    cd gittest
>    git init
>    touch foo.txt
>    git add .
>    git commit -m "foo"
>    echo "bar" > foo.txt
> 2. emacs -Q
> 3. C-x v d
> 4. C-x v u
> 5. Confirm discarding changes
>
> Debugger entered--Lisp error: (file-missing "Cannot open load file" "No such file or directory" "vc-nil")
>   require(vc-nil)

I get the same error...  but if I move point down to the foo.txt line,
then I get no error.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #13 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>, 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Fri, 18 Sep 2020 12:00:12 +0300

On 17.09.2020 10:29, Andrii Kolomoiets wrote:
> Debugger entered--Lisp error: (file-missing "Cannot open load file" "No such file or directory" "vc-nil")
>    require(vc-nil)
>    vc-find-backend-function(nil make-version-backups-p)
>    vc-call-backend(nil make-version-backups-p "/private/tmp/gittest/")
>    vc-version-backup-file("/private/tmp/gittest/")
>    vc-revert-file("/private/tmp/gittest/")
>    vc-revert()
>    funcall-interactively(vc-revert)
>    call-interactively(vc-revert nil nil)
>    command-execute(vc-revert)
> 
> At least for `hg` and `git` backends.  Maybe because `vc-registered` for
> repo root is nil.

I don't think we support vc-revrt on directories, repo root or not.

Message #16 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Fri, 18 Sep 2020 12:30:42 +0300

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 17.09.2020 10:29, Andrii Kolomoiets wrote:
>> Debugger entered--Lisp error: (file-missing "Cannot open load file" "No such file or directory" "vc-nil")
>>    require(vc-nil)
>>    vc-find-backend-function(nil make-version-backups-p)
>>    vc-call-backend(nil make-version-backups-p "/private/tmp/gittest/")
>>    vc-version-backup-file("/private/tmp/gittest/")
>>    vc-revert-file("/private/tmp/gittest/")
>>    vc-revert()
>>    funcall-interactively(vc-revert)
>>    call-interactively(vc-revert nil nil)
>>    command-execute(vc-revert)
>> At least for `hg` and `git` backends.  Maybe because `vc-registered`
>> for
>> repo root is nil.
>
> I don't think we support vc-revrt on directories, repo root or not.

Calling `vc-git-revert` or `vc-hg-revert` directly with repo root or any
directory within repo works fine.

Message #19 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464 <at> debbugs.gnu.org, Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Fri, 18 Sep 2020 15:18:07 +0200

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 17.09.2020 10:29, Andrii Kolomoiets wrote:
>> Debugger entered--Lisp error: (file-missing "Cannot open load file"
>> "No such file or directory" "vc-nil")
>>    require(vc-nil)
>>    vc-find-backend-function(nil make-version-backups-p)
>>    vc-call-backend(nil make-version-backups-p "/private/tmp/gittest/")
>>    vc-version-backup-file("/private/tmp/gittest/")
>>    vc-revert-file("/private/tmp/gittest/")
>>    vc-revert()
>>    funcall-interactively(vc-revert)
>>    call-interactively(vc-revert nil nil)
>>    command-execute(vc-revert)
>> At least for `hg` and `git` backends.  Maybe because `vc-registered`
>> for
>> repo root is nil.
>
> I don't think we support vc-revrt on directories, repo root or not.

Ah,  we basically get this error message whenever we type `C-x v u' in a
vc-dir buffer when not on a file (or there are no marked files)?

That seems like a suboptimal error message in this case, though.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #22 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Fri, 18 Sep 2020 16:30:05 +0300

On 18.09.2020 12:30, Andrii Kolomoiets wrote:
> Calling `vc-git-revert` or `vc-hg-revert` directly with repo root or any
> directory within repo works fine.

True. That's how Git and Hg work anyway.

I'm not saying the current situation is ideal, but we'd either have to 
give up any attempt to revert a directory (with a more appropriate 
message), or somehow differentiate between different backends where it 
would or wouldn't work.

Message #25 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Fri, 18 Sep 2020 18:45:08 +0300

[Message part 1 (text/plain, inline)]

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 18.09.2020 12:30, Andrii Kolomoiets wrote:
>> Calling `vc-git-revert` or `vc-hg-revert` directly with repo root or any
>> directory within repo works fine.
>
> True. That's how Git and Hg work anyway.
>
> I'm not saying the current situation is ideal, but we'd either have to
> give up any attempt to revert a directory (with a more appropriate
> message), or somehow differentiate between different backends where it
> would or wouldn't work.

BTW vc-revert is also works fine in Git repo when point is on
subdirectory.  So for the vc-git only reverting repo root is not
working.

Please see attached patch which make it possible for vc-hg to revert
directory.

[0001-vc-Treat-directory-as-registered.patch (text/x-patch, attachment)]

Message #28 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Tue, 22 Sep 2020 22:48:32 +0300

On 18.09.2020 18:45, Andrii Kolomoiets wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
> 
>> On 18.09.2020 12:30, Andrii Kolomoiets wrote:
>>> Calling `vc-git-revert` or `vc-hg-revert` directly with repo root or any
>>> directory within repo works fine.
>>
>> True. That's how Git and Hg work anyway.
>>
>> I'm not saying the current situation is ideal, but we'd either have to
>> give up any attempt to revert a directory (with a more appropriate
>> message), or somehow differentiate between different backends where it
>> would or wouldn't work.
> 
> BTW vc-revert is also works fine in Git repo when point is on
> subdirectory.  So for the vc-git only reverting repo root is not
> working.

That's an interesting observation.

> Please see attached patch which make it possible for vc-hg to revert
> directory.

Could you explain both changes in that patch?

And also: how does it change, or not change, the behavior of vc-revert 
in backends that are not Git or Hg?

Message #31 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Thu, 24 Sep 2020 10:15:21 +0300

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 18.09.2020 18:45, Andrii Kolomoiets wrote:
>> BTW vc-revert is also works fine in Git repo when point is on
>> subdirectory.  So for the vc-git only reverting repo root is not
>> working.
>
> That's an interesting observation.

And the most interest part of that observation is that I can't reproduce
it :)

>> Please see attached patch which make it possible for vc-hg to revert
>> directory.
>
> Could you explain both changes in that patch?

The idea is to make the 'vc-backend' function to return backend for
directory.  'vc-backend' function uses the 'vc-registered' function.
The change for vc-hg.el makes 'vc-hg-registered' return t for directory.
The change to vc.el makes the 'vc-register' function called on directory
to not error with "already registered" message.

Chances that the patch is completely wrong.  Perhaps `vc-backend` should
return correct backend for directory and the behavior of `vc-registered`
must be kept unchanged.

> And also: how does it change, or not change, the behavior of vc-revert
> in backends that are not Git or Hg?

Looks like everybody is ready for reverting dirs.

bzr, mtn, svn - Should be fine reverting directory
dav - do nothing on vc-revert
rcs - reverting directory is added in c22b0a7da32360e34f6f0ff86a886c9028b3d863
sccs - reverting directory is added in e7290559824406d111d306069b36dde8ced847f9
src - reverting directory is supported initially 1e81f6769013e1a0df9e10d7c5d0a3e3ca131143
cvs - passing directory to 'unedit' command should be fine

Message #34 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Wed, 30 Sep 2020 12:13:41 +0300

[Message part 1 (text/plain, inline)]

Andrii Kolomoiets <andreyk.mad <at> gmail.com> writes:

> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>> Could you explain both changes in that patch?
>
> The idea is to make the 'vc-backend' function to return backend for
> directory.
>
> `vc-backend` should return correct backend for directory and the
> behavior of `vc-registered` must be kept unchanged.

Please see attached patch.

[0001-vc-backend-for-directory.patch (text/x-patch, attachment)]

Message #37 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Mon, 5 Oct 2020 01:32:10 +0300

On 24.09.2020 10:15, Andrii Kolomoiets wrote:

>> On 18.09.2020 18:45, Andrii Kolomoiets wrote:
>>> BTW vc-revert is also works fine in Git repo when point is on
>>> subdirectory.  So for the vc-git only reverting repo root is not
>>> working.
>>
>> That's an interesting observation.
> 
> And the most interest part of that observation is that I can't reproduce
> it :)

Oh well. :-)

>>> Please see attached patch which make it possible for vc-hg to revert
>>> directory.
>>
>> Could you explain both changes in that patch?
> 
> The idea is to make the 'vc-backend' function to return backend for
> directory.  'vc-backend' function uses the 'vc-registered' function.
> The change for vc-hg.el makes 'vc-hg-registered' return t for directory.
> The change to vc.el makes the 'vc-register' function called on directory
> to not error with "already registered" message.

Thanks.

Where is that vc-backend called from, in our scenario?

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'm not 100% sure about the original design, but vc-responsible-backend 
*does* work on directories.

Its downside is it doesn't cache the result to VC properties (hence it 
would be unwise to use it everywhere). But it's also an upside, because 
said properties are invalidated in a few strategic places like 
find-file-hook, and that never happens for directories (hence that cache 
will tend to get out of date, sooner or later).

>> And also: how does it change, or not change, the behavior of vc-revert
>> in backends that are not Git or Hg?
> 
> Looks like everybody is ready for reverting dirs.
> 
> bzr, mtn, svn - Should be fine reverting directory
> dav - do nothing on vc-revert
> rcs - reverting directory is added in c22b0a7da32360e34f6f0ff86a886c9028b3d863
> sccs - reverting directory is added in e7290559824406d111d306069b36dde8ced847f9
> src - reverting directory is supported initially 1e81f6769013e1a0df9e10d7c5d0a3e3ca131143
> cvs - passing directory to 'unedit' command should be fine

Very good.

Message #40 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Mon, 05 Oct 2020 09:02:35 +0300

Dmitry Gutov <dgutov <at> 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.

> 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

Message #43 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Mon, 5 Oct 2020 13:19:19 +0300

On 05.10.2020 09:02, Andrii Kolomoiets wrote:
> Dmitry Gutov <dgutov <at> 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.

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

Message #46 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Wed, 07 Oct 2020 16:16:13 +0300

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 05.10.2020 09:02, Andrii Kolomoiets wrote:
>> Dmitry Gutov <dgutov <at> 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'?  In this case the function 'vc-version-backup-file'
must be changed as well.

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

Message #49 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Thu, 8 Oct 2020 01:47:29 +0300

[Message part 1 (text/plain, inline)]

On 07.10.2020 16:16, Andrii Kolomoiets wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
> 
>> On 05.10.2020 09:02, Andrii Kolomoiets wrote:
>>> Dmitry Gutov <dgutov <at> 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.

[vc-revert-dirs.diff (text/x-patch, attachment)]

Message #52 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Sun, 11 Oct 2020 23:28:08 +0300

Dmitry Gutov <dgutov <at> yandex.ru> writes:

>> Can you please advice me what this change should look like?  Get rid
>> of calling 'vc-call'?
>
> Yes. How about the attached patch?

Small fix: THEN and ELSE blocks of the '(if dir...' should be swapped.

Does those kind of changes should be applied to any function that uses
'vc-call' and can be called on dirs?

Is there any reason to use 'vc-backend' at all?
'vc-responsible-backend' will call 'vc-backend' on a file that is not a
directory.

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

Looks like it make sense for CVS backend.  Take a look at
'vc-cvs-stay-local-p'.

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

In this case `vc-call` must use that function, right?

> Also, your patch makes vc-registered work on directories.

How is that?  'vc-registered' is still returns nil for directories.  The
changes affects only the side effect of it.

Message #55 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 43464 <at> debbugs.gnu.org
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Tue, 13 Oct 2020 14:59:38 +0300

On 11.10.2020 23:28, Andrii Kolomoiets wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
> 
>>> Can you please advice me what this change should look like?  Get rid
>>> of calling 'vc-call'?
>>
>> Yes. How about the attached patch?
> 
> Small fix: THEN and ELSE blocks of the '(if dir...' should be swapped.

Ah yes, thanks for noticing.

> Does those kind of changes should be applied to any function that uses
> 'vc-call' and can be called on dirs?

I think so. Since none of them should work on directories now, it should 
be accompanied with some doc changes as well.

> Is there any reason to use 'vc-backend' at all?
>
> 'vc-responsible-backend' will call 'vc-backend' on a file that is not a
> directory.

Well, vc-backend is certain to be faster, on average. That is one 
advantage. Minus one disk loopup, or minus extra network interaction 
with Tramp (that's where it might really hurt).

I don't have a very strong argument here, except the current code works, 
and it should be easier to annotate the exceptions where we do want to 
handle directories. And while we do that, consider the performance 
implications for each case.

>>> 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?
> 
> Looks like it make sense for CVS backend.  Take a look at
> 'vc-cvs-stay-local-p'.

It might be desired for CVS (to cut down on network traffic), but how 
would it work? The function is supposed to return a backup file name. 
But we don't create backup files for whole directories. Only for 
individual files.

>> 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.
> 
> In this case `vc-call` must use that function, right?

No, vc-call won't be used. Like in the patch I sent previously.

>> Also, your patch makes vc-registered work on directories.
> 
> How is that?  'vc-registered' is still returns nil for directories.  The
> changes affects only the side effect of it.

Oh, now I finally understand what it's doing.

You can probably see how it's not ideal control flow (call a function, 
see it return nil, and then rely on its undocumented side-effect).

So if we can avoid doing that and still fix the bug, the alternative 
approach should be preferable.

Message #62 received at 43464-done <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 43464-done <at> debbugs.gnu.org, Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Mon, 17 Feb 2025 15:42:13 +0800

Hello,

On Tue 13 Oct 2020 at 02:59pm +03, Dmitry Gutov wrote:

> On 11.10.2020 23:28, Andrii Kolomoiets wrote:
>
>> Does those kind of changes should be applied to any function that uses
>> 'vc-call' and can be called on dirs?
>
> I think so. Since none of them should work on directories now, it should be
> accompanied with some doc changes as well.

I went through all uses of vc-call in emacs.git and found that the only
two which would make sense to apply to directories are vc-rename-file
and vc-revert-file.  I've added a FIXME to the former.

>>> Also, your patch makes vc-registered work on directories.
>> How is that?  'vc-registered' is still returns nil for directories.  The
>> changes affects only the side effect of it.
>
> Oh, now I finally understand what it's doing.
>
> You can probably see how it's not ideal control flow (call a function, see it
> return nil, and then rely on its undocumented side-effect).
>
> So if we can avoid doing that and still fix the bug, the alternative approach
> should be preferable.

I'm not sure what you meant here, because Andrii's patch still has the
problem of creating cache entries for directories that will never be
invalidated.  That's more significant than the control flow issue.

I've now installed a change to fix this bug based on Dmitry's approach.

-- 
Sean Whitton

Message #70 received at 43464 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: 43464 <at> debbugs.gnu.org, spwhitton <at> spwhitton.name, andreyk.mad <at> gmail.com
Subject: Re: bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
Date: Mon, 17 Feb 2025 22:13:06 +0200

On 17/02/2025 09:42, Sean Whitton wrote:
>> On 11.10.2020 23:28, Andrii Kolomoiets wrote:
>>
>>> Does those kind of changes should be applied to any function that uses
>>> 'vc-call' and can be called on dirs?
>> I think so. Since none of them should work on directories now, it should be
>> accompanied with some doc changes as well.
> I went through all uses of vc-call in emacs.git and found that the only
> two which would make sense to apply to directories are vc-rename-file
> and vc-revert-file.  I've added a FIXME to the former.
> 
>>>> Also, your patch makes vc-registered work on directories.
>>> How is that?  'vc-registered' is still returns nil for directories.  The
>>> changes affects only the side effect of it.
>> Oh, now I finally understand what it's doing.
>>
>> You can probably see how it's not ideal control flow (call a function, see it
>> return nil, and then rely on its undocumented side-effect).
>>
>> So if we can avoid doing that and still fix the bug, the alternative approach
>> should be preferable.
> I'm not sure what you meant here, because Andrii's patch still has the
> problem of creating cache entries for directories that will never be
> invalidated.  That's more significant than the control flow issue.
> 
> I've now installed a change to fix this bug based on Dmitry's approach.

Thanks! Looking good.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.