bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 4097 bytes --]

Hello,

This seems to be a regression from emacs 25.x.

If I have set delete-by-moving-to-trash to t AND trash-directory to a
custom value (there's a qualifier though, read further), I get this error,
but that does not trigger backtrace even after toggle-debug-on-error!

=====
Trashing...
(file-error Non-regular file Is a directory /home/kmodi/.emacs.d/foo)
=====

I was though able to force a backtrace (shown below) after trying to
recreate the same error after evaluating:

  (setq debug-on-message "Non-regular")

=====
Debugger entered--Lisp error: "(file-error Non-regular file Is a directory
/home/kmodi/.emacs.d/foo)\n"
  message("%s" "(file-error Non-regular file Is a directory
/home/kmodi/.emacs.d/foo)\n")
  dired-log-summary("1 of 1 deletion failed" ("/home/kmodi/.emacs.d/foo"))
  dired-internal-do-deletions((("/home/kmodi/.emacs.d/foo" . 2827)) nil t)
  dired-do-delete(nil)
  funcall-interactively(dired-do-delete nil)
  call-interactively(dired-do-delete nil nil)
  command-execute(dired-do-delete)
=====

Here is the recipe to recreate this in emacs -Q:

1. First evaluate the below

(progn
  (setq delete-by-moving-to-trash t)
  (setq trash-directory
        (let ((dir (concat temporary-file-directory
                           (getenv "USER") "/.trash_emacs/"))) ;Must end
with /
          (mkdir dir :parents)
          dir))
  (setq debug-on-message "Non-regular"))

2. Now evaluate

;; CASE A
(let ((dir (concat user-emacs-directory "foo/")))
  (mkdir dir :parents)
  (dired (concat dir "..")))

Now when you move the point to the foo dir, hit D and type yes, you will
get the above pasted backtrace.

Now here is the qualifier piece.. the same error does not happen if I try
to delete a directory in the temporary-file-directory!

So when I evaluate:

;; CASE B
(let ((dir (concat temporary-file-directory "foo/")))
  (mkdir dir :parents)
  (dired (concat dir "..")))

Then move the point to the foo dir, hit D and type yes, the deletion
(trashing) of that foo directory happens fine without that error.

So with this, CASE A gives error, CASE B does not.

Here's a twist..

Now let's move the trash directory from temporary-file-directory to
user-emacs-directory .. eval the below:

  (setq trash-directory
        (let ((dir (concat user-emacs-directory "/.trash_emacs/"))) ;Must
end with /
          (mkdir dir :parents)
          dir))

NOW, CASE A above will work fine, and CASE B will give that error.

My machine is RHEL 6.6. And the drives are NFS mounts. So I don't know
which of the below is true:

- Gives an error if the ORIG directory and TRASH directory are on possibly
different mounts.
- Gives an error if the ORIG and TRASH directory do not have the same root
directory (/home vs /tmp in the above example).

So that is the main bug report.

Side bug report on the fact that the below error does not generate a
backtrace with toggle-debug-on-error.. why is that?

(file-error Non-regular file Is a directory /home/kmodi/.emacs.d/foo)

(Thanks for reading through this whole bug report.. hopefully someone is
able to reproduce this issue and fix it.)


-----
In GNU Emacs 26.0.60 (build 26, x86_64-pc-linux-gnu, GTK+ Version 2.24.23)
 of 2017-10-11
Repository revision: 419a371f8b3d4ba200770be07136f909c7984ece
Windowing system distributor 'The X.Org Foundation', version 11.0.60900000
System Description: Red Hat Enterprise Linux Workstation release 6.6
(Santiago)

Configured using:
 'configure --with-modules
 --prefix=/home/kmodi/usr_local/apps/6/emacs/emacs-26
 '--program-transform-name=s/^ctags$/ctags_emacs/'
 'CPPFLAGS=-I/home/kmodi/usr_local/6/include -I/usr/include/freetype2
 -I/usr/include' 'CFLAGS=-ggdb3 -O0' 'CXXFLAGS=-ggdb3 -O0'
 'LDFLAGS=-L/home/kmodi/usr_local/6/lib -L/home/kmodi/usr_local/6/lib64
 -ggdb3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK2 X11 MODULES

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix


-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 6102 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

I'd like to add that if I leave the value of trash-directory to nil, then I
believe the default system trash directory is created somewhere in my /home.

