* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
@ 2020-09-17 7:29 Andrii Kolomoiets
2020-09-17 16:10 ` Lars Ingebrigtsen
2020-09-18 9:00 ` Dmitry Gutov
0 siblings, 2 replies; 17+ messages in thread
From: Andrii Kolomoiets @ 2020-09-17 7:29 UTC (permalink / raw)
To: 43464
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-17 7:29 bug#43464: 28.0.50; vc: Error calling vc-revert for repo root Andrii Kolomoiets
@ 2020-09-17 16:10 ` Lars Ingebrigtsen
2020-09-18 9:00 ` Dmitry Gutov
1 sibling, 0 replies; 17+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-17 16:10 UTC (permalink / raw)
To: Andrii Kolomoiets; +Cc: 43464
Andrii Kolomoiets <andreyk.mad@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
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-17 7:29 bug#43464: 28.0.50; vc: Error calling vc-revert for repo root Andrii Kolomoiets
2020-09-17 16:10 ` Lars Ingebrigtsen
@ 2020-09-18 9:00 ` Dmitry Gutov
2020-09-18 9:30 ` Andrii Kolomoiets
2020-09-18 13:18 ` Lars Ingebrigtsen
1 sibling, 2 replies; 17+ messages in thread
From: Dmitry Gutov @ 2020-09-18 9:00 UTC (permalink / raw)
To: Andrii Kolomoiets, 43464
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-18 9:00 ` Dmitry Gutov
@ 2020-09-18 9:30 ` Andrii Kolomoiets
2020-09-18 13:30 ` Dmitry Gutov
2020-09-18 13:18 ` Lars Ingebrigtsen
1 sibling, 1 reply; 17+ messages in thread
From: Andrii Kolomoiets @ 2020-09-18 9:30 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 43464
Dmitry Gutov <dgutov@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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-18 9:00 ` Dmitry Gutov
2020-09-18 9:30 ` Andrii Kolomoiets
@ 2020-09-18 13:18 ` Lars Ingebrigtsen
1 sibling, 0 replies; 17+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-18 13:18 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 43464, Andrii Kolomoiets
Dmitry Gutov <dgutov@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
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-18 9:30 ` Andrii Kolomoiets
@ 2020-09-18 13:30 ` Dmitry Gutov
2020-09-18 15:45 ` Andrii Kolomoiets
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2020-09-18 13:30 UTC (permalink / raw)
To: Andrii Kolomoiets; +Cc: 43464
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-18 13:30 ` Dmitry Gutov
@ 2020-09-18 15:45 ` Andrii Kolomoiets
2020-09-22 19:48 ` Dmitry Gutov
0 siblings, 1 reply; 17+ messages in thread
From: Andrii Kolomoiets @ 2020-09-18 15:45 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 43464
[-- Attachment #1: Type: text/plain, Size: 709 bytes --]
Dmitry Gutov <dgutov@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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vc-Treat-directory-as-registered.patch --]
[-- Type: text/x-patch, Size: 1705 bytes --]
From 47adbf50d6d3719a9fe9e350897b30917bc1220d Mon Sep 17 00:00:00 2001
From: Andrii Kolomoiets <andreyk.mad@gmail.com>
Date: Fri, 18 Sep 2020 18:38:19 +0300
Subject: [PATCH] vc: Treat directory as registered
* lisp/vc/vc-hg.el (vc-hg-registered): Treat directory as registered
* lisp/vc/vc.el (vc-register): Treat directory as unregistered
---
lisp/vc/vc-hg.el | 5 +++--
lisp/vc/vc.el | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index cb0657e70a..fec2235446 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -222,8 +222,9 @@ vc-hg-update-on-retrieve-tag
(defun vc-hg-registered (file)
"Return non-nil if FILE is registered with hg."
(when (vc-hg-root file) ; short cut
- (let ((state (vc-hg-state file))) ; expensive
- (and state (not (memq state '(ignored unregistered)))))))
+ (or (file-directory-p file)
+ (let ((state (vc-hg-state file))) ; expensive
+ (and state (not (memq state '(ignored unregistered))))))))
(defun vc-hg-state (file)
"Hg-specific version of `vc-state'."
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 3852a64550..77ba97fb73 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1370,7 +1370,8 @@ vc-register
(let ((bname (get-file-buffer fname)))
(unless fname
(setq fname buffer-file-name))
- (when (vc-call-backend backend 'registered fname)
+ (when (and (not (file-directory-p fname))
+ (vc-call-backend backend 'registered fname))
(error "This file is already registered"))
;; Watch out for new buffers of size 0: the corresponding file
;; does not exist yet, even though buffer-modified-p is nil.
--
2.15.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-18 15:45 ` Andrii Kolomoiets
@ 2020-09-22 19:48 ` Dmitry Gutov
2020-09-24 7:15 ` Andrii Kolomoiets
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2020-09-22 19:48 UTC (permalink / raw)
To: Andrii Kolomoiets; +Cc: 43464
On 18.09.2020 18:45, Andrii Kolomoiets wrote:
> Dmitry Gutov <dgutov@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?
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-22 19:48 ` Dmitry Gutov
@ 2020-09-24 7:15 ` Andrii Kolomoiets
2020-09-30 9:13 ` Andrii Kolomoiets
2020-10-04 22:32 ` Dmitry Gutov
0 siblings, 2 replies; 17+ messages in thread
From: Andrii Kolomoiets @ 2020-09-24 7:15 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 43464
Dmitry Gutov <dgutov@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
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-24 7:15 ` Andrii Kolomoiets
@ 2020-09-30 9:13 ` Andrii Kolomoiets
2020-10-04 22:32 ` Dmitry Gutov
1 sibling, 0 replies; 17+ messages in thread
From: Andrii Kolomoiets @ 2020-09-30 9:13 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 43464
[-- Attachment #1: Type: text/plain, Size: 378 bytes --]
Andrii Kolomoiets <andreyk.mad@gmail.com> writes:
> Dmitry Gutov <dgutov@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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vc-backend-for-directory.patch --]
[-- Type: text/x-patch, Size: 1865 bytes --]
From 8b097942a3fbaf5b6d27fa61b2d3e5357d79beb8 Mon Sep 17 00:00:00 2001
From: Andrii Kolomoiets <andreyk.mad@gmail.com>
Date: Wed, 30 Sep 2020 12:02:21 +0300
Subject: [PATCH] vc-backend for directory
* lisp/vc/vc-hooks.el (vc-registered): Use 'responsible-p' vc backend function
to find backend for directories.
(vc-backend): Don't depend on 'vc-registered' result because it nil for
directories; always read 'vc-backend' property.
---
lisp/vc/vc-hooks.el | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index f09ceddcb3..b20e2e0b00 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -336,12 +336,15 @@ vc-registered
;; There is no file name handler.
;; Try vc-BACKEND-registered for each handled BACKEND.
(catch 'found
- (let ((backend (vc-file-getprop file 'vc-backend)))
+ (let ((backend (vc-file-getprop file 'vc-backend))
+ (fn-result (if (file-directory-p file)
+ '(responsible-p)
+ '(registered . t))))
(mapc
(lambda (b)
- (and (vc-call-backend b 'registered file)
+ (and (vc-call-backend b (car fn-result) file)
(vc-file-setprop file 'vc-backend b)
- (throw 'found t)))
+ (throw 'found (cdr fn-result))))
(if (or (not backend) (eq backend 'none))
vc-handled-backends
(cons backend vc-handled-backends))))
@@ -361,9 +364,8 @@ vc-backend
(cond ((eq property 'none) nil)
(property)
;; vc-registered sets the vc-backend property
- (t (if (vc-registered file-or-list)
- (vc-file-getprop file-or-list 'vc-backend)
- nil)))))
+ (t (vc-registered file-or-list)
+ (vc-file-getprop file-or-list 'vc-backend)))))
((and file-or-list (listp file-or-list))
(vc-backend (car file-or-list)))
(t
--
2.15.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-09-24 7:15 ` Andrii Kolomoiets
2020-09-30 9:13 ` Andrii Kolomoiets
@ 2020-10-04 22:32 ` Dmitry Gutov
2020-10-05 6:02 ` Andrii Kolomoiets
1 sibling, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2020-10-04 22:32 UTC (permalink / raw)
To: Andrii Kolomoiets; +Cc: 43464
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-10-04 22:32 ` Dmitry Gutov
@ 2020-10-05 6:02 ` Andrii Kolomoiets
2020-10-05 10:19 ` Dmitry Gutov
0 siblings, 1 reply; 17+ messages in thread
From: Andrii Kolomoiets @ 2020-10-05 6:02 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 43464
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.
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-10-05 6:02 ` Andrii Kolomoiets
@ 2020-10-05 10:19 ` Dmitry Gutov
2020-10-07 13:16 ` Andrii Kolomoiets
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2020-10-05 10:19 UTC (permalink / raw)
To: Andrii Kolomoiets; +Cc: 43464
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.
>> 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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-10-05 10:19 ` Dmitry Gutov
@ 2020-10-07 13:16 ` Andrii Kolomoiets
2020-10-07 22:47 ` Dmitry Gutov
0 siblings, 1 reply; 17+ messages in thread
From: Andrii Kolomoiets @ 2020-10-07 13:16 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 43464
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'? 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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-10-07 13:16 ` Andrii Kolomoiets
@ 2020-10-07 22:47 ` Dmitry Gutov
2020-10-11 20:28 ` Andrii Kolomoiets
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2020-10-07 22:47 UTC (permalink / raw)
To: Andrii Kolomoiets; +Cc: 43464
[-- 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)))))
^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-10-07 22:47 ` Dmitry Gutov
@ 2020-10-11 20:28 ` Andrii Kolomoiets
2020-10-13 11:59 ` Dmitry Gutov
0 siblings, 1 reply; 17+ messages in thread
From: Andrii Kolomoiets @ 2020-10-11 20:28 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 43464
Dmitry Gutov <dgutov@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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#43464: 28.0.50; vc: Error calling vc-revert for repo root
2020-10-11 20:28 ` Andrii Kolomoiets
@ 2020-10-13 11:59 ` Dmitry Gutov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Gutov @ 2020-10-13 11:59 UTC (permalink / raw)
To: Andrii Kolomoiets; +Cc: 43464
On 11.10.2020 23:28, Andrii Kolomoiets wrote:
> Dmitry Gutov <dgutov@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.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-10-13 11:59 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 7:29 bug#43464: 28.0.50; vc: Error calling vc-revert for repo root Andrii Kolomoiets
2020-09-17 16:10 ` Lars Ingebrigtsen
2020-09-18 9:00 ` Dmitry Gutov
2020-09-18 9:30 ` Andrii Kolomoiets
2020-09-18 13:30 ` Dmitry Gutov
2020-09-18 15:45 ` Andrii Kolomoiets
2020-09-22 19:48 ` Dmitry Gutov
2020-09-24 7:15 ` Andrii Kolomoiets
2020-09-30 9:13 ` Andrii Kolomoiets
2020-10-04 22:32 ` Dmitry Gutov
2020-10-05 6:02 ` Andrii Kolomoiets
2020-10-05 10:19 ` Dmitry Gutov
2020-10-07 13:16 ` Andrii Kolomoiets
2020-10-07 22:47 ` Dmitry Gutov
2020-10-11 20:28 ` Andrii Kolomoiets
2020-10-13 11:59 ` Dmitry Gutov
2020-09-18 13:18 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).