Remove all existing file notification watches from Emacs

Remove all existing file notification watches from Emacs

[Michael Albinus]

While working on bug#51146, I found the following command useful, which
removes all file notification watches from an Emacs instance:

--8<---------------cut here---------------start------------->8---
(defun file-notify-rm-all-watches ()
  "Remove all existing file notification watches from Emacs."
  (interactive)
  (maphash
   (lambda (key _value)
     (file-notify-rm-watch key))
   file-notify-descriptors))
--8<---------------cut here---------------end--------------->8---

The code is simple enough to avoid regressions. I'd like to add it to
the emacs-28 branch (plus doc), any objection?

Best regards, Michael.

Re: Remove all existing file notification watches from Emacs

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Sat, 16 Oct 2021 09:14:17 +0200
> 
> While working on bug#51146, I found the following command useful, which
> removes all file notification watches from an Emacs instance:
> 
> --8<---------------cut here---------------start------------->8---
> (defun file-notify-rm-all-watches ()
>   "Remove all existing file notification watches from Emacs."
>   (interactive)
>   (maphash
>    (lambda (key _value)
>      (file-notify-rm-watch key))
>    file-notify-descriptors))
> --8<---------------cut here---------------end--------------->8---
> 
> The code is simple enough to avoid regressions. I'd like to add it to
> the emacs-28 branch (plus doc), any objection?

I don't necessarily object, but why is it important to have this
command in Emacs 28?  Just being useful is not enough, IMO: there are
quite a few features that have this property and are orthogonal to the
rest of the code, so allowing this addition will then beg the question
why not those others as well?

Re: Remove all existing file notification watches from Emacs

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

> I don't necessarily object, but why is it important to have this
> command in Emacs 28?  Just being useful is not enough, IMO: there are
> quite a few features that have this property and are orthogonal to the
> rest of the code, so allowing this addition will then beg the question
> why not those others as well?

I have no answer for "why is it important". I found it just convenient;
if you see the disadvantage in adding it to emacs-28, I follow.
Personally, I know how to define that command in Emacs 28 while bug
hunting :-)

Bringing it to master then. In case of high demand it could still be
cherry-picked for emacs-28.

Best regards, Michael.

Re: Remove all existing file notification watches from Emacs

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: emacs-devel@gnu.org
> Date: Sat, 16 Oct 2021 09:36:08 +0200
> 
> > I don't necessarily object, but why is it important to have this
> > command in Emacs 28?  Just being useful is not enough, IMO: there are
> > quite a few features that have this property and are orthogonal to the
> > rest of the code, so allowing this addition will then beg the question
> > why not those others as well?
> 
> I have no answer for "why is it important". I found it just convenient;
> if you see the disadvantage in adding it to emacs-28, I follow.
> Personally, I know how to define that command in Emacs 28 while bug
> hunting :-)

I understand.  But we must draw a line somewhere, if we want to
release Emacs 28 sooner rather than later.

Re: Remove all existing file notification watches from Emacs

[Stefan Monnier]

> While working on bug#51146, I found the following command useful, which
> removes all file notification watches from an Emacs instance:
>
> --8<---------------cut here---------------start------------->8---
> (defun file-notify-rm-all-watches ()
>   "Remove all existing file notification watches from Emacs."
>   (interactive)
>   (maphash
>    (lambda (key _value)
>      (file-notify-rm-watch key))
>    file-notify-descriptors))
> --8<---------------cut here---------------end--------------->8---

