bug#22736: 25.0.91; file-notify calls wrong callbacks

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Tassilo Horn]

I use inotify as `file-notify--library' on a GNU/Linux box and a very
recent emacs-25 branch version of emacs.

The problem is that when there is a directory watch on some directory
and a file watch on a file in that directory, the callback for the
directory watch gets an event with the descriptor of the file watch.

For example, here's an entry from my `file-notify-descriptors'
hashtable.

	       3
	       ("/home/horn/Repos/el/emacs/lisp"
		(nil . magit-filenotify--callback)
		("filenotify.el" . auto-revert-notify-handler))

As you can see, the emacs/lisp directory is watched and changes are to
be reported to `magit-filenotify--callback'.  When that registered the
watch, it was told the descriptor is (3).

Additionally, the file filenotify.el in that directory has a watch, too.
Changes are to be handled by `auto-revert-notify-handler', and the
descriptor at registration time was the cons (3 . "filenotify.el").

So now I change filenotify.el and save.  What happens is that
`magit-filenotify--callback' is called with an event whose descriptor is
(3 . "filenotify.el").  That's wrong.  Callbacks should not have to
explicitly ignore events they haven't registered for.  And the docs also
state that the callbacks are called with events whose descriptors are
the same objects as the one returned by `file-notify-add-watch'.



In GNU Emacs 25.0.91.3 (x86_64-unknown-linux-gnu, GTK+ Version 3.18.7)
 of 2016-02-18 built on thinkpad-t440p
Repository revision: fa8fd65ef40eacb808583a968c3621c228bf021c
Windowing system distributor 'The X.Org Foundation', version 11.0.11801000
System Description:	Arch Linux

Configured using:
 'configure --with-modules --with-xwidgets 'CFLAGS=-g -ggdb3 -Og''

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

Important settings:
  value of $LC_MONETARY: de_DE.utf8
  value of $LC_NUMERIC: de_DE.utf8
  value of $LC_TIME: de_DE.utf8
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: Summary

Minor modes in effect:
  debbugs-gnu-summary-mode: t
  hl-line-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  async-bytecomp-package-mode: t
  diff-auto-refine-mode: t
  global-company-mode: t
  shell-dirtrack-mode: t
  global-aggressive-indent-mode: t
  pdf-occur-global-minor-mode: t
  recentf-mode: t
  beacon-mode: t
  global-subword-mode: t
  subword-mode: t
  save-place-mode: t
  savehist-mode: t
  show-paren-mode: t
  ivy-mode: t
  minibuffer-depth-indicate-mode: t
  electric-pair-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  global-prettify-symbols-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
 [2 times]
Auto-Revert mode enabled in current buffer
 [2 times]
