GNU bug report logs - #68174
30.0.50; Project and VC disagree about what repository we're in

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 68174 in the body.
You can then email your comments to 68174 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: Sean Whitton <spwhitton <at> spwhitton.name>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Project and VC disagree about what repository we're in
Date: Sun, 31 Dec 2023 13:26:06 +0000

X-debbugs-cc: juri <at> linkov.net

Hello,

1. emacs -q
2. (setopt vc-deduce-backend-nonvc-modes t)
3. C-x C-f ~/src/dotfiles/.emacs.d/init.el RET
4. C-x p p ~/src/emacs/ RET o C-x v L
5. (project-root (project-current)) => "~/src/emacs/"
6. C-x v +
   - git pull is attempted in ~/src/dotfiles/.

You're in a *vc-change-log* for emacs.git, and (project-current) returns
the correct project.  But C-x v + operates in ~/src/dotfiles/.

-- 
Sean Whitton

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Sean Whitton <spwhitton <at> spwhitton.name>, 68174 <at> debbugs.gnu.org
Cc: juri <at> linkov.net
Subject: Re: bug#68174: 30.0.50; Project and VC disagree about what repository
 we're in
Date: Tue, 2 Jan 2024 05:31:20 +0200

Hi!

On 31/12/2023 15:26, Sean Whitton wrote:
> X-debbugs-cc:juri <at> linkov.net
> 
> Hello,
> 
> 1. emacs -q
> 2. (setopt vc-deduce-backend-nonvc-modes t)
> 3. C-x C-f ~/src/dotfiles/.emacs.d/init.el RET
> 4. C-x p p ~/src/emacs/  RET o C-x v L
> 5. (project-root (project-current)) => "~/src/emacs/"
> 6. C-x v +
>     - git pull is attempted in ~/src/dotfiles/.
> 
> You're in a*vc-change-log*  for emacs.git, and (project-current) returns
> the correct project.  But C-x v + operates in ~/src/dotfiles/.

Thanks for the report.

I general, the problem looks unsolvable (or at least hard) because 
various VC-related buffers look back on the fileset that spawned them 
through the variable vc-parent-buffer. And that one is often just set to 
the buffer that was previous before the current one was created (in 
vc-setup-buffer).

This particular case though seems fixed with this patch, please test:

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 3cd835a9d6b..82051740b8f 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1074,6 +1074,7 @@ vc-expand-dirs

 (defvar vc-dir-backend)
 (defvar log-view-vc-backend)
+(defvar log-view-vc-fileset)
 (defvar log-edit-vc-backend)
 (defvar diff-vc-backend)
 (defvar diff-vc-revisions)
@@ -1155,6 +1156,8 @@ vc-deduce-fileset
 	      (vc-state buffer-file-name)
 	      (vc-checkout-model backend buffer-file-name))
 	(list backend (list buffer-file-name))))