I can see its use while debugging but I can't imagine it being of any
use in real ELisp code because it's much too blunt.
[ Personally I'd put a `--` in its name and/or add some blurb in the
docstring explaining it's only for debugging.  ]

> The code is simple enough to avoid regressions. I'd like to add it to
> the emacs-28 branch (plus doc), any objection?

I don't see any reason why it needs to be in emacs-28.
It's not a bugfix and there's no hurry.


        Stefan

Re: Remove all existing file notification watches from Emacs

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

> I can see its use while debugging but I can't imagine it being of any
> use in real ELisp code because it's much too blunt.

Not only in code. It's an interactive command.

> [ Personally I'd put a `--` in its name and/or add some blurb in the
> docstring explaining it's only for debugging.  ]

Nope. There have been bug reports in the past, that Emacs was stalled
after creating hundreds of file notifications. One solution is to
stop/restart Emacs. The other solution is this command.

>> The code is simple enough to avoid regressions. I'd like to add it to
>> the emacs-28 branch (plus doc), any objection?
>
> I don't see any reason why it needs to be in emacs-28.
> It's not a bugfix and there's no hurry.

After advice from Eli, I've pushed it to master.

>         Stefan

Best regards, Michael.

Re: Remove all existing file notification watches from Emacs

[Stefan Monnier]

>> [ Personally I'd put a `--` in its name and/or add some blurb in the
>> docstring explaining it's only for debugging.  ]
>
> Nope.  There have been bug reports in the past, that Emacs was stalled
> after creating hundreds of file notifications. One solution is to
> stop/restart Emacs.  The other solution is this command.

That qualifies as debugging for me ;-)

I mean, using this instead of restarting Emacs may work sometimes, but
it can have undesirable (and hard to predict, for the end user) side
effects since packages have had their watches killed from under them.

So it's OK to use when the alternative is to kill Emacs, but it should
come with some warnings (in a pessimistic scenario I can see some blog
mentioning this command alongside the usual "set your GC threshold super
high" and a few other things, in the toolbox of "make your Emacs more
faster").


        Stefan

Re: Remove all existing file notification watches from Emacs

[Michael Albinus]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

> I mean, using this instead of restarting Emacs may work sometimes, but
> it can have undesirable (and hard to predict, for the end user) side
> effects since packages have had their watches killed from under them.

Like this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 212 bytes --]

(defun file-notify-rm-all-watches ()
  "Remove all existing file notification watches from Emacs."
  (interactive)
  (maphash
   (lambda (key _value)
     (file-notify-rm-watch key))
   file-notify-descriptors))

[-- Attachment #3: Type: text/plain, Size: 466 bytes --]


> So it's OK to use when the alternative is to kill Emacs, but it should
> come with some warnings

Do you want a confirmation à la "Are you really really sure?"

> (in a pessimistic scenario I can see some blog
> mentioning this command alongside the usual "set your GC threshold super
> high" and a few other things, in the toolbox of "make your Emacs more
> faster").

Such blogs will always happen ...

>         Stefan

Best regards, Michael.

Re: Remove all existing file notification watches from Emacs

[Stefan Kangas]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I can see its use while debugging but I can't imagine it being of any
> use in real ELisp code because it's much too blunt.
> [ Personally I'd put a `--` in its name and/or add some blurb in the
> docstring explaining it's only for debugging.  ]

Would it make sense to have something like 'debug-elisp.el' where we
could put all/most functions that are only useful when debugging?
Then we would decrease the risk of users stumbling upon our sharper
tools unless they actively go looking for them.  In this case, I can
imagine running this command by mistake or mistakenly messing up quite
a few things in a typical user session.

Orthogonally, could we place all/most such functions under some prefix
like "debug-"?

I'm not sure this makes sense, as I can't think of any more commands
that fall in that category, but the 'bidi-display-reordering' variable
is one candidate for a prefix, I think.

Re: Remove all existing file notification watches from Emacs

[Michael Albinus]

Stefan Kangas <stefan@marxist.se> writes:

Hi Stefan,

> Would it make sense to have something like 'debug-elisp.el' where we
> could put all/most functions that are only useful when debugging?
> Then we would decrease the risk of users stumbling upon our sharper
> tools unless they actively go looking for them.  In this case, I can
> imagine running this command by mistake or mistakenly messing up quite
> a few things in a typical user session.

I don't object, but as I said the other message, this command isn't only
for debugging purposes.

Best regards, Michael.

Re: Remove all existing file notification watches from Emacs

[Michael Albinus]

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

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Stefan,

>> I mean, using this instead of restarting Emacs may work sometimes, but
>> it can have undesirable (and hard to predict, for the end user) side
>> effects since packages have had their watches killed from under them.
>
> Like this?

Oops, I meant


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 554 bytes --]

diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index e3dcd6c778..5323f5f9a1 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -3232,6 +3232,10 @@ File Notifications

 @deffn Command file-notify-rm-all-watches
 Removes all existing file notification watches from Emacs.
+
+Use this command with caution, because it could have unexpected side
+effects on packages relying on file watches.  It is intended mainly
+for debugging purposes, or when Emacs has been stalled.
 @end deffn

 @defun file-notify-valid-p descriptor

[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


Best regards, Michael.

Re: Remove all existing file notification watches from Emacs

[Stefan Monnier]

> Oops, I meant
>
> diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
> index e3dcd6c778..5323f5f9a1 100644
> --- a/doc/lispref/os.texi
> +++ b/doc/lispref/os.texi
> @@ -3232,6 +3232,10 @@ File Notifications
>
>  @deffn Command file-notify-rm-all-watches
>  Removes all existing file notification watches from Emacs.
> +
> +Use this command with caution, because it could have unexpected side
> +effects on packages relying on file watches.  It is intended mainly
> +for debugging purposes, or when Emacs has been stalled.
>  @end deffn
>
>  @defun file-notify-valid-p descriptor

Sounds fine to me, yes.


        Stefan