Because then, based on my earlier email, CASE A (deleting directory in my
emacs user dir works fine, but CASE B (deleting directory in my /tmp) gives
that error.

So far, at least on my machine, it looks like if

- delete-by-moving-to-trash is t, AND
- trash-directory is in /tmp and to-be-deleted-dir in is HOME (or
vice-versa)

this error occurs.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1372 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 3156 bytes --]

On Wed, Oct 11, 2017 at 11:35 PM Kaushal Modi <kaushal.modi@gmail.com>
wrote:

> I'd like to add that if I leave the value of trash-directory to nil, then
> I believe the default system trash directory is created somewhere in my
> /home.
>
> Because then, based on my earlier email, CASE A (deleting directory in my
> emacs user dir works fine, but CASE B (deleting directory in my /tmp) gives
> that error.
>
> So far, at least on my machine, it looks like if
>
> - delete-by-moving-to-trash is t, AND
> - trash-directory is in /tmp and to-be-deleted-dir in is HOME (or
> vice-versa)
>
> this error occurs.
>

The situation unfolds further!

Turns out this has nothing to do with dired. So taking off Tino from the
To: list (sorry for doing that earlier).

Now I think that this probably has to do with the rename-file series of
commits (
http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-26&id=446e92548f932f18d57924573b49b5e6f4ae70c4
).
So copying Paul.

Here are even simpler recipes verified in emacs -Q:

1. First eval this one form to catch the error back trace
(setq debug-on-message "Non-regular")

2. Now eval the 3 forms below one by one.. the last one should fail as the
trash-directory and to-be-del directory do not share the same root?/mount?

;; WORKS
;; Both trash-directory and to-be-del dirs in temporary-file-directory
(let ((trash-directory (concat temporary-file-directory
                               (getenv "USER") "/.trash_emacs/"))
      (to-be-del (concat temporary-file-directory "foo/")))
  (mkdir trash-directory :parents)
  (mkdir to-be-del :parents)
  (move-file-to-trash to-be-del))

;; WORKS
;; Both trash-directory and to-be-del dirs in user-emacs-directory
(let ((trash-directory (concat user-emacs-directory "/.trash_emacs/"))
      (to-be-del (concat user-emacs-directory "foo/")))
  (mkdir trash-directory :parents)
  (mkdir to-be-del :parents)
  (move-file-to-trash to-be-del))

;; FAILS!!
;; trash-directory in temporary-file-directory but to-be-del dir in
;; user-emacs-directory
(let ((trash-directory (concat user-emacs-directory "/.trash_emacs/"))
      (to-be-del (concat temporary-file-directory "foo/")))
  (mkdir trash-directory :parents)
  (mkdir to-be-del :parents)
  (move-file-to-trash to-be-del))

When I eval this 3rd form, I get this error back trace:

Debugger entered--Lisp error: (file-error "Non-regular file" "Is a
directory" "/tmp/foo")
  rename-file("/tmp/foo" "/home/kmodi/.emacs.d/.trash_emacs/foo.~1~")
  move-file-to-trash("/tmp/foo/")
  (let ((trash-directory (concat user-emacs-directory "/.trash_emacs/"))
(to-be-del (concat temporary-file-directory "foo/"))) (mkdir
trash-directory :parents) (mkdir to-be-del :parents) (move-file-to-trash
to-be-del))
  eval((let ((trash-directory (concat user-emacs-directory
"/.trash_emacs/")) (to-be-del (concat temporary-file-directory "foo/")))
(mkdir trash-directory :parents) (mkdir to-be-del :parents)
(move-file-to-trash to-be-del)) nil)
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 5161 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Göktuğ Kayaalp]

Quite recently I've opened and retreated the similar bug#28791, trying
to trash directories recursively.  I did not use a custom
trash-directory, but I got the same error.  Turns out that it was
delete-by-moving-to-trash that provoked the error (with an indentical
message to yours).  Emptying the ~/.local/share/Trash/ folder solved the
issue for me.  I observed that in ~/.local/share/Trash/info/ some
.trashinfo files were created with the name of the directory (say
<dirname>) I was trying to delete, one <dirname>.trashinfo and many
<dirname><random bits>.trashinfo.  I've tried to understand
sth. inspecting the recent changes to rename-file in src/fileio.c using
vc-annotate, and some recent commits like a1cea94c0b seem relevant,
altho my C knowledge is very basic, and even more so of the Emacs style
and C codebase.  I've experienced this on master since about a couple
weeks, since my second-last build thereof, but my third-last build was
quite some time ago.

-- 
İ. Göktuğ Kayaalp	<http://www.gkayaalp.com/>
			024C 30DD 597D 142B 49AC
			40EB 465C D949 B101 2427

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Eli Zaretskii]