Mark set
Quit
Contacting host: debbugs.gnu.org:80 [3 times]
20160218T194758.086> Opening nndoc server on /tmp/gnus-temp-group-53519uC-ephemeral...
20160218T194758.086> Opening nndoc server on /tmp/gnus-temp-group-53519uC-ephemeral...done
EVENT: ((3) changed "/home/horn/Repos/el/emacs/lisp/#filenotify.el#")
DATA: ("/home/horn/Repos/el/emacs/lisp/" #<buffer *magit: emacs>)

Load-path shadows:
None found.

Features:
(shadow emacsbug sendmail sort smiley gnus-cite gnus-bcklg nndoc
url-handlers gnus-dup mm-archive debbugs-gnu debbugs soap-client
warnings rng-xsd rng-dt rng-util xsd-regexp gnus-async qp gnus-ml
hl-line nndraft nnmh rot13 utf-7 network-stream nsm starttls nnml nnnil
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-cache
gnus-demon nntp spam spam-stat gnus-uu yenc gnus-msg gnus-gravatar
mail-extr gravatar gnus-topic nnir gnus-registry registry eieio-compat
eieio-base th-private eieio-opt speedbar sb-image ezimage dframe
character-fold misearch multi-isearch linum magit-filenotify magit-blame
magit-stash magit-bisect magit-remote magit-commit magit-sequence magit
magit-apply magit-wip magit-log magit-diff smerge-mode magit-core
magit-autorevert autorevert filenotify magit-process magit-popup
magit-mode magit-git crm magit-section magit-utils git-commit log-edit
pcvs-util add-log with-editor async-bytecomp async smex ido vc
vc-dispatcher vc-git diff-mode colir color company-oddmuse
company-keywords company-etags company-gtags company-dabbrev-code
company-dabbrev company-files company-capf company-cmake company-xcode
company-clang company-semantic company-eclim company-template
company-css company-nxml company-bbdb highlight-indent-guides company
stratego-mode tg-mode generic preview-latex tex-site auto-loads
clj-refactor pkg-info url-http url-auth url-gw json map lisp-mnt epl
inflections html-mode-expansions sgml-mode edn peg multiple-cursors-core
s dash cider tramp-sh cider-debug cider-browse-ns cider-inspector
cider-mode cider-resolve cider-interaction arc-mode archive-mode
cider-overlays cider-repl cider-test cider-stacktrace cider-doc
org-table the-org-mode-expansions org org-macro org-footnote
org-pcomplete org-list org-faces org-entities org-version ob-emacs-lisp
ob ob-tangle ob-ref ob-lob ob-table ob-exp org-src ob-keys ob-comint
ob-core ob-eval org-compat org-macs org-loaddefs cal-menu calendar
cal-loaddefs cider-grimoire cider-popup cider-eldoc cider-client
cider-common cider-util nrepl-client tramp tramp-compat tramp-loaddefs
trampver ucs-normalize shell pcomplete queue ewoc cider-compat spinner
clojure-mode-expansions clojure-mode align paredit aggressive-indent
epa-file epa rdictcc google-contacts-message google-contacts xml
url-cache url url-proxy url-privacy url-expand url-methods url-history
url-cookie url-domsuf url-util google-oauth google-contacts-gnus
gnus-art mm-uu mml2015 mm-view mml-smime smime dig mailcap gnus-sum
gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source tls
gnutls utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-range gnus-win
gnus gnus-ems nnheader dired-x em-term term ehelp esh-opt esh-ext
esh-util highlight-symbol boxquote rect ecomplete message rfc822 mml
mml-sec epg mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 ietf-drums mailabbrev mail-utils gmm-utils mailheader server
yasnippet disp-table noutline outline pdf-occur ibuf-ext ibuffer tablist
tablist-filter semantic/wisent/comp semantic/wisent
semantic/wisent/wisent semantic/util-modes semantic/util semantic
semantic/tag semantic/lex semantic/fw mode-local find-func cedet dired
pdf-isearch let-alist pdf-misc imenu pdf-tools compile comint ansi-color
cus-edit cus-start cus-load pdf-view bookmark pp jka-compr pdf-cache
pdf-info tq pdf-util format-spec image-mode browse-kill-ring derived
recentf tree-widget wid-edit expand-region text-mode-expansions
er-basic-expansions expand-region-core expand-region-custom beacon seq
highlight-parentheses easy-mmode cl iedit iedit-lib hydra lv counsel
etags xref project swiper cap-words superword subword saveplace savehist
paren ivy delsel ffap thingatpt url-parse auth-source eieio byte-opt
bytecomp byte-compile cl-extra cconv eieio-core cl-macs gv gnus-util
mm-util help-fns help-mode mail-prsvr password-cache url-vars icomplete
mb-depth ace-window avy ring smart-mode-line-respectful-theme
smart-mode-line-dark-theme cl-seq smart-mode-line rich-minority rx bs
windmove elec-pair edmacro kmacro cl-loaddefs cl-lib subr-x pcase
tsdh-dark-theme finder-inf memory-usage-autoloads advice info package
easymenu epg-config time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
frame cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan
thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese charscript
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote dbusbind inotify
dynamic-setting system-font-setting font-render-setting xwidget-internal
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 818216 119839)
 (symbols 48 60404 16)
 (miscs 40 525 1053)
 (strings 32 194650 65781)
 (string-bytes 1 6408650)
 (vectors 16 87142)
 (vector-slots 8 2026074 102176)
 (floats 8 1116 652)
 (intervals 56 8874 4719)
 (buffers 976 43)
 (heap 1024 110928 12933))

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Eli Zaretskii]

> From: Tassilo Horn <tsdh@gnu.org>
> Date: Thu, 18 Feb 2016 20:04:40 +0100
> 
> The problem is that when there is a directory watch on some directory
> and a file watch on a file in that directory, the callback for the
> directory watch gets an event with the descriptor of the file watch.

