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

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

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

>> + A watched hard-link for some other file may not receive its events,
>>   due to string-equal being used for file-comparisons.  Shouldn't
>>   file-equal be used instead ?
>
> How does inotify (and the other libraries) work in this case? Does it
> support hard links? We should not add more logic than the native
> libraries offer.

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.

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.

>
>> + 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)))

>> + Why the seemingly arbitrary exclusion of backup-files in
>>   file-notify-callback ?  What if someone wants to track the creation of
>>   said files ?
>
> When a file under supervision is renamed during backup, the supervision
> might be stopped. This case must be handled.

Yes, I see.

>> + 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.

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.

Thanks for you response,
-ap





  reply	other threads:[~2017-03-18 20:37 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 [this message]
2017-03-19  9:39                 ` Michael Albinus
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=878to21fty.fsf@luca \
    --to=politza@hochschule-trier.de \
    --cc=26126@debbugs.gnu.org \
    --cc=michael.albinus@gmx.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).