> From: Göktuğ Kayaalp <self@gkayaalp.com>
> Date: Thu, 12 Oct 2017 15:50:26 +0300
> Cc: Paul Eggert <eggert@cs.ucla.edu>, 28792@debbugs.gnu.org
> 
> Quite recently I've opened and retreated the similar bug#28791, trying
> to trash directories recursively.  I did not use a custom
> trash-directory, but I got the same error.  Turns out that it was
> delete-by-moving-to-trash that provoked the error (with an indentical
> message to yours).  Emptying the ~/.local/share/Trash/ folder solved the
> issue for me.  I observed that in ~/.local/share/Trash/info/ some
> .trashinfo files were created with the name of the directory (say
> <dirname>) I was trying to delete, one <dirname>.trashinfo and many
> <dirname><random bits>.trashinfo.

This use case raises an interesting question: what should be the
behavior of delete-by-moving-to-trash when the Trash directory already
includes a directory by the same name as the non-directory file being
deleted?  Are files in the Trash directory generally unimportant
enough to disregard these situations, or does this use case run afoul
of the ability to restore the trashed files later?

I don't know the answers, as I intentionally avoid using the system
trash.

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

On Thu, Oct 12, 2017 at 8:58 AM Eli Zaretskii <eliz@gnu.org> wrote:

> This use case raises an interesting question: what should be the
> behavior of delete-by-moving-to-trash when the Trash directory already
> includes a directory by the same name as the non-directory file being
> deleted?  Are files in the Trash directory generally unimportant
> enough to disregard these situations, or does this use case run afoul
> of the ability to restore the trashed files later?
>

The fact that the user deleted the files means that the files were not
important. If the user deleted them by mistake, then the trash serves as a
last-resort to restore the files from. Trash is not a "backup".. so unlike
the Emacs backup, there shouldn't be a need to store multiple revisions of
trash.

IMO, if a file or a directory exists by the same name in trash, the
move-file-to-trash should just overwrite that.. if a foo file already
exists and a foo/ dir is being trashed, then just delete the old trashed
foo file and replace with the newly trashed foo/ dir.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1481 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Göktuğ Kayaalp]

On 2017-10-12 15:58 +03, Eli Zaretskii <eliz@gnu.org> wrote:
> This use case raises an interesting question: what should be the
> behavior of delete-by-moving-to-trash when the Trash directory already
> includes a directory by the same name as the non-directory file being
> deleted?  Are files in the Trash directory generally unimportant
> enough to disregard these situations, or does this use case run afoul
> of the ability to restore the trashed files later?
>
> I don't know the answers, as I intentionally avoid using the system
> trash.
>

The Freedesktop spec [1] says (under "Contents of a trash directory"):

    A trash directory contains two subdirectories, named info and files.
    
    The $trash/files directory contains the files and directories that
    were trashed. When a file or directory is trashed, it MUST be moved
    into this directory5 . The names of files in this directory are to
    be determined by the implementation; the only limitation is that
    they must be unique within the directory. _Even if a file with the
    same name and location gets trashed many times, each subsequent
    trashing must not overwrite a previous copy (a)_. The access rights,
    access time, modification time and extended attributes (if any) for
    a file/directory in $trash/files SHOULD be the same as the
    file/directory had before getting trashed.
    
    IMPORTANT NOTE. While an implementation may choose to base filenames
    in the $trash/files directory on the original filenames, this is
    never to be taken for granted6. A filename in the $trash/files
    directory MUST NEVER be used to recover the original filename; use
    the info file (see below) for that. (If an info file corresponding
    to a file/directory in $trash/files is not available, this is an
    emergency case, and MUST be clearly presented as such to the user or
    to the system administrator).
    
    The $trash/info directory contains an “information file” for every
    file and directory in $trash/files. This file MUST have exactly the
    same name as the file or directory in $trash/files, plus the
    extension “.trashinfo”7.