I think this is because when you ask to watch a file, we actually
watch its parent directory.

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> The problem is that when there is a directory watch on some directory
>> and a file watch on a file in that directory, the callback for the
>> directory watch gets an event with the descriptor of the file watch.
>
> I think this is because when you ask to watch a file, we actually
> watch its parent directory.

Although we watch the parent directory internally (except with kqueue),
the given scenario by Tassilo shall work as he expected. There shall be
two file notification events in the given case, calling
magit-filenotify--callback and auto-revert-notify-handler with the
respective descriptor.

I'll check.

Tassilo, would you be willing to write an ert test case for the problem?

Best regards, Michael.

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Tassilo Horn]

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

Hi Michael and Eli,

>>> The problem is that when there is a directory watch on some
>>> directory and a file watch on a file in that directory, the callback
>>> for the directory watch gets an event with the descriptor of the
>>> file watch.
>>
>> I think this is because when you ask to watch a file, we actually
>> watch its parent directory.
>
> Although we watch the parent directory internally (except with
> kqueue), the given scenario by Tassilo shall work as he expected.
> There shall be two file notification events in the given case, calling
> magit-filenotify--callback and auto-revert-notify-handler with the
> respective descriptor.

Yes, that's exactly what I had expected.

> I'll check.

Thanks!

> Tassilo, would you be willing to write an ert test case for the problem?

Sure, I'm already working on it. :-)

Bye,
Tassilo

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Michael Albinus]

Tassilo Horn <tsdh@gnu.org> writes:

Hi Tassilo,

>> Tassilo, would you be willing to write an ert test case for the problem?
>
> Sure, I'm already working on it. :-)

Thanks for the patch. I have modified it a little bit, the major problem
was that your (should ...) forms in the two callbacks have no effect,
because the callbacks run out of the scope of the ert test. I have
modified them in order to use the mechanism from
file-notify--test-event-handler.

I'll continue to check what's up.

> Bye,
> Tassilo

Best regards, Michael.

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Tassilo Horn]

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

Hi Michael,

>>> Tassilo, would you be willing to write an ert test case for the problem?
>>
>> Sure, I'm already working on it. :-)
>
> Thanks for the patch. I have modified it a little bit, the major
> problem was that your (should ...) forms in the two callbacks have no
> effect, because the callbacks run out of the scope of the ert test.

Ups.  I've tested it and got failures where (1) was expected but (1
. "fileX1234") was received which seemed correct. :-)

> I have modified them in order to use the mechanism from
> file-notify--test-event-handler.

Thanks!

Bye,
Tassilo

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Michael Albinus]

Tassilo Horn <tsdh@gnu.org> writes:

> Hi Michael,

Hi Tassilo,

>> I have modified them in order to use the mechanism from
>> file-notify--test-event-handler.
>
> Thanks!

I've pushed a patch to the emacs-25 branch which fixes the problem. The
test passes successfully, using the inotify backend. Could you, please,
crosscheck, that your use case runs also?

There are still problems with the `stopped' events in this
constellation, but that is outside the scope of this bug report. I'll
continue to check.

And I will run also the test with the other backends, afterwards.

> Bye,
> Tassilo

Best regards, Michael.

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Tassilo Horn]

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

Hi Michael,

> I've pushed a patch to the emacs-25 branch which fixes the problem.
> The test passes successfully, using the inotify backend. Could you,
> please, crosscheck, that your use case runs also?

Yes, now it's working fine again.

Bye,
Tassilo

bug#22736: 25.0.91; file-notify calls wrong callbacks

[Michael Albinus]

Tassilo Horn <tsdh@gnu.org> writes:

> Hi Michael,

Hi Tassilo,

>> I've pushed a patch to the emacs-25 branch which fixes the problem.
>> The test passes successfully, using the inotify backend. Could you,
>> please, crosscheck, that your use case runs also?
>
> Yes, now it's working fine again.

Thanks, I'm marking this bug as done.

I've finished also my tests with different backends (except for cygwin);
there were some minor glitches to fix.

And I've pimped file-notify-test08-watched-file-in-watched-dir; it
checks now the expected events.

> Bye,
> Tassilo

Best regards, Michael.