GNU bug report logs -
#26126
26.0.50; file-notify-rm-watch removes arbitrary watches
Previous Next
Reported by: Andreas Politz <politza <at> hochschule-trier.de>
Date: Thu, 16 Mar 2017 14:16:02 UTC
Severity: normal
Tags: fixed
Found in version 26.0.50
Fixed in version 26.1
Done: Michael Albinus <michael.albinus <at> gmx.de>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26126 in the body.
You can then email your comments to 26126 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Thu, 16 Mar 2017 14:16:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Andreas Politz <politza <at> hochschule-trier.de>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Thu, 16 Mar 2017 14:16:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
The descriptor returned by file-notify-add-watch is of the form (ID . DIR)
(at least when using inotify) and these descriptors are stored in a
hash-table using equal as comparator.
If multiple clients watch the same file and one of them removes its watch
via file-notify-rm-watch, the result is unpredictable. I.e. the function
removes some watch (I believe it is the last one added), but not necessarily
the one associated with the client calling file-notify-rm-watch on the
descriptor it got from calling file-notify-add-watch.
I've attached a test case for this.
Am I missing something here ?
-ap
[file-notify-bug-1.el (application/emacs-lisp, attachment)]
[Message part 3 (text/plain, inline)]
In GNU Emacs 26.0.50 (build 1, x86_64-unknown-linux-gnu, GTK+ Version 3.22.9)
of 2017-03-15 built on luca
Repository revision: 2f972349bdc99d5d9ebf63169c00e24b119aa38d
Windowing system distributor 'The X.Org Foundation', version 11.0.11902000
System Description: Arch Linux
Recent messages:
Delete all tests? (y or n) y
file-notify-rm-out-of-order
Ran 1 tests, 0 results were as expected
Quit
Undo! [2 times]
Mark set
C-c C-t is undefined
Test result: FAILED ()
Quit
Mark set [2 times]
Configured using:
'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
--localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-xwidgets
'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe
-fstack-protector-strong'
LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro
CPPFLAGS=-D_FORTIFY_SOURCE=2'
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 XWIDGETS LIBSYSTEMD
Important settings:
value of $LC_MESSAGES: en_US.UTF-8
value of $LANG: de_DE.UTF-8
locale-coding-system: utf-8-unix
Major mode: Ert
Minor modes in effect:
TeX-PDF-mode: t
diff-auto-refine-mode: t
yas-minor-mode: t
sow-scroll-other-window-mode: t
synclient-fix-tapping-mode: t
show-paren-mode: t
check-parens-mode: t
pdf-occur-global-minor-mode: t
override-global-mode: t
recentf-mode: t
shell-dirtrack-mode: t
save-place-mode: t
ewm-desktop-mode: t
ewm-mode: t
ewm-bindings-mode: t
ewm-compat-mode: t
savehist-mode: t
ekey-mode: t
desktop-save-mode: t
tooltip-mode: t
global-eldoc-mode: t
eldoc-mode: t
electric-indent-mode: t
mouse-wheel-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
line-number-mode: t
transient-mark-mode: t
Load-path shadows:
/home/politza/src/ewm/ewm-ruleset hides /home/politza/.emacs.d/elpa/ewm-1.0/ewm-ruleset
/home/politza/src/ewm/dev hides /home/politza/.emacs.d/elpa/ewm-1.0/dev
/home/politza/src/ewm/ewm-layout hides /home/politza/.emacs.d/elpa/ewm-1.0/ewm-layout
/home/politza/src/ewm/ewm-frame hides /home/politza/.emacs.d/elpa/ewm-1.0/ewm-frame
/home/politza/src/ewm/ewm hides /home/politza/.emacs.d/elpa/ewm-1.0/ewm
/home/politza/src/ewm/ewm-buffer hides /home/politza/.emacs.d/elpa/ewm-1.0/ewm-buffer
/home/politza/src/ewm/ewm-configuration hides /home/politza/.emacs.d/elpa/ewm-1.0/ewm-configuration
/home/politza/src/ewm/ewm-util hides /home/politza/.emacs.d/elpa/ewm-1.0/ewm-util
/home/politza/src/ewm/ewm-compat hides /home/politza/.emacs.d/elpa/ewm-1.0/ewm-compat
~/fh_notebook/politza/.emacs.d/plugins/s hides /home/politza/.emacs.d/elpa/s-20160928.636/s
~/fh_notebook/politza/.emacs.d/plugins/term hides /usr/share/emacs/26.0.50/lisp/term
~/fh_notebook/politza/.emacs.d/plugins/imenu hides /usr/share/emacs/26.0.50/lisp/imenu
~/fh_notebook/politza/.emacs.d/plugins/saveplace hides /usr/share/emacs/26.0.50/lisp/saveplace
/home/politza/.emacs.d/elpa/org-20170210/ox-md hides /usr/share/emacs/26.0.50/lisp/org/ox-md
/home/politza/.emacs.d/elpa/org-20170210/org-rmail hides /usr/share/emacs/26.0.50/lisp/org/org-rmail
/home/politza/.emacs.d/elpa/org-20170210/org-capture hides /usr/share/emacs/26.0.50/lisp/org/org-capture
/home/politza/.emacs.d/elpa/org-20170210/ob-fortran hides /usr/share/emacs/26.0.50/lisp/org/ob-fortran
/home/politza/.emacs.d/elpa/org-20170210/org-list hides /usr/share/emacs/26.0.50/lisp/org/org-list
/home/politza/.emacs.d/elpa/org-20170210/org-table hides /usr/share/emacs/26.0.50/lisp/org/org-table
/home/politza/.emacs.d/elpa/org-20170210/ob-screen hides /usr/share/emacs/26.0.50/lisp/org/ob-screen
/home/politza/.emacs.d/elpa/org-20170210/org-pcomplete hides /usr/share/emacs/26.0.50/lisp/org/org-pcomplete
/home/politza/.emacs.d/elpa/org-20170210/ob-ditaa hides /usr/share/emacs/26.0.50/lisp/org/ob-ditaa
/home/politza/.emacs.d/elpa/org-20170210/ox-texinfo hides /usr/share/emacs/26.0.50/lisp/org/ox-texinfo
/home/politza/.emacs.d/elpa/org-20170210/ob-plantuml hides /usr/share/emacs/26.0.50/lisp/org/ob-plantuml
/home/politza/.emacs.d/elpa/org-20170210/org-w3m hides /usr/share/emacs/26.0.50/lisp/org/org-w3m
/home/politza/.emacs.d/elpa/org-20170210/ob-ref hides /usr/share/emacs/26.0.50/lisp/org/ob-ref
/home/politza/.emacs.d/elpa/org-20170210/ob-lilypond hides /usr/share/emacs/26.0.50/lisp/org/ob-lilypond
/home/politza/.emacs.d/elpa/org-20170210/ox-odt hides /usr/share/emacs/26.0.50/lisp/org/ox-odt
/home/politza/.emacs.d/elpa/org-20170210/ob-latex hides /usr/share/emacs/26.0.50/lisp/org/ob-latex
/home/politza/.emacs.d/elpa/org-20170210/ox-ascii hides /usr/share/emacs/26.0.50/lisp/org/ox-ascii
/home/politza/.emacs.d/elpa/org-20170210/ob-sqlite hides /usr/share/emacs/26.0.50/lisp/org/ob-sqlite
/home/politza/.emacs.d/elpa/org-20170210/ob-lisp hides /usr/share/emacs/26.0.50/lisp/org/ob-lisp
/home/politza/.emacs.d/elpa/org-20170210/org-archive hides /usr/share/emacs/26.0.50/lisp/org/org-archive
/home/politza/.emacs.d/elpa/org-20170210/ox-latex hides /usr/share/emacs/26.0.50/lisp/org/ox-latex
/home/politza/.emacs.d/elpa/org-20170210/org-macro hides /usr/share/emacs/26.0.50/lisp/org/org-macro
/home/politza/.emacs.d/elpa/org-20170210/org-macs hides /usr/share/emacs/26.0.50/lisp/org/org-macs
/home/politza/.emacs.d/elpa/org-20170210/ob-makefile hides /usr/share/emacs/26.0.50/lisp/org/ob-makefile
/home/politza/.emacs.d/elpa/org-20170210/ob-core hides /usr/share/emacs/26.0.50/lisp/org/ob-core
/home/politza/.emacs.d/elpa/org-20170210/ox hides /usr/share/emacs/26.0.50/lisp/org/ox
/home/politza/.emacs.d/elpa/org-20170210/ob-awk hides /usr/share/emacs/26.0.50/lisp/org/ob-awk
/home/politza/.emacs.d/elpa/org-20170210/ob-js hides /usr/share/emacs/26.0.50/lisp/org/ob-js
/home/politza/.emacs.d/elpa/org-20170210/ob-table hides /usr/share/emacs/26.0.50/lisp/org/ob-table
/home/politza/.emacs.d/elpa/org-20170210/ox-man hides /usr/share/emacs/26.0.50/lisp/org/ox-man
/home/politza/.emacs.d/elpa/org-20170210/ob-dot hides /usr/share/emacs/26.0.50/lisp/org/ob-dot
/home/politza/.emacs.d/elpa/org-20170210/org-attach hides /usr/share/emacs/26.0.50/lisp/org/org-attach
/home/politza/.emacs.d/elpa/org-20170210/ox-publish hides /usr/share/emacs/26.0.50/lisp/org/ox-publish
/home/politza/.emacs.d/elpa/org-20170210/org-clock hides /usr/share/emacs/26.0.50/lisp/org/org-clock
/home/politza/.emacs.d/elpa/org-20170210/ob-matlab hides /usr/share/emacs/26.0.50/lisp/org/ob-matlab
/home/politza/.emacs.d/elpa/org-20170210/org-habit hides /usr/share/emacs/26.0.50/lisp/org/org-habit
/home/politza/.emacs.d/elpa/org-20170210/ob-picolisp hides /usr/share/emacs/26.0.50/lisp/org/ob-picolisp
/home/politza/.emacs.d/elpa/org-20170210/org-faces hides /usr/share/emacs/26.0.50/lisp/org/org-faces
/home/politza/.emacs.d/elpa/org-20170210/ob-css hides /usr/share/emacs/26.0.50/lisp/org/ob-css
/home/politza/.emacs.d/elpa/org-20170210/org-irc hides /usr/share/emacs/26.0.50/lisp/org/org-irc
/home/politza/.emacs.d/elpa/org-20170210/ob-C hides /usr/share/emacs/26.0.50/lisp/org/ob-C
/home/politza/.emacs.d/elpa/org-20170210/ob-eval hides /usr/share/emacs/26.0.50/lisp/org/ob-eval
/home/politza/.emacs.d/elpa/org-20170210/org-bbdb hides /usr/share/emacs/26.0.50/lisp/org/org-bbdb
/home/politza/.emacs.d/elpa/org-20170210/org-ctags hides /usr/share/emacs/26.0.50/lisp/org/org-ctags
/home/politza/.emacs.d/elpa/org-20170210/ob-sass hides /usr/share/emacs/26.0.50/lisp/org/ob-sass
/home/politza/.emacs.d/elpa/org-20170210/ob-perl hides /usr/share/emacs/26.0.50/lisp/org/ob-perl
/home/politza/.emacs.d/elpa/org-20170210/ox-org hides /usr/share/emacs/26.0.50/lisp/org/ox-org
/home/politza/.emacs.d/elpa/org-20170210/org-docview hides /usr/share/emacs/26.0.50/lisp/org/org-docview
/home/politza/.emacs.d/elpa/org-20170210/ob-maxima hides /usr/share/emacs/26.0.50/lisp/org/ob-maxima
/home/politza/.emacs.d/elpa/org-20170210/org-datetree hides /usr/share/emacs/26.0.50/lisp/org/org-datetree
/home/politza/.emacs.d/elpa/org-20170210/org-mobile hides /usr/share/emacs/26.0.50/lisp/org/org-mobile
/home/politza/.emacs.d/elpa/org-20170210/ox-icalendar hides /usr/share/emacs/26.0.50/lisp/org/ox-icalendar
/home/politza/.emacs.d/elpa/org-20170210/ob-octave hides /usr/share/emacs/26.0.50/lisp/org/ob-octave
/home/politza/.emacs.d/elpa/org-20170210/org-footnote hides /usr/share/emacs/26.0.50/lisp/org/org-footnote
/home/politza/.emacs.d/elpa/org-20170210/ob-scala hides /usr/share/emacs/26.0.50/lisp/org/ob-scala
/home/politza/.emacs.d/elpa/org-20170210/ob hides /usr/share/emacs/26.0.50/lisp/org/ob
/home/politza/.emacs.d/elpa/org-20170210/ob-ocaml hides /usr/share/emacs/26.0.50/lisp/org/ob-ocaml
/home/politza/.emacs.d/elpa/org-20170210/org-eshell hides /usr/share/emacs/26.0.50/lisp/org/org-eshell
/home/politza/.emacs.d/elpa/org-20170210/org-bibtex hides /usr/share/emacs/26.0.50/lisp/org/org-bibtex
/home/politza/.emacs.d/elpa/org-20170210/ob-mscgen hides /usr/share/emacs/26.0.50/lisp/org/ob-mscgen
/home/politza/.emacs.d/elpa/org-20170210/ob-haskell hides /usr/share/emacs/26.0.50/lisp/org/ob-haskell
/home/politza/.emacs.d/elpa/org-20170210/org-timer hides /usr/share/emacs/26.0.50/lisp/org/org-timer
/home/politza/.emacs.d/elpa/org-20170210/org-entities hides /usr/share/emacs/26.0.50/lisp/org/org-entities
/home/politza/.emacs.d/elpa/org-20170210/ob-clojure hides /usr/share/emacs/26.0.50/lisp/org/ob-clojure
/home/politza/.emacs.d/elpa/org-20170210/org-plot hides /usr/share/emacs/26.0.50/lisp/org/org-plot
/home/politza/.emacs.d/elpa/org-20170210/ob-io hides /usr/share/emacs/26.0.50/lisp/org/ob-io
/home/politza/.emacs.d/elpa/org-20170210/ob-java hides /usr/share/emacs/26.0.50/lisp/org/ob-java
/home/politza/.emacs.d/elpa/org-20170210/ob-calc hides /usr/share/emacs/26.0.50/lisp/org/ob-calc
/home/politza/.emacs.d/elpa/org-20170210/ob-emacs-lisp hides /usr/share/emacs/26.0.50/lisp/org/ob-emacs-lisp
/home/politza/.emacs.d/elpa/org-20170210/org-indent hides /usr/share/emacs/26.0.50/lisp/org/org-indent
/home/politza/.emacs.d/elpa/org-20170210/org-src hides /usr/share/emacs/26.0.50/lisp/org/org-src
/home/politza/.emacs.d/elpa/org-20170210/org hides /usr/share/emacs/26.0.50/lisp/org/org
/home/politza/.emacs.d/elpa/org-20170210/ob-lob hides /usr/share/emacs/26.0.50/lisp/org/ob-lob
/home/politza/.emacs.d/elpa/org-20170210/ob-shen hides /usr/share/emacs/26.0.50/lisp/org/ob-shen
/home/politza/.emacs.d/elpa/org-20170210/org-inlinetask hides /usr/share/emacs/26.0.50/lisp/org/org-inlinetask
/home/politza/.emacs.d/elpa/org-20170210/ox-beamer hides /usr/share/emacs/26.0.50/lisp/org/ox-beamer
/home/politza/.emacs.d/elpa/org-20170210/ob-asymptote hides /usr/share/emacs/26.0.50/lisp/org/ob-asymptote
/home/politza/.emacs.d/elpa/org-20170210/org-colview hides /usr/share/emacs/26.0.50/lisp/org/org-colview
/home/politza/.emacs.d/elpa/org-20170210/ob-sql hides /usr/share/emacs/26.0.50/lisp/org/ob-sql
/home/politza/.emacs.d/elpa/org-20170210/org-mhe hides /usr/share/emacs/26.0.50/lisp/org/org-mhe
/home/politza/.emacs.d/elpa/org-20170210/org-agenda hides /usr/share/emacs/26.0.50/lisp/org/org-agenda
/home/politza/.emacs.d/elpa/org-20170210/ob-python hides /usr/share/emacs/26.0.50/lisp/org/ob-python
/home/politza/.emacs.d/elpa/org-20170210/org-crypt hides /usr/share/emacs/26.0.50/lisp/org/org-crypt
/home/politza/.emacs.d/elpa/org-20170210/ob-gnuplot hides /usr/share/emacs/26.0.50/lisp/org/ob-gnuplot
/home/politza/.emacs.d/elpa/org-20170210/ob-org hides /usr/share/emacs/26.0.50/lisp/org/ob-org
/home/politza/.emacs.d/elpa/org-20170210/org-id hides /usr/share/emacs/26.0.50/lisp/org/org-id
/home/politza/.emacs.d/elpa/org-20170210/ob-ledger hides /usr/share/emacs/26.0.50/lisp/org/ob-ledger
/home/politza/.emacs.d/elpa/org-20170210/org-protocol hides /usr/share/emacs/26.0.50/lisp/org/org-protocol
/home/politza/.emacs.d/elpa/org-20170210/ox-html hides /usr/share/emacs/26.0.50/lisp/org/ox-html
/home/politza/.emacs.d/elpa/org-20170210/ob-comint hides /usr/share/emacs/26.0.50/lisp/org/ob-comint
/home/politza/.emacs.d/elpa/org-20170210/org-compat hides /usr/share/emacs/26.0.50/lisp/org/org-compat
/home/politza/.emacs.d/elpa/org-20170210/ob-R hides /usr/share/emacs/26.0.50/lisp/org/ob-R
/home/politza/.emacs.d/elpa/org-20170210/org-feed hides /usr/share/emacs/26.0.50/lisp/org/org-feed
/home/politza/.emacs.d/elpa/org-20170210/org-mouse hides /usr/share/emacs/26.0.50/lisp/org/org-mouse
/home/politza/.emacs.d/elpa/org-20170210/org-info hides /usr/share/emacs/26.0.50/lisp/org/org-info
/home/politza/.emacs.d/elpa/org-20170210/ob-keys hides /usr/share/emacs/26.0.50/lisp/org/ob-keys
/home/politza/.emacs.d/elpa/org-20170210/ob-ruby hides /usr/share/emacs/26.0.50/lisp/org/ob-ruby
/home/politza/.emacs.d/elpa/org-20170210/org-install hides /usr/share/emacs/26.0.50/lisp/org/org-install
/home/politza/.emacs.d/elpa/org-20170210/org-version hides /usr/share/emacs/26.0.50/lisp/org/org-version
/home/politza/.emacs.d/elpa/org-20170210/ob-exp hides /usr/share/emacs/26.0.50/lisp/org/ob-exp
/home/politza/.emacs.d/elpa/org-20170210/ob-scheme hides /usr/share/emacs/26.0.50/lisp/org/ob-scheme
/home/politza/.emacs.d/elpa/org-20170210/org-loaddefs hides /usr/share/emacs/26.0.50/lisp/org/org-loaddefs
/home/politza/.emacs.d/elpa/org-20170210/org-gnus hides /usr/share/emacs/26.0.50/lisp/org/org-gnus
/home/politza/.emacs.d/elpa/org-20170210/ob-tangle hides /usr/share/emacs/26.0.50/lisp/org/ob-tangle
/home/politza/.emacs.d/elpa/org-20170210/org-element hides /usr/share/emacs/26.0.50/lisp/org/org-element
Features:
(shadow sort gnus-cite mail-extr nnir nndraft nnmh utf-7 nnfolder nnnil
gnus-agent gnus-srvr gnus-score score-mode nnvirtual nntp gnus-cache
gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum
gnus-group gnus-undo gnus-start gnus-cloud gnus-spec emacsbug sendmail
gxref apropos timezone eieio-opt speedbar sb-image ezimage dframe
reposition cl-print edebug help-fns radix-tree xref project ert debug
hippie-exp misearch multi-isearch browse-kill-ring bash colir color ivy
delsel ivy-overlay ffap mm-archive conf-mode image-file org-rmail
org-mhe org-irc org-info org-gnus org-docview doc-view org-bibtex bibtex
org-bbdb org-w3m undo-tree diff vc-dir ewoc vc autorevert filenotify
sh-script smie executable autoconf autoconf-mode vc-dispatcher vc-svn
preview prv-emacs tex-buf font-latex latex tex-ispell tex-style tex dbus
xml crm tex-mode dired-aux vc-git diff-mode map view haskell-snippets
yasnippet network-stream starttls url-http url-gw nsm url-auth sow
which-key url-cache url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap paren rtags popup
repeat thingatpt cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs pdf-dev pdf-occur 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 cedet pdf-isearch
let-alist pdf-misc pdf-tools compile cus-edit cus-start cus-load
pdf-view bookmark pp jka-compr pdf-cache pdf-info tq pdf-util ob-shell
ob-plantuml org-www-bookmark org-protocol org-element avl-tree org
org-macro org-footnote org-pcomplete org-list org-faces org-entities
noutline outline org-version ob-emacs-lisp ob ob-tangle org-src ob-ref
ob-lob ob-table ob-keys ob-exp ob-comint ob-core ob-eval org-compat
org-macs org-loaddefs disp-table image-mode lib-kbd gnus-nnimap-format
nnimap nnmail gnus-int gnus-range mail-source message puny rfc822 mml
mml-sec epa epg mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader tls gnutls utf7 netrc nnoo
parse-time-rfc2822 info-look gnus-win iedit iedit-lib
multiple-cursors-core rect bind-key graphviz-dot-mode recentf
tree-widget tramp tramp-compat tramp-loaddefs trampver ucs-normalize
shell pcomplete comint ring parse-time format-spec advice derived
saveplace ewm edmacro kmacro ewm-configuration ewm-frame ewm-buffer
ibuf-macs ibuf-ext ibuffer ibuffer-loaddefs ewm-ruleset ewm-layout
find-func ewm-compat gnus nnheader gnus-util rmail rmail-loaddefs
rfc2047 rfc2045 ietf-drums mail-utils mm-util mail-prsvr wid-edit
cal-menu calendar cal-loaddefs calc calc-loaddefs calc-macs ewm-util
savehist server tsdh-dark-theme ekey dash dired-imenu imenu dired-x
dired dired-loaddefs desktop frameset easy-mmode ansi-color finder-inf
tex-site cl info package epg-config url-handlers url-parse auth-source
cl-seq eieio eieio-core cl-macs eieio-loaddefs password-cache url-vars
seq byte-opt subr-x gv bytecomp byte-compile cl-extra help-mode easymenu
cconv cl-loaddefs pcase cl-lib time-date mule-util tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors 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 composite charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray 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 1130920 1240746)
(symbols 48 72148 48)
(miscs 40 1245 8418)
(strings 32 198145 372410)
(string-bytes 1 7746812)
(vectors 16 94275)
(vector-slots 8 2251986 397758)
(floats 8 590 1337)
(intervals 56 63082 4479)
(buffers 968 342))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Fri, 17 Mar 2017 14:42:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
Thanks for the bug report.
> The descriptor returned by file-notify-add-watch is of the form (ID . DIR)
> (at least when using inotify) and these descriptors are stored in a
> hash-table using equal as comparator.
Yes.
> If multiple clients watch the same file and one of them removes its watch
> via file-notify-rm-watch, the result is unpredictable. I.e. the function
> removes some watch (I believe it is the last one added), but not necessarily
> the one associated with the client calling file-notify-rm-watch on the
> descriptor it got from calling file-notify-add-watch.
With the returned descriptors, it cannot be decided which watch has to
be removed, because both descriptors are equal. So we could decide
either to remove all watches for a given file if such a descriptor is
passed to `file-notify-rm-watch', or we must adapt
`file-notify-add-watch' to return different descriptors. I'm undecided
yet.
Note, that this problem seems to be specific to inotify. Other libraries
do not suffer from this problem, I believe.
> I've attached a test case for this.
Thanks. I've added this, modified, to `file-notify-test02-rm-watch' for
my internal tests. Will be pushed to master once I have a solution for
the problem.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Fri, 17 Mar 2017 15:00:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> With the returned descriptors, it cannot be decided which watch has to
> be removed, because both descriptors are equal. So we could decide
> either to remove all watches for a given file if such a descriptor is
> passed to `file-notify-rm-watch', or we must adapt
> `file-notify-add-watch' to return different descriptors. I'm undecided
> yet.
Surely, we want the ability of two or more different packages being able
to watch the same file, without interfering with each other. Why are
you undecided ?
I could try and find a solution, if you don't have time or anything.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Fri, 17 Mar 2017 16:09:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
> Michael Albinus <michael.albinus <at> gmx.de> writes:
>
>> With the returned descriptors, it cannot be decided which watch has to
>> be removed, because both descriptors are equal. So we could decide
>> either to remove all watches for a given file if such a descriptor is
>> passed to `file-notify-rm-watch', or we must adapt
>> `file-notify-add-watch' to return different descriptors. I'm undecided
>> yet.
>
> Surely, we want the ability of two or more different packages being able
> to watch the same file, without interfering with each other. Why are
> you undecided ?
I need to dig into inotify.c. IIRC, it wasn't simple to achieve this;
that's why we have this crude solution. Maybe it could be solved only on
filenotify.el level, don't remember the details.
> I could try and find a solution, if you don't have time or anything.
I don't have time, indeed. If you want to work on this, I would
appreciate. You could always ping me if you have questions.
Pls don't forget, whatever you propose must work for at least 6
different backends: inotify, kqueue, gfilenotify, w32notify and the
remote gvfs-monitor-dir and inotifywait. Heavy tests, always.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Fri, 17 Mar 2017 17:47:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> I need to dig into inotify.c. IIRC, it wasn't simple to achieve this;
> that's why we have this crude solution. Maybe it could be solved only on
> filenotify.el level, don't remember the details.
What I would do, is box the internal descriptor in file-notify-add-watch,
to make it unique to the caller and unbox it in file-notify-rm-watch and
file-notify-valid-p.
A different question is, whether file-notify-add-watch should behave
like add-hook with regards to adding a function multiple times. I don't
think it does at the moment, so I probably leave it as such (?).
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 18 Mar 2017 08:31:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
> What I would do, is box the internal descriptor in file-notify-add-watch,
> to make it unique to the caller and unbox it in file-notify-rm-watch and
> file-notify-valid-p.
Sounds OK to me. Do you want to do this only for inotify, or also for
the other backends?
> A different question is, whether file-notify-add-watch should behave
> like add-hook with regards to adding a function multiple times. I don't
> think it does at the moment, so I probably leave it as such (?).
I believe adding the *same* function twice for the same file/directory
doesn't make sense. Maybe we should document this in the Elisp manual,
and test it also in `file-notify-test01-add-watch'.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 18 Mar 2017 13:33:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>> A different question is, whether file-notify-add-watch should behave
>> like add-hook [...]
>
> I believe adding the *same* function twice for the same file/directory
> doesn't make sense. Maybe we should document this in the Elisp manual,
> and test it also in `file-notify-test01-add-watch'.
There is a check for this in file-notify-add-watch:
(unless (member entry (cdr registered))
(puthash desc ... file-notify-descriptors))
So, I should probably stay this way. (Even so, one might favor it being
differently, because file-notify-add-watch returns a cookie and thus has
more like a transactional semantics, for lack of a better word).
I took a deeper look filenotify.el and found some problems/have some
further questions. Note that I use inotify.
+ 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 ?
+ Watching a /dir/file may receive events (e.g. touch /dir) for dir.
+ Why the seemingly arbitrary exclusion of backup-files in
file-notify-callback ? What if someone wants to track the creation of
said files ?
+ 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.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 18 Mar 2017 19:29:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 26126 <at> debbugs.gnu.org (full text, mbox):
I have another question:
Do you remember what this comment from file-notify-callback is talking
about ?
#+BEGIN_SRC emacs-lisp
((memq action '(moved rename))
;; The kqueue rename event does not return file1 in
;; case a file monitor is established.
(if (setq file1 (file-notify--event-file1-name event))
'renamed 'deleted))
#+END_SRC
And file-notify--event-file1-name basically returns nil, if the file1
argument of event is not a string. But this seems to contradict the
comment, which states that file1 *is* nil, implying that the stored
filename should be used, or not ?
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 18 Mar 2017 19:37:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
> I took a deeper look filenotify.el and found some problems/have some
> further questions. Note that I use inotify.
>
> + 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.
> + Watching a /dir/file may receive events (e.g. touch /dir) for dir.
Could you pls give an example?
> + 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.
> + 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?
> -ap
Best rewgards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 18 Mar 2017 19:50:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
> Do you remember what this comment from file-notify-callback is talking
> about ?
>
> #+BEGIN_SRC emacs-lisp
> ((memq action '(moved rename))
> ;; The kqueue rename event does not return file1 in
> ;; case a file monitor is established.
> (if (setq file1 (file-notify--event-file1-name event))
> 'renamed 'deleted))
> #+END_SRC
kqueue is the oldest of he supported native libraries. It is the only
one (IIRC), which does not return source and target file name in case of
a rename. That's why that the kqueue `rename' event is transformed into
a `deleted' event. This is what the comment is speaking about.
I admit it would be more understandable if there would be an overview of
the events and their arguments, the different libraries are sending.
> And file-notify--event-file1-name basically returns nil, if the file1
> argument of event is not a string. But this seems to contradict the
> comment, which states that file1 *is* nil, implying that the stored
> filename should be used, or not ?
The fourth slot of an event structure could be either a string (file1)
or a cookie, or nil. This is tried to check.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 18 Mar 2017 20:39:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 18 Mar 2017 20:50:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> [...] the kqueue `rename' event is transformed into a `deleted' event
OK, I misunderstood the comment then.
> The fourth slot of an event structure could be either a string (file1)
> or a cookie, or nil. This is tried to check.
Yes, I see.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sun, 19 Mar 2017 09:40:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sun, 19 Mar 2017 11:15:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
>>>> + Watching a /dir/file may receive events (e.g. touch /dir) for dir.
> Yes, I remember. For all backends except kqueue, we watch the
> directory. This is intended.
>
Sure, the back-ends mostly watch directories, except for kqueue. But is
this behavior also intended to be propagated to the clients of
filenotify.el ?
>>>> + 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.
>> 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.
Sorry, for not being clear: I was exclusively talking about
file-name-handler here. Passing the intended filename is more general
then passing the directory only. Think of some program foonotify, which
is similarly limited like kqueue. Granted, this scenario probably won't
come up very soon.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sun, 19 Mar 2017 19:25:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
> Sure, the back-ends mostly watch directories, except for kqueue. But is
> this behavior also intended to be propagated to the clients of
> filenotify.el ?
Yes.
>> 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.
>
> Sorry, for not being clear: I was exclusively talking about
> file-name-handler here. Passing the intended filename is more general
> then passing the directory only. Think of some program foonotify, which
> is similarly limited like kqueue. Granted, this scenario probably won't
> come up very soon.
We have only two different programs for the remote case, gvfs-monitor-dir
and inotifywait. Coincidentally, I'm also the Tramp maintainer, so I
know what I am saying :-)
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sun, 19 Mar 2017 22:06:01 GMT)
Full text and
rfc822 format available.
Message #50 received at 26126 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
I think there are two ways to fix the main problem discussed here.
1. Change inotify.c and make it return/receive a unique descriptor per client.
2. Wrap inotify's descriptor inside file-notify-add-watch, similar to
how it's currently done. This is what I was refering to as
boxing/unboxing earlier.
The 2nd approach is problematic in the context of file-name-handler, so
I attempted to solve it the other way: Instead of a single number, use a
cons of
(INOTIFY-DESCRIPTOR . ID)
, which is unique across all active watches. I.e. this makes inotify
work like all the other back-ends/handler. Here is a first draft of a
corresponding patch, let me know what you think.
[Message part 2 (text/x-diff, inline)]
diff --git a/lisp/filenotify.el b/lisp/filenotify.el
index 7eb6229976..beae94c2c2 100644
--- a/lisp/filenotify.el
+++ b/lisp/filenotify.el
@@ -55,9 +55,8 @@ file-notify--rm-descriptor
"Remove DESCRIPTOR from `file-notify-descriptors'.
DESCRIPTOR should be an object returned by `file-notify-add-watch'.
If it is registered in `file-notify-descriptors', a stopped event is sent."
- (let* ((desc (if (consp descriptor) (car descriptor) descriptor))
- (registered (gethash desc file-notify-descriptors))
- (file (if (consp descriptor) (cdr descriptor) (cl-caadr registered)))
+ (let* ((registered (gethash descriptor file-notify-descriptors))
+ (file (cl-caadr registered))
(dir (car registered)))
(when (consp registered)
@@ -69,12 +68,12 @@ file-notify--rm-descriptor
;; Modify `file-notify-descriptors'.
(if (not file)
- (remhash desc file-notify-descriptors)
+ (remhash descriptor file-notify-descriptors)
(setcdr registered
(delete (assoc file (cdr registered)) (cdr registered)))
(if (null (cdr registered))
- (remhash desc file-notify-descriptors)
- (puthash desc registered file-notify-descriptors))))))
+ (remhash descriptor file-notify-descriptors)
+ (puthash descriptor registered file-notify-descriptors))))))
;; This function is used by `inotify', `kqueue', `gfilenotify' and
;; `w32notify' events.
@@ -102,9 +101,9 @@ file-notify--pending-event
(defun file-notify--event-watched-file (event)
"Return file or directory being watched.
Could be different from the directory watched by the backend library."
- (let* ((desc (if (consp (car event)) (caar event) (car event)))
- (registered (gethash desc file-notify-descriptors))
- (file (if (consp (car event)) (cdar event) (cl-caadr registered)))
+ (let* ((descriptor (car event))
+ (registered (gethash descriptor file-notify-descriptors))
+ (file (cl-caadr registered))
(dir (car registered)))
(if file (expand-file-name file dir) dir)))
@@ -133,17 +132,11 @@ file-notify--event-cookie
;; `inotify' returns the same descriptor when the file (directory)
;; uses the same inode. We want to distinguish, and apply a virtual
;; descriptor which make the difference.
-(defun file-notify--descriptor (desc file)
+(defun file-notify--descriptor (desc _file)
"Return the descriptor to be used in `file-notify-*-watch'.
For `gfilenotify' and `w32notify' it is the same descriptor as
used in the low-level file notification package."
- (if (and (natnump desc) (eq file-notify--library 'inotify))
- (cons desc
- (and (stringp file)
- (car (assoc
- (file-name-nondirectory file)
- (gethash desc file-notify-descriptors)))))
- desc))
+ desc)
;; The callback function used to map between specific flags of the
;; respective file notifications, and the ones we return.
@@ -396,7 +389,6 @@ file-notify-add-watch
;; Modify `file-notify-descriptors'.
(setq file (unless (file-directory-p file) (file-name-nondirectory file))
- desc (if (consp desc) (car desc) desc)
registered (gethash desc file-notify-descriptors)
entry `(,file . ,callback))
(unless (member entry (cdr registered))
@@ -408,9 +400,8 @@ file-notify-add-watch
(defun file-notify-rm-watch (descriptor)
"Remove an existing watch specified by its DESCRIPTOR.
DESCRIPTOR should be an object returned by `file-notify-add-watch'."
- (let* ((desc (if (consp descriptor) (car descriptor) descriptor))
- (file (if (consp descriptor) (cdr descriptor)))
- (registered (gethash desc file-notify-descriptors))
+ (let* ((file nil)
+ (registered (gethash descriptor file-notify-descriptors))
(dir (car registered))
(handler (and (stringp dir)
(find-file-name-handler dir 'file-notify-rm-watch))))
@@ -432,7 +423,7 @@ file-notify-rm-watch
((eq file-notify--library 'kqueue) 'kqueue-rm-watch)
((eq file-notify--library 'gfilenotify) 'gfile-rm-watch)
((eq file-notify--library 'w32notify) 'w32notify-rm-watch))
- desc))
+ descriptor))
(file-notify-error nil)))
;; Modify `file-notify-descriptors'.
@@ -441,9 +432,8 @@ file-notify-rm-watch
(defun file-notify-valid-p (descriptor)
"Check a watch specified by its DESCRIPTOR.
DESCRIPTOR should be an object returned by `file-notify-add-watch'."
- (let* ((desc (if (consp descriptor) (car descriptor) descriptor))
- (file (if (consp descriptor) (cdr descriptor)))
- (registered (gethash desc file-notify-descriptors))
+ (let* ((file nil)
+ (registered (gethash descriptor file-notify-descriptors))
(dir (car registered))
handler)
@@ -464,7 +454,7 @@ file-notify-valid-p
((eq file-notify--library 'kqueue) 'kqueue-valid-p)
((eq file-notify--library 'gfilenotify) 'gfile-valid-p)
((eq file-notify--library 'w32notify) 'w32notify-valid-p))
- desc))
+ descriptor))
t))))
;; The end:
diff --git a/src/inotify.c b/src/inotify.c
index 61ef615328..302f52225e 100644
--- a/src/inotify.c
+++ b/src/inotify.c
@@ -51,10 +51,47 @@ static int inotifyfd = -1;
static Lisp_Object watch_list;
static Lisp_Object
-make_watch_descriptor (int wd)
+add_watch_descriptor (int wd, Lisp_Object filename, Lisp_Object callback)
{
- /* TODO replace this with a Misc Object! */
- return make_number (wd);
+ Lisp_Object descriptor = make_number (wd);
+ Lisp_Object elt = Fassoc (descriptor, watch_list);
+ Lisp_Object list = Fcdr (elt);
+ Lisp_Object watch, watch_id;
+ int id = 0;
+
+ while (! NILP (list))
+ {
+ id = max (id, 1 + XINT (XCAR (XCAR (list))));
+ list = XCDR (list);
+ }
+
+ watch_id = make_number (id);
+ watch = list3 (watch_id, filename, callback);
+
+ if (NILP (elt))
+ watch_list = Fcons (Fcons (descriptor, Fcons (watch, Qnil)),
+ watch_list);
+ else
+ XSETCDR (elt, Fcons (watch, XCDR (elt)));
+
+ return Fcons (descriptor, watch_id);
+}
+
+static void
+remove_watch_descriptor (Lisp_Object descriptor, Lisp_Object id)
+{
+ Lisp_Object watches = Fassoc (descriptor, watch_list);
+
+ if (CONSP (watches))
+ {
+ Lisp_Object watch = Fassoc (id, XCDR (watches));
+
+ if (CONSP (watch))
+ XSETCDR (watches, Fdelete (watch, XCDR (watches)));
+
+ if (NILP (XCDR (watches)))
+ watch_list = Fdelete (watches, watch_list);
+ }
}
static Lisp_Object
@@ -96,7 +133,7 @@ mask_to_aspects (uint32_t mask) {
}
static Lisp_Object
-inotifyevent_to_event (Lisp_Object watch_object, struct inotify_event const *ev)
+inotifyevent_to_event (Lisp_Object watch, struct inotify_event const *ev)
{
Lisp_Object name = Qnil;
if (ev->len > 0)
@@ -106,13 +143,13 @@ inotifyevent_to_event (Lisp_Object watch_object, struct inotify_event const *ev)
name = DECODE_FILE (name);
}
else
- name = XCAR (XCDR (watch_object));
+ name = XCAR (XCDR (watch));
- return list2 (list4 (make_watch_descriptor (ev->wd),
+ return list2 (list4 (Fcons (make_number (ev->wd), XCAR (watch)),
mask_to_aspects (ev->mask),
name,
make_number (ev->cookie)),
- Fnth (make_number (2), watch_object));
+ Fnth (make_number (2), watch));
}
/* This callback is called when the FD is available for read. The inotify
@@ -121,7 +158,6 @@ static void
inotify_callback (int fd, void *_)
{
struct input_event event;
- Lisp_Object watch_object;
int to_read;
char *buffer;
ssize_t n;
@@ -146,20 +182,23 @@ inotify_callback (int fd, void *_)
while (i < (size_t)n)
{
struct inotify_event *ev = (struct inotify_event *) &buffer[i];
+ Lisp_Object descriptor = make_number (ev->wd);
+ Lisp_Object watches = Fassoc (descriptor, watch_list);
- watch_object = Fassoc (make_watch_descriptor (ev->wd), watch_list);
- if (!NILP (watch_object))
+ if (CONSP (watches))
{
- event.arg = inotifyevent_to_event (watch_object, ev);
+ for (Lisp_Object elt = XCDR (watches); CONSP (elt); elt = XCDR (elt))
+ {
+ event.arg = inotifyevent_to_event (XCAR (elt), ev);
+ if (!NILP (event.arg))
+ kbd_buffer_store_event (&event);
+ }
/* If event was removed automatically: Drop it from watch list. */
if (ev->mask & IN_IGNORED)
- watch_list = Fdelete (watch_object, watch_list);
-
- if (!NILP (event.arg))
- kbd_buffer_store_event (&event);
+ for (Lisp_Object elt = XCDR (watches); CONSP (elt); elt = XCDR (elt))
+ remove_watch_descriptor (descriptor, XCAR (elt));
}
-
i += sizeof (*ev) + ev->len;
}
@@ -292,14 +331,12 @@ renames (moved-from and moved-to).
See inotify(7) and inotify_add_watch(2) for further information. The inotify fd
is managed internally and there is no corresponding inotify_init. Use
`inotify-rm-watch' to remove a watch.
- */)
+ */)
(Lisp_Object file_name, Lisp_Object aspect, Lisp_Object callback)
{
uint32_t mask;
- Lisp_Object watch_object;
Lisp_Object encoded_file_name;
- Lisp_Object watch_descriptor;
- int watchdesc = -1;
+ int wd = -1;
CHECK_STRING (file_name);
@@ -314,22 +351,11 @@ is managed internally and there is no corresponding inotify_init. Use
mask = aspect_to_inotifymask (aspect);
encoded_file_name = ENCODE_FILE (file_name);
- watchdesc = inotify_add_watch (inotifyfd, SSDATA (encoded_file_name), mask);
- if (watchdesc == -1)
+ wd = inotify_add_watch (inotifyfd, SSDATA (encoded_file_name), mask);
+ if (wd == -1)
report_file_notify_error ("Could not add watch for file", file_name);
- watch_descriptor = make_watch_descriptor (watchdesc);
-
- /* Delete existing watch object. */
- watch_object = Fassoc (watch_descriptor, watch_list);
- if (!NILP (watch_object))
- watch_list = Fdelete (watch_object, watch_list);
-
- /* Store watch object in watch list. */
- watch_object = list3 (watch_descriptor, encoded_file_name, callback);
- watch_list = Fcons (watch_object, watch_list);
-
- return watch_descriptor;
+ return add_watch_descriptor (wd, file_name, callback);
}
DEFUN ("inotify-rm-watch", Finotify_rm_watch, Sinotify_rm_watch, 1, 1, 0,
@@ -341,16 +367,24 @@ See inotify_rm_watch(2) for more information.
*/)
(Lisp_Object watch_descriptor)
{
- Lisp_Object watch_object;
- int wd = XINT (watch_descriptor);
- if (inotify_rm_watch (inotifyfd, wd) == -1)
- report_file_notify_error ("Could not rm watch", watch_descriptor);
+ Lisp_Object descriptor, id;
- /* Remove watch descriptor from watch list. */
- watch_object = Fassoc (watch_descriptor, watch_list);
- if (!NILP (watch_object))
- watch_list = Fdelete (watch_object, watch_list);
+ if (! (CONSP (watch_descriptor)
+ && INTEGERP (XCAR (watch_descriptor))
+ && INTEGERP (XCDR (watch_descriptor))))
+ report_file_notify_error ("Invalid descriptor ", watch_descriptor);
+
+ descriptor = XCAR (watch_descriptor);
+ id = XCDR (watch_descriptor);
+ remove_watch_descriptor (descriptor, id);
+
+ if (NILP (Fassoc (descriptor, watch_list)))
+ {
+ int wd = XINT (descriptor);
+ if (inotify_rm_watch (inotifyfd, wd) == -1)
+ report_file_notify_error ("Could not rm watch", watch_descriptor);
+ }
/* Cleanup if no more files are watched. */
if (NILP (watch_list))
@@ -374,10 +408,34 @@ reason. Removing the watch by calling `inotify-rm-watch' also makes
it invalid. */)
(Lisp_Object watch_descriptor)
{
- Lisp_Object watch_object = Fassoc (watch_descriptor, watch_list);
- return NILP (watch_object) ? Qnil : Qt;
+ Lisp_Object watches;
+
+ if (! (CONSP (watch_descriptor)
+ && INTEGERP (XCAR (watch_descriptor))
+ && INTEGERP (XCDR (watch_descriptor))))
+ return Qnil;
+
+ watches = Fassoc (XCAR (watch_descriptor), watch_list);
+
+ if (! CONSP (watches))
+ return Qnil;
+ return CONSP (Fassoc (XCDR (watch_descriptor), XCDR (watches)));
+}
+
+#ifdef DEBUG
+DEFUN ("inotify-watch-list", Finotify_watch_list, Sinotify_watch_list, 0, 0, 0,
+ doc: /* Return a copy of the internal watch_list. */)
+{
+ return Fcopy_sequence (watch_list);
}
+DEFUN ("inotify-allocated-p", Finotify_allocated_p, Sinotify_allocated_p, 0, 0, 0,
+ doc: /* Return non-nil, if a inotify instance is allocated. */)
+{
+ return inotifyfd < 0 ? Qnil : Qt;
+}
+#endif
+
void
syms_of_inotify (void)
{
@@ -414,6 +472,10 @@ syms_of_inotify (void)
defsubr (&Sinotify_rm_watch);
defsubr (&Sinotify_valid_p);
+#ifdef DEBUG
+ defsubr (&Sinotify_watch_list);
+ defsubr (&Sinotify_allocated_p);
+#endif
staticpro (&watch_list);
Fprovide (intern_c_string ("inotify"), Qnil);
[Message part 3 (text/plain, inline)]
Apart from that, the following is a list containing all the problems
I've found that I still think are relevant. Some of which we already
discussed earlier.
* Don't discriminate remote handler based on the local library used.
Already discussed.
[Message part 4 (text/x-diff, inline)]
diff -u --label /home/politza/src/emacs/lisp/filenotify.el --label \#\<buffer\ filenotify.el\> /home/politza/src/emacs/lisp/filenotify.el /tmp/buffer-content-142424Gt
--- /home/politza/src/emacs/lisp/filenotify.el
+++ #<buffer filenotify.el>
@@ -342,10 +342,7 @@
;; 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)
+ dir
flags callback))
;; Check, whether Emacs has been compiled with file notification
Diff finished. Sun Mar 19 23:05:00 2017
[Message part 5 (text/plain, inline)]
* The value of file-notify--pending-event is reset after the first
client was processed in the outer loop in file-notify-callback,
resulting in clients watching for the same file being treated
differently. Note that this problem would be solved by not having
multiple clients per descriptor.
[pending-event-test.el (application/emacs-lisp, inline)]
[Message part 7 (text/plain, inline)]
* inotify_add_watch internally uses a single watch per directory, which
may be shared by multiple clients (using filenotify.el). The problem
here seems to be that these clients may use different FLAGS as
argument to file-notify-add-watch. Currently, the last added client's
FLAGS become the effective mask for the underlying C-descriptor,
meaning that clients added before may not receive change or
attribute-change events if the mask was modified accordingly.
* It seems to me that the right word here is "unified".
[Message part 8 (text/x-diff, inline)]
diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index 9b6752c5e1..4f7d47305f 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -2707,7 +2707,7 @@ File Notifications
Since all these libraries emit different events on notified file
changes, there is the Emacs library @code{filenotify} which provides a
-unique interface.
+unified interface.
@defun file-notify-add-watch file flags callback
Add a watch for filesystem events pertaining to @var{file}. This
[Message part 9 (text/plain, inline)]
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Mon, 20 Mar 2017 20:41:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>
>> Sure, the back-ends mostly watch directories, except for kqueue. But is
>> this behavior also intended to be propagated to the clients of
>> filenotify.el ?
>
> Yes.
Why is that ?
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Tue, 21 Mar 2017 08:46:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
>>> Sure, the back-ends mostly watch directories, except for kqueue. But is
>>> this behavior also intended to be propagated to the clients of
>>> filenotify.el ?
>>
>> Yes.
>
> Why is that ?
We've discussed this a while ago, main reason (IIRC) was to achieve
same behaviour for the different libraries.
I cannot find the discussion just now, likely it was in one of the
bugs. The main reason was the w32notify library, which works only
watching directories.
Later on we've added the kqueue library, which works reliably only
watching files. This breaks the unification attempt, but I don't believe
we shall change the behaviour again.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Tue, 21 Mar 2017 13:06:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
Thanks for your hard work!
> I think there are two ways to fix the main problem discussed here.
>
> 1. Change inotify.c and make it return/receive a unique descriptor per client.
>
> 2. Wrap inotify's descriptor inside file-notify-add-watch, similar to
> how it's currently done. This is what I was refering to as
> boxing/unboxing earlier.
>
> The 2nd approach is problematic in the context of file-name-handler, so
> I attempted to solve it the other way: Instead of a single number, use a
> cons of
>
> (INOTIFY-DESCRIPTOR . ID)
>
> , which is unique across all active watches. I.e. this makes inotify
> work like all the other back-ends/handler.
I agree with you, that's the best choice.
> Here is a first draft of a
> corresponding patch, let me know what you think.
I've applied the patch, and filenotify-tests.el passes all tests
(except `file-notify-test04-autorevert-remote', but that's another
story). So I believe it is OK to apply it to master, and see how it goes
(waiting for feedback).
Some comments:
> diff --git a/lisp/filenotify.el b/lisp/filenotify.el
> index 7eb6229976..beae94c2c2 100644
> --- a/lisp/filenotify.el
> +++ b/lisp/filenotify.el
> @@ -133,17 +132,11 @@ file-notify--event-cookie
> ;; `inotify' returns the same descriptor when the file (directory)
> ;; uses the same inode. We want to distinguish, and apply a virtual
> ;; descriptor which make the difference.
> -(defun file-notify--descriptor (desc file)
> +(defun file-notify--descriptor (desc _file)
> "Return the descriptor to be used in `file-notify-*-watch'.
> For `gfilenotify' and `w32notify' it is the same descriptor as
> used in the low-level file notification package."
> - (if (and (natnump desc) (eq file-notify--library 'inotify))
> - (cons desc
> - (and (stringp file)
> - (car (assoc
> - (file-name-nondirectory file)
> - (gethash desc file-notify-descriptors)))))
> - desc))
> + desc)
In this case, we shall remove `file-notify--descriptor', and replace all
calls by the `desc' argument.
> @@ -408,9 +400,8 @@ file-notify-add-watch
> (defun file-notify-rm-watch (descriptor)
> "Remove an existing watch specified by its DESCRIPTOR.
> DESCRIPTOR should be an object returned by `file-notify-add-watch'."
> - (let* ((desc (if (consp descriptor) (car descriptor) descriptor))
> - (file (if (consp descriptor) (cdr descriptor)))
> - (registered (gethash desc file-notify-descriptors))
> + (let* ((file nil)
> + (registered (gethash descriptor file-notify-descriptors))
I'm not sure we can eliminate the `file' binding. I believe, it is
needed for the kqueue library. Maybe you add a TODO comment for
retesting instead.
(My virtual machine running BSD is in a bad shape. I should reanimate it.)
> @@ -441,9 +432,8 @@ file-notify-rm-watch
> (defun file-notify-valid-p (descriptor)
> "Check a watch specified by its DESCRIPTOR.
> DESCRIPTOR should be an object returned by `file-notify-add-watch'."
> - (let* ((desc (if (consp descriptor) (car descriptor) descriptor))
> - (file (if (consp descriptor) (cdr descriptor)))
> - (registered (gethash desc file-notify-descriptors))
> + (let* ((file nil)
> + (registered (gethash descriptor file-notify-descriptors))
dito.
> diff --git a/src/inotify.c b/src/inotify.c
> index 61ef615328..302f52225e 100644
> --- a/src/inotify.c
> +++ b/src/inotify.c
> +#ifdef DEBUG
Please use a more specific flag, like INOTIFY_DEBUG.
> Apart from that, the following is a list containing all the problems
> I've found that I still think are relevant. Some of which we already
> discussed earlier.
>
> * Don't discriminate remote handler based on the local library used.
> Already discussed.
In general I agree it looks like a bug. But prior removing, I would like
to retest with the kqueue library.
> * The value of file-notify--pending-event is reset after the first
> client was processed in the outer loop in file-notify-callback,
> resulting in clients watching for the same file being treated
> differently. Note that this problem would be solved by not having
> multiple clients per descriptor.
Makes sense.
> (ert-deftest file-notify-test03c-events ()
> "Check that rename events are propagated to all watches."
> (skip-unless (file-notify--test-local-enabled))
> (unwind-protect
> (progn
> (setq file-notify--test-tmpfile (file-notify--test-make-temp-name)
> file-notify--test-tmpfile1 (file-notify--test-make-temp-name))
> (with-temp-file file-notify--test-tmpfile1)
> (setq file-notify--test-desc (file-notify-add-watch
> file-notify--test-tmpfile
> '(change attribute-change)
> #'file-notify--test-event-handler)
> file-notify--test-desc1 (file-notify-add-watch
> file-notify--test-tmpfile
> '(change attribute-change)
> (lambda (event)
> (file-notify--test-event-handler event))))
> (file-notify--test-with-events '(renamed renamed)
> (rename-file file-notify--test-tmpfile1
> file-notify--test-tmpfile))
> (file-notify-rm-watch file-notify--test-desc)
> (file-notify-rm-watch file-notify--test-desc1)
> (file-notify--test-cleanup-p))
> (file-notify--test-cleanup)))
I'm a little bit undecided, whether we shall add this as extra test
case, or whether we shall integrate it into
`file-notify-test03-events'. The former might be better, but it would
also mean that we shall break down `file-notify-test03-events' into
smaller tests.
> * inotify_add_watch internally uses a single watch per directory, which
> may be shared by multiple clients (using filenotify.el). The problem
> here seems to be that these clients may use different FLAGS as
> argument to file-notify-add-watch. Currently, the last added client's
> FLAGS become the effective mask for the underlying C-descriptor,
> meaning that clients added before may not receive change or
> attribute-change events if the mask was modified accordingly.
I'm aware of this problem (it happens also for other libraries, I
believe). No idea yet whether it is important to fix it. But maybe you
add a TODO entry at the end of filenotify.el.
> * It seems to me that the right word here is "unified".
>
> diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
> index 9b6752c5e1..4f7d47305f 100644
> --- a/doc/lispref/os.texi
> +++ b/doc/lispref/os.texi
> @@ -2707,7 +2707,7 @@ File Notifications
>
> Since all these libraries emit different events on notified file
> changes, there is the Emacs library @code{filenotify} which provides a
> -unique interface.
> +unified interface.
>
> @defun file-notify-add-watch file flags callback
> Add a watch for filesystem events pertaining to @var{file}. This
My English is not good enough to decide what's better. But I don't
object if you want to change.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Tue, 21 Mar 2017 15:07:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>> 1. Change inotify.c and make it return/receive a unique descriptor per client.
>
> I agree with you, that's the best choice.
>
Ok.
>> Here is a first draft of a
>> corresponding patch, let me know what you think.
>
> I've applied the patch, and filenotify-tests.el passes all tests
> (except `file-notify-test04-autorevert-remote', but that's another
> story). So I believe it is OK to apply it to master, and see how it goes
> (waiting for feedback).
Let me work on this a little more. I think, I'm not removing the
descriptors in inotify.c correctly.
>
> Some comments:
>
>> diff --git a/lisp/filenotify.el b/lisp/filenotify.el
>> -(defun file-notify--descriptor (desc file)
>> +(defun file-notify--descriptor (desc _file)
> In this case, we shall remove `file-notify--descriptor', and replace all
> calls by the `desc' argument.
Yes, and since (with the patch added) we now have a one-to-one relation
between clients and descriptors across all implementations, we could
also simplify the hash values.
>> @@ -408,9 +400,8 @@ file-notify-add-watch
>> (defun file-notify-rm-watch (descriptor)
>> "Remove an existing watch specified by its DESCRIPTOR.
>> DESCRIPTOR should be an object returned by `file-notify-add-watch'."
>> - (let* ((desc (if (consp descriptor) (car descriptor) descriptor))
>> - (file (if (consp descriptor) (cdr descriptor)))
>> - (registered (gethash desc file-notify-descriptors))
>> + (let* ((file nil)
>> + (registered (gethash descriptor file-notify-descriptors))
>
> I'm not sure we can eliminate the `file' binding. I believe, it is
> needed for the kqueue library. Maybe you add a TODO comment for
> retesting instead.
Shouldn't be, since kqueue, w32notify and gfilenotify all return a
pointer wrapped in a Lisp-Integer, i.e. for these back-ends the file
value was already nil all the time.
> (My virtual machine running BSD is in a bad shape. I should reanimate
> it.)
We should have a server, doing this sort of thing.
>> diff --git a/src/inotify.c b/src/inotify.c
>> index 61ef615328..302f52225e 100644
>> --- a/src/inotify.c
>> +++ b/src/inotify.c
>> +#ifdef DEBUG
>
> Please use a more specific flag, like INOTIFY_DEBUG.
Will do.
>> (ert-deftest file-notify-test03c-events () [...]
> I'm a little bit undecided, whether we shall add this as extra test
> case, or whether we shall integrate it into
> `file-notify-test03-events'. The former might be better, but it would
> also mean that we shall break down `file-notify-test03-events' into
> smaller tests.
I think it would be better to split those tests into smaller units. For
once it makes it easier to determine which should-form actually failed.
And secondly, it makes it easier to add a new test (especially for
people not to familiar with the code), without being anxious about
interfering with existing ones.
>> * inotify_add_watch internally uses a single watch per directory, which
>> may be shared by multiple clients (using filenotify.el). The problem
>> here seems to be that these clients may use different FLAGS as
>> argument to file-notify-add-watch. Currently, the last added client's
>> FLAGS become the effective mask for the underlying C-descriptor,
>> meaning that clients added before may not receive change or
>> attribute-change events if the mask was modified accordingly.
>
> I'm aware of this problem (it happens also for other libraries, I
> believe). No idea yet whether it is important to fix it. But maybe you
> add a TODO entry at the end of filenotify.el.
I think, it is important. For example, auto-revert's file-notify
mechanism (using '(change attribute-change) as flags) would break, if
some other package decides to watch the same file, but for
attribute-changes only.
It seems to me that this only affects inotify, since all other back-ends
return a newly created descriptor, but I haven't explicitly checked
this.
>
>> * It seems to me that the right word here is "unified".
>> Since all these libraries emit different events on notified file
>> changes, there is the Emacs library @code{filenotify} which provides a
>> -unique interface.
>> +unified interface.
> My English is not good enough to decide what's better. But I don't
> object if you want to change.
I would translate it in this context as "einzigartig"
vs. "vereinheitlicht". Native speakers to the rescue !
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Tue, 21 Mar 2017 15:39:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 26126 <at> debbugs.gnu.org (full text, mbox):
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Date: Tue, 21 Mar 2017 09:44:52 +0100
> Cc: 26126 <at> debbugs.gnu.org
>
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>
> >>> Sure, the back-ends mostly watch directories, except for kqueue. But is
> >>> this behavior also intended to be propagated to the clients of
> >>> filenotify.el ?
> >>
> >> Yes.
> >
> > Why is that ?
>
> We've discussed this a while ago, main reason (IIRC) was to achieve
> same behaviour for the different libraries.
That discussion was about local notifications, where indeed the
situation is like you describe.
But Andreas asks about calling remote handlers, about which we by
definition know much less. In that context, it might indeed make
sense to pass the file, not its parent directory, because the handler
can easily reconstruct the parent directory if that's what it needs.
By contrast, there's no way for the handler to intuit the file which
was stripped.
WDYT?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Tue, 21 Mar 2017 15:55:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 26126 <at> debbugs.gnu.org (full text, mbox):
> From: Andreas Politz <politza <at> hochschule-trier.de>
> Date: Tue, 21 Mar 2017 16:06:22 +0100
> Cc: 26126 <at> debbugs.gnu.org
>
> >> * It seems to me that the right word here is "unified".
>
> >> Since all these libraries emit different events on notified file
> >> changes, there is the Emacs library @code{filenotify} which provides a
> >> -unique interface.
> >> +unified interface.
>
> > My English is not good enough to decide what's better. But I don't
> > object if you want to change.
>
> I would translate it in this context as "einzigartig"
> vs. "vereinheitlicht". Native speakers to the rescue !
I'm not a native speaker of English, but "unified" is the correct word
here.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Tue, 21 Mar 2017 15:58:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>
>>>> Sure, the back-ends mostly watch directories, except for kqueue. But is
>>>> this behavior also intended to be propagated to the clients of
>>>> filenotify.el ?
> We've discussed this a while ago, main reason (IIRC) was to achieve
> same behaviour for the different libraries.
>
> I cannot find the discussion just now, likely it was in one of the
> bugs. The main reason was the w32notify library, which works only
> watching directories.
>
> Later on we've added the kqueue library, which works reliably only
> watching files. This breaks the unification attempt, but I don't believe
> we shall change the behaviour again.
Ok, I try to find it, but note that this behavior was introduced with
the following (kqueue-only) commit
commit 7bf54d01159eb09bae3c9cd86f2af0812d9afdf6
Author: Michael Albinus <michael.albinus <at> gmx.de>
Date: Fri Jan 22 19:56:09 2016 +0100
Backport kqueue integration from master
Maybe we can work on unifying the behavior across back-ends at a later
time, while taking some pointers from other projects, e.g.
https://github.com/emcrisostomo/fswatch .
If it stays this way, it should probably be documented.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Tue, 21 Mar 2017 19:01:01 GMT)
Full text and
rfc822 format available.
Message #74 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> But Andreas asks about calling remote handlers, about which we by
> definition know much less. In that context, it might indeed make
> sense to pass the file, not its parent directory, because the handler
> can easily reconstruct the parent directory if that's what it needs.
> By contrast, there's no way for the handler to intuit the file which
> was stripped.
>
> WDYT?
Actually, I was talking about both aspects and incidentally, in this
context about the other one. I refer to packages using filenotify.el as
clients.
But, yes, what you're expressing, is what I was trying to get at before.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 12:58:01 GMT)
Full text and
rfc822 format available.
Message #77 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
>> Later on we've added the kqueue library, which works reliably only
>> watching files. This breaks the unification attempt, but I don't believe
>> we shall change the behaviour again.
>
> Ok, I try to find it, but note that this behavior was introduced with
> the following (kqueue-only) commit
>
> commit 7bf54d01159eb09bae3c9cd86f2af0812d9afdf6
> Author: Michael Albinus <michael.albinus <at> gmx.de>
> Date: Fri Jan 22 19:56:09 2016 +0100
>
> Backport kqueue integration from master
My FreeBSD 10 VM has been reanimated. Last time I've used it was June
2016, so everything is dusty there.
As time permits, I'll check the kqueue vs remote handlers status.
> Maybe we can work on unifying the behavior across back-ends at a later
> time, while taking some pointers from other projects, e.g.
> https://github.com/emcrisostomo/fswatch .
Looks interesting, but I have no idea about its state. Last commit is
from 23 Jul 2016; is it still maintained actively?
A similar approach like fswatch is the already integrated gfilenotify
library. Honestly, this is the most problematic backend.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 13:18:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
>> I've applied the patch, and filenotify-tests.el passes all tests
>> (except `file-notify-test04-autorevert-remote', but that's another
>> story). So I believe it is OK to apply it to master, and see how it goes
>> (waiting for feedback).
>
> Let me work on this a little more. I think, I'm not removing the
> descriptors in inotify.c correctly.
OK. Take your time.
>> I'm not sure we can eliminate the `file' binding. I believe, it is
>> needed for the kqueue library. Maybe you add a TODO comment for
>> retesting instead.
>
> Shouldn't be, since kqueue, w32notify and gfilenotify all return a
> pointer wrapped in a Lisp-Integer, i.e. for these back-ends the file
> value was already nil all the time.
We shall recheck, once your changed inotify implementation has hit the repo.
>> I'm a little bit undecided, whether we shall add this as extra test
>> case, or whether we shall integrate it into
>> `file-notify-test03-events'. The former might be better, but it would
>> also mean that we shall break down `file-notify-test03-events' into
>> smaller tests.
>
> I think it would be better to split those tests into smaller units. For
> once it makes it easier to determine which should-form actually failed.
> And secondly, it makes it easier to add a new test (especially for
> people not to familiar with the code), without being anxious about
> interfering with existing ones.
Likely yes. I have the same feeling, but haven't done due to lack of
energy and time.
For the time being, I have added a modified version of your test
removing watch descriptors out of order to
`file-notify-test02-rm-watch'. Since this fails for inotify, I've added
an :expected-result tag to this test. Can be removed, when fixed in
inotify.c.
>>> * inotify_add_watch internally uses a single watch per directory, which
>>> may be shared by multiple clients (using filenotify.el). The problem
>>> here seems to be that these clients may use different FLAGS as
>>> argument to file-notify-add-watch. Currently, the last added client's
>>> FLAGS become the effective mask for the underlying C-descriptor,
>>> meaning that clients added before may not receive change or
>>> attribute-change events if the mask was modified accordingly.
>>
>> I'm aware of this problem (it happens also for other libraries, I
>> believe). No idea yet whether it is important to fix it. But maybe you
>> add a TODO entry at the end of filenotify.el.
>
> I think, it is important. For example, auto-revert's file-notify
> mechanism (using '(change attribute-change) as flags) would break, if
> some other package decides to watch the same file, but for
> attribute-changes only.
>
> It seems to me that this only affects inotify, since all other back-ends
> return a newly created descriptor, but I haven't explicitly checked
> this.
So I would let it for you to implement it in inotify.c. When there is
also a respective test, we will see whether it is a problem for other
backends, too.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 13:24:01 GMT)
Full text and
rfc822 format available.
Message #83 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> We've discussed this a while ago, main reason (IIRC) was to achieve
>> same behaviour for the different libraries.
>
> That discussion was about local notifications, where indeed the
> situation is like you describe.
>
> But Andreas asks about calling remote handlers, about which we by
> definition know much less.
Nope. We know exactly which remote handlers are called, and how they
behave. Do you expect other implementations of remote handlers? I don't,
because I'm not aware of other candidates but inotifywait and gvfs-monitor-dir.
> In that context, it might indeed make sense to pass the file, not its
> parent directory, because the handler can easily reconstruct the
> parent directory if that's what it needs. By contrast, there's no way
> for the handler to intuit the file which was stripped.
>
> WDYT?
I still don't understand what's the difference between local and remote
events in your eyes. I've tried to implement remote handlers to behave
exactly like the local ones. That's the Tramp philosophy.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 15:46:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 26126 <at> debbugs.gnu.org (full text, mbox):
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: politza <at> hochschule-trier.de, 26126 <at> debbugs.gnu.org
> Date: Wed, 22 Mar 2017 14:23:47 +0100
>
> > But Andreas asks about calling remote handlers, about which we by
> > definition know much less.
>
> Nope. We know exactly which remote handlers are called, and how they
> behave.
We know that about our handlers, yes. But that doesn't have to be the
end of the story. Emacs is extensible.
> Do you expect other implementations of remote handlers?
Yes, why not? It's much easier to do that in Lisp than in C, where
the local handlers should be implemented.
> > In that context, it might indeed make sense to pass the file, not its
> > parent directory, because the handler can easily reconstruct the
> > parent directory if that's what it needs. By contrast, there's no way
> > for the handler to intuit the file which was stripped.
> >
> > WDYT?
>
> I still don't understand what's the difference between local and remote
> events in your eyes.
See above. Admittedly, this is a minor point, so not worth arguing if
you disagree with my POV.
> I've tried to implement remote handlers to behave exactly like the
> local ones. That's the Tramp philosophy.
Right, but in this case there are 2 flavors of local handlers, and the
question is on which of them to model the remote ones.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 16:02:01 GMT)
Full text and
rfc822 format available.
Message #89 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> I've tried to implement remote handlers to behave exactly like the
>> local ones. That's the Tramp philosophy.
>
> Right, but in this case there are 2 flavors of local handlers, and the
> question is on which of them to model the remote ones.
IIRC, it was driven by you to let all handlers behave like directory
monitors. The kqueue case came later, and it is the only exception as of
today.
II also RC, there was the idea to offer another function, which returns
the type of a monitor, being a file monitor or a directory monitor. This
idea was given up after all monitors were forced to behave like a
directory monitor; prior the existence of kqueue. Maybe we shall rethink
about this idea? Is this of practical use?
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 16:14:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 26126 <at> debbugs.gnu.org (full text, mbox):
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: politza <at> hochschule-trier.de, 26126 <at> debbugs.gnu.org
> Date: Wed, 22 Mar 2017 17:01:13 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> I've tried to implement remote handlers to behave exactly like the
> >> local ones. That's the Tramp philosophy.
> >
> > Right, but in this case there are 2 flavors of local handlers, and the
> > question is on which of them to model the remote ones.
>
> IIRC, it was driven by you to let all handlers behave like directory
> monitors.
Yes, that's right. But I was surely only thinking about local
handlers.
> II also RC, there was the idea to offer another function, which returns
> the type of a monitor, being a file monitor or a directory monitor. This
> idea was given up after all monitors were forced to behave like a
> directory monitor; prior the existence of kqueue. Maybe we shall rethink
> about this idea? Is this of practical use?
I wouldn't start working on this before there's a real need.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 16:25:02 GMT)
Full text and
rfc822 format available.
Message #95 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> II also RC, there was the idea to offer another function, which returns
>> the type of a monitor, being a file monitor or a directory monitor. This
>> idea was given up after all monitors were forced to behave like a
>> directory monitor; prior the existence of kqueue. Maybe we shall rethink
>> about this idea? Is this of practical use?
>
> I wouldn't start working on this before there's a real need.
Agreed, until somebody shows the real need for this distinction.
> Thanks.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 17:35:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> A similar approach like fswatch is the already integrated gfilenotify
> library. Honestly, this is the most problematic backend.
In terms of "unintegratedness", i.e. does behave differently depending
on the back-end used ? (I know from the tests that it may use polling
every 5 secs.)
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 17:44:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>>>> * inotify_add_watch internally uses a single watch per directory, which
>>>> may be shared by multiple clients (using filenotify.el). The problem
>>>> here seems to be that these clients may use different FLAGS as
>>>> argument to file-notify-add-watch. [...]
> So I would let it for you to implement it in inotify.c. When there is
> also a respective test, we will see whether it is a problem for other
> backends, too.
This touches the question whether we're operating under the assumption,
that other Lisp-code (apart from filenotify.el) may freely call
inotify-add-watch or not. Because if it does, this has to be handled in
inotify.c. Otherwise we may handle it in filenotify.el .
If I do it in inotify.c: Is it possible to store a bit-mask (of type
uint32_t) in a Lisp_Object ? Since this would be the easiest make this
change.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 18:50:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
>> A similar approach like fswatch is the already integrated gfilenotify
>> library. Honestly, this is the most problematic backend.
>
> In terms of "unintegratedness", i.e. does behave differently depending
> on the back-end used ? (I know from the tests that it may use polling
> every 5 secs.)
Yes, this was also a problem. Sometimes, the promised polling on mounted
filesystems didn't work reliably.
There were also bugs which have restricted use of gfilenotify.
And there are also reservations by people, who do not want to use glib /
gio infrastructure.
Due to the existence of inotify and kqueue monitors in Emacs, the only
major use of gfilenotify seems to be in Cygwin.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 18:58:01 GMT)
Full text and
rfc822 format available.
Message #107 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
> This touches the question whether we're operating under the assumption,
> that other Lisp-code (apart from filenotify.el) may freely call
> inotify-add-watch or not. Because if it does, this has to be handled in
> inotify.c. Otherwise we may handle it in filenotify.el .
It is still possible to call inotify-add-watch and companions. However,
we don't force this use; the Elisp manual decribes only filenoty.el
functionality.
And I don't know any example people are using initofy-add-watch directly.
> If I do it in inotify.c: Is it possible to store a bit-mask (of type
> uint32_t) in a Lisp_Object ? Since this would be the easiest make this
> change.
Sure, but Emacs integers guarantee you only 30 bits. But why do you want
to do this? You could map your bit mask into a complex Lisp object, a
vector with 32 slots, or whatever. This makes access/manipulation in
Lisp much more simple.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 19:41:01 GMT)
Full text and
rfc822 format available.
Message #110 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
Hi Andreas,
>> * Don't discriminate remote handler based on the local library used.
>> Already discussed.
>
> In general I agree it looks like a bug. But prior removing, I would like
> to retest with the kqueue library.
I've tested now with kqueue. There was an error indeed; I've fixed this
in master.
There are still some other issues running filenotify-tests.el; looks
like I should spend more time for kqueue related tests.
>> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Wed, 22 Mar 2017 20:04:01 GMT)
Full text and
rfc822 format available.
Message #113 received at 26126 <at> debbugs.gnu.org (full text, mbox):
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Date: Wed, 22 Mar 2017 19:57:10 +0100
> Cc: 26126 <at> debbugs.gnu.org
>
> > This touches the question whether we're operating under the assumption,
> > that other Lisp-code (apart from filenotify.el) may freely call
> > inotify-add-watch or not. Because if it does, this has to be handled in
> > inotify.c. Otherwise we may handle it in filenotify.el .
>
> It is still possible to call inotify-add-watch and companions. However,
> we don't force this use; the Elisp manual decribes only filenoty.el
> functionality.
IMO, Lisp programs should always go through filenotify.el, never via
backend-specific APIs.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Thu, 23 Mar 2017 07:37:02 GMT)
Full text and
rfc822 format available.
Message #116 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Michael Albinus <michael.albinus <at> gmx.de>
>> Date: Wed, 22 Mar 2017 19:57:10 +0100
>> Cc: 26126 <at> debbugs.gnu.org
>>
>> > This touches the question whether we're operating under the assumption,
>> > that other Lisp-code (apart from filenotify.el) may freely call
>> > inotify-add-watch or not. Because if it does, this has to be handled in
>> > inotify.c. Otherwise we may handle it in filenotify.el .
>>
>> It is still possible to call inotify-add-watch and companions. However,
>> we don't force this use; the Elisp manual decribes only filenoty.el
>> functionality.
>
> IMO, Lisp programs should always go through filenotify.el, never via
> backend-specific APIs.
Then we shall emphasize this in the manual. Like this?
--8<---------------cut here---------------start------------->8---
diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index 9b6752c..09f7c18 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -2707,7 +2707,8 @@ File Notifications
Since all these libraries emit different events on notified file
changes, there is the Emacs library @code{filenotify} which provides a
-unique interface.
+unified interface. It is highly recommended to use this library
+instead of the native ones.
@defun file-notify-add-watch file flags callback
Add a watch for filesystem events pertaining to @var{file}. This
--8<---------------cut here---------------end--------------->8---
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Thu, 23 Mar 2017 15:23:01 GMT)
Full text and
rfc822 format available.
Message #119 received at 26126 <at> debbugs.gnu.org (full text, mbox):
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: politza <at> hochschule-trier.de, 26126 <at> debbugs.gnu.org
> Date: Thu, 23 Mar 2017 08:36:39 +0100
>
> > IMO, Lisp programs should always go through filenotify.el, never via
> > backend-specific APIs.
>
> Then we shall emphasize this in the manual. Like this?
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
> index 9b6752c..09f7c18 100644
> --- a/doc/lispref/os.texi
> +++ b/doc/lispref/os.texi
> @@ -2707,7 +2707,8 @@ File Notifications
>
> Since all these libraries emit different events on notified file
> changes, there is the Emacs library @code{filenotify} which provides a
> -unique interface.
> +unified interface. It is highly recommended to use this library
> +instead of the native ones.
I'd say that even more definitively:
Lisp programs that want to receive file notifications should always
use this library in preference to the native ones.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Thu, 23 Mar 2017 16:11:01 GMT)
Full text and
rfc822 format available.
Message #122 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> I'd say that even more definitively:
>
> Lisp programs that want to receive file notifications should always
> use this library in preference to the native ones.
Committed to master.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Fri, 24 Mar 2017 19:55:02 GMT)
Full text and
rfc822 format available.
Message #125 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Michael Albinus <michael.albinus <at> gmx.de>
>> Do you expect other implementations of remote handlers?
>
> Yes, why not? It's much easier to do that in Lisp than in C, where
> the local handlers should be implemented.
I just wanted to add, that file-handler do not necessarily have to refer
to files, e.g. think of a /proc kind of handler making internal Emacs
state available via filenames.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Fri, 24 Mar 2017 20:45:02 GMT)
Full text and
rfc822 format available.
Message #128 received at 26126 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hey !
Below is a second version of the previous patch. It is somewhat
conservative, since neither did I attempt to
+ further simplify filenotify.el nor
+ handle differing masks in inotify.c .
--
I also thought about the test-cases and more generally about how to
develop a specification for this library, i.e. how do we want this to
behave. Do we have the desire that it works uniformly across all
participating back-ends ? And is that even possible ?
I think it is to easy to adapt the tests for each back-end, until they
succeed and thereby potentially masking actual bugs.
One way to go about this would be to write a series of definitive
unit-tests which specify the intended behavior. Then, allow them to fail
for a specific back-end, until someone has fixed potential bugs for it
and confirmed that the test succeeds. This would allow for an
incremental improvement on fairly solid grounds. I'm assuming that
people of the future are interested in improving their used back-end
(e.g. make kqueue watch directories properly, if that is possible).
Anyway, I was bored today, so I took a look at what events these
libraries actually produce, the result of which you may also find below.
Finally, I'm tempted to suggest to get rid of the flags argument of
file-notify-add-watch. As it is, things are already complicated enough
and we don't seem to have many people working on this. I think we could
make it backward-compatible to a certain degree. Note also, that many
file operations trigger both kinds of events anyway.
--
[Message part 2 (test/x-patch, inline)]
[Message part 3 (text/plain, inline)]
--
[Message part 4 (text/plain, inline)]
inotify w32notify kqueue gfilenotify gvfs-monitor-dir inotifywait
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
file-create file created file created file skipped created file created file created file
deleted file deleted file deleted file attr-changed file attr-changed file
stopped file stopped file stopped file deleted file deleted file
stopped file stopped file
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
file-read file deleted file deleted file deleted file deleted file deleted file deleted file
stopped file stopped file stopped file stopped file stopped file stopped file
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
file-write file changed file changed file attr-changed file changed file changed file changed file
deleted file changed file changed file changed file changed file changed file
stopped file deleted file deleted file deleted file attr-changed file attr-changed file
stopped file stopped file stopped file deleted file deleted file
stopped file stopped file
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
file-attrib file attr-changed file changed file attr-changed file attr-changed file attr-changed file attr-changed file
deleted file deleted file deleted file deleted file deleted file deleted file
stopped file stopped file stopped file stopped file stopped file stopped file
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
file-rename dir renamed dir/src dir/dest deleted dir/dest deleted dir renamed dir/src dir/dest renamed dir/src dir/dest renamed dir/src dir/dest
created dir/dest renamed dir/src dir/dest stopped dir deleted dir/dest attr-changed dir/dest attr-changed dir/dest
deleted dir/dest timedout 12 deleted dir attr-changed dir/dest attr-changed dir/dest
deleted dir stopped dir stopped dir deleted dir/dest deleted dir/dest
stopped dir deleted dir deleted dir
stopped dir stopped dir
dir/src deleted dir timedout 12 deleted dir/src renamed dir/src dir/dest renamed dir/src dir/dest renamed dir/src dir/dest
stopped dir/src stopped dir/src stopped dir/src stopped dir/src stopped dir/src stopped dir/src
deleted dir/src
dir/dest renamed dir/src dir/dest deleted dir/dest deleted dir/dest renamed dir/src dir/dest renamed dir/src dir/dest renamed dir/src dir/dest
deleted dir/dest stopped dir/dest stopped dir/dest deleted dir/dest attr-changed dir/dest attr-changed dir/dest
stopped dir/dest stopped dir/dest attr-changed dir/dest attr-changed dir/dest
deleted dir/dest deleted dir/dest
stopped dir/dest stopped dir/dest
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dir-create dir created dir created dir skipped created dir created dir created anon-1
deleted dir deleted dir deleted dir deleted dir deleted anon-1
stopped dir stopped dir stopped dir stopped dir timedout 12
stopped dir
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dir-read dir deleted dir timedout 12 deleted dir deleted dir deleted dir deleted dir
stopped dir stopped dir stopped dir stopped dir stopped dir stopped dir
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dir-create-file dir deleted dir/file deleted dir/file deleted dir deleted dir/file deleted dir/file deleted dir/file
deleted dir timedout 12 stopped dir deleted dir deleted dir deleted dir
stopped dir stopped dir stopped dir stopped dir stopped dir
dir/file deleted dir/file deleted dir/file deleted dir/file deleted dir/file deleted dir/file deleted dir/file
stopped dir/file stopped dir/file stopped dir/file stopped dir/file stopped dir/file stopped dir/file
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dir-create-dir dir deleted dir/dir deleted dir/dir deleted dir deleted dir/dir deleted dir/dir deleted dir
deleted dir timedout 12 stopped dir deleted dir deleted dir stopped dir
stopped dir stopped dir stopped dir stopped dir
dir/dir deleted dir/dir timedout 12 deleted dir/dir deleted dir/dir deleted dir/dir deleted dir/dir
stopped dir/dir stopped dir/dir stopped dir/dir stopped dir/dir stopped dir/dir stopped dir/dir
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dir-attrib dir attr-changed dir timedout 12 attr-changed dir attr-changed dir attr-changed dir attr-changed dir
deleted dir stopped dir deleted dir deleted dir deleted dir deleted dir
stopped dir stopped dir stopped dir stopped dir stopped dir
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dir-rename dir renamed dir/src dir/dest deleted dir/src skipped renamed dir/src dir/dest deleted dir/src renamed dir anon-1
deleted dir timedout 12 deleted dir deleted dir deleted dir
stopped dir stopped dir stopped dir stopped dir stopped dir
dir/src deleted dir/src timedout 12 skipped deleted dir/src deleted dir/src deleted dir/src
stopped dir/src stopped dir/src stopped dir/src stopped dir/src stopped dir/src
dir/dest renamed dir/src dir/dest created dir/dest skipped created dir/dest created dir/dest attr-changed anon-1
deleted dir/dest deleted dir/dest deleted dir/dest attr-changed dir/dest attr-changed anon-1
stopped dir/dest stopped dir/dest stopped dir/dest attr-changed dir/dest deleted anon-1
deleted dir/dest deleted anon-1
stopped dir/dest timedout 12
stopped dir/dest
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
;; Local Variables:
;; truncate-lines: t
;; End:
[Message part 5 (text/plain, inline)]
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 06:37:02 GMT)
Full text and
rfc822 format available.
Message #131 received at 26126 <at> debbugs.gnu.org (full text, mbox):
> From: Andreas Politz <politza <at> hochschule-trier.de>
> Date: Fri, 24 Mar 2017 21:44:28 +0100
> Cc: 26126 <at> debbugs.gnu.org
>
> I also thought about the test-cases and more generally about how to
> develop a specification for this library, i.e. how do we want this to
> behave. Do we have the desire that it works uniformly across all
> participating back-ends ?
In general, yes.
> And is that even possible ?
Almost. Some corner cases (like what happens when the watched
directory is deleted) behave differently.
> One way to go about this would be to write a series of definitive
> unit-tests which specify the intended behavior. Then, allow them to fail
> for a specific back-end, until someone has fixed potential bugs for it
> and confirmed that the test succeeds.
That's how these tests were developed. What you see is the result; if
you have specific comments about some known failures, please spell
them out.
> Anyway, I was bored today, so I took a look at what events these
> libraries actually produce, the result of which you may also find below.
Thanks, but I have difficulty reading it. Could you please provide a
short legend?
> Finally, I'm tempted to suggest to get rid of the flags argument of
> file-notify-add-watch. As it is, things are already complicated enough
> and we don't seem to have many people working on this. I think we could
> make it backward-compatible to a certain degree. Note also, that many
> file operations trigger both kinds of events anyway.
The flags are there for the operations where the differences matter.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 08:59:02 GMT)
Full text and
rfc822 format available.
Message #134 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> Thanks, but I have difficulty reading it. Could you please provide a
> short legend?
Sorry, I forgot to do that.
The first column is the name of the test. The 2nd lists all watched
entities mapped to symbolic names. Further columns list the events for
specific back ends, which may refer to the same names.
Because kqueue does not support watching nonexistent files, in most
cases I did create them before starting to watch. That's why there are
usually no creation events.
In every test, I deleted all participating files as the last step. A
`(timeout n)' event means just that: We waited for n seconds without
receiving a stopped event.
If the event lists a name anon-xyz, it means that it contained the
filename of a non-watched file. This may signify a bug.
All test ran with the last patched I've posted applied.
>> Finally, I'm tempted to suggest to get rid of the flags argument of
>> file-notify-add-watch.
> The flags are there for the operations where the differences matter.
When does it matter ? The callback can just ignore the events its not
interested in. The sole advantage would be reduced complexity, but, of
course, it shouldn't be done, if this is not seen as a sufficient reason.
Also: I think in the end we want to add a layer above filenotify.el,
with the added ability of multiplexing events in a directory to multiple
watches, watching files in that directory. The current implementation
(including the patch) returns a new descriptor for every watch. This is
good for a low-level api, because it is easy to manage.
The downside is, that it does not scale very well. For example every
tramp watch starts a new process and (it seems to me) every w32 watch a
new thread.
And, coming back to the original point, in this case we usually need to
watch with all flags enabled anyway.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 12:52:01 GMT)
Full text and
rfc822 format available.
Message #137 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
Hi Andreas,
> I just wanted to add, that file-handler do not necessarily have to refer
> to files, e.g. think of a /proc kind of handler making internal Emacs
> state available via filenames.
I don't understand. File name handlers in Emacs provide alternative
implementations of basic file manipulation operations, see the list in
(info "(elisp) Magic File Names")
Do you propose to add a new file name handler for the "/proc" virtual
file system?
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 14:00:02 GMT)
Full text and
rfc822 format available.
Message #140 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Do you propose to add a new file name handler for the "/proc" virtual
> file system?
No, I propose that it is possible to do that.
And it seems that this is already a reality, e.g. url-file-handler.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 14:05:02 GMT)
Full text and
rfc822 format available.
Message #143 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
> Hey !
Hi Andreas,
> Below is a second version of the previous patch. It is somewhat
> conservative, since neither did I attempt to
>
> + further simplify filenotify.el nor
>
> + handle differing masks in inotify.c .
Thanks for this. Next time you provide a patch, could you pls merge
emacs recent changes from master first? Your patch was rejected partly,
and I had to apply rejected hunks manually. Comments:
> diff --git a/lisp/filenotify.el b/lisp/filenotify.el
> index 7eb6229976..5dea67b580 100644
> --- a/lisp/filenotify.el
> +++ b/lisp/filenotify.el
> @@ -25,6 +25,20 @@
> ;; file notification packages `inotify', `kqueue', `gfilenotify' and
> ;; `w32notify'.
>
> +;; TODO:
Pls move TODOs at the end of the file.
> +;; * "inotify_add_watch adds a new watch, or modifies an existing watch"
> +;; We need to make sure that different watches for the same directory
> +;; don't set the mask in a conflicting way regarding changed/attribute-changed
> +;; * Also check which other inotify flags are problematic
> +;; for concurrent use of the underlying descriptor
Well, I had always the hope to modify inotify watches in this case. If
there is a watch with flags f1, and a new watch for the same file is
requested with flags f2, and f2 contains a flag which is not part of f1,
then either the existing watch shall be adapted, or the existing watch
shall be removed, and a new shall be installed. Don't know what's
possible in inotify.
> @@ -48,16 +62,14 @@ file-notify-descriptors
>
> (DIR (FILE . CALLBACK) (FILE . CALLBACK) ...)
>
> -Several values for a given DIR happen only for `inotify', when
> -different files from the same directory are watched.")
> +Several values for a given DIR should currently not occur.")
Remove "currently". Every docstring speaks about current state.
> (defun file-notify-rm-watch (descriptor)
> "Remove an existing watch specified by its DESCRIPTOR.
> DESCRIPTOR should be an object returned by `file-notify-add-watch'."
> - (let* ((desc (if (consp descriptor) (car descriptor) descriptor))
> - (file (if (consp descriptor) (cdr descriptor)))
> - (registered (gethash desc file-notify-descriptors))
> + (let* ((file nil)
> (defun file-notify-valid-p (descriptor)
> "Check a watch specified by its DESCRIPTOR.
> DESCRIPTOR should be an object returned by `file-notify-add-watch'."
> - (let* ((desc (if (consp descriptor) (car descriptor) descriptor))
> - (file (if (consp descriptor) (cdr descriptor)))
> - (registered (gethash desc file-notify-descriptors))
> + (let* ((file nil)
In both functions I believe we don't need to bind `file'. The code could
be simplified, because (or (not file) ...) always succeeds.
Your changes look good; "make -C test filenotify-tests
SELECTOR='$(SELECTOR_DEFAULT)'" passes all tests. Even if there is room
for improvement I believe you could push your patch to master now, in
order to get feedback from other developers.
> I also thought about the test-cases and more generally about how to
> develop a specification for this library, i.e. how do we want this to
> behave. Do we have the desire that it works uniformly across all
> participating back-ends ? And is that even possible ?
As Eli said, that's the intention. But we cannot reach this goal
completely due to the different behaviour of the libraries.
> I think it is to easy to adapt the tests for each back-end, until they
> succeed and thereby potentially masking actual bugs.
That's what file-notify-test.el intends to do. Well, the code has
evolved over the time, and it is somehow hard to read. Improvements are
welcome!
> One way to go about this would be to write a series of definitive
> unit-tests which specify the intended behavior. Then, allow them to fail
> for a specific back-end, until someone has fixed potential bugs for it
> and confirmed that the test succeeds. This would allow for an
> incremental improvement on fairly solid grounds. I'm assuming that
> people of the future are interested in improving their used back-end
> (e.g. make kqueue watch directories properly, if that is possible).
Could you show an example how this shall look like?
> Anyway, I was bored today, so I took a look at what events these
> libraries actually produce, the result of which you may also find below.
Thanks; I'll review it next days.
> Finally, I'm tempted to suggest to get rid of the flags argument of
> file-notify-add-watch. As it is, things are already complicated enough
> and we don't seem to have many people working on this. I think we could
> make it backward-compatible to a certain degree. Note also, that many
> file operations trigger both kinds of events anyway.
I agree with you. I haven't seen any different use of the flags yet (but
I maybe wrong).
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 14:10:01 GMT)
Full text and
rfc822 format available.
Message #146 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
> And it seems that this is already a reality, e.g. url-file-handler.
Which works also over file names, like "http://host/path/to/file".
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 14:19:01 GMT)
Full text and
rfc822 format available.
Message #149 received at 26126 <at> debbugs.gnu.org (full text, mbox):
> From: Andreas Politz <politza <at> hochschule-trier.de>
> Cc: michael.albinus <at> gmx.de, 26126 <at> debbugs.gnu.org
> Date: Sat, 25 Mar 2017 09:57:52 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > Thanks, but I have difficulty reading it. Could you please provide a
> > short legend?
>
> Sorry, I forgot to do that.
>
> The first column is the name of the test. The 2nd lists all watched
> entities mapped to symbolic names. Further columns list the events for
> specific back ends, which may refer to the same names.
That still leaves a lot of unclear entries there. maybe it isn't
important.
> >> Finally, I'm tempted to suggest to get rid of the flags argument of
> >> file-notify-add-watch.
>
> > The flags are there for the operations where the differences matter.
>
> When does it matter ?
Why should we care? That's for the programmer to decide; we just give
them the tools.
> Also: I think in the end we want to add a layer above filenotify.el,
> with the added ability of multiplexing events in a directory to multiple
> watches, watching files in that directory.
That was the original plan, AFAIR, but this facility still has too few
users to try abstracting that higher layer, IMO.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 16:20:02 GMT)
Full text and
rfc822 format available.
Message #152 received at 26126 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>> + further simplify filenotify.el nor
>>
>> + handle differing masks in inotify.c .
>
> Thanks for this. Next time you provide a patch, could you pls merge
> emacs recent changes from master first? Your patch was rejected partly,
> and I had to apply rejected hunks manually. Comments:
Sorry, I forgot to pull.
Anyway, here is the more progressive version of the patch, adding both
of the above points. (I guess, I'm to conservative sometimes and/or
seeing only problems everywhere.)
>> +;; * "inotify_add_watch adds a new watch, or modifies an existing watch"
>> +;; We need to make sure that different watches for the same directory
>> +;; don't set the mask in a conflicting way regarding changed/attribute-changed
>> +;; * Also check which other inotify flags are problematic
>> +;; for concurrent use of the underlying descriptor
>
> Well, I had always the hope to modify inotify watches in this case. If
> there is a watch with flags f1, and a new watch for the same file is
> requested with flags f2, and f2 contains a flag which is not part of f1,
> then either the existing watch shall be adapted, or the existing watch
> shall be removed, and a new shall be installed. Don't know what's
> possible in inotify.
I implemented it by always using constantly watching for all events
(IN_ALL_EVENTS) and storing the given user-flags with the callback etc.
When an event occurs, I check whether it matches the given mask.
For that to work, I had to restrict the flag-usage by the user to those
not having an effect on the shared descriptor.
I also added IN_EXCL_UNLINK as a default. This avoids reporting events
for already deleted filenames, which are still opened by some process,
which seems what we want as a default.
> Your changes look good; "make -C test filenotify-tests
> SELECTOR='$(SELECTOR_DEFAULT)'" passes all tests. Even if there is room
> for improvement I believe you could push your patch to master now, in
> order to get feedback from other developers.
I ran the non-expensive tests for inotify, kqueue and they succeeded.
I have no push privileges.
[Message part 2 (test/x-patch, inline)]
[Message part 3 (text/plain, inline)]
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 16:22:01 GMT)
Full text and
rfc822 format available.
Message #155 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> That's what file-notify-test.el intends to do. Well, the code has
> evolved over the time, and it is somehow hard to read. Improvements are
> welcome!
OK, I'm going to work a little bit on this.
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 16:28:02 GMT)
Full text and
rfc822 format available.
Message #158 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>
>> And it seems that this is already a reality, e.g. url-file-handler.
>
> Which works also over file names, like "http://host/path/to/file".
If you think that's a file, how about this:
(url-handler-mode)
(find-file "http://www.google.com/?s=foo")
(file-notify-add-watch "http://www.google.com/?s=foo" '(change) #'ignore)
=> File notification error: "Directory does not exist", "http://www.google.com"
or this
(hypothetical-emacsfs-mode)
(find-file "/efs/recentf/some-recent-file")
(find-file "/efs/bookmarks/some-bookmark")
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 16:35:02 GMT)
Full text and
rfc822 format available.
Message #161 received at 26126 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
> That still leaves a lot of unclear entries there. maybe it isn't
> important.
Mmh. I attached the file which generated the table.
>> >> Finally, I'm tempted to suggest to get rid of the flags argument of
>> >> file-notify-add-watch.
>>
>> > The flags are there for the operations where the differences matter.
>>
>> When does it matter ?
>
> Why should we care? That's for the programmer to decide; we just give
> them the tools.
But this is not a restriction on the part of the programmer, as he can
still filter out the events he's not interested in.
>> Also: I think in the end we want to add a layer above filenotify.el,
> That was the original plan, ...
That's good to hear.
-ap
[check-filenotify.el (application/emacs-lisp, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 16:38:02 GMT)
Full text and
rfc822 format available.
Message #164 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
>> Which works also over file names, like "http://host/path/to/file".
>
> If you think that's a file, how about this:
I don't think it's a file; beause I don't know anything about it. I
think it's a file name, which makes a difference.
> (url-handler-mode)
> (find-file "http://www.google.com/?s=foo")
> (file-notify-add-watch "http://www.google.com/?s=foo" '(change) #'ignore)
> => File notification error: "Directory does not exist", "http://www.google.com"
What's wrong with this? `url-handler-mode' hasn't implemented
`file-notify-add-watch'. In such cases, the original implementation is
applied. And `file-notify-add-watch' is right in saying that this
"directory" doesn't exist, from its pov.
> or this
>
> (hypothetical-emacsfs-mode)
> (find-file "/efs/recentf/some-recent-file")
> (find-file "/efs/bookmarks/some-bookmark")
No problem. Write a file name handler which takes responsibility for all
files whose names start with "/efs", and here we are. It would still be
a handler which is called based on the file *name*.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 17:10:02 GMT)
Full text and
rfc822 format available.
Message #167 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
> Anyway, here is the more progressive version of the patch, adding both
> of the above points. (I guess, I'm to conservative sometimes and/or
> seeing only problems everywhere.)
Thanks!
>> Well, I had always the hope to modify inotify watches in this case. If
>> there is a watch with flags f1, and a new watch for the same file is
>> requested with flags f2, and f2 contains a flag which is not part of f1,
>> then either the existing watch shall be adapted, or the existing watch
>> shall be removed, and a new shall be installed. Don't know what's
>> possible in inotify.
>
> I implemented it by always using constantly watching for all events
> (IN_ALL_EVENTS) and storing the given user-flags with the callback etc.
> When an event occurs, I check whether it matches the given mask.
Sounds good.
> For that to work, I had to restrict the flag-usage by the user to those
> not having an effect on the shared descriptor.
What does this mean in practice? Any restriction we need to document?
> I also added IN_EXCL_UNLINK as a default. This avoids reporting events
> for already deleted filenames, which are still opened by some process,
> which seems what we want as a default.
OK.
> I have no push privileges.
I'm willing to push the patch in your name, if you provide me a ChangeLog
style commit message. For the future, I recommend to obtain push privileges.
Some nitpicks:
> --- a/lisp/filenotify.el
> +++ b/lisp/filenotify.el
> +(defun file-notify--watch-absolute-filename (watch)
This deserves a docstring.
> +handler. The value in the hash table is file-notify--watch
> +struct.")
Please quote `file-notify--watch'.
> (defun file-notify--rm-descriptor (descriptor)
> +DESCRIPTOR should be an object returned by
> +`file-notify-add-watch'. If it is registered in
> +`file-notify-descriptors', a stopped event is sent."
Don't reformat the docstring, keep the first line as complete sentence.
> - (dolist (action actions)
> + (while actions
> + (let ((action (pop actions)))
Being curious: why did you change this?
> --- a/src/inotify.c
> +++ b/src/inotify.c
> @@ -264,10 +360,6 @@ close
> The following symbols can also be added to a list of aspects:
>
> dont-follow
> -excl-unlink
> -mask-add
> -oneshot
> -onlydir
Maybe we shall say explicitely, that those inotify events are not supported.
> -COOKIE is an object that can be compared using `equal' to identify two matching
> +COOKIE is an object that can be compared using `equal' to identify two matchingt
Typo.
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 17:13:02 GMT)
Full text and
rfc822 format available.
Message #170 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Andreas Politz <politza <at> hochschule-trier.de> writes:
>
>>> Which works also over file names, like "http://host/path/to/file".
>>
>> If you think that's a file, how about this:
>
> I don't think it's a file; beause I don't know anything about it.
Then I'm confused, since I didn't mean to suggest that file-name-handler
work on anything other then strings.
>> (file-notify-add-watch "http://www.google.com/?s=foo" '(change) #'ignore)
>> => File notification error: "Directory does not exist", "http://www.google.com"
>
> What's wrong with this? `url-handler-mode' hasn't implemented
> `file-notify-add-watch'.
Yes, but the point is how would you ? By restricting watches to
directories, you are also forcing every handler to have a concept of
such an entity and to be able to watch it; rather then the original
"filename".
>> (hypothetical-emacsfs-mode)
> No problem.
Let's backtrack.
I was suggesting that providing the given filename as an argument to the
file-handler is more general than using it's directory. Your point as I
understood it, was, that the existing file-notify-add-watch
tramp-handler work just fine with directories, therefore such a change
is not needed.
My argument is that there may be other ways of using the
file-name-handler machinery, and I was presenting some examples.
I also fail to see any disadvantages (of using the given filename).
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 17:27:02 GMT)
Full text and
rfc822 format available.
Message #173 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
>> For that to work, I had to restrict the flag-usage by the user to those
>> not having an effect on the shared descriptor.
>
> What does this mean in practice? Any restriction we need to document?
I don't think so, apart from inotify-add-watch's doc-string. It means a
user of inotify can't use the following flags
IN_EXCL_UNLINK - already mentioned
IN_MASK_ADD - modifies the (shared) descriptor
IN_ONESHOT - monitor for one event only
IN_ONLYDIR - From the man page:
"Watch pathname only if it is a directory. Using this flag provides an applica‐
tion with a race-free way of ensuring that the monitored object is a directory."
This sounded esoteric enough for it to be excluded, i.e. I don't know
the exact behavior on a already existing descriptor and it does not
seam to be useful for our use-case.
> I'm willing to push the patch in your name, if you provide me a ChangeLog
> style commit message.
OK, will do.
> This deserves a docstring.
>
> Please quote `file-notify--watch'.
>
> Don't reformat the docstring, keep the first line as complete
> sentence.
OK.
>> - (dolist (action actions)
>
>> + (while actions
>> + (let ((action (pop actions)))
>
> Being curious: why did you change this?
actions is set to nil at one point inside the loop, but dolist creates
an alias for it, such that setting the variable would have no effect.
>> -excl-unlink
>> -mask-add
>> -oneshot
>> -onlydir
>
> Maybe we shall say explicitely, that those inotify events are not supported.
> Typo.
OK.
Thanks for the feedback,
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 18:19:01 GMT)
Full text and
rfc822 format available.
Message #176 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
>> (defun file-notify--rm-descriptor (descriptor)
>> +DESCRIPTOR should be an object returned by
>> +`file-notify-add-watch'. If it is registered in
>> +`file-notify-descriptors', a stopped event is sent."
>
> Don't reformat the docstring, keep the first line as complete sentence.
It's the second line:
(defun file-notify--rm-descriptor (descriptor)
"Remove DESCRIPTOR from `file-notify-descriptors'.
-DESCRIPTOR should be an object returned by `file-notify-add-watch'.
+DESCRIPTOR should be an object returned by
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 18:38:01 GMT)
Full text and
rfc822 format available.
Message #179 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
>>> (file-notify-add-watch "http://www.google.com/?s=foo" '(change) #'ignore)
>>> => File notification error: "Directory does not exist",
>>> "http://www.google.com"
>>
>> What's wrong with this? `url-handler-mode' hasn't implemented
>> `file-notify-add-watch'.
>
> Yes, but the point is how would you ? By restricting watches to
> directories, you are also forcing every handler to have a concept of
> such an entity and to be able to watch it; rather then the original
> "filename".
The error message says, that the upper directory of
"http://www.google.com/?s=foo", "http://www.google.com", does not
exist. This would be a problem for any file notification library. This
does not mean that the handler is forced to watch a directory only.
>>> (hypothetical-emacsfs-mode)
>
>> No problem.
>
> Let's backtrack.
>
> I was suggesting that providing the given filename as an argument to the
> file-handler is more general than using it's directory. Your point as I
> understood it, was, that the existing file-notify-add-watch
> tramp-handler work just fine with directories, therefore such a change
> is not needed.
>
> My argument is that there may be other ways of using the
> file-name-handler machinery, and I was presenting some examples.
>
> I also fail to see any disadvantages (of using the given filename).
Then I completely misunderstood your argumentation. What's your point
with (hypothetical-emacsfs-mode) ?
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 18:41:01 GMT)
Full text and
rfc822 format available.
Message #182 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
>> Don't reformat the docstring, keep the first line as complete sentence.
>
> It's the second line:
Ah, yes. But still, the docstring doesn't need to be
reformatted. Nitpick, as said ...
> -ap
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sat, 25 Mar 2017 19:36:02 GMT)
Full text and
rfc822 format available.
Message #185 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> This does not mean that the handler is forced to watch a directory
> only.
But it is:
#+BEGIN_SRC emacs-lisp
(defun file-notify-add-watch (file flags callback)
;;....
(let* ((handler (find-file-name-handler file 'file-notify-add-watch))
(dir (directory-file-name
(if (file-directory-p file)
file
(file-name-directory file))))
desc func l-flags)
(unless (file-directory-p dir)
(signal 'file-notify-error `("Directory does not exist" ,dir)))
(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 dir flags callback))
;;---------------------------------------------------------^
#+END_SRC
-ap
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Sun, 26 Mar 2017 07:09:02 GMT)
Full text and
rfc822 format available.
Message #188 received at 26126 <at> debbugs.gnu.org (full text, mbox):
Andreas Politz <politza <at> hochschule-trier.de> writes:
> Michael Albinus <michael.albinus <at> gmx.de> writes:
>
>> This does not mean that the handler is forced to watch a directory
>> only.
>
> But it is:
I was speaking solely about the message.
And yes, the file name handlers watch a directory. I wouldn't change
this, until there is the need to support a file name handler which
cannot implement it.
> -ap
Best regards, Michael.
Added tag(s) fixed.
Request was from
Michael Albinus <michael.albinus <at> gmx.de>
to
control <at> debbugs.gnu.org.
(Sun, 26 Mar 2017 09:44:02 GMT)
Full text and
rfc822 format available.
bug marked as fixed in version 26.1, send any further explanations to
26126 <at> debbugs.gnu.org and Andreas Politz <politza <at> hochschule-trier.de>
Request was from
Michael Albinus <michael.albinus <at> gmx.de>
to
control <at> debbugs.gnu.org.
(Sun, 26 Mar 2017 09:44:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#26126; Package
emacs.
(Thu, 30 Mar 2017 18:16:02 GMT)
Full text and
rfc822 format available.
Message #195 received at 26126 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thanks for writing and reviewing that code. I looked at inotify.c after
the patch was applied and found a few races and integer-overflow issues,
most of which have likely been lurking there for a while. I fixed the
problems that I found by installing the attached patch into master;
please give it a try when you have the time.
[0001-Some-inotify-cleanup.txt (text/plain, attachment)]
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Fri, 28 Apr 2017 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 6 years and 363 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.