It seems that the file name under <trash dir>/files/ is not important
and only an identifier, used to match the corresponding
<file name>.trashinfo file, which contains the path the file originally
was.  Thus, it should be possible to have that <file name> be a random
string with a sensible prefix (the name of the deleted file), allowing
to delete files at identical full-paths without trouble.  IMO we can
never know how important the files under the Trash/files directory might
or might not be, so it would be better to err on the safe side.

My trash can looks like this:

    /home/g/.local/share/Trash
    ├── files
    │   └── testdir
    └── info
        └── testdir.trashinfo

And info/testdir.trashinfo like this:

    [Trash Info]
    Path=/home/g/testdir
    DeletionDate=2017-10-12T15:01:27

I beleive the feature is useful, I myself do quite a bit of mistaken
deletings, and even though most of the more important stuff is version
controlled, things happen..

[1] https://specifications.freedesktop.org/trash-spec/trashspec-latest.html

-- 
İ. Göktuğ Kayaalp	<http://www.gkayaalp.com/>
			024C 30DD 597D 142B 49AC
			40EB 465C D949 B101 2427

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

On Thu, Oct 12, 2017 at 8:50 AM Göktuğ Kayaalp <self@gkayaalp.com> wrote:

> Quite recently I've opened and retreated the similar bug#28791, trying
> to trash directories recursively.  I did not use a custom
> trash-directory, but I got the same error.  Turns out that it was
> delete-by-moving-to-trash that provoked the error (with an indentical
> message to yours).  Emptying the ~/.local/share/Trash/ folder solved the
> issue for me.  I observed that in ~/.local/share/Trash/info/ some
> .trashinfo files were created with the name of the directory (say
> <dirname>) I was trying to delete, one <dirname>.trashinfo and many
> <dirname><random bits>.trashinfo.
>

That is still a bug in Emacs, at least because the error message is not
helpful. It is also in a non-standard format with incorrect capitalizations
in-between and doesn't even generate a backtrace by default.

Also this issue did not happen on emacs 25.x.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1400 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Thu, 12 Oct 2017 12:58:57 +0000
> Cc: Paul Eggert <eggert@cs.ucla.edu>, 28792@debbugs.gnu.org
> 
> That is still a bug in Emacs, at least because the error message is not helpful. It is also in a non-standard
> format with incorrect capitalizations in-between and doesn't even generate a backtrace by default.

FWIW, I see no problem with the error message, we produce such error
messages in quite a few other places in fileio.c.  The capitalization
seems okay, too.

As for lack of backtrace, that's because the error comes from a C
primitive.

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On Thu, Oct 12, 2017 at 9:16 AM Eli Zaretskii <eliz@gnu.org> wrote:

> FWIW, I see no problem with the error message, we produce such error
> messages in quite a few other places in fileio.c.  The capitalization
> seems okay, too.
>

OK, but it seems non-standard compared to error messages from Elisp land.

Shouldn't

(file-error Non-regular file Is a directory /home/kmodi/.emacs.d/foo)

look like:

file-error: Non-regular file: /home/kmodi/.emacs.d/foo is a directory

- Why those parentheses?
- Why are "N" and "I" capitalized in-between that "sentence".. error
messages are usually sentences without ending in period, right?
- Above instead looks like a list printed with 3 elements.

Also, the error is not-informative.. the user is trying to trash foo/ and
knows that foo/ is a directory.. so how would

(file-error Non-regular file Is a directory /home/kmodi/.emacs.d/foo)

help?


> As for lack of backtrace, that's because the error comes from a C
> primitive.
>

For the lack of better understanding, isn't it possible to set the
"severity" of certain messages to be of Error status (or something like
that), so that a backtrace automatically analogous to what I get on my
above-mentioned debug-on-message setting?
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2043 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Thu, 12 Oct 2017 13:31:49 +0000
> Cc: self@gkayaalp.com, eggert@cs.ucla.edu, 28792@debbugs.gnu.org
> 
>  FWIW, I see no problem with the error message, we produce such error
>  messages in quite a few other places in fileio.c.  The capitalization
>  seems okay, too.
> 
> OK, but it seems non-standard compared to error messages from Elisp land.

When the error comes from a primitive, and quotes a system error
message, it always looks like this.  E.g., try evaluating this:

  (insert-file-contents "no-such-file")

