GNU bug report logs - #70593
30.0.50; Dired: buffers of renamed dirs are broken

Previous Next

To reply to this bug, email your comments to 70593 AT debbugs.gnu.org.

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

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Dired: buffers of renamed dirs are broken
Date: Fri, 26 Apr 2024 13:59:16 +0200

Hello,

I'm really, really happy that I finally nailed down this not
insignificant dired problem:

When you revert a dired buffer whose default-directory has been renamed,
the buffer is erased and an error message is displayed.  Marks and stuff
are permanently lost.  This even happens when the directory had been
renamed with means of dired.

It follows a recipe for emacs -Q; feel free to skip to the following
analysis of the problem... (we care about this case, but the code is
broken - see below)

[ Recipe:

I have two dired buffers, one showing "~", the other one a subdirectory
"~/test" in the other window.  You can put some trash files into that
directory and mark some of them.

Now I select the "~" dired buffer and enable wdired.  I rename "test"
to, say, "test1", and confirm.

Now select the other dired buffer.  The directory name in the first line
has even been updated.  But when I hit g, the buffer is erased and an
error is raised:

| insert-directory: Setting current directory: No such file or directory,
| /home/micha/test2/

Recipe end ]

The problem is that we partially fail to update the affected buffers.

The code wants to do this in `dired-rename-subdir-1':

    (if (equal dir default-directory)
	;; if top level directory was renamed, lots of things have to be
	;; updated:

DIR is the old directory name.  But the `equality' test fails simply
because default-directory is "~/test/" but DIR has the tilde expanded.


TIA,

Michael.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 70593 <at> debbugs.gnu.org
Subject: Re: bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken
Date: Fri, 26 Apr 2024 17:55:52 +0300

> Date: Fri, 26 Apr 2024 13:59:16 +0200
> From:  Michael Heerdegen via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> The problem is that we partially fail to update the affected buffers.
> 
> The code wants to do this in `dired-rename-subdir-1':
> 
>     (if (equal dir default-directory)
> 	;; if top level directory was renamed, lots of things have to be
> 	;; updated:
> 
> DIR is the old directory name.  But the `equality' test fails simply
> because default-directory is "~/test/" but DIR has the tilde expanded.

So all we need to do is call expand-file-name on both of them?  Or do
we need also to call file-truename?

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70593 <at> debbugs.gnu.org
Subject: Re: bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken
Date: Mon, 29 Apr 2024 16:42:18 +0200

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

Eli Zaretskii <eliz <at> gnu.org> writes:

> > The code wants to do this in `dired-rename-subdir-1':
> >
> >     (if (equal dir default-directory)
> > 	;; if top level directory was renamed, lots of things have to be
> > 	;; updated:
> >
> > DIR is the old directory name.  But the `equality' test fails simply
> > because default-directory is "~/test/" but DIR has the tilde expanded.
>
> So all we need to do is call expand-file-name on both of them?  Or do
> we need also to call file-truename?

A sign of life with intermediate results:

Turns out this is a difficult and not even the only question (see
below).  It looks to me like this branch of the code (after the always
failing test) was never in use, or only a very long time ago.  Because
that unreachable branch is broken in several ways.

I needed to add a `file-name-as-directory' wrapper, else dired wouldn't
recognize the new directory as a directory and treats the file name as a
wildcard instead.

I would also add `abbreviate-file-name' calls so that the format of the
file names is like before (abbreviated).  Finally I don't understand why
the existing code computes `default-directory' and `dired-directory'
differently, so I reduced that to one computation for now, but also see
below for more.

This took me to this first actually working version for the
simplest case:

[0001-WIP-Try-to-fix-70593.patch (text/x-diff, inline)]

From aa6146e69d8d9d97424ccaf4d69db9eea64d0011 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen <at> web.de>
Date: Mon, 29 Apr 2024 16:10:29 +0200
Subject: [PATCH] WIP: Try to fix #70593

---
 lisp/dired-aux.el | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index e340f98a551..60a0f16bfe4 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -2331,18 +2331,21 @@ dired-rename-subdir-1
       (if (dired-in-this-tree-p (car elt) expanded-dir)
 	  ;; ELT's subdir is affected by the rename
 	  (dired-rename-subdir-2 elt dir to)))
