GNU bug report logs - #60146
file-exists-in-trash-p needs better name or semantics

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 60146 in the body.
You can then email your comments to 60146 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: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: file-exists-in-trash-p needs better name or semantics
Date: Fri, 16 Dec 2022 21:17:13 -0800

The recently-added function (file-exists-in-trash-p FILE) is poorly 
named since it's not really related to trash - it's simply checking for 
the existence of a directory entry named FILE.

How about extending file-exists-p instead? (file-exists-p FILE t) would 
be like (file-exists-p FILE) except it would not follow symlinks. This 
extension can be implemented via a single system call on POSIX systems, 
and this would be more efficient and would avoid a race in the current 
implementation of file-exists-in-trash-p. (Though of course pretty much 
any use of this new function makes one vulnerable to races....)

If extending file-exists-p is too much, at least please rename 
file-exists-in-trash-p to something like files--exists-nofollow-p, to 
indicate that it's private to files.el and to say better what it means.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 60146 <at> debbugs.gnu.org
Subject: Re: bug#60146: file-exists-in-trash-p needs better name or semantics
Date: Sat, 17 Dec 2022 10:47:45 +0200

> Date: Fri, 16 Dec 2022 21:17:13 -0800
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> 
> The recently-added function (file-exists-in-trash-p FILE) is poorly 
> named since it's not really related to trash - it's simply checking for 
> the existence of a directory entry named FILE.
> 
> How about extending file-exists-p instead? (file-exists-p FILE t) would 
> be like (file-exists-p FILE) except it would not follow symlinks. This 
> extension can be implemented via a single system call on POSIX systems, 
> and this would be more efficient and would avoid a race in the current 
> implementation of file-exists-in-trash-p. (Though of course pretty much 
> any use of this new function makes one vulnerable to races....)
> 
> If extending file-exists-p is too much, at least please rename 
> file-exists-in-trash-p to something like files--exists-nofollow-p, to 
> indicate that it's private to files.el and to say better what it means.

I don't really mind renaming that function, but the reason I called it
like I did was that apparently no one needed such a functionality in
Emacs until now, except in this obscure use case of moving to trash.
That seems to tell me that extending file-exists-p would be a solution
waiting for the problem, something that we don't like doing.

In any case, if we do decide to extend file-exists-p, it would need to
be done on master, as that is not a trivial change, which has to be
done in C.

Lars, Stefan, WDYT?

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 60146 <at> debbugs.gnu.org
Subject: Re: bug#60146: file-exists-in-trash-p needs better name or semantics
Date: Sat, 17 Dec 2022 11:01:35 +0100

Paul Eggert <eggert <at> cs.ucla.edu> writes:

Hi Paul,

> How about extending file-exists-p instead? (file-exists-p FILE t)
> would be like (file-exists-p FILE) except it would not follow
> symlinks. This extension can be implemented via a single system call
> on POSIX systems, and this would be more efficient and would avoid a
> race in the current implementation of file-exists-in-trash-p. (Though
> of course pretty much any use of this new function makes one
> vulnerable to races....)

Pls don't do it in the emacs-29 branch. file-exists-p checks for file
name handlers, with all consequences for a changed signature.

Best regards, Michael.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Paul Eggert <eggert <at> cs.ucla.edu>,
 60146 <at> debbugs.gnu.org
Subject: Re: bug#60146: file-exists-in-trash-p needs better name or semantics
Date: Sat, 17 Dec 2022 10:51:38 -0500

> I don't really mind renaming that function, but the reason I called it
> like I did was that apparently no one needed such a functionality in
> Emacs until now, except in this obscure use case of moving to trash.
> That seems to tell me that extending file-exists-p would be a solution
> waiting for the problem, something that we don't like doing.

Hmm... arguing about names, eh?
Let me throw that hat into the ring: IIUC this is a variant of
`file-exists-p` (currently) used only when dealing the trash, so I'd
call it `trash--file-exists-p`.

> In any case, if we do decide to extend file-exists-p, it would need to
> be done on master, as that is not a trivial change, which has to be
> done in C.

Indeed, I see no need for that on emacs-29.


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: larsi <at> gnus.org, eggert <at> cs.ucla.edu, 60146 <at> debbugs.gnu.org
Subject: Re: bug#60146: file-exists-in-trash-p needs better name or semantics
Date: Sat, 17 Dec 2022 19:45:02 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Paul Eggert <eggert <at> cs.ucla.edu>,  Lars Ingebrigtsen <larsi <at> gnus.org>,
>   60146 <at> debbugs.gnu.org
> Date: Sat, 17 Dec 2022 10:51:38 -0500
> 
> Let me throw that hat into the ring: IIUC this is a variant of
> `file-exists-p` (currently) used only when dealing the trash, so I'd
> call it `trash--file-exists-p`.

Well, the function is defined in files.el, so starting the name with
"trash" would be frowned upon...

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 60146 <at> debbugs.gnu.org
Subject: Re: bug#60146: file-exists-in-trash-p needs better name or semantics
Date: Sat, 17 Dec 2022 11:57:39 -0800

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

On 12/17/22 00:47, Eli Zaretskii wrote:
> I don't really mind renaming that function, but the reason I called it
> like I did was that apparently no one needed such a functionality in
> Emacs until now,

I have a bit of a different take. First, there's adequate functionality 
in Emacs now if we don't mind inefficiency, and it's already used; just 
call file-attributes. (Gnus does this, in nnmaildir-unlink at least, and 
I expect there are other uses.) Second, I wouldn't be surprised if other 
uses of file-exists-p have problems similar to the one you found in 
move-file-to-trash. Not that any of us have time to go scout for them 
right now.

It's a minor point. Still, the name file-exists-in-trash-p really needs 
to go somehow as it's not a name that should be user-visible. How about 
the attached patch?

[0001-Remove-file-exists-in-trash-p.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: larsi <at> gnus.org, monnier <at> iro.umontreal.ca, 60146 <at> debbugs.gnu.org
Subject: Re: bug#60146: file-exists-in-trash-p needs better name or semantics
Date: Sat, 17 Dec 2022 22:09:43 +0200

> Date: Sat, 17 Dec 2022 11:57:39 -0800
> Cc: 60146 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> 
> I have a bit of a different take. First, there's adequate functionality 
> in Emacs now if we don't mind inefficiency, and it's already used; just 
> call file-attributes. (Gnus does this, in nnmaildir-unlink at least, and 
> I expect there are other uses.) Second, I wouldn't be surprised if other 
> uses of file-exists-p have problems similar to the one you found in 
> move-file-to-trash. Not that any of us have time to go scout for them 
> right now.
> 
> It's a minor point. Still, the name file-exists-in-trash-p really needs 
> to go somehow as it's not a name that should be user-visible. How about 
> the attached patch?

If you can verify that it works in the scenario of bug#59986, I'm okay
with this change.

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, monnier <at> iro.umontreal.ca, 60146-done <at> debbugs.gnu.org
Subject: Re: bug#60146: file-exists-in-trash-p needs better name or semantics
Date: Sat, 17 Dec 2022 15:55:43 -0800

On 12/17/22 12:09, Eli Zaretskii wrote:
> If you can verify that it works in the scenario of bug#59986, I'm okay
> with this change.

Thanks, I checked that the patch works in that scenario (dangling 
symlink in the trash directory), and installed it. Closing the bug report.

Previous Next

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