GNU bug report logs - #77306
31.0.50; C-x v L from *vc-change-log* buffer leaves wrong default-directory

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 77306 in the body.
You can then email your comments to 77306 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: 31.0.50; C-x v L from *vc-change-log* buffer leaves wrong
 default-directory
Date: Thu, 27 Mar 2025 19:13:40 +0800

X-debbugs-cc: dmitry <at> gutov.dev, monnier <at> iro.umontreal.ca

A similar problem to #53626 crops up with C-x v L, too.

1. emacs -q
2. Visit a version controlled file.
3. C-x v L
4. Without switching buffer, C-x p p ~/some/other/repo RET o C-x v L
5. RET to toggle display of the log message.

Result, in the case of git, is "fatal: bad revision" because
default-directory is still the first VC repository's root.

This problem is caused by the let binding of default-directory
established by vc-print-root-log.  vc-setup-buffer wants to set
default-directory in the *vc-change-log* buffer to the new repository's
root, but ends up just adjusting the let binding's value.

The same thing probably occurs with C-x v D.

Could we reformulate the command definitions so that they didn't need to
bind default-directory?  Should vc-setup-buffer use the same
run-with-timer trick implemented in xref--ensure-default-directory?

CCing Stefan since this is a dynamic binding issue.

-- 
Sean Whitton

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: 77306-done <at> debbugs.gnu.org
Cc: dmitry <at> gutov.dev, monnier <at> iro.umontreal.ca
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Thu, 03 Apr 2025 16:35:25 +0800

Version: 31.1

Hello,

No-one has had any better ideas yet here so I've installed another
run-with-timer workaround, with an explanatory comment.
Hopefully we can improve on this, but the bug is worth fixing now
-- I run into it daily, somehow.

-- 
Sean Whitton

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Sean Whitton <spwhitton <at> spwhitton.name>, 77306 <at> debbugs.gnu.org
Cc: monnier <at> iro.umontreal.ca
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Thu, 3 Apr 2025 15:14:28 +0300

On 27/03/2025 13:13, Sean Whitton wrote:
> This problem is caused by the let binding of default-directory
> established by vc-print-root-log.  vc-setup-buffer wants to set
> default-directory in the*vc-change-log* buffer to the new repository's
> root, but ends up just adjusting the let binding's value.
> 
> The same thing probably occurs with C-x v D.
> 
> Could we reformulate the command definitions so that they didn't need to
> bind default-directory?  Should vc-setup-buffer use the same
> run-with-timer trick implemented in xref--ensure-default-directory?

Yeah, I don't have a better suggestion.

The key here seem to be that the buffer already exists and is selected 
when the command is invoked. Perhaps we could recreate the buffer 
instead of reusing it? But then we'd also need to put it in the same 
window, fixup the buffer list to use the same position, and etc...

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

From: Robert Pluim <rpluim <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 77306 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Thu, 03 Apr 2025 14:58:24 +0200

>>>>> On Thu, 3 Apr 2025 15:14:28 +0300, Dmitry Gutov <dmitry <at> gutov.dev> said:

    Dmitry> On 27/03/2025 13:13, Sean Whitton wrote:
    >> This problem is caused by the let binding of default-directory
    >> established by vc-print-root-log.  vc-setup-buffer wants to set
    >> default-directory in the*vc-change-log* buffer to the new repository's
    >> root, but ends up just adjusting the let binding's value.
    >> The same thing probably occurs with C-x v D.
    >> Could we reformulate the command definitions so that they didn't
    >> need to
    >> bind default-directory?  Should vc-setup-buffer use the same
    >> run-with-timer trick implemented in xref--ensure-default-directory?

    Dmitry> Yeah, I don't have a better suggestion.

Doe `setq-local' not adjust the buffer-local value? What am I missing?

Robert
-- 

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, 77306 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Thu, 03 Apr 2025 22:36:51 +0800

Hello,

On Thu 03 Apr 2025 at 02:58pm +02, Robert Pluim wrote:

> Does `setq-local' not adjust the buffer-local value? What am I missing?

Indeed it does not.

-- 
Sean Whitton

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

From: Robert Pluim <rpluim <at> gmail.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, 77306 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Thu, 03 Apr 2025 17:47:36 +0200

