Using glib's g_file_monitor_file and g_file_monitor_directory

Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Hi,

for GNU/Linux systems, we have inotify support now. As discussed when it
was added, there are some drawbacks. So it seems to work for local file
systems only; network file systems might work, or not.

There is an alternative: glib's functions g_file_monitor_file and
g_file_monitor_directory. They will use inotify when it is
applicable. Otherwise, they implement polling, which is something we
would need also in Emacs, when no native file monitoring is possible.

g_file_monitor_file works even for gvfs mounted filesystems (I've tested
with mounts of kind davs://..., smb://... and ftp://...). Likely, via
polling.

And it also has a kqueue backend for NetBSD (see
https://mail.gnome.org/archives/commits-list/2012-November/msg03867.html). I
don't know whether this is used widely, 'tho.

What about adding this interface to Emacs, on C level? It could be in
parallel to, or replacing the inotify interface.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Ted Zlatanov]

On Wed, 20 Mar 2013 21:40:26 +0100 Michael Albinus <michael.albinus@gmx.de> wrote: 

MA> for GNU/Linux systems, we have inotify support now. As discussed when it
MA> was added, there are some drawbacks. So it seems to work for local file
MA> systems only; network file systems might work, or not.

MA> There is an alternative: glib's functions g_file_monitor_file and
MA> g_file_monitor_directory. They will use inotify when it is
MA> applicable. Otherwise, they implement polling, which is something we
MA> would need also in Emacs, when no native file monitoring is possible.

MA> g_file_monitor_file works even for gvfs mounted filesystems (I've tested
MA> with mounts of kind davs://..., smb://... and ftp://...). Likely, via
MA> polling.

MA> And it also has a kqueue backend for NetBSD (see
MA> https://mail.gnome.org/archives/commits-list/2012-November/msg03867.html). I
MA> don't know whether this is used widely, 'tho.

MA> What about adding this interface to Emacs, on C level? It could be in
MA> parallel to, or replacing the inotify interface.

I am in favor, replacing the existing inotify backend and keeping the
same API if possible, but there should be a way to avoid polling, even
disable it by default, and I hope it's asynchronous to some degree.  You
can imagine how slow polling a ftp connection can be...

Ted

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Ted Zlatanov <tzz@lifelogs.com> writes:

> I am in favor, replacing the existing inotify backend and keeping the
> same API if possible, but there should be a way to avoid polling, even
> disable it by default, and I hope it's asynchronous to some degree.  You
> can imagine how slow polling a ftp connection can be...

As far as I understand the API, polling is started by g_file_monitor_*
only in case there is no native monitoring library, which could do the
job. If you don't monitor an ftp:// mounted file, there's problem. It's up
to you :-)

> Ted

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Ted Zlatanov]

On Thu, 21 Mar 2013 15:54:04 +0100 Michael Albinus <michael.albinus@gmx.de> wrote: 

MA> Ted Zlatanov <tzz@lifelogs.com> writes:
>> I am in favor, replacing the existing inotify backend and keeping the
>> same API if possible, but there should be a way to avoid polling, even
>> disable it by default, and I hope it's asynchronous to some degree.  You
>> can imagine how slow polling a ftp connection can be...

MA> As far as I understand the API, polling is started by g_file_monitor_*
MA> only in case there is no native monitoring library, which could do the
MA> job. If you don't monitor an ftp:// mounted file, there's problem. It's up
MA> to you :-)

My point is, currently, monitoring an ftp:// mount with inotify is
harmless, right?  You won't poll and it fails instantly?

Now suddenly we may start to poll.  This is unexpected behavior and
should be off by default, to prevent unwanted network connections and
general lag in Emacs.  I think that's generally desirable.

Ted

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Ted Zlatanov <tzz@lifelogs.com> writes:

> My point is, currently, monitoring an ftp:// mount with inotify is
> harmless, right?  You won't poll and it fails instantly?

Yes.

> Now suddenly we may start to poll.  This is unexpected behavior and
> should be off by default, to prevent unwanted network connections and
> general lag in Emacs.  I think that's generally desirable.

There won't be unwanted network connections. If you start to monitor
ftp://..., and this is not mounted already by gvfs, it won't be mounted
in the background just because of monitoring. You must mount it
explicitly by the gvfs-mount command, or by a file manager like nautilus
or Tramp.

> Ted

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

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

Hi,

> What about adding this interface to Emacs, on C level? It could be in
> parallel to, or replacing the inotify interface.

It would be helpful if Emacs maintainers would react. If there's no
interest, I could stop further work on this.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Jan Djärv]

Hello.

1 apr 2013 kl. 13:36 skrev Michael Albinus <michael.albinus@gmx.de>:

> Michael Albinus <michael.albinus@gmx.de> writes:
> 
> Hi,
> 
>> What about adding this interface to Emacs, on C level? It could be in
>> parallel to, or replacing the inotify interface.
> 
> It would be helpful if Emacs maintainers would react. If there's no
> interest, I could stop further work on this.

Not a maintainer, but it could be an alternative implementation when Glib is used.
How about implementing it (with suitable configure checks), and post a patch?

I don't see any technical reasons for not accepting it, if it enhances current interfaces.

	Jan D.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Stefan Monnier]

>> What about adding this interface to Emacs, on C level? It could be in
>> parallel to, or replacing the inotify interface.

That sounds good.  AFAIK Emacs almost always links to glib indirectly
nowadays, so it would probably be OK to replace the inotify version with
the glib version.


        Stefan

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[James Cloos]

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

SM> AFAIK Emacs almost always links to glib indirectly nowadays, so it
SM> would probably be OK to replace the inotify version with the glib
SM> version.

Please don't.  Not every box where emacs is used runs x11, and glib
should not be a forced dependency.  Inotify(7) support is welcome
even on headless systems.

It is OK for systems where glib is linked anyway to make more use of it,
but loosing existing features on glib-less systems isn't welcome.

For comparison, the version of trunk on my workstation starts at 300M of
VM; the nox version of trunk Julien packages for deb starts at 58M (both
amd64 compiles) and the 32-bit nox version at 15M.  RSS is of course less,
but the difference remains important for right-sized servers with 512M or
less (sometimes a lot less) of ram.

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: James Cloos <cloos@jhcloos.com>
> Date: Wed, 03 Apr 2013 15:21:20 -0400
> Cc: Michael Albinus <michael.albinus@gmx.de>, emacs-devel@gnu.org
> 
> For comparison, the version of trunk on my workstation starts at 300M of
> VM; the nox version of trunk Julien packages for deb starts at 58M (both
> amd64 compiles)

You mean, the memory footprint of "emacs -Q"?  If so, I'm surprised it
is so large.

Perhaps this includes all the libraries mapped into the program?  If
so, I don't think you should count them, because other X programs are
likely to share them, at least in part.

The 32-bit w32 Emacs starts at 13M, FWIW.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[James Cloos]

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

EZ> You mean, the memory footprint of "emacs -Q"?  If so, I'm surprised it
EZ> is so large.

Not -Q; what is actually used is more relevant.  I did just try -q -nox
though on the workstation.  That reduces the startup VM to 270M, for a
more fair comparison to what I run on the servers.  (I don't copy my
main .emacs over to them; they have radically different needs.)

EZ> Perhaps this includes all the libraries mapped into the program?

Yes.

EZ> If so, I don't think you should count them, because other X
EZ> programs are likely to share them, at least in part.

On GUI systems, but the point was to keep the ram and disk footprints
small -- or at least no larger then they already are -- on headless
systems, which wouldn't have anything else linking in glib.

EZ> The 32-bit w32 Emacs starts at 13M, FWIW.

The datapoint is welcome.

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

James Cloos <cloos@jhcloos.com> writes:

> SM> AFAIK Emacs almost always links to glib indirectly nowadays, so it
> SM> would probably be OK to replace the inotify version with the glib
> SM> version.
>
> Please don't.  Not every box where emacs is used runs x11, and glib
> should not be a forced dependency.  Inotify(7) support is welcome
> even on headless systems.

It shall be configurable. Glib's implementation might the primary
choice, pure inotify the second choice, and none of them the third
one. Except systems which use w32notify, of course.

I'll try to prepare a patch before my vacations (starting later in
April).

I will also think about a unified Lisp interface for the different file
notification implementations. During the discussion last autumn, there
was a proposal (by Oscar? by Eli?) to distinguish between "file contents
changes" and "file metadata changes". This sounds to me like a good
starting point, abstracting from the different events raised by those
notification packages.

And of course, respective file name handlers and there needs shall not
be forgotten.

> -JimC

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[James Cloos]

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

>> Inotify(7) support is welcome even on headless systems.

MA> It shall be configurable. Glib's implementation might the primary
MA> choice, pure inotify the second choice, and none of them the third
MA> one. Except systems which use w32notify, of course.

That works.

MA> I will also think about a unified Lisp interface for the different file
MA> notification implementations.

That also is welcome.

MA> And of course, respective file name handlers and there needs shall not
MA> be forgotten.

Thanks,

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Stefan Monnier]

> Please don't.  Not every box where emacs is used runs x11, and glib
> should not be a forced dependency.

AFAIK glib does not require X11.  Are you confusing it with Gtk, maybe?

   % aptitude show libglib2.0-0
   [...]
   Dépend: libc6 (>= 2.9), libffi5 (>= 3.0.4), libpcre3 (>= 8.10),
           libselinux1 (>= 1.32), zlib1g (>= 1:1.2.2)
   [...]
   Description : GLib library of C routines
    GLib is a library containing many useful C routines for things such
    as trees, hashes, lists, and strings.  It is a useful
    general-purpose C library used by projects such as GTK+, GIMP,
    and GNOME. 
    
    This package contains the shared libraries.  Site : http://www.gtk.org/


-- Stefan

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

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

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

>>> What about adding this interface to Emacs, on C level? It could be in
>>> parallel to, or replacing the inotify interface.
>
> That sounds good.  AFAIK Emacs almost always links to glib indirectly
> nowadays, so it would probably be OK to replace the inotify version with
> the glib version.

I've appended a patch which I have tested successfully under GNU/Linux.
There is a new gfilenotify.c, which uses glib's functionality. There is
also a new config option --with-file-notification=LIB, LIB can be "yes",
"no", "gfile" or "inotify". Default on non-w32 machines is "gfile" if
applicable. For w32 machines, this option is not taken into account yet,
emacs undonditionally links w32notify (maybe we want also support "no"
there?).

Furthermore, all file notification libraries use now the same lisp event
`file-notify' instead of `file-inotify' and `file-w32notify'.

Changelog entries and proper documentation are not part of this patch yet.

Comments on the patch are welcome. And we should also start to design an
upper layer over gfilenotify.c, inotify.c and w32notify.c, which
abstracts from the respective file notification events. They are very
different for every library, which complicates a convenient use in other
elisp packages. See the changes in autorevert.el in that patch.

>         Stefan

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bzr.diff --]
[-- Type: text/x-diff, Size: 24254 bytes --]

=== modified file 'autogen/config.in'
--- autogen/config.in	2013-05-16 16:37:05 +0000
+++ autogen/config.in	2013-05-21 08:25:51 +0000
@@ -132,7 +132,7 @@
    #if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__
    # define _FORTIFY_SOURCE 2
    #endif
-  
+
 
 /* Define to 1 if futimesat mishandles a NULL file name. */
 #undef FUTIMESAT_NULL_BUG
@@ -623,6 +623,9 @@
 /* Define to 1 if you have inet sockets. */
 #undef HAVE_INET_SOCKETS
 
+/* Define to 1 to use glib's notify. */
+#undef HAVE_GFILENOTIFY
+
 /* Define to 1 to use inotify. */
 #undef HAVE_INOTIFY
 
@@ -1836,4 +1839,3 @@
 mode: c
 End:
 */
-