> Shouldn't
> 
> (file-error Non-regular file Is a directory /home/kmodi/.emacs.d/foo)
> 
> look like:
> 
> file-error: Non-regular file: /home/kmodi/.emacs.d/foo is a directory

No, we put the file last because there could be a list of additional
objects to print.

> - Why those parentheses?

It depends on how did you get the error and where did you pick it up
from: echo area, *Messages*, *scratch*, something else?

> - Why are "N" and "I" capitalized in-between that "sentence".. error messages are usually sentences without
> ending in period, right?

The "error message" in this case is concocted from several parts, each
one has its own capitalization.  We used to lower-case some of that,
but doing sop has its own problems, because the system error messages
can be localized.

> - Above instead looks like a list printed with 3 elements.

That's because it is.

> Also, the error is not-informative.. the user is trying to trash foo/ and knows that foo/ is a directory.. so how
> would
> 
> (file-error Non-regular file Is a directory /home/kmodi/.emacs.d/foo) 
> 
> help?

It tells you that /home/kmodi/.emacs.d/foo is a directory, not a
regular file.

I'm not saying that the message couldn't be improved, but doing so is
only possible in move-file-to-trash, not in primitives below it.

>  As for lack of backtrace, that's because the error comes from a C
>  primitive.
> 
> For the lack of better understanding, isn't it possible to set the "severity" of certain messages to be of Error
> status (or something like that), so that a backtrace automatically analogous to what I get on my
> above-mentioned debug-on-message setting?

What you see _is_ an error already.

Once again, I think this issue should be resolved so as not to fail
file deletion, but let's not muddy the waters by unrelated aspects,
such as the error message formatting, which works as designed here.
the real problem is elsewhere.

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Andreas Schwab]

On Okt 12 2017, Kaushal Modi <kaushal.modi@gmail.com> wrote:

> OK, but it seems non-standard compared to error messages from Elisp land.
>
> Shouldn't
>
> (file-error Non-regular file Is a directory /home/kmodi/.emacs.d/foo)
>
> look like:
>
> file-error: Non-regular file: /home/kmodi/.emacs.d/foo is a directory
>
> - Why those parentheses?
> - Why are "N" and "I" capitalized in-between that "sentence".. error
> messages are usually sentences without ending in period, right?
> - Above instead looks like a list printed with 3 elements.

No, it contains 4 elements:

- the error symbol (file-error)
- a descriptive string ("Non-regular file")
- the system error as a string ("Is a directory")
- the offending object ("/home/kmodi/.emacs.d/foo")

> Also, the error is not-informative.. the user is trying to trash foo/ and
> knows that foo/ is a directory.. so how would

copy-file does not support copying non-regular files.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

On Thu, Oct 12, 2017 at 10:02 AM Andreas Schwab <schwab@suse.de> wrote:

> No, it contains 4 elements:
>
> - the error symbol (file-error)
> - a descriptive string ("Non-regular file")
> - the system error as a string ("Is a directory")
> - the offending object ("/home/kmodi/.emacs.d/foo")
>

Thanks.


> > Also, the error is not-informative.. the user is trying to trash foo/ and
> > knows that foo/ is a directory.. so how would
>
> copy-file does not support copying non-regular files.
>

But it seems to work in some cases i.e. trashing a dir works*. See my
recipe in my earlier message in this debbugs thread[1]. Somehow this error
has a correlation with relative locations of the user-set trash dir and
to-be-deleted dir..

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28792#11
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1484 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

Hello all,

I have created a new debbugs to discuss and resolve the issue with the
error message formatting and it not generating a backtrace:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28797

For the issue pertaining to this bug, the last relevant updates were:

- https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28792#11
- https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28792#44

Let's continue from there... why does that error show up on trashing (i.e.
moving, not deleting) a directory, dependent on the value of
trash-directory and the location of the directory I am trying to trash?

Sorry for taking discussion off-topic.

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1367 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Thu, 12 Oct 2017 15:07:35 +0000
> Cc: self@gkayaalp.com, Noam Postavsky <npostavs@users.sourceforge.net>, 
> 	Drew Adams <drew.adams@oracle.com>
> 
> why does that error show up on trashing (i.e. moving, not deleting) a directory,
> dependent on the value of trash-directory and the location of the directory I am trying to trash?

Because one directory already includes a file/directory of the same
names as the one you are trying to trash, whereas the other one does
not.

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Andreas Schwab]