>>>>> On Thu, 03 Apr 2025 22:36:51 +0800, Sean Whitton <spwhitton <at> spwhitton.name> said:

    Sean> Hello,
    Sean> On Thu 03 Apr 2025 at 02:58pm +02, Robert Pluim wrote:

    >> Does `setq-local' not adjust the buffer-local value? What am I missing?

    Sean> Indeed it does not.

Youʼre right, but I donʼt understand why youʼre right, which means my
model of buffer-local variables is wrong. Oh well.

Robert
-- 

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, 77306 <at> debbugs.gnu.org,
 Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Thu, 03 Apr 2025 12:52:51 -0400

>     Dmitry> Yeah, I don't have a better suggestion.
> Doe `setq-local' not adjust the buffer-local value? What am I missing?

It does, but the `let` is also currently affecting just that
buffer-local value.  IOW, it's a `setq-local` within a `let-local`,
which thus behaves just like a normal `setq` within a normal `let`: the
effect of the `setq` disappears when we exit the `let`.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: dmitry <at> gutov.dev, 77306 <at> debbugs.gnu.org
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Thu, 03 Apr 2025 14:25:14 -0400

> Could we reformulate the command definitions so that they didn't need to
> bind default-directory?

We could do something like the patch below, I guess.


        Stefan


diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 2db2a017525..b0161270d4f 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -3095,7 +3095,6 @@ vc-print-root-log
     (t
      (list (when (> vc-log-show-limit 0) vc-log-show-limit)))))
   (let* ((backend (vc-deduce-backend))
-	 (default-directory default-directory)
 	 (with-diff (and (eq limit 1) revision))
 	 (vc-log-short-style (unless with-diff vc-log-short-style))
 	 rootdir)
@@ -3105,12 +3104,14 @@ vc-print-root-log
       (setq backend (vc-responsible-backend rootdir))
       (unless backend
         (error "Directory is not version controlled")))
-    (setq default-directory rootdir)
-    (vc-print-log-internal backend (list rootdir) revision revision limit
-                           (when with-diff 'with-diff))
-    ;; We're looking at the root, so displaying " from <some-file>" in
-    ;; the mode line isn't helpful.
-    (setq vc-parent-buffer-name nil)))
+    (with-temp-buffer
+      (setq default-directory rootdir)
+      (vc-print-log-internal backend (list rootdir) revision revision limit
+                             (when with-diff 'with-diff))
+      ;; We're looking at the root, so displaying " from <some-file>" in
+      ;; the mode line isn't helpful.
+      ;; FIXME: In this eval'd in the appropriate buffer?
+      (setq vc-parent-buffer-name nil))))
 
 ;;;###autoload
 (defun vc-print-branch-log (branch)

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: dmitry <at> gutov.dev, 77306 <at> debbugs.gnu.org
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Sat, 05 Apr 2025 11:03:37 +0800

Hello,

Avoiding this problem by using a temporary buffer makes sense to me,
though, the FIXME would need to be resolved first :)

-- 
Sean Whitton

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: dmitry <at> gutov.dev, 77306 <at> debbugs.gnu.org
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Fri, 04 Apr 2025 23:17:51 -0400

> Avoiding this problem by using a temporary buffer makes sense to me,
> though, the FIXME would need to be resolved first :)

FWIW, the FIXME applies to the current code as much as it does to the
patched code, I think.

IOW, my understanding is that this `setq` is meant to be executed in the
buffer in which the print-log has been inserted, but I'm not completely
sure that `vc-print-log-internal` will always switch to that buffer
before exiting, hence the question.


        Stefan

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: dmitry <at> gutov.dev, 77306 <at> debbugs.gnu.org
Subject: Re: bug#77306: 31.0.50; C-x v L from *vc-change-log* buffer leaves
 wrong default-directory
Date: Sat, 05 Apr 2025 15:47:49 +0800

Hello,

On Fri 04 Apr 2025 at 11:17pm -04, Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:

>> Avoiding this problem by using a temporary buffer makes sense to me,
>> though, the FIXME would need to be resolved first :)
>
> FWIW, the FIXME applies to the current code as much as it does to the
> patched code, I think.
>
> IOW, my understanding is that this `setq` is meant to be executed in the
> buffer in which the print-log has been inserted, but I'm not completely
> sure that `vc-print-log-internal` will always switch to that buffer
> before exiting, hence the question.

Okay.  I think I'd be inclined to leave things how they are for now.
If some other aberrant behaviour, it may then become clear what to do.

-- 
Sean Whitton

Previous Next

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