unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Michael Albinus <michael.albinus@gmx.de>
To: Andreas Politz <politza@hochschule-trier.de>
Cc: 26126@debbugs.gnu.org
Subject: bug#26126: 26.0.50; file-notify-rm-watch removes arbitrary watches
Date: Sun, 19 Mar 2017 10:39:11 +0100	[thread overview]
Message-ID: <87wpbl1u80.fsf@detlef> (raw)
In-Reply-To: <878to21fty.fsf@luca> (Andreas Politz's message of "Sat, 18 Mar 2017 21:37:45 +0100")

Andreas Politz <politza@hochschule-trier.de> writes:

Hi Andreas,

> inotify seems to be inode-based, so it emits notifications independent
> of the filename used.  I think that hard-links are not commonly used
> under win32, but I don't know.

Yes, inotify watches inodes.

> I we stay with string-equal, we are watching filenames, not files.
> Which is probably sufficient/OK.  Maybe this should be mentioned in the
> manual, I made a note of it.

The docstring speaks about filesystems. This is a hint, that the
behaviour of the libraries might differ.

>>> + Watching a /dir/file may receive events (e.g. touch /dir) for dir.
>>
>> Could you pls give an example?
>
> With inotify: 
>
> (let ((desc (file-notify-add-watch
>              "/tmp/file" '(attribute-change)
>              (lambda (_) (cl-assert nil)))))
>   (unwind-protect
>       (progn
>         (shell-command "touch /tmp")
>         (sit-for 3))
>     (file-notify-rm-watch desc)))

Yes, I remember. For all backends except kqueue, we watch the
directory. This is intended.

>>> + Why is the existence of kqueue checked for the handler in
>>>   file-notify-add-watch ? After all we don't know how this handler will
>>>   operate.
>>
>> Why don't we know what kqueue does?
>
> This:
>
>     (if handler
> 	;; A file name handler could exist even if there is no local
> 	;; file notification support.
> 	(setq desc (funcall
> 		    handler 'file-notify-add-watch
>                     ;; kqueue does not report file changes in
>                     ;; directory monitor.  So we must watch the file
>                     ;; itself.
>                     (if (eq file-notify--library 'kqueue) file dir)
>                     flags callback))
>
> Why should we assume that handler is somehow related to kqueue ? I think
> its just a copy and paste error.  The comment should be moved to the
> actual library call below this.

This looks like an error, indeed. I will check.

> I also wonder, if the passed argument should not always be the filename
> for which the watch was requested, as opposed to its directory.  After
> all we should not make assumptions about the abilities of the underlying
> mechanism.  For example it could work similar to kqueue, i.e. with an
> inability to watch directories.

We've discussed this years ago, maybe you find it in the archives. There
are problems when you watch only the file. This doesn't work for example
when you want to watch a file which does not exist yet. Or which
disappears, and reappears.

The agreement was to watch the upper directory. This works for all
backends except kqueue.

> Thanks for you response,
> -ap

Best regards, Michael.





  reply	other threads:[~2017-03-19  9:39 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-16 14:14 bug#26126: 26.0.50; file-notify-rm-watch removes arbitrary watches Andreas Politz
2017-03-17 14:41 ` Michael Albinus
2017-03-17 14:59   ` Andreas Politz
2017-03-17 16:08     ` Michael Albinus
2017-03-17 17:45       ` Andreas Politz
2017-03-18  8:30         ` Michael Albinus
2017-03-18 13:32           ` Andreas Politz
2017-03-18 19:36             ` Michael Albinus
2017-03-18 20:37               ` Andreas Politz
2017-03-19  9:39                 ` Michael Albinus [this message]
2017-03-19 11:14                   ` Andreas Politz
2017-03-19 19:23                     ` Michael Albinus
2017-03-20 20:39                       ` Andreas Politz
2017-03-21  8:44                         ` Michael Albinus
2017-03-21 15:37                           ` Eli Zaretskii
2017-03-21 18:59                             ` Andreas Politz
2017-03-22 13:23                             ` Michael Albinus
2017-03-22 15:44                               ` Eli Zaretskii
2017-03-22 16:01                                 ` Michael Albinus
2017-03-22 16:13                                   ` Eli Zaretskii
2017-03-22 16:23                                     ` Michael Albinus
2017-03-24 19:54                                 ` Andreas Politz
2017-03-25 12:50                                   ` Michael Albinus
2017-03-25 13:59                                     ` Andreas Politz
2017-03-25 14:08                                       ` Michael Albinus
2017-03-25 16:27                                         ` Andreas Politz
2017-03-25 16:37                                           ` Michael Albinus
2017-03-25 17:12                                             ` Andreas Politz
2017-03-25 18:36                                               ` Michael Albinus
2017-03-25 19:34                                                 ` Andreas Politz
2017-03-26  7:08                                                   ` Michael Albinus
2017-03-21 15:56                           ` Andreas Politz
2017-03-22 12:56                             ` Michael Albinus
2017-03-22 17:34                               ` Andreas Politz
2017-03-22 18:49                                 ` Michael Albinus
2017-03-19 22:05               ` Andreas Politz
2017-03-21 13:05                 ` Michael Albinus
2017-03-21 15:06                   ` Andreas Politz
2017-03-21 15:54                     ` Eli Zaretskii
2017-03-22 13:17                     ` Michael Albinus
2017-03-22 17:43                       ` Andreas Politz
2017-03-22 18:57                         ` Michael Albinus
2017-03-22 20:02                           ` Eli Zaretskii
2017-03-23  7:36                             ` Michael Albinus
2017-03-23 15:22                               ` Eli Zaretskii
2017-03-23 16:10                                 ` Michael Albinus
2017-03-22 19:40                   ` Michael Albinus
2017-03-24 20:44                 ` Andreas Politz
2017-03-25  6:35                   ` Eli Zaretskii
2017-03-25  8:57                     ` Andreas Politz
2017-03-25 14:17                       ` Eli Zaretskii
2017-03-25 16:34                         ` Andreas Politz
2017-03-25 14:04                   ` Michael Albinus
2017-03-25 16:19                     ` Andreas Politz
2017-03-25 17:09                       ` Michael Albinus
2017-03-25 17:26                         ` Andreas Politz
2017-03-25 18:18                         ` Andreas Politz
2017-03-25 18:40                           ` Michael Albinus
2017-03-25 16:21                     ` Andreas Politz
2017-03-18 19:28           ` Andreas Politz
2017-03-18 19:49             ` Michael Albinus
2017-03-18 20:48               ` Andreas Politz
2017-03-30 18:15 ` Paul Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wpbl1u80.fsf@detlef \
    --to=michael.albinus@gmx.de \
    --cc=26126@debbugs.gnu.org \
    --cc=politza@hochschule-trier.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).