On Okt 12 2017, Kaushal Modi <kaushal.modi@gmail.com> wrote:

> But it seems to work in some cases i.e. trashing a dir works*. See my
> recipe in my earlier message in this debbugs thread[1]. Somehow this error
> has a correlation with relative locations of the user-set trash dir and
> to-be-deleted dir..

That depend on whether the trash is on the same file system as the
directory to trash.  rename-file now refuses to copy directory across
file systems unless the target name ends in a slash.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

On Thu, Oct 12, 2017 at 11:27 AM Andreas Schwab <schwab@suse.de> wrote:

> On Okt 12 2017, Kaushal Modi <kaushal.modi@gmail.com> wrote:
>
> > But it seems to work in some cases i.e. trashing a dir works*. See my
> > recipe in my earlier message in this debbugs thread[1]. Somehow this
> error
> > has a correlation with relative locations of the user-set trash dir and
> > to-be-deleted dir..
>
> That depend on whether the trash is on the same file system as the
> directory to trash.  rename-file now refuses to copy directory across
> file systems unless the target name ends in a slash.
>

That means that as dired-do-delete is a wrapper at some level over
move-file-to-trash, that slash appending should be done in dired somewhere?
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1229 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On 10/12/2017 08:27 AM, Andreas Schwab wrote:
> rename-file now refuses to copy directory across
> file systems unless the target name ends in a slash.

Thanks for the diagnosis. This is an unintended consequence of the 
race-condition patch for rename-file that I installed last month. I 
installed the attached patch into the emacs-26 branch to fix the bug.


[-- Attachment #2: 0001-Let-rename-file-rename-dirs-across-filesystems.patch --]
[-- Type: text/x-patch, Size: 1901 bytes --]

From 21df0eec6d056c88c7ffbd5a1c0f76661702d5cf Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 12 Oct 2017 13:08:53 -0700
Subject: [PATCH] Let rename-file rename dirs across filesystems

Problem diagnosed by Andreas Schwab (Bug#28792#65).
This fixes a bug that I introduced in
2017-09-10T22:39:24@eggert@cs.ucla.edu
"Fix race with rename-file etc. with dir NEWNAME".
* src/fileio.c (Frename_file): Copy a source directory across
file system boundaries even if its name does not end in slash.
---
 src/fileio.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index 4bbcec6f17..e57bf46015 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2260,7 +2260,7 @@ This is what happens in interactive use with M-x.  */)
   (Lisp_Object file, Lisp_Object newname, Lisp_Object ok_if_already_exists)
 {
   Lisp_Object handler;
-  Lisp_Object encoded_file, encoded_newname, symlink_target;
+  Lisp_Object encoded_file, encoded_newname;
 
   file = Fexpand_file_name (file, Qnil);
 
@@ -2334,12 +2334,22 @@ This is what happens in interactive use with M-x.  */)
   if (rename_errno != EXDEV)
     report_file_errno ("Renaming", list2 (file, newname), rename_errno);
 
+  struct stat file_st;
   bool dirp = !NILP (Fdirectory_name_p (file));
+  if (!dirp)
+    {
+      if (lstat (SSDATA (encoded_file), &file_st) != 0)
+	report_file_error ("Renaming", list2 (file, newname));
+      dirp = S_ISDIR (file_st.st_mode) != 0;
+    }
   if (dirp)
     call4 (Qcopy_directory, file, newname, Qt, Qnil);
   else
     {
-      symlink_target = Ffile_symlink_p (file);
+      Lisp_Object symlink_target
+	= (S_ISLNK (file_st.st_mode)
+	   ? emacs_readlinkat (AT_FDCWD, SSDATA (encoded_file))
+	   : Qnil);
       if (!NILP (symlink_target))
 	Fmake_symbolic_link (symlink_target, newname, ok_if_already_exists);
       else
-- 
2.13.6

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

On Thu, Oct 12, 2017 at 4:25 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> On 10/12/2017 08:27 AM, Andreas Schwab wrote:
> > rename-file now refuses to copy directory across
> > file systems unless the target name ends in a slash.
>
> Thanks for the diagnosis. This is an unintended consequence of the
> race-condition patch for rename-file that I installed last month. I
> installed the attached patch into the emacs-26 branch to fix the bug.
>

Awesome! Thank you! I confirm the bug fix.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 905 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Paul Eggert]

Kaushal Modi wrote:
> I confirm the bug fix.

Thanks for checking; closing the bug report.

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 375 bytes --]