+     ((derived-mode-p 'log-view-mode)
+      (list log-view-vc-backend log-view-vc-fileset))
      ((and (buffer-live-p vc-parent-buffer)
            ;; FIXME: Why this test?  --Stef
            (or (buffer-file-name vc-parent-buffer)

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 68174 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#68174: 30.0.50; Project and VC disagree about what
 repository we're in
Date: Tue, 02 Jan 2024 08:55:42 +0000

Hello,

On Tue 02 Jan 2024 at 05:31am +02, Dmitry Gutov wrote:

> I general, the problem looks unsolvable (or at least hard) because various
> VC-related buffers look back on the fileset that spawned them through the
> variable vc-parent-buffer. And that one is often just set to the buffer that
> was previous before the current one was created (in vc-setup-buffer).

Hmm.  This would be unfortunate.

> This particular case though seems fixed with this patch, please test:
>
> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> index 3cd835a9d6b..82051740b8f 100644
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
> @@ -1074,6 +1074,7 @@ vc-expand-dirs
>
>  (defvar vc-dir-backend)
>  (defvar log-view-vc-backend)
> +(defvar log-view-vc-fileset)
>  (defvar log-edit-vc-backend)
>  (defvar diff-vc-backend)
>  (defvar diff-vc-revisions)
> @@ -1155,6 +1156,8 @@ vc-deduce-fileset
>  	      (vc-state buffer-file-name)
>  	      (vc-checkout-model backend buffer-file-name))
>  	(list backend (list buffer-file-name))))
> +     ((derived-mode-p 'log-view-mode)
> +      (list log-view-vc-backend log-view-vc-fileset))
>       ((and (buffer-live-p vc-parent-buffer)
>             ;; FIXME: Why this test?  --Stef
>             (or (buffer-file-name vc-parent-buffer)

Yup, that works, thanks!

-- 
Sean Whitton

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Sean Whitton <spwhitton <at> spwhitton.name>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 68174 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#68174: 30.0.50; Project and VC disagree about what repository
 we're in
Date: Tue, 2 Jan 2024 15:10:53 +0200

On 02/01/2024 10:55, Sean Whitton wrote:
>> This particular case though seems fixed with this patch, please test:
>>
>> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
>> index 3cd835a9d6b..82051740b8f 100644
>> --- a/lisp/vc/vc.el
>> +++ b/lisp/vc/vc.el
>> @@ -1074,6 +1074,7 @@ vc-expand-dirs
>>
>>   (defvar vc-dir-backend)
>>   (defvar log-view-vc-backend)
>> +(defvar log-view-vc-fileset)
>>   (defvar log-edit-vc-backend)
>>   (defvar diff-vc-backend)
>>   (defvar diff-vc-revisions)
>> @@ -1155,6 +1156,8 @@ vc-deduce-fileset
>>   	      (vc-state buffer-file-name)
>>   	      (vc-checkout-model backend buffer-file-name))
>>   	(list backend (list buffer-file-name))))
>> +     ((derived-mode-p 'log-view-mode)
>> +      (list log-view-vc-backend log-view-vc-fileset))
>>        ((and (buffer-live-p vc-parent-buffer)
>>              ;; FIXME: Why this test?  --Stef
>>              (or (buffer-file-name vc-parent-buffer)
> Yup, that works, thanks!

Very good.

Eli, do you think we could add this to emacs-29 as well?

It would help the users of 29.2 avoid the described bug when/if they 
upgrade to the latest project.el. Probably not very urgent, but OTOH the 
change looks pretty safe.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: juri <at> linkov.net, 68174 <at> debbugs.gnu.org, spwhitton <at> spwhitton.name
Subject: Re: bug#68174: 30.0.50; Project and VC disagree about what repository
 we're in
Date: Tue, 02 Jan 2024 15:28:59 +0200

> Date: Tue, 2 Jan 2024 15:10:53 +0200
> Cc: 68174 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> On 02/01/2024 10:55, Sean Whitton wrote:
> >> This particular case though seems fixed with this patch, please test:
> >>
> >> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> >> index 3cd835a9d6b..82051740b8f 100644
> >> --- a/lisp/vc/vc.el
> >> +++ b/lisp/vc/vc.el
> >> @@ -1074,6 +1074,7 @@ vc-expand-dirs
> >>
> >>   (defvar vc-dir-backend)
> >>   (defvar log-view-vc-backend)
> >> +(defvar log-view-vc-fileset)
> >>   (defvar log-edit-vc-backend)
> >>   (defvar diff-vc-backend)
> >>   (defvar diff-vc-revisions)
> >> @@ -1155,6 +1156,8 @@ vc-deduce-fileset
> >>   	      (vc-state buffer-file-name)
> >>   	      (vc-checkout-model backend buffer-file-name))
> >>   	(list backend (list buffer-file-name))))
> >> +     ((derived-mode-p 'log-view-mode)
> >> +      (list log-view-vc-backend log-view-vc-fileset))
> >>        ((and (buffer-live-p vc-parent-buffer)
> >>              ;; FIXME: Why this test?  --Stef
> >>              (or (buffer-file-name vc-parent-buffer)
> > Yup, that works, thanks!
> 
> Very good.
> 
> Eli, do you think we could add this to emacs-29 as well?
> 
> It would help the users of 29.2 avoid the described bug when/if they 
> upgrade to the latest project.el. Probably not very urgent, but OTOH the 
> change looks pretty safe.

If it's safe, you'd need to convince me.  Because just by looking at
it, I don't have a clear idea what else it could affect.  And the use
case is pretty rare, AFAICT.  So I tend to be happier with this on
master.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> linkov.net, 68174-done <at> debbugs.gnu.org, spwhitton <at> spwhitton.name
Subject: Re: bug#68174: 30.0.50; Project and VC disagree about what repository
 we're in
Date: Wed, 3 Jan 2024 02:13:07 +0200

Version: 30.1

On 02/01/2024 15:28, Eli Zaretskii wrote:
>> Date: Tue, 2 Jan 2024 15:10:53 +0200
>> Cc: 68174 <at> debbugs.gnu.org, juri <at> linkov.net
>> From: Dmitry Gutov <dmitry <at> gutov.dev>
>>
>> On 02/01/2024 10:55, Sean Whitton wrote:
>>>> This particular case though seems fixed with this patch, please test:
>>>>
>>>> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
>>>> index 3cd835a9d6b..82051740b8f 100644
>>>> --- a/lisp/vc/vc.el
>>>> +++ b/lisp/vc/vc.el
>>>> @@ -1074,6 +1074,7 @@ vc-expand-dirs
>>>>
>>>>    (defvar vc-dir-backend)
>>>>    (defvar log-view-vc-backend)
>>>> +(defvar log-view-vc-fileset)
>>>>    (defvar log-edit-vc-backend)
>>>>    (defvar diff-vc-backend)
>>>>    (defvar diff-vc-revisions)
>>>> @@ -1155,6 +1156,8 @@ vc-deduce-fileset
>>>>    	      (vc-state buffer-file-name)
>>>>    	      (vc-checkout-model backend buffer-file-name))
>>>>    	(list backend (list buffer-file-name))))
>>>> +     ((derived-mode-p 'log-view-mode)
>>>> +      (list log-view-vc-backend log-view-vc-fileset))
>>>>         ((and (buffer-live-p vc-parent-buffer)
>>>>               ;; FIXME: Why this test?  --Stef
>>>>               (or (buffer-file-name vc-parent-buffer)
>>> Yup, that works, thanks!
>>
>> Very good.
>>
>> Eli, do you think we could add this to emacs-29 as well?
>>
>> It would help the users of 29.2 avoid the described bug when/if they
>> upgrade to the latest project.el. Probably not very urgent, but OTOH the
>> change looks pretty safe.
> 
> If it's safe, you'd need to convince me.  Because just by looking at
> it, I don't have a clear idea what else it could affect.

It's not a trivial transformation, but it's semantically sound: 
log-view-mode saves -- or one might say caches -- the backend and the 
fileset in its own variables locally (referenced in the diff above). The 
commands in log-view.el (of which there are several) use those vars 
rather than look up vc-parent-buffer, so the values should be correct.

> And the use
> case is pretty rare, AFAICT.  So I tend to be happier with this on
> master.

Ok, I'll push it to master for now.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: juri <at> linkov.net, 68174 <at> debbugs.gnu.org, spwhitton <at> spwhitton.name
Subject: Re: bug#68174: 30.0.50; Project and VC disagree about what repository
 we're in
Date: Wed, 03 Jan 2024 14:41:03 +0200

> Date: Wed, 3 Jan 2024 02:13:07 +0200
> Cc: spwhitton <at> spwhitton.name, 68174-done <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> > If it's safe, you'd need to convince me.  Because just by looking at
> > it, I don't have a clear idea what else it could affect.
> 
> It's not a trivial transformation, but it's semantically sound: 
> log-view-mode saves -- or one might say caches -- the backend and the 
> fileset in its own variables locally (referenced in the diff above). The 
> commands in log-view.el (of which there are several) use those vars 
> rather than look up vc-parent-buffer, so the values should be correct.

Thanks.  Let's see if there are some use cases which actually benefit
from an ad-hoc decision at that place instead of using the stashed
values.

Previous Next

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