=== modified file 'configure.ac'
--- configure.ac	2013-05-27 02:03:18 +0000
+++ configure.ac	2013-05-28 09:28:06 +0000
@@ -200,7 +200,23 @@
 OPTION_DEFAULT_ON([gsettings],[don't compile with GSettings support])
 OPTION_DEFAULT_ON([selinux],[don't compile with SELinux support])
 OPTION_DEFAULT_ON([gnutls],[don't use -lgnutls for SSL/TLS support])
-OPTION_DEFAULT_ON([inotify],[don't compile with inotify (file-watch) support])
+
+## In case of mingw32, it doesn't matter.
+AC_ARG_WITH([file-notification],[AS_HELP_STRING([--with-file-notification=LIB],
+ [use a file notification library (LIB one of: yes or gfile, inotify, no)])],
+ [	  case "${withval}" in
+	    y | ye | yes )	val=gfile ;;
+	    n | no )		val=no  ;;
+	    g | gf | gfi | gfil | gfile )	val=gfile ;;
+	    i | in | ino | inot | inoti | inotif | inotify )	val=inotify ;;
+	    * )
+AC_MSG_ERROR([`--with-file-notification=$withval' is invalid;
+this option's value should be `yes', `no', `gfile' or `inotify'.
+`yes' and `gfile' are synonyms.])
+	    ;;
+	  esac
+	  with_file_notification=$val
+])
 
 ## For the times when you want to build Emacs but don't have
 ## a suitable makeinfo, and can live without the manuals.
@@ -1668,7 +1684,6 @@
     W32_RES_LINK="-Wl,emacs.res"
   else
     W32_OBJ="$W32_OBJ w32.o w32console.o w32heap.o w32inevt.o w32proc.o"
-    W32_OBJ="$W32_OBJ w32notify.o"
     W32_LIBS="$W32_LIBS -lwinmm -lgdi32 -lcomdlg32"
     W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32 -lusp10"
     W32_RES_LINK="\$(EMACSRES)"
@@ -2294,16 +2309,38 @@
 AC_SUBST(LIBGNUTLS_LIBS)
 AC_SUBST(LIBGNUTLS_CFLAGS)
 
+dnl g_file_monitor exists since glib 2.18.  It has been tested under
+dnl GNU/Linux only.  We take precedence over inotify, but this makes
+dnl only sense when glib has been compiled with inotify support.  How
+dnl to check?
+NOTIFY_OBJ=
+NOTIFY_SUMMARY=no
+if test "${with_file_notification}" = "gfile" || test -z "${with_file_notification}"; then
+   PKG_CHECK_MODULES(GFILENOTIFY,  gio-2.0 >= 2.18, HAVE_GFILENOTIFY=yes, HAVE_GFILENOTIFY=no)
+   if test "$HAVE_GFILENOTIFY" = "yes"; then
+      AC_DEFINE(HAVE_GFILENOTIFY, 1, [Define to 1 if using GFile.])
+      LIBS="$LIBS $GFILENOTIFY_LIBS"
+      NOTIFY_OBJ=gfilenotify.o
+      NOTIFY_SUMMARY="yes -lgio (gfile)"
+   fi
+fi
 dnl inotify is only available on GNU/Linux.
-if test "${with_inotify}" = "yes"; then
+if test "${with_file_notification}" = "inotify" || (test -z "${with_file_notification}" && test -z "$NOTIFY_OBJ"); then
    AC_CHECK_HEADERS(sys/inotify.h)
    if test "$ac_cv_header_sys_inotify_h" = yes ; then
-     AC_CHECK_FUNC(inotify_init1)
-   fi
-fi
-if test "$ac_cv_func_inotify_init1" = yes; then
-  AC_DEFINE(HAVE_INOTIFY, 1, [Define to 1 to use inotify.])
-fi
+       AC_CHECK_FUNC(inotify_init1)
+       if test "$ac_cv_func_inotify_init1" = yes; then
+           AC_DEFINE(HAVE_INOTIFY, 1, [Define to 1 to use inotify.])
+	   NOTIFY_OBJ=inotify.o
+	   NOTIFY_SUMMARY="yes -lglibc (inotify)"
+	fi
+  fi
+fi
+if "${opsys}" = "mingw32"; then
+   NOTIFY_OBJ=w32notify.o
+   NOTIFY_SUMMARY="yes"
+fi
+AC_SUBST(NOTIFY_OBJ)
 
 dnl Do not put whitespace before the #include statements below.
 dnl Older compilers (eg sunos4 cc) choke on it.
@@ -4682,6 +4719,7 @@
 echo "  Does Emacs use -ldbus?                                  ${HAVE_DBUS}"
 echo "  Does Emacs use -lgconf?                                 ${HAVE_GCONF}"
 echo "  Does Emacs use GSettings?                               ${HAVE_GSETTINGS}"
+echo "  Does Emacs use a file notification library?             ${NOTIFY_SUMMARY}"
 echo "  Does Emacs use -lselinux?                               ${HAVE_LIBSELINUX}"
 echo "  Does Emacs use -lgnutls?                                ${HAVE_GNUTLS}"
 echo "  Does Emacs use -lxml2?                                  ${HAVE_LIBXML2}"

=== modified file 'lisp/autorevert.el'
--- lisp/autorevert.el	2013-05-22 14:47:19 +0000
+++ lisp/autorevert.el	2013-05-22 15:03:33 +0000
@@ -271,7 +271,7 @@
   :version "24.4")
 
 (defconst auto-revert-notify-enabled
-  (or (featurep 'inotify) (featurep 'w32notify))
+  (or (featurep 'gfilenotify) (featurep 'inotify) (featurep 'w32notify))
   "Non-nil when Emacs has been compiled with file notification support.")
 
 (defcustom auto-revert-use-notify auto-revert-notify-enabled
@@ -502,9 +502,12 @@
 	     (puthash key value auto-revert-notify-watch-descriptor-hash-list)
 	   (remhash key auto-revert-notify-watch-descriptor-hash-list)
 	   (ignore-errors
-	     (funcall (if (fboundp 'inotify-rm-watch)
-			  'inotify-rm-watch 'w32notify-rm-watch)
-		      auto-revert-notify-watch-descriptor)))))
+	     (funcall
+	      (cond
+	       ((fboundp 'gfile-rm-watch) 'gfile-rm-watch)
+	       ((fboundp 'inotify-rm-watch) 'inotify-rm-watch)
+	       ((fboundp 'w32notify-rm-watch) 'w32notify-rm-watch))
+	      auto-revert-notify-watch-descriptor)))))
      auto-revert-notify-watch-descriptor-hash-list)
     (remove-hook 'kill-buffer-hook 'auto-revert-notify-rm-watch))
   (setq auto-revert-notify-watch-descriptor nil
@@ -519,12 +522,18 @@
 
   (when (and buffer-file-name auto-revert-use-notify
 	     (not auto-revert-notify-watch-descriptor))
-    (let ((func (if (fboundp 'inotify-add-watch)
-		    'inotify-add-watch 'w32notify-add-watch))
-	  ;; `attrib' is needed for file modification time.
-	  (aspect (if (fboundp 'inotify-add-watch)
-		      '(attrib create modify moved-to) '(size last-write-time)))
-	  (file (if (fboundp 'inotify-add-watch)
+    (let ((func
+	   (cond
+	    ((fboundp 'gfile-add-watch) 'gfile-add-watch)
+	    ((fboundp 'inotify-add-watch) 'inotify-add-watch)
+	    ((fboundp 'w32notify-add-watch) 'w32notify-add-watch)))
+	  (aspect
+	   (cond
+	    ((fboundp 'gfile-add-watch) '(watch-mounts))
+	    ;; `attrib' is needed for file modification time.
+	    ((fboundp 'inotify-add-watch) '(attrib create modify moved-to))
+	    ((fboundp 'w32notify-add-watch) '(size last-write-time))))
+	  (file (if (or (fboundp 'gfile-add-watch) (fboundp 'inotify-add-watch))
 		    (directory-file-name (expand-file-name default-directory))
 		  (buffer-file-name))))
       (setq auto-revert-notify-watch-descriptor
@@ -545,10 +554,13 @@
 
 (defun auto-revert-notify-event-p (event)
   "Check that event is a file notification event."
-  (cond ((featurep 'inotify)
-	 (and (listp event) (= (length event) 4)))
-	((featurep 'w32notify)
-	 (and (listp event) (= (length event) 3) (stringp (nth 2 event))))))
+  (and (listp event)
+       (cond ((featurep 'gfilenotify)
+	      (and (>= (length event) 3) (stringp (nth 2 event))))
+	     ((featurep 'inotify)
+	      (= (length event) 4))
+	     ((featurep 'w32notify)
+	      (and (= (length event) 3) (stringp (nth 2 event)))))))
 
 (defun auto-revert-notify-event-descriptor (event)
   "Return watch descriptor of file notification event, or nil."
@@ -561,7 +573,8 @@
 (defun auto-revert-notify-event-file-name (event)
   "Return file name of file notification event, or nil."
   (and (auto-revert-notify-event-p event)
-       (cond ((featurep 'inotify) (nth 3 event))
+       (cond ((featurep 'gfilenotify) (nth 2 event))
+	     ((featurep 'inotify) (nth 3 event))
 	     ((featurep 'w32notify) (nth 2 event)))))
 
 (defun auto-revert-notify-handler (event)
@@ -576,12 +589,18 @@
 	;; Check, that event is meant for us.
 	;; TODO: Filter events which stop watching, like `move' or `removed'.
 	(cl-assert descriptor)
-	(when (featurep 'inotify)
+	(cond
+	 ((featurep 'gfilenotify)
+	  (cl-assert (or (eq 'attribute-changed action)
+			 (eq 'changed action)
+			 (eq 'created action)
+			 (eq 'deleted action))))
+	 ((featurep 'inotify)
 	  (cl-assert (or (memq 'attrib action)
 			 (memq 'create action)
 			 (memq 'modify action)
 			 (memq 'moved-to action))))
-	(when (featurep 'w32notify) (cl-assert (eq 'modified action)))
+	 ((featurep 'w32notify) (cl-assert (eq 'modified action))))
 	;; Since we watch a directory, a file name must be returned.
 	(cl-assert (stringp file))
 	(dolist (buffer buffers)

=== modified file 'lisp/subr.el'
--- lisp/subr.el	2013-05-25 16:05:19 +0000
+++ lisp/subr.el	2013-05-27 06:24:20 +0000
@@ -4432,32 +4432,16 @@
 \f
 ;;;; Support for watching filesystem events.
 
-(defun inotify-event-p (event)
-  "Check if EVENT is an inotify event."
-  (and (listp event)
-       (>= (length event) 3)
-       (eq (car event) 'file-inotify)))
-
-;;;###autoload
-(defun inotify-handle-event (event)
-  "Handle inotify file system monitoring event.
-If EVENT is an inotify filewatch event, call its callback.
-Otherwise, signal a `filewatch-error'."
-  (interactive "e")
-  (unless (inotify-event-p event)
-    (signal 'filewatch-error (cons "Not a valid inotify event" event)))
-  (funcall (nth 2 event) (nth 1 event)))
-
-(defun w32notify-handle-event (event)
-  "Handle MS-Windows file system monitoring event.
-If EVENT is an MS-Windows filewatch event, call its callback.
-Otherwise, signal a `filewatch-error'."
-  (interactive "e")
-  (if (and (eq (car event) 'file-w32notify)
-	   (= (length event) 3))
+(defun file-notify-handle-event (event)
+  "Handle file system monitoring event.
+If EVENT is a filewatch event, call its callback.
+Otherwise, signal a `filewatch-error'."
+  (interactive "e")
+  (if (and (eq (car event) 'file-notify)
+	   (>= (length event) 3))
       (funcall (nth 2 event) (nth 1 event))
     (signal 'filewatch-error
-	    (cons "Not a valid MS-Windows file-notify event" event))))
+	    (cons "Not a valid file-notify event" event))))
 
 \f
 ;;;; Comparing version strings.

=== modified file 'src/Makefile.in'
--- src/Makefile.in	2013-05-16 09:58:56 +0000
+++ src/Makefile.in	2013-05-28 09:35:00 +0000
@@ -156,6 +156,11 @@
 ## gtkutil.o if USE_GTK, else empty.
 GTK_OBJ=@GTK_OBJ@
 
+## gfilenotify.o if HAVE_GFILENOTIFY.
+## inotify.o if HAVE_INOTIFY.
+## w32notify.o for mingw32.
+NOTIFY_OBJ = @NOTIFY_OBJ@
+
 ## -ltermcap, or -lncurses, or -lcurses, or "".
 LIBS_TERMCAP=@LIBS_TERMCAP@
 ## terminfo.o if TERMINFO, else tparam.o.
@@ -363,7 +368,7 @@
 	syntax.o $(UNEXEC_OBJ) bytecode.o \
 	process.o gnutls.o callproc.o \
 	region-cache.o sound.o atimer.o \
-	doprnt.o intervals.o textprop.o composite.o xml.o inotify.o \
+	doprnt.o intervals.o textprop.o composite.o xml.o $(NOTIFY_OBJ) \
 	profiler.o \
 	$(MSDOS_OBJ) $(MSDOS_X_OBJ) $(NS_OBJ) $(CYGWIN_OBJ) $(FONT_OBJ) \
 	$(W32_OBJ) $(WINDOW_SYSTEM_OBJ)

=== modified file 'src/emacs.c'
--- src/emacs.c	2013-03-19 14:09:05 +0000
+++ src/emacs.c	2013-04-10 09:27:58 +0000
@@ -1409,6 +1409,10 @@
       syms_of_gnutls ();
 #endif
 
+#ifdef HAVE_GFILENOTIFY
+      syms_of_gfilenotify ();
+#endif /* HAVE_GFILENOTIFY */
+
 #ifdef HAVE_INOTIFY
       syms_of_inotify ();
 #endif /* HAVE_INOTIFY */

=== added file 'src/gfilenotify.c'
--- src/gfilenotify.c	1970-01-01 00:00:00 +0000
+++ src/gfilenotify.c	2013-05-22 15:04:04 +0000
@@ -0,0 +1,266 @@
+/* Elisp bindings for D-Bus.
+   Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+
+#ifdef HAVE_GFILENOTIFY
+#include <stdio.h>
+#include <gio/gio.h>
+#include "lisp.h"
+#include "coding.h"
+#include "frame.h"
+#include "termhooks.h"
+#include "keyboard.h"
+#include "process.h"
+
+\f
+/* Subroutines.  */
+static Lisp_Object Qgfile_add_watch;
+static Lisp_Object Qgfile_rm_watch;
+
+/* Filter objects.  */
+static Lisp_Object Qwatch_mounts;      /* G_FILE_MONITOR_WATCH_MOUNTS  */
+static Lisp_Object Qsend_moved;        /* G_FILE_MONITOR_SEND_MOVED  */
+
+/* Event types.  */
+static Lisp_Object Qchanged;           /* G_FILE_MONITOR_EVENT_CHANGED  */
+static Lisp_Object Qchanges_done_hint; /* G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT  */
+static Lisp_Object Qdeleted;           /* G_FILE_MONITOR_EVENT_DELETED  */
+static Lisp_Object Qcreated;           /* G_FILE_MONITOR_EVENT_CREATED  */
+static Lisp_Object Qattribute_changed; /* G_FILE_MONITOR_EVENT_ATTRIBUTE_CHANGED  */
+static Lisp_Object Qpre_unmount;       /* G_FILE_MONITOR_EVENT_PRE_UNMOUNT  */
+static Lisp_Object Qunmounted;         /* G_FILE_MONITOR_EVENT_UNMOUNTED  */
+static Lisp_Object Qmoved;             /* G_FILE_MONITOR_EVENT_MOVED  */
+
+static Lisp_Object watch_list;
+
+/* This is the callback function for arriving signals from
+   g_file_monitor.  It shall create a Lisp event, and put it into
+   Emacs input queue.  */
+static gboolean
+dir_monitor_callback (GFileMonitor* monitor,
+		      GFile* file,
+		      GFile* other_file,
+		      GFileMonitorEvent event_type,
+		      gpointer user_data)
+{
+  Lisp_Object symbol, watch_object;
+  char *name = g_file_get_parse_name (file);
+  char *oname = other_file ? g_file_get_parse_name (other_file) : NULL;
+
+  /* Determine event symbol.  */
+  switch (event_type)
+    {
+    case G_FILE_MONITOR_EVENT_CHANGED:
+      symbol = Qchanged;
+      break;
+    case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT:
+      symbol = Qchanges_done_hint;
+      break;
+    case G_FILE_MONITOR_EVENT_DELETED:
+      symbol = Qdeleted;
+      break;
+    case G_FILE_MONITOR_EVENT_CREATED:
+      symbol = Qcreated;
+      break;
+    case G_FILE_MONITOR_EVENT_ATTRIBUTE_CHANGED:
+      symbol = Qattribute_changed;
+      break;
+    case G_FILE_MONITOR_EVENT_PRE_UNMOUNT:
+      symbol = Qpre_unmount;
+      break;
+    case G_FILE_MONITOR_EVENT_UNMOUNTED:
+      symbol = Qunmounted;
+      break;
+    case G_FILE_MONITOR_EVENT_MOVED:
+      symbol = Qmoved;
+      break;
+    default:
+      goto cleanup;
+    }
+
+  /* Determine callback function.  */
+  watch_object = Fassoc (XIL ((EMACS_INT) monitor), watch_list);
+
+  if (FUNCTIONP (CDR_SAFE (watch_object)))
+    {
+      /* Construct an event.  */
+      struct input_event event;
+      EVENT_INIT (event);
+      event.kind = FILE_NOTIFY_EVENT;
+      event.frame_or_window = Qnil;
+      event.arg = oname
+	? list2 (list4 (XIL ((EMACS_INT) monitor), symbol,
+			build_string (name), build_string (oname)),
+		 CDR_SAFE (watch_object))
+	: list2 (list3 (XIL ((EMACS_INT) monitor), symbol, build_string (name)),
+		 CDR_SAFE (watch_object));
+
+      /* Store it into the input event queue.  */
+      kbd_buffer_store_event (&event);
+    }
+
+  /* Cleanup.  */
+ cleanup:
+  g_free (name);
+  g_free (oname);
+
+  return TRUE;
+}
+
+DEFUN ("gfile-add-watch", Fgfile_add_watch, Sgfile_add_watch, 3, 3, 0,
+       doc: /* Add a watch for filesystem events pertaining to FILE.
+
+This arranges for filesystem events pertaining to FILE to be reported
+to Emacs.  Use `gfile-rm-watch' to cancel the watch.
+
+Value is a descriptor for the added watch.  If the file cannot be
+watched for some reason, this function signals a `file-error' error.
+
+FLAGS is a list of conditions to set what will be watched for.  It can
+include the following symbols:
+
+  'watch-mounts' -- watch for mount events
+  'send-moved'   -- pair 'deleted' and 'created' events caused by file
+                    renames (moves) and send a single 'event-moved'
+                    event instead
+
+When any event happens, Emacs will call the CALLBACK function passing
+it a single argument EVENT, which is of the form
+
+  (DESCRIPTOR ACTION FILE [FILE1])
+
+DESCRIPTOR is the same object as the one returned by this function.
+ACTION is the description of the event.  It could be any one of the
+following:
+
+  'changed'           -- FILE has changed
+  'changes-done-hint' -- a hint that this was probably the last change
+                         in a set of changes
+  'deleted'           -- FILE was deleted
+  'created'           -- FILE was created
+  'attribute-changed' -- a FILE attribute was changed
+  'pre-unmount'       -- the FILE location will soon be unmounted
+  'unmounted'         -- the FILE location was unmounted
+  'moved'             -- FILE was moved to FILE1
+
+FILE is the name of the file whose event is being reported.  FILE1
+will be reported only in case of the 'moved' event.  */)
+  (Lisp_Object file, Lisp_Object flags, Lisp_Object callback)
+{
+  Lisp_Object watch_descriptor, watch_object;
+  GFile *gfile;
+  GFileMonitor* monitor;
+  GFileMonitorFlags gflags = G_FILE_MONITOR_NONE;
+
+  /* Check parameters.  */
+  CHECK_STRING (file);
+  file = Fdirectory_file_name (Fexpand_file_name (file, Qnil));
+  if (NILP (Ffile_exists_p (file)))
+    report_file_error ("File does not exists", Fcons (file, Qnil));
+
+  CHECK_LIST (flags);
+
+  if (!FUNCTIONP (callback))
+    wrong_type_argument (Qinvalid_function, callback);
+
+  /* Create GFile name.  */
+  gfile = g_file_new_for_path (SSDATA (ENCODE_FILE (file)));
+
+  /* Assemble flags.  */
+  if (!NILP (Fmember (Qwatch_mounts, flags)))
+    gflags |= G_FILE_MONITOR_WATCH_MOUNTS;
+  if (!NILP (Fmember (Qsend_moved, flags)))
+    gflags |= G_FILE_MONITOR_SEND_MOVED;
+
+  /* Enable watch.  */
+  monitor = g_file_monitor (gfile, gflags, NULL, NULL);
+  if (monitor != NULL)
+    g_signal_connect (monitor, "changed",
+		      (GCallback) dir_monitor_callback, NULL);
+  else
+    report_file_error ("Cannot watch file", Fcons (file, Qnil));
+
+  /* Store watch object in watch list.  */
+  watch_descriptor = XIL ((EMACS_INT) monitor);
+  watch_object = Fcons (watch_descriptor, callback);
+  watch_list = Fcons (watch_object, watch_list);
+
+  return watch_descriptor;
+}
+
+DEFUN ("gfile-rm-watch", Fgfile_rm_watch, Sgfile_rm_watch, 1, 1, 0,
+       doc: /* Remove an existing WATCH-DESCRIPTOR.
+
+WATCH-DESCRIPTOR should be an object returned by `gfile-add-watch'.  */)
+     (Lisp_Object watch_descriptor)
+{
+  Lisp_Object watch_object;
+  GFileMonitor *monitor = (GFileMonitor *) XLI (watch_descriptor);
+
+  watch_object = Fassoc (watch_descriptor, watch_list);
+  if (NILP (watch_object))
+    report_file_error ("Not a watch descriptor",
+		       Fcons (watch_descriptor, Qnil));
+
+  if (!g_file_monitor_cancel (monitor))
+    report_file_error ("Could not rm watch",
+		       Fcons (watch_descriptor, Qnil));
+
+  /* Remove watch descriptor from watch list. */
+  watch_list = Fdelete (watch_object, watch_list);
+
+  /* Cleanup.  */
+  g_object_unref (monitor);
+
+  return Qt;
+}
+
+\f
+void
+syms_of_gfilenotify (void)
+{
+
+  g_type_init ();
+
+  DEFSYM (Qgfile_add_watch, "gfile-add-watch");
+  defsubr (&Sgfile_add_watch);
+
+  DEFSYM (Qgfile_rm_watch, "gfile-rm-watch");
+  defsubr (&Sgfile_rm_watch);
+
+  DEFSYM (Qwatch_mounts, "watch-mounts");
+  DEFSYM (Qsend_moved, "send-moved");
+  DEFSYM (Qchanged, "changed");
+  DEFSYM (Qchanges_done_hint, "changes-done-hint");
+  DEFSYM (Qdeleted, "deleted");
+  DEFSYM (Qcreated, "created");
+  DEFSYM (Qattribute_changed, "attribute-changed");
+  DEFSYM (Qpre_unmount, "pre-unmount");
+  DEFSYM (Qunmounted, "unmounted");
+  DEFSYM (Qmoved, "moved");
+
+  /* Initialize internal objects.  */
+  watch_list = Qnil;
+  staticpro (&watch_list);
+
+  Fprovide (intern_c_string ("gfilenotify"), Qnil);
+
+}
+
+#endif /* HAVE_GFILENOTIFY  */

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2013-05-23 18:49:57 +0000
+++ src/keyboard.c	2013-05-24 09:51:35 +0000
@@ -308,18 +308,15 @@
 Lisp_Object Qmouse_click;
 #ifdef HAVE_NTGUI
 Lisp_Object Qlanguage_change;
-#ifdef WINDOWSNT
-Lisp_Object Qfile_w32notify;
-#endif
 #endif
 static Lisp_Object Qdrag_n_drop;
 static Lisp_Object Qsave_session;
 #ifdef HAVE_DBUS
 static Lisp_Object Qdbus_event;
 #endif
-#ifdef HAVE_INOTIFY
-static Lisp_Object Qfile_inotify;
-#endif /* HAVE_INOTIFY */
+#if defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY || defined WINDOWSNT
+static Lisp_Object Qfile_notify;
+#endif /* defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY || defined WINDOWSNT */
 static Lisp_Object Qconfig_changed_event;
 
 /* Lisp_Object Qmouse_movement; - also an event header */
@@ -4017,7 +4014,7 @@
       else if (event->kind == FILE_NOTIFY_EVENT)
 	{
 	  /* Make an event (file-notify (DESCRIPTOR ACTION FILE) CALLBACK).  */
-	  obj = Fcons (Qfile_w32notify,
+	  obj = Fcons (Qfile_notify,
 		       list2 (list3 (make_number (event->code),
 				     XCAR (event->arg),
 				     XCDR (event->arg)),
@@ -4082,7 +4079,7 @@
 	  kbd_fetch_ptr = event + 1;
 	}
 #endif
-#ifdef HAVE_INOTIFY
+#if defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY
       else if (event->kind == FILE_NOTIFY_EVENT)
         {
           obj = make_lispy_event (event);
@@ -5991,12 +5988,12 @@
       }
 #endif /* HAVE_DBUS */
 
-#ifdef HAVE_INOTIFY
+#if defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY
     case FILE_NOTIFY_EVENT:
       {
-        return Fcons (Qfile_inotify, event->arg);
+        return Fcons (Qfile_notify, event->arg);
       }
-#endif /* HAVE_INOTIFY */
+#endif /* defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY */
 
     case CONFIG_CHANGED_EVENT:
 	return Fcons (Qconfig_changed_event,
@@ -11006,17 +11003,13 @@
   DEFSYM (Qlanguage_change, "language-change");
 #endif
 
-#ifdef WINDOWSNT
-  DEFSYM (Qfile_w32notify, "file-w32notify");
-#endif
-
 #ifdef HAVE_DBUS
   DEFSYM (Qdbus_event, "dbus-event");
 #endif
 
-#ifdef HAVE_INOTIFY
-  DEFSYM (Qfile_inotify, "file-inotify");
-#endif /* HAVE_INOTIFY */
+#if defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY || defined WINDOWSNT
+  DEFSYM (Qfile_notify, "file-notify");
+#endif /* defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY || defined WINDOWSNT */
 
   DEFSYM (QCenable, ":enable");
   DEFSYM (QCvisible, ":visible");
@@ -11762,20 +11755,18 @@
 			    "dbus-handle-event");
 #endif
 
-#ifdef HAVE_INOTIFY
-  /* Define a special event which is raised for inotify callback
+#if defined HAVE_GFILENOTIFY || HAVE_INOTIFY || defined WINDOWSNT
+  /* Define a special event which is raised for notification callback
      functions.  */
-  initial_define_lispy_key (Vspecial_event_map, "file-inotify",
-                            "inotify-handle-event");
-#endif /* HAVE_INOTIFY */
+  initial_define_lispy_key (Vspecial_event_map, "file-notify",
+                            "file-notify-handle-event");
+#endif /* defined HAVE_GFILENOTIFY || HAVE_INOTIFY || defined WINDOWSNT */
 
   initial_define_lispy_key (Vspecial_event_map, "config-changed-event",
 			    "ignore");
 #if defined (WINDOWSNT)
   initial_define_lispy_key (Vspecial_event_map, "language-change",
 			    "ignore");
-  initial_define_lispy_key (Vspecial_event_map, "file-w32notify",
-			    "w32notify-handle-event");
 #endif
 }
 

=== modified file 'src/lisp.h'
--- src/lisp.h	2013-05-15 18:54:49 +0000
+++ src/lisp.h	2013-05-21 08:25:51 +0000
@@ -3728,6 +3728,11 @@
 extern void syms_of_w32notify (void);
 #endif
 
+/* Defined in gfilenotify.c */
+#ifdef HAVE_GFILENOTIFY
+extern void syms_of_gfilenotify (void);
+#endif
+
 /* Defined in inotify.c */
 #ifdef HAVE_INOTIFY
 extern void syms_of_inotify (void);

=== modified file 'src/termhooks.h'
--- src/termhooks.h	2013-03-11 01:17:40 +0000
+++ src/termhooks.h	2013-04-10 09:24:04 +0000
@@ -212,7 +212,7 @@
   , NS_NONKEY_EVENT
 #endif
 
-#if defined (HAVE_INOTIFY) || defined (HAVE_NTGUI)
+#if defined (HAVE_GFILENOTIFY) || defined (HAVE_INOTIFY) || defined (HAVE_NTGUI)
   /* File or directory was changed.  */
   , FILE_NOTIFY_EVENT
 #endif

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

On 05/28/2013 03:39 AM, Michael Albinus wrote:
> Comments on the patch are welcome. And we should also start to design an
> upper layer over gfilenotify.c, inotify.c and w32notify.c, which
> abstracts from the respective file notification events

I've been running into a similar issue with monitoring
child processes; see Bug#14474.  A couple of dumb questions:

Should the issues of file-change and process-change notifications
be combined, or kept separate?

Would it make sense, at the low level, for Emacs to rely on the
glib primitives in child-process area?  I worry about race
conditions (SIGCHLD arriving in the middle of memory allocation,
that sort of thing).

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> Date: Tue, 28 May 2013 07:44:57 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> Should the issues of file-change and process-change notifications
> be combined, or kept separate?

What are the advantages of combining them?  The uses and the use-cases
are quite different, it seems.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

On 05/28/13 08:33, Eli Zaretskii wrote:
> What are the advantages of combining them?  The uses and the use-cases
> are quite different, it seems.

I'm thinking about this at a fairly low level, not at the Lisp level.
At the low level, Emacs is often best thought of as an event handler,
where events can be keyboard presses, file-change events, process-change
events, and so forth.  And glib uses a similar mechanism to handle
file-change and process-change events.  So it seems natural to
think about how these should be unified rather than kept separate,
independently of how they're used.

I don't have anything specific in mind, as far as implementation goes;
I was just looking for a bit of advice, from someone who knows more
about glib than I do.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Stefan Monnier]

> events, and so forth.  And glib uses a similar mechanism to handle
> file-change and process-change events.  So it seems natural to
> think about how these should be unified rather than kept separate,
> independently of how they're used.

In the same vein, there's the D-Bus message handler.

Currently d-bus messages generate events in the key-event queue, which means
that processing of dbus messages may be delayed by processing of key
presses.  I think this should be changed to "use a separate queue", just
like process events don't go through the key-event queue.


        Stefan

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: Eli Zaretskii <eliz@gnu.org>, michael.albinus@gmx.de, emacs-devel@gnu.org
> Date: Tue, 28 May 2013 12:21:33 -0400
> 
> Currently d-bus messages generate events in the key-event queue, which means
> that processing of dbus messages may be delayed by processing of key
> presses.  I think this should be changed to "use a separate queue", just
> like process events don't go through the key-event queue.

But doesn't Emacs call 'select' only when no keyboard input is ready?
If so, process events can also be delayed by processing a key, no?

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Stefan Monnier]

>> Currently d-bus messages generate events in the key-event queue, which means
>> that processing of dbus messages may be delayed by processing of key
>> presses.  I think this should be changed to "use a separate queue", just
>> like process events don't go through the key-event queue.
> But doesn't Emacs call 'select' only when no keyboard input is ready?
> If so, process events can also be delayed by processing a key, no?

Process events wait until the next "idle" moment, yes.
But they don't wait until we've processed all key-events that arrived before.

So `accept-process-output' can keep receiving/processing new output even
if the user has already hit the keys that will run the next command.


        Stefan

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Currently d-bus messages generate events in the key-event queue, which means
> that processing of dbus messages may be delayed by processing of key
> presses.  I think this should be changed to "use a separate queue", just
> like process events don't go through the key-event queue.

I believe all events which are not raised by an user action (keyboard,
mouse, etc) shall be handled by such a separate queue. Currently,
TIMER_EVENT, BUFFER_SWITCH_EVENT, DBUS_EVENT and FILE_NOTIFY_EVENT would
fall into this category. Maybe also USER_SIGNAL_EVENT.

If processes would send events when changing the state, in addition to
calling their process-sentinel, their event type would also be a candidate.

>         Stefan

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Stefan Monnier]

> I believe all events which are not raised by an user action (keyboard,
> mouse, etc) shall be handled by such a separate queue. Currently,
> TIMER_EVENT, BUFFER_SWITCH_EVENT, DBUS_EVENT and FILE_NOTIFY_EVENT would
> fall into this category. Maybe also USER_SIGNAL_EVENT.

I'm not sure I'd put BUFFER_SWITCH_EVENT in there, but I agree about the
other ones.


        Stefan

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

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

>> I believe all events which are not raised by an user action (keyboard,
>> mouse, etc) shall be handled by such a separate queue. Currently,
>> TIMER_EVENT, BUFFER_SWITCH_EVENT, DBUS_EVENT and FILE_NOTIFY_EVENT would
>> fall into this category. Maybe also USER_SIGNAL_EVENT.
>
> I'm not sure I'd put BUFFER_SWITCH_EVENT in there, but I agree about the
> other ones.

I haven't used it yet, but the doc in termhooks.h says

  BUFFER_SWITCH_EVENT,        /* A process filter has switched buffers.  */

This doesn't sound like triggered by a user action.

>         Stefan

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Stefan Monnier]

>>> I believe all events which are not raised by an user action (keyboard,
>>> mouse, etc) shall be handled by such a separate queue. Currently,
>>> TIMER_EVENT, BUFFER_SWITCH_EVENT, DBUS_EVENT and FILE_NOTIFY_EVENT would
>>> fall into this category. Maybe also USER_SIGNAL_EVENT.
>> I'm not sure I'd put BUFFER_SWITCH_EVENT in there, but I agree about the
>> other ones.
> I haven't used it yet, but the doc in termhooks.h says
>   BUFFER_SWITCH_EVENT,        /* A process filter has switched buffers.  */
> This doesn't sound like triggered by a user action.

AFAIK this event is only inserted when we're waiting for user input
(see record_asynch_buffer_change).  Furthermore, I'd argue that the
right way to handle such switches is to consider them as bugs (i.e. we
could just throw BUFFER_SWITCH_EVENT away).


        Stefan

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> AFAIK this event is only inserted when we're waiting for user input
> (see record_asynch_buffer_change).  Furthermore, I'd argue that the
> right way to handle such switches is to consider them as bugs (i.e. we
> could just throw BUFFER_SWITCH_EVENT away).

According to the comments, record_asynch_buffer_change is also used for
forcing a redisplay in xterm.c and w32term.c. This must be implemented
otherwise, if we throw BUFFER_SWITCH_EVENT away.

>         Stefan

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Stefan Monnier]

>> AFAIK this event is only inserted when we're waiting for user input
>> (see record_asynch_buffer_change).  Furthermore, I'd argue that the
>> right way to handle such switches is to consider them as bugs (i.e. we
>> could just throw BUFFER_SWITCH_EVENT away).
> According to the comments, record_asynch_buffer_change is also used for
> forcing a redisplay in xterm.c and w32term.c. This must be implemented
> otherwise, if we throw BUFFER_SWITCH_EVENT away.

Indeed.  But the fact that remains that it being linked to the "key
event queue" is not a problem, so it doesn't need to (and shouldn't) be
moved to some other event queue, contrary to dbus, etc...


        Stefan

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> Date: Tue, 28 May 2013 09:02:06 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: michael.albinus@gmx.de, emacs-devel@gnu.org
> 
> I'm thinking about this at a fairly low level, not at the Lisp level.
> At the low level, Emacs is often best thought of as an event handler,
> where events can be keyboard presses, file-change events, process-change
> events, and so forth.  And glib uses a similar mechanism to handle
> file-change and process-change events.  So it seems natural to
> think about how these should be unified rather than kept separate,
> independently of how they're used.

So you are asking whether it would make sense to replace use of
'select' for process notifications by events that are put on the
keyboard queue?

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

On 05/28/13 09:43, Eli Zaretskii wrote:
> So you are asking whether it would make sense to replace use of
> 'select' for process notifications by events that are put on the
> keyboard queue?

Something like that, yes.  It wouldn't be a keyboard queue,
so much as a unified event queue.  Currently Emacs's low
level is confusing, since it's getting notifications
via signals and via reads.  It'd be nicer if there was just
one way to get notified, and if most of the low-level code
didn't have to worry about being interrupted by signal handlers.

I don't have a concrete suggestion, and maybe the whole idea
is a pipe dream....

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> Date: Tue, 28 May 2013 09:52:31 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> On 05/28/13 09:43, Eli Zaretskii wrote:
> > So you are asking whether it would make sense to replace use of
> > 'select' for process notifications by events that are put on the
> > keyboard queue?
> 
> Something like that, yes.  It wouldn't be a keyboard queue,
> so much as a unified event queue.  Currently Emacs's low
> level is confusing, since it's getting notifications
> via signals and via reads.  It'd be nicer if there was just
> one way to get notified, and if most of the low-level code
> didn't have to worry about being interrupted by signal handlers.

But this eventually boils down to the mechanisms supported by the OS
for each kind of events, doesn't it?  E.g., we could use glib or
whatever for converting process notifications into readable events,
but if that library uses signals internally, our code still can be
interrupted by signals, right?

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

On 05/28/13 09:59, Eli Zaretskii wrote:
> E.g., we could use glib or
> whatever for converting process notifications into readable events,
> but if that library uses signals internally, our code still can be
> interrupted by signals, right?

Yes and no.  The glib SIGCHLD handler is async-signal-safe,
modifying only data that's private to glib and without doing anything
dangerous like malloc.  So Emacs's code can still be interrupted by
signals, but the only symptoms visible to Emacs should be that
syscalls will occasionally fail with errno==EINTR; this is
a bit of a hassle, but it's one we've by-and-large solved and is
a relatively minor issue compared to the potential races
that still plague Emacs's internals.

Currently, Emacs's child-watcher procedure is done inside
Emacs's SIGCHLD handler (in handle_child_signal).  But if we
let glib handle SIGCHLD, glib already has a table of child
processes it's waiting for and glib reaps the children,
so handle_child_signal needs to be partly bypassed.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 05/28/2013 03:39 AM, Michael Albinus wrote:
>> Comments on the patch are welcome. And we should also start to design an
>> upper layer over gfilenotify.c, inotify.c and w32notify.c, which
>> abstracts from the respective file notification events
>
> I've been running into a similar issue with monitoring
> child processes; see Bug#14474.  A couple of dumb questions:
>
> Should the issues of file-change and process-change notifications
> be combined, or kept separate?

I agree with Eli: these are different topics, which shall be treated
different.

Wrt Bug#14474: if you want to suppress the autostart of the D-Bus
session bus, you might set a dummy address for $DBUS_SESSION_BUS_ADDRESS.
See (info "(emacs)General Variables")

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

On 05/28/13 08:58, Michael Albinus wrote:
> Wrt Bug#14474: if you want to suppress the autostart of the D-Bus
> session bus, you might set a dummy address for $DBUS_SESSION_BUS_ADDRESS.
> See (info "(emacs)General Variables")

Thanks for this suggestion.  I have followed up at
<http://bugs.gnu.org/bug=14474#29> by suggesting
that DBUS_SESSION_BUS_ADDRESS be set to the empty string
if it's not already set and if gtk is being used.

The empty string is the shortest dummy address I could think of;
if some other string would be a better choice, please let me know.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 05/28/13 08:58, Michael Albinus wrote:
>> Wrt Bug#14474: if you want to suppress the autostart of the D-Bus
>> session bus, you might set a dummy address for $DBUS_SESSION_BUS_ADDRESS.
>> See (info "(emacs)General Variables")
>
> Thanks for this suggestion.  I have followed up at
> <http://bugs.gnu.org/bug=14474#29> by suggesting
> that DBUS_SESSION_BUS_ADDRESS be set to the empty string
> if it's not already set and if gtk is being used.
>
> The empty string is the shortest dummy address I could think of;
> if some other string would be a better choice, please let me know.

I believe this still allows autostart of the session bus. It needs a
"real" dummy address in DBUS_SESSION_BUS_ADDRESS with proper syntax,
like the example I have pointed to. Compare

# env DBUS_SESSION_BUS_ADDRESS="" emacs --eval "(dbus-init-bus :session)"

# env DBUS_SESSION_BUS_ADDRESS="unix:path=/tmp/foo" emacs --eval "(dbus-init-bus :session)"

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

On 05/28/13 13:31, Michael Albinus wrote:
> # env DBUS_SESSION_BUS_ADDRESS="unix:path=/tmp/foo" emacs --eval "(dbus-init-bus :session)"

Thanks.  What is the significance of "/tmp/foo" here?
Will any file name do, e.g.,
DBUS_SESSION_BUS_ADDRESS="unix:path=/"?
Or must it be a file that is guaranteed
to not exist?  If the latter, that might
be hard to arrange reliably.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Paul Eggert <eggert@cs.ucla.edu> writes:

>> # env DBUS_SESSION_BUS_ADDRESS="unix:path=/tmp/foo" emacs --eval
>> "(dbus-init-bus :session)"
>
> Thanks.  What is the significance of "/tmp/foo" here?
> Will any file name do, e.g.,
> DBUS_SESSION_BUS_ADDRESS="unix:path=/"?
> Or must it be a file that is guaranteed
> to not exist?  If the latter, that might
> be hard to arrange reliably.

DBUS_SESSION_BUS_ADDRESS="unix:path=/" works, but returns "Permission
denied". I have not checked what happens if "/" is writable for the caller.

DBUS_SESSION_BUS_ADDRESS="unix:path=/dev/null" seems to be reliable.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Tue, 28 May 2013 12:39:15 +0200
> Cc: emacs-devel@gnu.org
> 
> I've appended a patch which I have tested successfully under GNU/Linux.
> There is a new gfilenotify.c, which uses glib's functionality.

Thanks.

> There is
> also a new config option --with-file-notification=LIB, LIB can be "yes",
> "no", "gfile" or "inotify". Default on non-w32 machines is "gfile" if
> applicable. For w32 machines, this option is not taken into account yet,
> emacs undonditionally links w32notify (maybe we want also support "no"
> there?).

I'd rather not special-case w32 unless really necessary.  (The whole
point of using the Posix configury was to remove as much w32-specific
stuff as possible from the build process.)  So yes, let's by all means
support "no" for w32, although I'm not sure who'd want that.  And
please also include mingw32 in the same test harness where you test
for gfilenotify and inotify, i.e. test for a header (windows.h will
do), not the value of $opsys.

Btw, glib is available for Windows as well, and AFAICT its
gfilemonitor should work (it actually uses the same APIs as
w32notify.c does).  So "gfile" should not be precluded from the w32
build.

> +if "${opsys}" = "mingw32"; then
> +   NOTIFY_OBJ=w32notify.o
> +   NOTIFY_SUMMARY="yes"
> +fi

See above: I'd prefer this to be a real test, based on windows.h
presence.  And if the user selected "gfile", let them have it, do not
override with w32notify.o.

> +#if defined (HAVE_GFILENOTIFY) || defined (HAVE_INOTIFY) || defined (HAVE_NTGUI)

It sounds like a single symbol HAVE_FILE_NOTIFY, defined somewhere as
the OR of these 3, should make the sources more readable.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> I'd rather not special-case w32 unless really necessary.  (The whole
> point of using the Posix configury was to remove as much w32-specific
> stuff as possible from the build process.)  So yes, let's by all means
> support "no" for w32, although I'm not sure who'd want that.  And
> please also include mingw32 in the same test harness where you test
> for gfilenotify and inotify, i.e. test for a header (windows.h will
> do), not the value of $opsys.

Done.

> Btw, glib is available for Windows as well, and AFAICT its
> gfilemonitor should work (it actually uses the same APIs as
> w32notify.c does).  So "gfile" should not be precluded from the w32
> build.

OK.

> See above: I'd prefer this to be a real test, based on windows.h
> presence.  And if the user selected "gfile", let them have it, do not
> override with w32notify.o.

Also done.

>> +#if defined (HAVE_GFILENOTIFY) || defined (HAVE_INOTIFY) || defined (HAVE_NTGUI)
>
> It sounds like a single symbol HAVE_FILE_NOTIFY, defined somewhere as
> the OR of these 3, should make the sources more readable.

That's now USE_FILE_NOTIFY.

I will wait another couple of days (let's say until Sunday evening), if
there's no objection I will commit. I cannot test the MS Windows case;
here I trust on you.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Wed, 29 May 2013 15:30:57 +0200
> 
> I will wait another couple of days (let's say until Sunday evening), if
> there's no objection I will commit.

Thanks.

> I cannot test the MS Windows case; here I trust on you.

Can you post an updated patch?

Also, it would be good for someone on Cygwin to test this and/or
comment on the applicability of the options to the Cygwin builds.
Ken, Daniel, can you take a look, please?  TIA.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> I cannot test the MS Windows case; here I trust on you.
>
> Can you post an updated patch?

Appended.

> Also, it would be good for someone on Cygwin to test this and/or
> comment on the applicability of the options to the Cygwin builds.
> Ken, Daniel, can you take a look, please?  TIA.

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bzr.diff --]
[-- Type: text/x-diff, Size: 26354 bytes --]

=== modified file 'autogen/config.in'
--- autogen/config.in	2013-05-16 16:37:05 +0000
+++ autogen/config.in	2013-05-29 11:29:31 +0000
@@ -132,7 +132,7 @@
    #if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__
    # define _FORTIFY_SOURCE 2
    #endif
-
+

 /* Define to 1 if futimesat mishandles a NULL file name. */
 #undef FUTIMESAT_NULL_BUG
@@ -547,6 +547,9 @@
 /* Define to 1 if you have the `get_current_dir_name' function. */
 #undef HAVE_GET_CURRENT_DIR_NAME

+/* Define to 1 to use glib's notify. */
+#undef HAVE_GFILENOTIFY
+
 /* Define to 1 if you have a gif (or ungif) library. */
 #undef HAVE_GIF

@@ -1140,6 +1143,9 @@
 /* Define to 1 if you have the <vfork.h> header file. */
 #undef HAVE_VFORK_H

+/* Define to 1 to use w32notify. */
+#undef HAVE_W32NOTIFY
+
 /* Define to 1 if you have the <wchar.h> header file. */
 #undef HAVE_WCHAR_H

@@ -1496,6 +1502,9 @@
 /* Define to nonzero if you want access control list support. */
 #undef USE_ACL

+/* Define to 1 if if using file notifications. */
+#undef USE_FILE_NOTIFY
+
 /* Define to 1 if using GTK. */
 #undef USE_GTK

@@ -1836,4 +1845,3 @@
 mode: c
 End:
 */
-

=== modified file 'configure.ac'
--- configure.ac	2013-05-27 02:03:18 +0000
+++ configure.ac	2013-05-29 12:41:52 +0000
@@ -200,7 +200,23 @@
 OPTION_DEFAULT_ON([gsettings],[don't compile with GSettings support])
 OPTION_DEFAULT_ON([selinux],[don't compile with SELinux support])
 OPTION_DEFAULT_ON([gnutls],[don't use -lgnutls for SSL/TLS support])
-OPTION_DEFAULT_ON([inotify],[don't compile with inotify (file-watch) support])
+
+AC_ARG_WITH([file-notification],[AS_HELP_STRING([--with-file-notification=LIB],
+ [use a file notification library (LIB one of: yes, gfile, inotify, w32, no)])],
+ [ case "${withval}" in
+    y | ye | yes )	val=yes ;;
+    n | no )		val=no  ;;
+    g | gf | gfi | gfil | gfile )	val=gfile ;;
+    i | in | ino | inot | inoti | inotif | inotify )	val=inotify ;;
+    w | w3 | w32 )	val=w32 ;;
+    * ) AC_MSG_ERROR([`--with-file-notification=$withval' is invalid;
+this option's value should be `yes', `no', `gfile', `inotify' or `w32'.
+`yes' is a synonym for `w32' on MS-Windows, and for `gfile' otherwise.])
+    ;;
+   esac
+   with_file_notification=$val
+ ],
+ [with_file_notification=yes])

 ## For the times when you want to build Emacs but don't have
 ## a suitable makeinfo, and can live without the manuals.
@@ -1668,7 +1684,6 @@
     W32_RES_LINK="-Wl,emacs.res"
   else
     W32_OBJ="$W32_OBJ w32.o w32console.o w32heap.o w32inevt.o w32proc.o"
-    W32_OBJ="$W32_OBJ w32notify.o"
     W32_LIBS="$W32_LIBS -lwinmm -lgdi32 -lcomdlg32"
     W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32 -lusp10"
     W32_RES_LINK="\$(EMACSRES)"
@@ -2294,16 +2309,56 @@
 AC_SUBST(LIBGNUTLS_LIBS)
 AC_SUBST(LIBGNUTLS_CFLAGS)

+NOTIFY_OBJ=
+NOTIFY_SUMMARY=no
+
+dnl Set defaults of $with_file_notification.
+if test "${with_file_notification}" = "yes"; then
+   if test "${opsys}" = "mingw32"; then
+      with_file_notification=w32
+   else
+      with_file_notification=gfile
+   fi
+fi
+
+dnl g_file_monitor exists since glib 2.18.  It has been tested under
+dnl GNU/Linux only.  We take precedence over inotify, but this makes
+dnl only sense when glib has been compiled with inotify support.  How
+dnl to check?
+if test "${with_file_notification}" = "gfile"; then
+   PKG_CHECK_MODULES(GFILENOTIFY, gio-2.0 >= 2.18, HAVE_GFILENOTIFY=yes, HAVE_GFILENOTIFY=no)
+   if test "$HAVE_GFILENOTIFY" = "yes"; then
+      AC_DEFINE(HAVE_GFILENOTIFY, 1, [Define to 1 if using GFile.])
+      LIBS="$LIBS $GFILENOTIFY_LIBS"
+      NOTIFY_OBJ=gfilenotify.o
+      NOTIFY_SUMMARY="yes -lgio (gfile)"
+   fi
+fi
 dnl inotify is only available on GNU/Linux.
-if test "${with_inotify}" = "yes"; then
-   AC_CHECK_HEADERS(sys/inotify.h)
+if test "${with_file_notification}" = "inotify"; then
+   AC_CHECK_HEADER(sys/inotify.h)
    if test "$ac_cv_header_sys_inotify_h" = yes ; then
-     AC_CHECK_FUNC(inotify_init1)
+       AC_CHECK_FUNC(inotify_init1)
+       if test "$ac_cv_func_inotify_init1" = yes; then
+           AC_DEFINE(HAVE_INOTIFY, 1, [Define to 1 to use inotify.])
+	   NOTIFY_OBJ=inotify.o
+	   NOTIFY_SUMMARY="yes -lglibc (inotify)"
+	fi
+  fi
+fi
+dnl MS Windows native file monitor is available for mingw32 only.
+if test "${with_file_notification}" = "w32"; then
+   AC_CHECK_HEADER(windows.h)
+   if test "$ac_cv_header_windows_h" = yes ; then
+      AC_DEFINE(HAVE_W32NOTIFY, 1, [Define to 1 to use w32notify.])
+      NOTIFY_OBJ=w32notify.o
+      NOTIFY_SUMMARY="yes (w32)"
    fi
 fi
-if test "$ac_cv_func_inotify_init1" = yes; then
-  AC_DEFINE(HAVE_INOTIFY, 1, [Define to 1 to use inotify.])
+if test -n "$NOTIFY_OBJ"; then
+   AC_DEFINE(USE_FILE_NOTIFY, 1, [Define to 1 if using file notifications.])
 fi
+AC_SUBST(NOTIFY_OBJ)

 dnl Do not put whitespace before the #include statements below.
 dnl Older compilers (eg sunos4 cc) choke on it.
@@ -4682,6 +4737,7 @@
 echo "  Does Emacs use -ldbus?                                  ${HAVE_DBUS}"
 echo "  Does Emacs use -lgconf?                                 ${HAVE_GCONF}"
 echo "  Does Emacs use GSettings?                               ${HAVE_GSETTINGS}"
+echo "  Does Emacs use a file notification library?             ${NOTIFY_SUMMARY}"
 echo "  Does Emacs use -lselinux?                               ${HAVE_LIBSELINUX}"
 echo "  Does Emacs use -lgnutls?                                ${HAVE_GNUTLS}"
 echo "  Does Emacs use -lxml2?                                  ${HAVE_LIBXML2}"

=== modified file 'lisp/autorevert.el'
--- lisp/autorevert.el	2013-05-22 14:47:19 +0000
+++ lisp/autorevert.el	2013-05-22 15:03:33 +0000
@@ -271,7 +271,7 @@
   :version "24.4")

 (defconst auto-revert-notify-enabled
-  (or (featurep 'inotify) (featurep 'w32notify))
+  (or (featurep 'gfilenotify) (featurep 'inotify) (featurep 'w32notify))
   "Non-nil when Emacs has been compiled with file notification support.")

 (defcustom auto-revert-use-notify auto-revert-notify-enabled
@@ -502,9 +502,12 @@
 	     (puthash key value auto-revert-notify-watch-descriptor-hash-list)
 	   (remhash key auto-revert-notify-watch-descriptor-hash-list)
 	   (ignore-errors
-	     (funcall (if (fboundp 'inotify-rm-watch)
-			  'inotify-rm-watch 'w32notify-rm-watch)
-		      auto-revert-notify-watch-descriptor)))))
+	     (funcall
+	      (cond
+	       ((fboundp 'gfile-rm-watch) 'gfile-rm-watch)
+	       ((fboundp 'inotify-rm-watch) 'inotify-rm-watch)
+	       ((fboundp 'w32notify-rm-watch) 'w32notify-rm-watch))
+	      auto-revert-notify-watch-descriptor)))))
      auto-revert-notify-watch-descriptor-hash-list)
     (remove-hook 'kill-buffer-hook 'auto-revert-notify-rm-watch))
   (setq auto-revert-notify-watch-descriptor nil
@@ -519,12 +522,18 @@

   (when (and buffer-file-name auto-revert-use-notify
 	     (not auto-revert-notify-watch-descriptor))
-    (let ((func (if (fboundp 'inotify-add-watch)
-		    'inotify-add-watch 'w32notify-add-watch))
-	  ;; `attrib' is needed for file modification time.
-	  (aspect (if (fboundp 'inotify-add-watch)
-		      '(attrib create modify moved-to) '(size last-write-time)))
-	  (file (if (fboundp 'inotify-add-watch)
+    (let ((func
+	   (cond
+	    ((fboundp 'gfile-add-watch) 'gfile-add-watch)
+	    ((fboundp 'inotify-add-watch) 'inotify-add-watch)
+	    ((fboundp 'w32notify-add-watch) 'w32notify-add-watch)))
+	  (aspect
+	   (cond
+	    ((fboundp 'gfile-add-watch) '(watch-mounts))
+	    ;; `attrib' is needed for file modification time.
+	    ((fboundp 'inotify-add-watch) '(attrib create modify moved-to))
+	    ((fboundp 'w32notify-add-watch) '(size last-write-time))))
+	  (file (if (or (fboundp 'gfile-add-watch) (fboundp 'inotify-add-watch))
 		    (directory-file-name (expand-file-name default-directory))
 		  (buffer-file-name))))
       (setq auto-revert-notify-watch-descriptor
@@ -545,10 +554,13 @@

 (defun auto-revert-notify-event-p (event)
   "Check that event is a file notification event."
-  (cond ((featurep 'inotify)
-	 (and (listp event) (= (length event) 4)))
-	((featurep 'w32notify)
-	 (and (listp event) (= (length event) 3) (stringp (nth 2 event))))))
+  (and (listp event)
+       (cond ((featurep 'gfilenotify)
+	      (and (>= (length event) 3) (stringp (nth 2 event))))
+	     ((featurep 'inotify)
+	      (= (length event) 4))
+	     ((featurep 'w32notify)
+	      (and (= (length event) 3) (stringp (nth 2 event)))))))

 (defun auto-revert-notify-event-descriptor (event)
   "Return watch descriptor of file notification event, or nil."
@@ -561,7 +573,8 @@
 (defun auto-revert-notify-event-file-name (event)
   "Return file name of file notification event, or nil."
   (and (auto-revert-notify-event-p event)
-       (cond ((featurep 'inotify) (nth 3 event))
+       (cond ((featurep 'gfilenotify) (nth 2 event))
+	     ((featurep 'inotify) (nth 3 event))
 	     ((featurep 'w32notify) (nth 2 event)))))

 (defun auto-revert-notify-handler (event)
@@ -576,12 +589,18 @@
 	;; Check, that event is meant for us.
 	;; TODO: Filter events which stop watching, like `move' or `removed'.
 	(cl-assert descriptor)
-	(when (featurep 'inotify)
+	(cond
+	 ((featurep 'gfilenotify)
+	  (cl-assert (or (eq 'attribute-changed action)
+			 (eq 'changed action)
+			 (eq 'created action)
+			 (eq 'deleted action))))
+	 ((featurep 'inotify)
 	  (cl-assert (or (memq 'attrib action)
 			 (memq 'create action)
 			 (memq 'modify action)
 			 (memq 'moved-to action))))
-	(when (featurep 'w32notify) (cl-assert (eq 'modified action)))
+	 ((featurep 'w32notify) (cl-assert (eq 'modified action))))
 	;; Since we watch a directory, a file name must be returned.
 	(cl-assert (stringp file))
 	(dolist (buffer buffers)

=== modified file 'lisp/subr.el'
--- lisp/subr.el	2013-05-25 16:05:19 +0000
+++ lisp/subr.el	2013-05-27 06:24:20 +0000
@@ -4432,32 +4432,16 @@
 \f
 ;;;; Support for watching filesystem events.

-(defun inotify-event-p (event)
-  "Check if EVENT is an inotify event."
-  (and (listp event)
-       (>= (length event) 3)
-       (eq (car event) 'file-inotify)))
-
-;;;###autoload
-(defun inotify-handle-event (event)
-  "Handle inotify file system monitoring event.
-If EVENT is an inotify filewatch event, call its callback.
-Otherwise, signal a `filewatch-error'."
-  (interactive "e")
-  (unless (inotify-event-p event)
-    (signal 'filewatch-error (cons "Not a valid inotify event" event)))
-  (funcall (nth 2 event) (nth 1 event)))
-
-(defun w32notify-handle-event (event)
-  "Handle MS-Windows file system monitoring event.
-If EVENT is an MS-Windows filewatch event, call its callback.
-Otherwise, signal a `filewatch-error'."
-  (interactive "e")
-  (if (and (eq (car event) 'file-w32notify)
-	   (= (length event) 3))
+(defun file-notify-handle-event (event)
+  "Handle file system monitoring event.
+If EVENT is a filewatch event, call its callback.
+Otherwise, signal a `filewatch-error'."
+  (interactive "e")
+  (if (and (eq (car event) 'file-notify)
+	   (>= (length event) 3))
       (funcall (nth 2 event) (nth 1 event))
     (signal 'filewatch-error
-	    (cons "Not a valid MS-Windows file-notify event" event))))
+	    (cons "Not a valid file-notify event" event))))

 \f
 ;;;; Comparing version strings.

=== modified file 'src/Makefile.in'
--- src/Makefile.in	2013-05-16 09:58:56 +0000
+++ src/Makefile.in	2013-05-29 11:31:27 +0000
@@ -156,6 +156,11 @@
 ## gtkutil.o if USE_GTK, else empty.
 GTK_OBJ=@GTK_OBJ@

+## gfilenotify.o if HAVE_GFILENOTIFY.
+## inotify.o if HAVE_INOTIFY.
+## w32notify.o if HAVE_W32NOTIFY.
+NOTIFY_OBJ = @NOTIFY_OBJ@
+
 ## -ltermcap, or -lncurses, or -lcurses, or "".
 LIBS_TERMCAP=@LIBS_TERMCAP@
 ## terminfo.o if TERMINFO, else tparam.o.
@@ -363,7 +368,7 @@
 	syntax.o $(UNEXEC_OBJ) bytecode.o \
 	process.o gnutls.o callproc.o \
 	region-cache.o sound.o atimer.o \
-	doprnt.o intervals.o textprop.o composite.o xml.o inotify.o \
+	doprnt.o intervals.o textprop.o composite.o xml.o $(NOTIFY_OBJ) \
 	profiler.o \
 	$(MSDOS_OBJ) $(MSDOS_X_OBJ) $(NS_OBJ) $(CYGWIN_OBJ) $(FONT_OBJ) \
 	$(W32_OBJ) $(WINDOW_SYSTEM_OBJ)

=== modified file 'src/emacs.c'
--- src/emacs.c	2013-03-19 14:09:05 +0000
+++ src/emacs.c	2013-05-29 12:46:37 +0000
@@ -1252,7 +1252,9 @@

 #ifdef WINDOWSNT
   globals_of_w32 ();
+#ifdef HAVE_W32NOTIFY
   globals_of_w32notify ();
+#endif
   /* Initialize environment from registry settings.  */
   init_environment (argv);
   init_ntproc (dumping); /* must precede init_editfns.  */
@@ -1409,17 +1411,24 @@
       syms_of_gnutls ();
 #endif

+#ifdef HAVE_GFILENOTIFY
+      syms_of_gfilenotify ();
+#endif /* HAVE_GFILENOTIFY */
+
 #ifdef HAVE_INOTIFY
       syms_of_inotify ();
 #endif /* HAVE_INOTIFY */

+#ifdef HAVE_W32NOTIFY
+      syms_of_w32notify ();
+#endif /* HAVE_W32NOTIFY */
+
 #ifdef HAVE_DBUS
       syms_of_dbusbind ();
 #endif /* HAVE_DBUS */

 #ifdef WINDOWSNT
       syms_of_ntterm ();
-      syms_of_w32notify ();
 #endif /* WINDOWSNT */

       syms_of_profiler ();

=== added file 'src/gfilenotify.c'
--- src/gfilenotify.c	1970-01-01 00:00:00 +0000
+++ src/gfilenotify.c	2013-05-22 15:04:04 +0000
@@ -0,0 +1,266 @@
+/* Elisp bindings for D-Bus.
+   Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+
+#ifdef HAVE_GFILENOTIFY
+#include <stdio.h>
+#include <gio/gio.h>
+#include "lisp.h"
+#include "coding.h"
+#include "frame.h"
+#include "termhooks.h"
+#include "keyboard.h"
+#include "process.h"
+
+\f
+/* Subroutines.  */
+static Lisp_Object Qgfile_add_watch;
+static Lisp_Object Qgfile_rm_watch;
+
+/* Filter objects.  */
+static Lisp_Object Qwatch_mounts;      /* G_FILE_MONITOR_WATCH_MOUNTS  */
+static Lisp_Object Qsend_moved;        /* G_FILE_MONITOR_SEND_MOVED  */
+
+/* Event types.  */
+static Lisp_Object Qchanged;           /* G_FILE_MONITOR_EVENT_CHANGED  */
+static Lisp_Object Qchanges_done_hint; /* G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT  */
+static Lisp_Object Qdeleted;           /* G_FILE_MONITOR_EVENT_DELETED  */
+static Lisp_Object Qcreated;           /* G_FILE_MONITOR_EVENT_CREATED  */
+static Lisp_Object Qattribute_changed; /* G_FILE_MONITOR_EVENT_ATTRIBUTE_CHANGED  */
+static Lisp_Object Qpre_unmount;       /* G_FILE_MONITOR_EVENT_PRE_UNMOUNT  */
+static Lisp_Object Qunmounted;         /* G_FILE_MONITOR_EVENT_UNMOUNTED  */
+static Lisp_Object Qmoved;             /* G_FILE_MONITOR_EVENT_MOVED  */
+
+static Lisp_Object watch_list;
+
+/* This is the callback function for arriving signals from
+   g_file_monitor.  It shall create a Lisp event, and put it into
+   Emacs input queue.  */
+static gboolean
+dir_monitor_callback (GFileMonitor* monitor,
+		      GFile* file,
+		      GFile* other_file,
+		      GFileMonitorEvent event_type,
+		      gpointer user_data)
+{
+  Lisp_Object symbol, watch_object;
+  char *name = g_file_get_parse_name (file);
+  char *oname = other_file ? g_file_get_parse_name (other_file) : NULL;
+
+  /* Determine event symbol.  */
+  switch (event_type)
+    {
+    case G_FILE_MONITOR_EVENT_CHANGED:
+      symbol = Qchanged;
+      break;
+    case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT:
+      symbol = Qchanges_done_hint;
+      break;
+    case G_FILE_MONITOR_EVENT_DELETED:
+      symbol = Qdeleted;
+      break;
+    case G_FILE_MONITOR_EVENT_CREATED:
+      symbol = Qcreated;
+      break;
+    case G_FILE_MONITOR_EVENT_ATTRIBUTE_CHANGED:
+      symbol = Qattribute_changed;
+      break;
+    case G_FILE_MONITOR_EVENT_PRE_UNMOUNT:
+      symbol = Qpre_unmount;
+      break;
+    case G_FILE_MONITOR_EVENT_UNMOUNTED:
+      symbol = Qunmounted;
+      break;
+    case G_FILE_MONITOR_EVENT_MOVED:
+      symbol = Qmoved;
+      break;
+    default:
+      goto cleanup;
+    }
+
+  /* Determine callback function.  */
+  watch_object = Fassoc (XIL ((EMACS_INT) monitor), watch_list);
+
+  if (FUNCTIONP (CDR_SAFE (watch_object)))
+    {
+      /* Construct an event.  */
+      struct input_event event;
+      EVENT_INIT (event);
+      event.kind = FILE_NOTIFY_EVENT;
+      event.frame_or_window = Qnil;
+      event.arg = oname
+	? list2 (list4 (XIL ((EMACS_INT) monitor), symbol,
+			build_string (name), build_string (oname)),
+		 CDR_SAFE (watch_object))
+	: list2 (list3 (XIL ((EMACS_INT) monitor), symbol, build_string (name)),
+		 CDR_SAFE (watch_object));
+
+      /* Store it into the input event queue.  */
+      kbd_buffer_store_event (&event);
+    }
+
+  /* Cleanup.  */
+ cleanup:
+  g_free (name);
+  g_free (oname);
+
+  return TRUE;
+}
+
+DEFUN ("gfile-add-watch", Fgfile_add_watch, Sgfile_add_watch, 3, 3, 0,
+       doc: /* Add a watch for filesystem events pertaining to FILE.
+
+This arranges for filesystem events pertaining to FILE to be reported
+to Emacs.  Use `gfile-rm-watch' to cancel the watch.
+
+Value is a descriptor for the added watch.  If the file cannot be
+watched for some reason, this function signals a `file-error' error.
+
+FLAGS is a list of conditions to set what will be watched for.  It can
+include the following symbols:
+
+  'watch-mounts' -- watch for mount events
+  'send-moved'   -- pair 'deleted' and 'created' events caused by file
+                    renames (moves) and send a single 'event-moved'
+                    event instead
+
+When any event happens, Emacs will call the CALLBACK function passing
+it a single argument EVENT, which is of the form
+
+  (DESCRIPTOR ACTION FILE [FILE1])
+
+DESCRIPTOR is the same object as the one returned by this function.
+ACTION is the description of the event.  It could be any one of the
+following:
+
+  'changed'           -- FILE has changed
+  'changes-done-hint' -- a hint that this was probably the last change
+                         in a set of changes
+  'deleted'           -- FILE was deleted
+  'created'           -- FILE was created
+  'attribute-changed' -- a FILE attribute was changed
+  'pre-unmount'       -- the FILE location will soon be unmounted
+  'unmounted'         -- the FILE location was unmounted
+  'moved'             -- FILE was moved to FILE1
+
+FILE is the name of the file whose event is being reported.  FILE1
+will be reported only in case of the 'moved' event.  */)
+  (Lisp_Object file, Lisp_Object flags, Lisp_Object callback)
+{
+  Lisp_Object watch_descriptor, watch_object;
+  GFile *gfile;
+  GFileMonitor* monitor;
+  GFileMonitorFlags gflags = G_FILE_MONITOR_NONE;
+
+  /* Check parameters.  */
+  CHECK_STRING (file);
+  file = Fdirectory_file_name (Fexpand_file_name (file, Qnil));
+  if (NILP (Ffile_exists_p (file)))
+    report_file_error ("File does not exists", Fcons (file, Qnil));
+
+  CHECK_LIST (flags);
+
+  if (!FUNCTIONP (callback))
+    wrong_type_argument (Qinvalid_function, callback);
+
+  /* Create GFile name.  */
+  gfile = g_file_new_for_path (SSDATA (ENCODE_FILE (file)));
+
+  /* Assemble flags.  */
+  if (!NILP (Fmember (Qwatch_mounts, flags)))
+    gflags |= G_FILE_MONITOR_WATCH_MOUNTS;
+  if (!NILP (Fmember (Qsend_moved, flags)))
+    gflags |= G_FILE_MONITOR_SEND_MOVED;
+
+  /* Enable watch.  */
+  monitor = g_file_monitor (gfile, gflags, NULL, NULL);
+  if (monitor != NULL)
+    g_signal_connect (monitor, "changed",
+		      (GCallback) dir_monitor_callback, NULL);
+  else
+    report_file_error ("Cannot watch file", Fcons (file, Qnil));
+
+  /* Store watch object in watch list.  */
+  watch_descriptor = XIL ((EMACS_INT) monitor);
+  watch_object = Fcons (watch_descriptor, callback);
+  watch_list = Fcons (watch_object, watch_list);
+
+  return watch_descriptor;
+}
+
+DEFUN ("gfile-rm-watch", Fgfile_rm_watch, Sgfile_rm_watch, 1, 1, 0,
+       doc: /* Remove an existing WATCH-DESCRIPTOR.
+
+WATCH-DESCRIPTOR should be an object returned by `gfile-add-watch'.  */)
+     (Lisp_Object watch_descriptor)
+{
+  Lisp_Object watch_object;
+  GFileMonitor *monitor = (GFileMonitor *) XLI (watch_descriptor);
+
+  watch_object = Fassoc (watch_descriptor, watch_list);
+  if (NILP (watch_object))
+    report_file_error ("Not a watch descriptor",
+		       Fcons (watch_descriptor, Qnil));
+
+  if (!g_file_monitor_cancel (monitor))
+    report_file_error ("Could not rm watch",
+		       Fcons (watch_descriptor, Qnil));
+
+  /* Remove watch descriptor from watch list. */
+  watch_list = Fdelete (watch_object, watch_list);
+
+  /* Cleanup.  */
+  g_object_unref (monitor);
+
+  return Qt;
+}
+
+\f
+void
+syms_of_gfilenotify (void)
+{
+
+  g_type_init ();
+
+  DEFSYM (Qgfile_add_watch, "gfile-add-watch");
+  defsubr (&Sgfile_add_watch);
+
+  DEFSYM (Qgfile_rm_watch, "gfile-rm-watch");
+  defsubr (&Sgfile_rm_watch);
+
+  DEFSYM (Qwatch_mounts, "watch-mounts");
+  DEFSYM (Qsend_moved, "send-moved");
+  DEFSYM (Qchanged, "changed");
+  DEFSYM (Qchanges_done_hint, "changes-done-hint");
+  DEFSYM (Qdeleted, "deleted");
+  DEFSYM (Qcreated, "created");
+  DEFSYM (Qattribute_changed, "attribute-changed");
+  DEFSYM (Qpre_unmount, "pre-unmount");
+  DEFSYM (Qunmounted, "unmounted");
+  DEFSYM (Qmoved, "moved");
+
+  /* Initialize internal objects.  */
+  watch_list = Qnil;
+  staticpro (&watch_list);
+
+  Fprovide (intern_c_string ("gfilenotify"), Qnil);
+
+}
+
+#endif /* HAVE_GFILENOTIFY  */

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2013-05-23 18:49:57 +0000
+++ src/keyboard.c	2013-05-29 12:52:13 +0000
@@ -308,18 +308,15 @@
 Lisp_Object Qmouse_click;
 #ifdef HAVE_NTGUI
 Lisp_Object Qlanguage_change;
-#ifdef WINDOWSNT
-Lisp_Object Qfile_w32notify;
-#endif
 #endif
 static Lisp_Object Qdrag_n_drop;
 static Lisp_Object Qsave_session;
 #ifdef HAVE_DBUS
 static Lisp_Object Qdbus_event;
 #endif
-#ifdef HAVE_INOTIFY
-static Lisp_Object Qfile_inotify;
-#endif /* HAVE_INOTIFY */
+#ifdef USE_FILE_NOTIFY
+static Lisp_Object Qfile_notify;
+#endif /* USE_FILE_NOTIFY */
 static Lisp_Object Qconfig_changed_event;

 /* Lisp_Object Qmouse_movement; - also an event header */
@@ -4013,18 +4010,22 @@
 	  kbd_fetch_ptr = event + 1;
 	}
 #endif
-#ifdef WINDOWSNT
+#ifdef USE_FILE_NOTIFY
       else if (event->kind == FILE_NOTIFY_EVENT)
 	{
+#ifdef HAVE_W32NOTIFY
 	  /* Make an event (file-notify (DESCRIPTOR ACTION FILE) CALLBACK).  */
-	  obj = Fcons (Qfile_w32notify,
+	  obj = Fcons (Qfile_notify,
 		       list2 (list3 (make_number (event->code),
 				     XCAR (event->arg),
 				     XCDR (event->arg)),
 			      event->frame_or_window));
+#else
+          obj = make_lispy_event (event);
+#endif
 	  kbd_fetch_ptr = event + 1;
 	}
-#endif
+#endif /* USE_FILE_NOTIFY */
       else if (event->kind == SAVE_SESSION_EVENT)
         {
           obj = Fcons (Qsave_session, Fcons (event->arg, Qnil));
@@ -4082,13 +4083,6 @@
 	  kbd_fetch_ptr = event + 1;
 	}
 #endif
-#ifdef HAVE_INOTIFY
-      else if (event->kind == FILE_NOTIFY_EVENT)
-        {
-          obj = make_lispy_event (event);
-          kbd_fetch_ptr = event + 1;
-        }
-#endif
       else if (event->kind == CONFIG_CHANGED_EVENT)
 	{
 	  obj = make_lispy_event (event);
@@ -5991,12 +5985,12 @@
       }
 #endif /* HAVE_DBUS */

-#ifdef HAVE_INOTIFY
+#if defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY
     case FILE_NOTIFY_EVENT:
       {
-        return Fcons (Qfile_inotify, event->arg);
+        return Fcons (Qfile_notify, event->arg);
       }
-#endif /* HAVE_INOTIFY */
+#endif /* defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY */

     case CONFIG_CHANGED_EVENT:
 	return Fcons (Qconfig_changed_event,
@@ -11006,17 +11000,13 @@
   DEFSYM (Qlanguage_change, "language-change");
 #endif

-#ifdef WINDOWSNT
-  DEFSYM (Qfile_w32notify, "file-w32notify");
-#endif
-
 #ifdef HAVE_DBUS
   DEFSYM (Qdbus_event, "dbus-event");
 #endif

-#ifdef HAVE_INOTIFY
-  DEFSYM (Qfile_inotify, "file-inotify");
-#endif /* HAVE_INOTIFY */
+#ifdef USE_FILE_NOTIFY
+  DEFSYM (Qfile_notify, "file-notify");
+#endif /* USE_FILE_NOTIFY */

   DEFSYM (QCenable, ":enable");
   DEFSYM (QCvisible, ":visible");
@@ -11762,20 +11752,18 @@
 			    "dbus-handle-event");
 #endif

-#ifdef HAVE_INOTIFY
-  /* Define a special event which is raised for inotify callback
+#ifdef USE_FILE_NOTIFY
+  /* Define a special event which is raised for notification callback
      functions.  */
-  initial_define_lispy_key (Vspecial_event_map, "file-inotify",
-                            "inotify-handle-event");
-#endif /* HAVE_INOTIFY */
+  initial_define_lispy_key (Vspecial_event_map, "file-notify",
+                            "file-notify-handle-event");
+#endif /* USE_FILE_NOTIFY */

   initial_define_lispy_key (Vspecial_event_map, "config-changed-event",
 			    "ignore");
 #if defined (WINDOWSNT)
   initial_define_lispy_key (Vspecial_event_map, "language-change",
 			    "ignore");
-  initial_define_lispy_key (Vspecial_event_map, "file-w32notify",
-			    "w32notify-handle-event");
 #endif
 }


=== modified file 'src/lisp.h'
--- src/lisp.h	2013-05-15 18:54:49 +0000
+++ src/lisp.h	2013-05-29 12:39:19 +0000
@@ -3723,9 +3723,9 @@
 extern Lisp_Object Qfont_param;
 #endif

-#ifdef WINDOWSNT
-/* Defined on w32notify.c.  */
-extern void syms_of_w32notify (void);
+/* Defined in gfilenotify.c */
+#ifdef HAVE_GFILENOTIFY
+extern void syms_of_gfilenotify (void);
 #endif

 /* Defined in inotify.c */
@@ -3733,6 +3733,11 @@
 extern void syms_of_inotify (void);
 #endif

+#ifdef HAVE_W32NOTIFY
+/* Defined on w32notify.c.  */
+extern void syms_of_w32notify (void);
+#endif
+
 /* Defined in xfaces.c.  */
 extern Lisp_Object Qdefault, Qtool_bar, Qfringe;
 extern Lisp_Object Qheader_line, Qscroll_bar, Qcursor;

=== modified file 'src/termhooks.h'
--- src/termhooks.h	2013-03-11 01:17:40 +0000
+++ src/termhooks.h	2013-05-29 12:40:17 +0000
@@ -212,7 +212,7 @@
   , NS_NONKEY_EVENT
 #endif

-#if defined (HAVE_INOTIFY) || defined (HAVE_NTGUI)
+#ifdef USE_FILE_NOTIFY
   /* File or directory was changed.  */
   , FILE_NOTIFY_EVENT
 #endif

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Daniel Colascione]

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

On 5/29/2013 8:57 AM, Michael Albinus wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>>> I cannot test the MS Windows case; here I trust on you.
>>
>> Can you post an updated patch?
> 
> Appended.
> 
>> Also, it would be good for someone on Cygwin to test this and/or
>> comment on the applicability of the options to the Cygwin builds.
>> Ken, Daniel, can you take a look, please?  TIA.
> 

The change looks safe to me. I've been meaning to make file notifications work
under Cygwin for a while now, but I haven't had the time. Ideally, we'd provide
a generic notifications API instead of adding more Cygwin-specific code to Emacs.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Daniel Colascione <dancol@dancol.org> writes:

> Ideally, we'd provide a generic notifications API instead of adding
> more Cygwin-specific code to Emacs.

That's planned as next step, after the patch has landed in the trunk.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Ken Brown <kbrown@cornell.edu>,  Daniel Colascione <dancol@dancol.org>,  monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Wed, 29 May 2013 17:57:19 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I cannot test the MS Windows case; here I trust on you.
> >
> > Can you post an updated patch?
> 
> Appended.

Thanks, and sorry for the delay (I had a few other things on my
plate).

I tried this now; the testing was minimal and included a full
"configure && make" build, followed by invocation of auto-revert-mode
on some file.

First, I guess you tested this in a build that was linked against glib
regardless of gfilenotify.c.  Otherwise, I don't understand how it
compiled and linked for you under --with-file-notification=gfile,
because the compiler and linker flags reported by pkg-config for glib
were not propagated to src/Makefile.  The first 2 hunks below fix
that.  I don't think this is specific to w32, but feel free to point
out what I missed.

I also needed a few additional w32-specific changes to build without
any file notification mechanism; those changes are also below.

After that, both --with-file-notification=w32 and =no built fine, as
well as the default (without any switch), and they all seem to work as
expected.

When I used --with-file-notification=gfile, Emacs built OK, but
crashed in the bowels of glib the moment auto-revert-mode called the
glib monitor.  I post the backtrace below, for the record.  The glib
libraries I installed were stripped, so I couldn't step through their
code on the source level.  However, by comparing the disassembly with
the source code, I concluded that it crashes here:

  void
  _g_io_modules_ensure_extension_points_registered (void)
  {
    static gboolean registered_extensions = FALSE;
    GIOExtensionPoint *ep;

    G_LOCK (registered_extensions);   <<<<<<<<<<<<<<<<<<<<<<

G_LOCK has some convoluted expansion, of which there are several
variants, so I'm not sure which one exactly was used, but it is quite
clear that the crash was deliberate:

  (gdb) disassemble
  Dump of assembler code for function _g_io_modules_ensure_extension_points_registered:
     0x6d5a6044 <+0>:     push   %ebp
     0x6d5a6045 <+1>:     mov    %esp,%ebp
     0x6d5a6047 <+3>:     push   %esi
     0x6d5a6048 <+4>:     push   %ebx
     0x6d5a6049 <+5>:     sub    $0x10,%esp
     0x6d5a604c <+8>:     mov    0x6d64105c,%ebx
     0x6d5a6052 <+14>:    mov    0x4(%ebx),%esi
     0x6d5a6055 <+17>:    mov    0x6d632218,%eax
     0x6d5a605a <+22>:    test   %eax,%eax
     0x6d5a605c <+24>:    je     0x6d5a6158 <_g_io_modules_ensure_extension_points_registered+276>
     0x6d5a6062 <+30>:    mov    %eax,(%esp)
     0x6d5a6065 <+33>:    call   *%esi
  => 0x6d5a6067 <+35>:    mov    0x6d63221c,%eax

The value of ESI is zero.

Here's the last portion of the backtrace; as you see, the file name is
perfectly alright:

  Program received signal SIGSEGV, Segmentation fault.
  0x00000000 in ?? ()
  (gdb) bt
  #0  0x00000000 in ?? ()
  #1  0x6d5a6067 in _g_io_modules_ensure_extension_points_registered ()
     from D:\usr\bin\libgio-2.0-0.dll
  #2  0x6d5a618e in _g_io_modules_ensure_loaded ()
     from D:\usr\bin\libgio-2.0-0.dll
  #3  0x6d5c2a58 in get_default_vfs () from D:\usr\bin\libgio-2.0-0.dll
  #4  0x6860d8bb in g_once_impl () from D:\usr\bin\libglib-2.0-0.dll
  #5  0x6d5c2d5c in g_vfs_get_default () from D:\usr\bin\libgio-2.0-0.dll
  #6  0x6d595d67 in g_file_new_for_path () from D:\usr\bin\libgio-2.0-0.dll
  #7  0x01291727 in Fgfile_add_watch (file=85666001, flags=58732934,
      callback=86315250) at gfilenotify.c:183
  #8  0x011f14dd in Ffuncall (nargs=4, args=0x88e414) at eval.c:2681

  Lisp Backtrace:
  "gfile-add-watch" (0x88e418)
  "byte-code" (0x88e67c)
  "auto-revert-notify-add-watch" (0x88eae8)
  "auto-revert-buffers" (0x88eedc)
  "apply" (0x88eed8)
  "byte-code" (0x88f13c)
  "timer-event-handler" (0x88f5ac)
  (gdb) frame 7
  #7  0x01291727 in Fgfile_add_watch (file=85666001, flags=58732934,
      callback=86315250) at gfilenotify.c:183
  183       gfile = g_file_new_for_path (SSDATA (ENCODE_FILE (file)));
  (gdb) p file
  $1 = 85666001
  (gdb) xtype
  Lisp_String
  (gdb) xstring
  $2 = (struct Lisp_String *) 0x51b28d0
  "d:/usr/eli/utils/emacs-trunk_2013-05-27/src"

So I guess we won't be recommending glib file notifications to Emacs
users on Windows just yet ;-)

Perhaps someone who knows more about glib on Windows will be able to
explain the crash and suggest a solution.

Finally, here are the changes I suggest on top of yours:

--- configure.ac~1	2013-06-02 07:32:48.220647900 +0300
+++ configure.ac	2013-06-02 09:57:31.173191700 +0300
@@ -2329,7 +2329,6 @@
    PKG_CHECK_MODULES(GFILENOTIFY, gio-2.0 >= 2.18, HAVE_GFILENOTIFY=yes, HAVE_GFILENOTIFY=no)
    if test "$HAVE_GFILENOTIFY" = "yes"; then
       AC_DEFINE(HAVE_GFILENOTIFY, 1, [Define to 1 if using GFile.])
-      LIBS="$LIBS $GFILENOTIFY_LIBS"
       NOTIFY_OBJ=gfilenotify.o
       NOTIFY_SUMMARY="yes -lgio (gfile)"
    fi
@@ -2359,6 +2358,8 @@
    AC_DEFINE(USE_FILE_NOTIFY, 1, [Define to 1 if using file notifications.])
 fi
 AC_SUBST(NOTIFY_OBJ)
+AC_SUBST(GFILENOTIFY_CFLAGS)
+AC_SUBST(GFILENOTIFY_LIBS)
 
 dnl Do not put whitespace before the #include statements below.
 dnl Older compilers (eg sunos4 cc) choke on it.


--- src/Makefile.in~1	2013-06-02 07:32:48.236247900 +0300
+++ src/Makefile.in	2013-06-02 09:56:15.342077800 +0300
@@ -160,6 +160,8 @@
 ## inotify.o if HAVE_INOTIFY.
 ## w32notify.o if HAVE_W32NOTIFY.
 NOTIFY_OBJ = @NOTIFY_OBJ@
+GFILENOTIFY_CFLAGS = @GFILENOTIFY_CFLAGS@
+GFILENOTIFY_LIBS = @GFILENOTIFY_LIBS@
 
 ## -ltermcap, or -lncurses, or -lcurses, or "".
 LIBS_TERMCAP=@LIBS_TERMCAP@
@@ -342,7 +344,7 @@
   $(LIBXML2_CFLAGS) $(DBUS_CFLAGS) $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) \
   $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
   $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
-  $(LIBGNUTLS_CFLAGS) \
+  $(LIBGNUTLS_CFLAGS) $(GFILENOTIFY_CFLAGS) \
   $(WARN_CFLAGS) $(WERROR_CFLAGS) $(CFLAGS)
 ALL_OBJC_CFLAGS=$(ALL_CFLAGS) $(GNU_OBJC_CFLAGS)
 
@@ -424,7 +426,7 @@
    $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
    $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
    $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(LIB_PTHREAD_SIGMASK) \
-   $(LIB_MATH)
+   $(GFILENOTIFY_LIBS) $(LIB_MATH)
 
 all: emacs$(EXEEXT) $(OTHER_FILES)
 .PHONY: all


--- nt/config.nt~1	2013-05-09 07:10:33.000000000 +0300
+++ nt/config.nt	2013-06-02 07:49:56.258512600 +0300
@@ -547,6 +547,9 @@
 /* Define to 1 if you have the `gettimeofday' function. */
 #define HAVE_GETTIMEOFDAY 1
 
+/* Define to 1 if using GFile. */
+#undef HAVE_GFILENOTIFY
+
 /* Define to 1 if you have the `get_current_dir_name' function. */
 #undef HAVE_GET_CURRENT_DIR_NAME
 
@@ -1141,6 +1144,9 @@
 /* Define to 1 if you have the <vfork.h> header file. */
 #undef HAVE_VFORK_H
 
+/* Define to 1 to use w32notify. */
+#define HAVE_W32NOTIFY 1
+
 /* Define to 1 if you have the <wchar.h> header file. */
 #undef HAVE_WCHAR_H
 
@@ -1497,6 +1503,9 @@
 /* Define to nonzero if you want access control list support. */
 #undef USE_ACL
 
+/* Define to 1 if using file notifications. */
+#define USE_FILE_NOTIFY 1
+
 /* Define to 1 if using GTK. */
 #undef USE_GTK
 


--- src/w32inevt.c~1	2013-01-07 14:13:25.000000000 +0200
+++ src/w32inevt.c	2013-06-02 07:56:13.907896900 +0300
@@ -577,6 +577,7 @@
 		     0, 0, 0);
 }
 
+#if HAVE_W32NOTIFY
 static int
 handle_file_notifications (struct input_event *hold_quit)
 {
@@ -644,6 +645,13 @@
   leave_crit ();
   return nevents;
 }
+#else  /* !HAVE_W32NOTIFY */
+static int
+handle_file_notifications (struct input_event *hold_quit)
+{
+  return 0;
+}
+#endif	/* !HAVE_W32NOTIFY */
 
 /* Here's an overview of how Emacs input works in non-GUI sessions on
    MS-Windows.  (For description of the GUI input, see the commentary


--- src/w32term.c~1	2013-04-21 14:52:21.000000000 +0300
+++ src/w32term.c	2013-06-02 08:58:01.383496900 +0300
@@ -3210,6 +3210,8 @@
 }
 
 \f
+#if HAVE_W32NOTIFY
+
 /* File event notifications (see w32notify.c).  */
 
 Lisp_Object
@@ -3325,7 +3327,8 @@
   /* We've stuffed all the events ourselves, so w32_read_socket shouldn't.  */
   event->kind = NO_EVENT;
 }
-#endif
+#endif	/* WINDOWSNT */
+#endif	/* HAVE_W32NOTIFY */
 
 \f
 /* Function to report a mouse movement to the mainstream Emacs code.
@@ -4968,7 +4971,7 @@
 	  check_visibility = 1;
 	  break;
 
-#ifdef WINDOWSNT
+#if HAVE_W32NOTIFY
 	case WM_EMACS_FILENOTIFY:
 	  f = x_window_to_frame (dpyinfo, msg.msg.hwnd);
 	  if (f)

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

> First, I guess you tested this in a build that was linked against glib
> regardless of gfilenotify.c.  Otherwise, I don't understand how it
> compiled and linked for you under --with-file-notification=gfile,
> because the compiler and linker flags reported by pkg-config for glib
> were not propagated to src/Makefile.  The first 2 hunks below fix
> that.  I don't think this is specific to w32, but feel free to point
> out what I missed.

Under GNU/Linux, there are no additional cflags needed, so I didn't miss
them :-)

> When I used --with-file-notification=gfile, Emacs built OK, but
> crashed in the bowels of glib the moment auto-revert-mode called the
> glib monitor.  I post the backtrace below, for the record.  The glib
> libraries I installed were stripped, so I couldn't step through their
> code on the source level.  However, by comparing the disassembly with
> the source code, I concluded that it crashes here:
>
>   void
>   _g_io_modules_ensure_extension_points_registered (void)
>   {
>     static gboolean registered_extensions = FALSE;
>     GIOExtensionPoint *ep;
>
>     G_LOCK (registered_extensions);   <<<<<<<<<<<<<<<<<<<<<<

G_LOCK is a macro in gthread.h. I don't know, how multithreading of glib
works on MS Windows.

> So I guess we won't be recommending glib file notifications to Emacs
> users on Windows just yet ;-)

Yep. Maybe it is worth to be mentioned in etc/PROBLEMS. Would be
interesting to know how it behaves under cygwin, or osx.

> Perhaps someone who knows more about glib on Windows will be able to
> explain the crash and suggest a solution.

At least it would be great to find a test for configure.ac, that we can
kick off glib on Windows in case.

> Finally, here are the changes I suggest on top of yours:

Thanks, works fine here. I have applied my patch to the trunk. You could
apply your patch on top of it (or tell me to do it for you).

I will to add some few lines to etc/NEWS and to the doc. Afterwards, we
could start to think how a general file notifications interface shall
look like.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: kbrown@cornell.edu,  dancol@dancol.org,  monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 03 Jun 2013 15:21:15 +0200
> 
> >   void
> >   _g_io_modules_ensure_extension_points_registered (void)
> >   {
> >     static gboolean registered_extensions = FALSE;
> >     GIOExtensionPoint *ep;
> >
> >     G_LOCK (registered_extensions);   <<<<<<<<<<<<<<<<<<<<<<
> 
> G_LOCK is a macro in gthread.h.

Yes, I know, I looked up its expansion, which invokes another macro,
and another, etc.  Some of them have several variants.

> I don't know, how multithreading of glib works on MS Windows.

No matter how it works, it doesn't involve calling a function through
a NULL pointer ;-)

> I have applied my patch to the trunk. You could apply your patch on
> top of it (or tell me to do it for you).

Please commit my changes as well, I'm a bit busy at the moment.

Thanks.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Glenn Morris]

Fails to build (with or without Eli's changes) on RHEL6 using

configure --without-xaw3d --without-gconf --without-gsettings \
   --without-xim --with-x --with-x-toolkit=athena \
   --without-toolkit-scroll-bars

  Does Emacs use -lrsvg-2?                                yes
  Does Emacs use -lgconf?                                 no
  Does Emacs use GSettings?                               no
  Does Emacs use a file notification library?             yes -lgio (gfile)

rpm -q glib2
glib2-2.22.5-7.el6.x86_64

gfilenotify.c: In function 'dir_monitor_callback':
gfilenotify.c:90: error: 'G_FILE_MONITOR_EVENT_MOVED' undeclared (first
use in this function)
gfilenotify.c:90: error: (Each undeclared identifier is reported only once
gfilenotify.c:90: error: for each function it appears in.)
gfilenotify.c: In function 'Fgfile_add_watch':
gfilenotify.c:189: error: 'G_FILE_MONITOR_SEND_MOVED' undeclared (first
use in this function)
make[1]: *** [gfilenotify.o] Error 1

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: Michael Albinus <michael.albinus@gmx.de>,  dancol@dancol.org,  monnier@iro.umontreal.ca,  kbrown@cornell.edu,  emacs-devel@gnu.org
> Date: Mon, 03 Jun 2013 11:30:45 -0400
> 
> 
> Fails to build (with or without Eli's changes) on RHEL6 using
> 
> configure --without-xaw3d --without-gconf --without-gsettings \
>    --without-xim --with-x --with-x-toolkit=athena \
>    --without-toolkit-scroll-bars
> 
>   Does Emacs use -lrsvg-2?                                yes
>   Does Emacs use -lgconf?                                 no
>   Does Emacs use GSettings?                               no
>   Does Emacs use a file notification library?             yes -lgio (gfile)
> 
> rpm -q glib2
> glib2-2.22.5-7.el6.x86_64
> 
> gfilenotify.c: In function 'dir_monitor_callback':
> gfilenotify.c:90: error: 'G_FILE_MONITOR_EVENT_MOVED' undeclared (first
> use in this function)
> gfilenotify.c:90: error: (Each undeclared identifier is reported only once
> gfilenotify.c:90: error: for each function it appears in.)
> gfilenotify.c: In function 'Fgfile_add_watch':
> gfilenotify.c:189: error: 'G_FILE_MONITOR_SEND_MOVED' undeclared (first
> use in this function)
> make[1]: *** [gfilenotify.o] Error 1

FWIW, I tried with glib 2.28.  Maybe 2.22 is too old?

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> Date: Mon, 03 Jun 2013 19:29:51 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: dancol@dancol.org, michael.albinus@gmx.de, monnier@iro.umontreal.ca,
> 	kbrown@cornell.edu, emacs-devel@gnu.org
> 
> > From: Glenn Morris <rgm@gnu.org>
> > Cc: Michael Albinus <michael.albinus@gmx.de>,  dancol@dancol.org,  monnier@iro.umontreal.ca,  kbrown@cornell.edu,  emacs-devel@gnu.org
> > Date: Mon, 03 Jun 2013 11:30:45 -0400
> > 
> > 
> > Fails to build (with or without Eli's changes) on RHEL6 using
> > 
> > configure --without-xaw3d --without-gconf --without-gsettings \
> >    --without-xim --with-x --with-x-toolkit=athena \
> >    --without-toolkit-scroll-bars
> > 
> >   Does Emacs use -lrsvg-2?                                yes
> >   Does Emacs use -lgconf?                                 no
> >   Does Emacs use GSettings?                               no
> >   Does Emacs use a file notification library?             yes -lgio (gfile)
> > 
> > rpm -q glib2
> > glib2-2.22.5-7.el6.x86_64
> > 
> > gfilenotify.c: In function 'dir_monitor_callback':
> > gfilenotify.c:90: error: 'G_FILE_MONITOR_EVENT_MOVED' undeclared (first
> > use in this function)
> > gfilenotify.c:90: error: (Each undeclared identifier is reported only once
> > gfilenotify.c:90: error: for each function it appears in.)
> > gfilenotify.c: In function 'Fgfile_add_watch':
> > gfilenotify.c:189: error: 'G_FILE_MONITOR_SEND_MOVED' undeclared (first
> > use in this function)
> > make[1]: *** [gfilenotify.o] Error 1
> 
> FWIW, I tried with glib 2.28.  Maybe 2.22 is too old?

Alternatively, can you compile successfully if you ifdef away
G_FILE_MONITOR_EVENT_MOVED?  Perhaps this is a late addition.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Glenn Morris]

Eli Zaretskii wrote:

>> FWIW, I tried with glib 2.28.  Maybe 2.22 is too old?

I'm sure that's the case.

> Alternatively, can you compile successfully if you ifdef away
> G_FILE_MONITOR_EVENT_MOVED?  Perhaps this is a late addition.

Yes, but there is also G_FILE_MONITOR_SEND_MOVED missing. If this isn't
present the doc of gfile-add-watch will be incorrect, since the
send-moved case will be a no-op.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

> Eli Zaretskii wrote:
>
>>> FWIW, I tried with glib 2.28.  Maybe 2.22 is too old?
>
> I'm sure that's the case.

G_FILE_MONITOR_EVENT_MOVED has been added to gioenums.h in gio 2.24. Compare
<https://git.gnome.org/browse/glib/tree/gio/gioenums.h?h=glib-2-24>
<https://git.gnome.org/browse/glib/tree/gio/gioenums.h?h=glib-2-22>

I've adapted the check in configure.ac.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

I tried the new monitor and it made Emacs freeze.
I was running "emacs -nw", which I think may help explain the problem.
Fedora 17, x86-64.  Here's what I ran:

(gfile-add-watch "/tmp/foo" '() (lambda (ev) (message "%s" ev)))

Here are the diagnostics that were blatted
onto my screen before Emacs froze:

(process:11082): GLib-GObject-CRITICAL **: gtype.c:2722: You forgot to call g_type_init()
(process:11082): GLib-GObject-CRITICAL **: g_type_interface_add_prerequisite: assertion `G_TYPE_IS_INTERFACE (interface_type)'\
 failed
(process:11082): GLib-CRITICAL **: g_once_init_leave: assertion `result != 0' failed
(process:11082): GLib-GObject-CRITICAL **: gtype.c:2722: You forgot to call g_type_init()
(process:11082): GLib-CRITICAL **: g_once_init_leave: assertion `result != 0' failed
(process:11082): GLib-GObject-CRITICAL **: gtype.c:2722: You forgot to call g_type_init()
(process:11082): GLib-CRITICAL **: g_once_init_leave: assertion `result != 0' failed

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Paul Eggert <eggert@cs.ucla.edu> writes:

> I tried the new monitor and it made Emacs freeze.
> I was running "emacs -nw", which I think may help explain the problem.
> Fedora 17, x86-64.  Here's what I ran:
>
> (gfile-add-watch "/tmp/foo" '() (lambda (ev) (message "%s" ev)))
>
> Here are the diagnostics that were blatted
> onto my screen before Emacs froze:
>
> (process:11082): GLib-GObject-CRITICAL **: gtype.c:2722: You forgot to
> call g_type_init()

I've committed a patch to the trunk which prevents this crash. However,
if you call "emacs -nw", you won't get any notification. I suppose we
need an generalized g_main_loop integration, synchronized with the code
in gtkutil.c.

Likely, we must move this code out of gtkutil.c and gfilenotify.c. Jan?

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Jan Djärv]

Hello.

5 jun 2013 kl. 15:25 skrev Michael Albinus <michael.albinus@gmx.de>:

> Paul Eggert <eggert@cs.ucla.edu> writes:
> 
>> I tried the new monitor and it made Emacs freeze.
>> I was running "emacs -nw", which I think may help explain the problem.
>> Fedora 17, x86-64.  Here's what I ran:
>> 
>> (gfile-add-watch "/tmp/foo" '() (lambda (ev) (message "%s" ev)))
>> 
>> Here are the diagnostics that were blatted
>> onto my screen before Emacs froze:
>> 
>> (process:11082): GLib-GObject-CRITICAL **: gtype.c:2722: You forgot to
>> call g_type_init()
> 
> I've committed a patch to the trunk which prevents this crash. However,
> if you call "emacs -nw", you won't get any notification. I suppose we
> need an generalized g_main_loop integration, synchronized with the code
> in gtkutil.c.
> 
> Likely, we must move this code out of gtkutil.c and gfilenotify.c. Jan?


Which code?  The glib event loop integration is in xgselect.c.  I think it is called in the -nw case also.  It is only dependent on HAVE_GLIB.
The Gtk+ event loop integration is in xterm.c, it is a separate thing.

	Jan D.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Jan Djärv <jan.h.d@swipnet.se> writes:

> Hello.

Hi,

>> I've committed a patch to the trunk which prevents this crash. However,
>> if you call "emacs -nw", you won't get any notification. I suppose we
>> need an generalized g_main_loop integration, synchronized with the code
>> in gtkutil.c.
>> 
>> Likely, we must move this code out of gtkutil.c and gfilenotify.c. Jan?
>
>
> Which code?  The glib event loop integration is in xgselect.c.  I
> think it is called in the -nw case also.  It is only dependent on
> HAVE_GLIB.
> The Gtk+ event loop integration is in xterm.c, it is a separate thing.

I was thinking about the g_main_loop_run and g_main_loop_quit calls in
gtkutil.c. Reading the code in more detail, it looks like the calls have
a local scope only, so we don't need to sync.

gfilenotify.c needs a running glib event loop. If Emacs is called with
the "-nw" flag there is no such event loop; it must be started in a new
thread by gfilenotify.c I believe.

> 	Jan D.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Jan Djärv]

Hello.

6 jun 2013 kl. 13:06 skrev Michael Albinus <michael.albinus@gmx.de>:

> Jan Djärv <jan.h.d@swipnet.se> writes:
> 
>> Hello.
> 
> Hi,
> 
>>> I've committed a patch to the trunk which prevents this crash. However,
>>> if you call "emacs -nw", you won't get any notification. I suppose we
>>> need an generalized g_main_loop integration, synchronized with the code
>>> in gtkutil.c.
>>> 
>>> Likely, we must move this code out of gtkutil.c and gfilenotify.c. Jan?
>> 
>> 
>> Which code?  The glib event loop integration is in xgselect.c.  I
>> think it is called in the -nw case also.  It is only dependent on
>> HAVE_GLIB.
>> The Gtk+ event loop integration is in xterm.c, it is a separate thing.
> 
> I was thinking about the g_main_loop_run and g_main_loop_quit calls in
> gtkutil.c. Reading the code in more detail, it looks like the calls have
> a local scope only, so we don't need to sync.

Right, it is only used when popping up a dialog.

> 
> gfilenotify.c needs a running glib event loop. If Emacs is called with
> the "-nw" flag there is no such event loop; it must be started in a new
> thread by gfilenotify.c I believe.


No need for that, you can just apply the patch below to xgselect.c and you will have a glib handling events for -nw.  The first call to g_main_context_pending does not return true for file monitoring events, why I don't know.  I think the window check was added for bug 9754 (http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9754), but you have to remove it for -nw.

=== modified file 'src/xgselect.c'
--- src/xgselect.c      2013-06-03 18:47:35 +0000
+++ src/xgselect.c      2013-06-07 10:40:37 +0000
@@ -44,15 +44,12 @@
   int i, nfds, tmo_in_millisec;
   USE_SAFE_ALLOCA;
 
-  if (! (window_system_available (NULL)
-        && g_main_context_pending (context = g_main_context_default ())))
-    return pselect (fds_lim, rfds, wfds, efds, timeout, sigmask);
-
   if (rfds) all_rfds = *rfds;
   else FD_ZERO (&all_rfds);
   if (wfds) all_wfds = *wfds;
   else FD_ZERO (&all_wfds);
 
+  context = g_main_context_default ();
   n_gfds = g_main_context_query (context, G_PRIORITY_LOW, &tmo_in_millisec,
                                 gfds, gfds_size);
   if (gfds_size < n_gfds)


	Jan D.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Jan Djärv <jan.h.d@swipnet.se> writes:

> Hello.

Hi Jan,

> No need for that, you can just apply the patch below to xgselect.c and
> you will have a glib handling events for -nw.

Works like a charm, thanks! Do you install the patch, or shall I do it
in your name?

> 	Jan D.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Jan Djärv]

Hello.

7 jun 2013 kl. 14:18 skrev Michael Albinus <michael.albinus@gmx.de>:

> Jan Djärv <jan.h.d@swipnet.se> writes:
> 
>> Hello.
> 
> Hi Jan,
> 
>> No need for that, you can just apply the patch below to xgselect.c and
>> you will have a glib handling events for -nw.
> 
> Works like a charm, thanks! Do you install the patch, or shall I do it
> in your name?

I did not install it, because I didn't have time to investigate possible impact on bug 9754.

	Jan D.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Jan Djärv <jan.h.d@swipnet.se> writes:

> Hello.

Hi Jan,

>>> No need for that, you can just apply the patch below to xgselect.c and
>>> you will have a glib handling events for -nw.
>>
>> Works like a charm, thanks! Do you install the patch, or shall I do it
>> in your name?
>
> I did not install it, because I didn't have time to investigate
> possible impact on bug 9754.

I've read the messages in that bug report, and it looks to me like the
issue is solved even when not checking window_system_available. But you
are more familiar with that topic, it's always better that you
cross-check.

> 	Jan D.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Jan Djärv]

Hello.

7 jun 2013 kl. 16:50 skrev Michael Albinus <michael.albinus@gmx.de>:

> Jan Djärv <jan.h.d@swipnet.se> writes:
> 
>> Hello.
> 
> Hi Jan,
> 
>>>> No need for that, you can just apply the patch below to xgselect.c and
>>>> you will have a glib handling events for -nw.
>>> 
>>> Works like a charm, thanks! Do you install the patch, or shall I do it
>>> in your name?
>> 
>> I did not install it, because I didn't have time to investigate
>> possible impact on bug 9754.
> 
> I've read the messages in that bug report, and it looks to me like the
> issue is solved even when not checking window_system_available. But you
> are more familiar with that topic, it's always better that you
> cross-check.


I checked in the change.  There was a bug pointed out by 9754, but only in the 23-branch.
The discussion after that was really about a Cygwin bug.

	Jan D.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

A quick review of some of the C-level changes:

* The XIL ((EMACS_INT) monitor) trick doesn't work in
  general, since the monitor address could look like a
  tagged Emacs object and the garbage collector could
  go kaflooey on it.  Instead, perhaps use make_save_pointer,
  as it's designed for this sort of thing.

* Faster and simpler than FUNCTIONP (CDR_SAFE (watch_object))
  is CONSP (watch_object).  Since the user can't modify
  the watch list we can rely on it to be a list of pairs.

* report_file_error reads errno, so it's not suitable
  when errno's value is indeterminate.  
  xsignal... (Qfile_error, ...) is preferable.

Maybe something like the following patch?  I couldn't get
this stuff to work on my Fedora 17 host, either without or
with the patch, but it should be necessary to apply something
like the following.

=== modified file 'src/gfilenotify.c'
--- src/gfilenotify.c	2013-06-03 13:03:05 +0000
+++ src/gfilenotify.c	2013-06-04 06:43:23 +0000
@@ -95,21 +95,26 @@ dir_monitor_callback (GFileMonitor* moni
     }
 
   /* Determine callback function.  */
-  watch_object = Fassoc (XIL ((EMACS_INT) monitor), watch_list);
+  for (watch_object = watch_list;
+       CONSP (watch_object);
+       watch_object = XCDR (watch_object))
+    if (XSAVE_POINTER (XCAR (watch_object), 0) == monitor)
+      break;
 
-  if (FUNCTIONP (CDR_SAFE (watch_object)))
+  if (CONSP (watch_object))
     {
       /* Construct an event.  */
       struct input_event event;
       EVENT_INIT (event);
       event.kind = FILE_NOTIFY_EVENT;
       event.frame_or_window = Qnil;
-      event.arg = oname
-	? list2 (list4 (XIL ((EMACS_INT) monitor), symbol,
-			build_string (name), build_string (oname)),
-		 CDR_SAFE (watch_object))
-	: list2 (list3 (XIL ((EMACS_INT) monitor), symbol, build_string (name)),
-		 CDR_SAFE (watch_object));
+      event.arg = list2 (Fcons (XCAR (watch_object),
+				Fcons (symbol,
+				       Fcons (build_string (name),
+					      (oname
+					       ? list1 (build_string (oname))
+					       : Qnil)))),
+			 XCDR (watch_object));
 
       /* Store it into the input event queue.  */
       kbd_buffer_store_event (&event);
@@ -194,10 +199,10 @@ will be reported only in case of the 'mo
     g_signal_connect (monitor, "changed",
 		      (GCallback) dir_monitor_callback, NULL);
   else
-    report_file_error ("Cannot watch file", Fcons (file, Qnil));
+    xsignal2 (Qfile_error, build_string ("Cannot watch file"), file);
 
   /* Store watch object in watch list.  */
-  watch_descriptor = XIL ((EMACS_INT) monitor);
+  watch_descriptor = make_save_pointer (monitor);
   watch_object = Fcons (watch_descriptor, callback);
   watch_list = Fcons (watch_object, watch_list);
 
@@ -210,20 +215,20 @@ DEFUN ("gfile-rm-watch", Fgfile_rm_watch
 WATCH-DESCRIPTOR should be an object returned by `gfile-add-watch'.  */)
      (Lisp_Object watch_descriptor)
 {
-  Lisp_Object watch_object;
-  GFileMonitor *monitor = (GFileMonitor *) XLI (watch_descriptor);
+  GFileMonitor *monitor;
+  Lisp_Object watch_object = Fassq (watch_descriptor, watch_list);
 
-  watch_object = Fassoc (watch_descriptor, watch_list);
-  if (NILP (watch_object))
-    report_file_error ("Not a watch descriptor",
-		       Fcons (watch_descriptor, Qnil));
+  if (! CONSP (watch_object))
+    xsignal2 (Qfile_error, build_string ("Not a watch descriptor"),
+	      watch_descriptor);
 
+  monitor = XSAVE_POINTER (XCAR (watch_object), 0);
   if (!g_file_monitor_cancel (monitor))
-    report_file_error ("Could not rm watch",
-		       Fcons (watch_descriptor, Qnil));
+    xsignal2 (Qfile_error, build_string ("Could not rm watch"),
+	      watch_descriptor);
 
   /* Remove watch descriptor from watch list. */
-  watch_list = Fdelete (watch_object, watch_list);
+  watch_list = Fdelq (watch_object, watch_list);
 
   /* Cleanup.  */
   g_object_unref (monitor);

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> Date: Mon, 03 Jun 2013 23:45:30 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> * The XIL ((EMACS_INT) monitor) trick doesn't work in
>   general, since the monitor address could look like a
>   tagged Emacs object and the garbage collector could
>   go kaflooey on it.

You mean, if the monitor address is something other than a pointer?
Or do you mean that the alignment of that pointer might be not enough
to ensure that all the tag bits are zero?

>   Instead, perhaps use make_save_pointer,
>   as it's designed for this sort of thing.

AFAIR at the time Stefan didn't like this alternative (or any other,
for that matter).

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

On 06/04/2013 08:16 AM, Eli Zaretskii wrote:
> You mean, if the monitor address is something other than a pointer?
> Or do you mean that the alignment of that pointer might be not enough
> to ensure that all the tag bits are zero?

The latter.  Also, suppose we're using a host with MSB tags, or
are compiling --with-wide-int.  All of these could cause problems.

> AFAIR at the time Stefan didn't like this alternative (or any other,
> for that matter).

I don't much like it either, but it's better than crashing.

(Maybe I should dust off my bignum implementation?  :-)

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Stefan Monnier]

>> AFAIR at the time Stefan didn't like this alternative (or any other,
>> for that matter).

make_save_value is for objects which shouldn't be visible to Elisp.
It's not terrible if Elisp gets to see them, but it should not happen
"in the normal course of things".
If you need something like that, then create a new type for it (either
a new lisp_misc or a new pseudo vector).


        Stefan

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Paul Eggert]

On 06/04/2013 06:23 PM, Stefan Monnier wrote:
> If you need something like that, then create a new type for it (either
> a new lisp_misc or a new pseudo vector).

I installed a more-conservative change instead as trunk bzr 112869:
it causes Emacs to check that a file monitor is indeed a Lisp integer
after conversion, and causes gfile-add-watch to throw an exception
otherwise.  (Another possibility might be to emacs_abort () on this
sort of failure.)

If we run into a platform where glib file monitors aren't valid Lisp
integers, we can create a new Lisp_Misc etc. then, but perhaps we'll
get lucky and not need to do it.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> I have applied my patch to the trunk. You could apply your patch on
>> top of it (or tell me to do it for you).
>
> Please commit my changes as well, I'm a bit busy at the moment.

Done.

> Thanks.

Best regards, Michael.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Ken Brown]

On 5/29/2013 11:25 AM, Eli Zaretskii wrote:
>> From: Michael Albinus <michael.albinus@gmx.de>
>> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
>> Date: Wed, 29 May 2013 15:30:57 +0200
>>
>> I will wait another couple of days (let's say until Sunday evening), if
>> there's no objection I will commit.
>
> Thanks.
>
>> I cannot test the MS Windows case; here I trust on you.
>
> Can you post an updated patch?
>
> Also, it would be good for someone on Cygwin to test this and/or
> comment on the applicability of the options to the Cygwin builds.
> Ken, Daniel, can you take a look, please?  TIA.

I'm busy with another project right now, but I should be able to get to 
it within a week.

Ken

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Tue, 28 May 2013 12:39:15 +0200
> Cc: emacs-devel@gnu.org
> 
> There is a new gfilenotify.c, which uses glib's functionality. There is
> also a new config option --with-file-notification=LIB, LIB can be "yes",
> "no", "gfile" or "inotify". Default on non-w32 machines is "gfile" if
> applicable.

One more thing: the default on w32 should be "w32notify", of course.

Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> One more thing: the default on w32 should be "w32notify", of course.

For sure.