On Sun, Oct 15, 2017, 3:19 AM Paul Eggert

> Thanks for checking; closing the bug report.
>

Thanks. I had though already closed it by emailing to the -done address in
my last email.

I even received a bug-closing confirmation after that.

Is it that the bug got reopened automatically when Tino merged it with
another bug, and so it had to be re-closed?

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 876 bytes --]

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Noam Postavsky]

Kaushal Modi <kaushal.modi@gmail.com> writes:

> Is it that the bug got reopened automatically when Tino merged it
> with another bug, and so it had to be re-closed?

Looks like, yes.  The command was 'forcemerge 28828 28792' which puts
the open bug first, hence this bug gets reopened.

https://debbugs.gnu.org/cgi/bugreport.cgi?msg=78;bug=28792

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Noam Postavsky]

On Thu, Oct 12, 2017 at 9:16 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Kaushal Modi <kaushal.modi@gmail.com>
>> Date: Thu, 12 Oct 2017 12:58:57 +0000
>> Cc: Paul Eggert <eggert@cs.ucla.edu>, 28792@debbugs.gnu.org
>>
>> That is still a bug in Emacs, at least because the error message is not helpful. It is also in a non-standard
>> format with incorrect capitalizations in-between and doesn't even generate a backtrace by default.
>
> FWIW, I see no problem with the error message, we produce such error
> messages in quite a few other places in fileio.c.  The capitalization
> seems okay, too.
>
> As for lack of backtrace, that's because the error comes from a C
> primitive.

The lack of backtrace and odd formatting is because of a
condition-case in dired-internal-do-deletions:

(defun dired-internal-do-deletions (l arg &optional trash)
  ...
        (condition-case err
              ...
              (dired-delete-file fn dired-recursive-deletes trash)
              ...
          (error ;; catch errors from failed deletions
           (dired-log "%s\n" err)
           (setq failures (cons (car (car l)) failures)))))


Compare

  (condition-case err
      (signal 'file-error '("Non-regular file" "Is a directory" "/the/file/"))
    (error
     (message "%s\n" err)))

vs

  (condition-case err
      (signal 'file-error '("Non-regular file" "Is a directory" "/the/file/"))
    (error
     (message "%s: %s\n" (car err) (error-message-string err))))

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Drew Adams]

What Noam said.

I think the problem with the error message is not that
it was composed from multiple pieces.  It is that after
those pieces have been joined the result is not a
normal message.

In particular, there should be some punctuation or other
separation between the piece that says that the thing is
not a regular file ("Non-regular file" or whatever) and
the piece that says what is not regular about it ("Is a
directory" or whatever).

This message is not normal (it is confusing):

 (file-error Non-regular file Is a directory /the/file/)

This message is better (though not ideal):

 file-error: Non-regular file: Is a directory, /the/file/

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Andreas Schwab]

On Okt 12 2017, Drew Adams <drew.adams@oracle.com> wrote:

> In particular, there should be some punctuation or other
> separation between the piece that says that the thing is
> not a regular file ("Non-regular file" or whatever) and
> the piece that says what is not regular about it ("Is a
> directory" or whatever).

That can be accomplished by formatting it with "%S".

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Drew Adams]

> > In particular, there should be some punctuation or other
> > separation between the piece that says that the thing is
> > not a regular file ("Non-regular file" or whatever) and
> > the piece that says what is not regular about it ("Is a
> > directory" or whatever).
> 
> That can be accomplished by formatting it with "%S".

Of course it can be easily done.  It is not being done now.

bug#28792: 26.0.60; Deleting to a custom trash directory in Dired gives error

[Tino Calancha]

Kaushal Modi <kaushal.modi@gmail.com> writes:

I don't have strong opinion about the issue because I don't
use the trash either.

From a git bisect:

01c885f21f343045783eb9ad1ff5f9b83d6cd789 is the first bad commit
commit 01c885f21f343045783eb9ad1ff5f9b83d6cd789
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Sun Sep 10 15:39:24 2017 -0700

    Fix race with rename-file etc. with dir NEWNAME
    
    This changes the behavior of rename-file etc. slightly.
    The old behavior mostly disagreed with the documentation, and had
    a race condition bug that could allow attackers to modify victims'
    write-protected directories (Bug#27986).