-    (if (equal dir default-directory)
+    (if (string= dir (expand-file-name default-directory))
 	;; if top level directory was renamed, lots of things have to be
 	;; updated:
 	(progn
 	  (dired-unadvertise dir)	; we no longer dired DIR...
-	  (setq default-directory to
-		dired-directory (expand-file-name;; this is correct
-				 ;; with and without wildcards
-				 (file-name-nondirectory (if (stringp dired-directory)
-                                                             dired-directory
-                                                           (car dired-directory)))
-				 to))
+	  (setq dired-directory
+                (abbreviate-file-name
+                 (file-name-as-directory
+                  (expand-file-name ;; this is correct
+		   ;; with and without wildcards
+		   (file-name-nondirectory (if (stringp dired-directory)
+                                               dired-directory
+                                             (car dired-directory)))
+		   to)))
+                default-directory dired-directory)
 	  (let ((new-name (file-name-nondirectory
 			   (directory-file-name (if (stringp dired-directory)
                                                     dired-directory
--
2.39.2

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

Open questions:

(1) What do we need to do we do when `dired-directory' is a cons
(i.e. dir along with a file list)?  The existing code just threw away
the file list (that's why I unified the cases for now - the code didn't
make sense to me).

(2) What to do when symlinks are involved?  This question is also not
trivial.  It might be necessary to handle different cases and change
more than one part of the code.

Just calling `file-truename' before comparing as you (Eli) suggested
might fix more valid cases but will also resolve symlinks as side
effect: the dired buffer will afterwards visit a directory behind a
(probably unrelated) symlink when the visited directory name had not
been unresolved before.  This is not what we want.  Hope it's
understandable what I mean.

Another tricky case is when the user accidentally broke a symlink
and a dired buffer we handle visits a directory with a name that can't
be resolved any more.


Michael.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70593 <at> debbugs.gnu.org
Subject: Re: bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken
Date: Thu, 09 May 2024 15:46:18 +0200

Hello,

I'm CC'ing Drew.  Drew, do you happen to have any experience with this
case case (in general)?


In the meantime I've though about this particular question:

> (1) What do we need to do we do when `dired-directory' is a cons
> (i.e. dir along with a file list)?  The existing code just threw away
> the file list (that's why I unified the cases for now - the code didn't
> make sense to me).

I continue thinking like that.  But what should we do instead?  The file
list can contain anything - relative or absolute names.  And there can
be names affected by the renaming operation in the list even when the
`dired-directory' is not affected.  Would we want to exchange affected
file names in the list silently, or leave them as they are?


Michael.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
 of text editors" <bug-gnu-emacs <at> gnu.org>
Cc: 70593 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken
Date: Thu, 09 May 2024 16:05:11 +0200

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

Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
of text editors" <bug-gnu-emacs <at> gnu.org> writes:

> > (1) What do we need to do we do when `dired-directory' is a cons
> > (i.e. dir along with a file list)?

Here is an update of the former patch doing the minimum in that case:
update only the car of a consp `dired-directory' when the car is
affected by the renaming, and leave the rest untouched:

[0001-WIP-Try-to-fix-70593.patch (text/x-diff, inline)]

From 30d1546d30a75f5ff52a7aad10063c49a5e76789 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen <at> web.de>
Date: Mon, 29 Apr 2024 16:10:29 +0200
Subject: [PATCH] WIP: Try to fix #70593

---
 lisp/dired-aux.el | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index e340f98a551..9c7cd8aa1f4 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -2331,18 +2331,25 @@ dired-rename-subdir-1
       (if (dired-in-this-tree-p (car elt) expanded-dir)
 	  ;; ELT's subdir is affected by the rename
 	  (dired-rename-subdir-2 elt dir to)))
-    (if (equal dir default-directory)
+    (if (string= dir (expand-file-name default-directory))
 	;; if top level directory was renamed, lots of things have to be
 	;; updated:
 	(progn
 	  (dired-unadvertise dir)	; we no longer dired DIR...
-	  (setq default-directory to
-		dired-directory (expand-file-name;; this is correct
-				 ;; with and without wildcards
-				 (file-name-nondirectory (if (stringp dired-directory)
-                                                             dired-directory
-                                                           (car dired-directory)))
-				 to))
+	  (let* ((dired-dir (if (stringp dired-directory)
+                                dired-directory
+                              (car dired-directory)))
+                 (dired-dir-new
+                  (abbreviate-file-name
+                   (file-name-as-directory
+                    (expand-file-name ;; this is correct
+		     ;; with and without wildcards
+		     (file-name-nondirectory dired-dir)
+		     to)))))
+            (if (stringp dired-directory)
+                (setq dired-directory dired-dir-new)
+              (setcar dired-directory dired-dir-new))
+            (setq default-directory dired-dir-new))
 	  (let ((new-name (file-name-nondirectory
 			   (directory-file-name (if (stringp dired-directory)
                                                     dired-directory
--
2.39.2

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

Michael.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>, Eli Zaretskii <eliz <at> gnu.org>
Cc: "70593 <at> debbugs.gnu.org" <70593 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#70593: 30.0.50; Dired: buffers of renamed dirs
 are broken
Date: Thu, 9 May 2024 16:30:18 +0000

> I'm CC'ing Drew.  Drew, do you happen to have any experience with this
> case case (in general)?

Thanks for cc'ing me.

No, I have no particular experience with the
`dired-rename-subdir' code, other than using it in
typical ways.

In general, the support for a cons value of
`dired-directory' is not 100% wonderful.  I'm not
surprised when some uses of `dired-directory' don't
really handle the cons case or don't handle it well.

IMO the cons case is an important Dired feature,
but it's not taken into account well everywhere.

This is maybe partly the result of it being ignored
when changes have been made over the years to some
of the underlying basic functions.  IOW, use of a
cons `dired-directory' might not get tested after
some basic-level code change.

A Dired listing of arbitrary files (the cons case)
can be complicated, especially when it comes to
reverting the listing and in the face of changes
such as renamings.  Some of the basic plumbing
functions can be involved, and that's where some
of the complication lies.

Dunno whether it helps or would waste your time,
but in dired+.el I redefined some if the basic
functions to support a cons `dired-directory'.
E.g., you might look at `dired-buffers-for-dir'.
This is the comment for it in dired+.el:

;; REPLACE ORIGINAL in `dired.el'.
;;
;; 1. Allow for consp `dired-directory' too.
;; 2. Updated for Emacs 28: Added optional arg SUBDIRS-P.
;; 3. Fix for Emacs bug #52395.  Expand DIR with
;;    `default-directory', so `file-name-as-directory'
;;    gets applied to absolute name.  Otherwise,
;;    (dired-buffers-for-dir "~/some/dir/") returns nil,
;;    because the element in `dired-subdir-alist' is
;;    ("/the/home/dir/some/dir/" . #<buffer dir>), not
;;    ("~/some/dir/" . #<buffer dir>).

> In the meantime I've though about this particular question:
> 
> > (1) What do we need to do we do when `dired-directory' is a cons
> > (i.e. dir along with a file list)?  The existing code just threw away
> > the file list (that's why I unified the cases for now - the code didn't
> > make sense to me).

`dired-directory' is a buffer-local variable.
I don't think there's any support for subdir
listings that are of arbitrary files (i.e., cons
`dired-directory').  I don't recall, but I think
that's the case - in which case it shouldn't
matter that such a list of arbitrary files is
discarded here.  But it should be checked first.
And maybe there's a need for an error message
somewhere if things aren't as expected (?).

However, the explicit list of files can of course
include (arbitrary) directory names.  And in the
Dired buffer you can use `i' to insert a "subdir"
listing (which really means you can insert a
listing of any of the directories that are listed).

If you do that to insert a directory that's one of
the arbitrary files listed in `dired-directory',
then the files and dirs listed in that inserted
directory listing are not included in the
`dired-directory' cons value - it defines only the
main listing, not any inserted "subdir" listings.

So I think there's no problem with discarding the
explicit list of files in the case you raise.  But
I don't really know.

> I continue thinking like that.  But what should we do instead?  The file
> list can contain anything - relative or absolute names. 

Yes, the top-level listing (i.e., the value of
`dired-directory') can.

> And there can
> be names affected by the renaming operation in the list even when the
> `dired-directory' is not affected.  Would we want to exchange affected
> file names in the list silently, or leave them as they are?

Not sure I follow completely, but I think yes,
rename them in `dired-directory' also.  Maybe
if you showed some examples it would be clearer.

HTH somehow, though I doubt it.

Overall, I'd say that it's good to at least try
to test a bit with the cons `dired-directory'
case, whatever changes/fixes you might make.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Drew Adams via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: "70593 <at> debbugs.gnu.org" <70593 <at> debbugs.gnu.org>,
 Eli Zaretskii <eliz <at> gnu.org>, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken
Date: Thu, 09 May 2024 21:27:14 +0200

Drew Adams via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> `dired-directory' is a buffer-local variable.
> I don't think there's any support for subdir
> listings that are of arbitrary files (i.e., cons
> `dired-directory').  I don't recall, but I think
> that's the case - in which case it shouldn't
> matter that such a list of arbitrary files is
> discarded here.  But it should be checked first.

Not sure whether I can follow.

When I call dired with a cons DIRNAME, `dired-directory' will be bound
to that cons.  And it is consulted for reverting.

When I overwrite it with just `default-directory' and revert, the result
will be a "normal" dired buffer showing this directory - the explicit
listing is lost.

In the scenario of the bug this surely makes a difference.  My latest
patch seems to work as expected.  Any comment about the patch btw?


Subdir insertion in cons value `dired-directory' buffers are not really
supported currently it seems so I took this complication aside for now.
The code does handle the subdir alists, though.


Thanks for your comments so far Drew,

Michael.

Previous Next

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