bug#16519: 24.3.50; gfile notifications not received in batch mode

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: 16519@debbugs.gnu.org
> Date: Thu, 30 Jan 2014 11:03:33 +0100
> 
> > No, it seems to be read-event, see below.
> > 
> `sit-for' calls `read-event', if `input-pending-p' returns nil. So it
> shall be safe to call.

Obviously, input-pending-p returns non-nil on Windows, so read-event
is never called.

If we want sit-for to call read-event, then let's call read-event
directly.  Why risk platform-specific behavior in delicate functions
like input-pending-p? what does it gain us here?  sit-for does nothing
useful except call read-event.

> All interactive cases work. In the noninteractive case, it works also
> for inotify. For w32notify and gfilenotify it doesn't work yet, that's
> why there is the expected result :fail.

My crystal ball says that you've done all your testing on GNU/Linux
with a build that has D-Bus support compiled in.  If so, please try
a --without-dbus build: I'm sure you will see that read-event in batch
mode doesn't return in such a build even if there is a time-out.

The reason is this part in kbd_buffer_get_event:

  #ifndef HAVE_DBUS  /* We want to read D-Bus events in batch mode.  */
    if (noninteractive
        /* In case we are running as a daemon, only do this before
           detaching from the terminal.  */
        || (IS_DAEMON && daemon_pipe[1] >= 0))
      {
        int c = getchar ();
        XSETINT (obj, c);
        *kbp = current_kboard;
        return obj;
      }
  #endif        /* ! HAVE_DBUS  */

So, if we are in batch mode and D-Bus is not compiled in (which
happens on w32), we are going to call 'getchar', and remain stuck in a
system call until someone types something on the keyboard.

(You may think that binding 'noninteractive' to nil on the Lisp level
should skip this fragment, but if you look in emacs.c, you will see
that the Lisp variable 'noninteractive' changes the value of a C
variable 'noninteractive1', and thus largely has no effect on the C
level.  Not sure why we are doing this, perhaps to prevent users from
messing up Emacs, but in any case this has been the case since about
forever.)

If I #ifdef away the above fragment, file-notify-test02 passes on w32
in batch mode as well, provided that there's a call to read-event in
file-notify--wait-for-events, as I've shown yesterday.

> So it seems to be a problem read the events inside Emacs. The difference
> between the two test cases is, that in the autorevert case there are
> active timers. Maybe those timers trigger the event handling.

That's possible: timers change the control flow in the input
processing code.  But test02 also has at least one timer: the one
launched by with-timeout.

> > If we don't call read-event, the notifications are not read, since
> > evidently the read_socket_hook isn't called.
> > 
> Strange. Two days ago, I've installed the recent w32 snapshot (dated
> 20140119, IIRC) on an old Windows XP machine at work, plus the changed
> file-notify-tests.el. All tests succeeded in the GUI case.

I don't see how this could be possible, sorry.  I tried this on 2
different systems with several versions of file-notify-tests.el from
the past week, including just now.  The behavior is consistent: test02
fails.  (Autorevert tests indeed succeed.)  I get this in the *ert*
buffer:

  Selector: "2"
  Passed:  0
  Failed:  1 (1 unexpected)
  Skipped: 1
  Total:   2/2

  Started at:   2014-01-30 19:00:35+0200
  Finished.
  Finished at:  2014-01-30 19:00:40+0200

  Fs

  F file-notify-test02-events
      Check file creation/removal notifications.
      (ert-test-failed
       ((should file-notify--test-results)
        :form file-notify--test-results :value nil))

Please try with running emacs.exe, not runemacs.exe, and see if it
still succeeds.  I run like this (from cmd prompt):

  cd test\automated
  ..\..\src\emacs -Q -batch -l file-notify-tests.el -f ert

then type "2 RET" when prompted in the minibuffer.  The results are
one test skipped and one unexpected failure.

If you still don't see the failure, is it possible that you have some
local uncommitted changes?

> > Btw, why did you need to bind noninteractive to nil?
> > 
> See the code of `sit-for'. When `noninteractive' is non-nil, it simply
> calls `sleep-for'.

Again, if we want to force sit-for into some specific mode of
operation, I think calling that code directly is better.  E.g., it
won't break if sit-for is changed.

> That was proper in the past, but these days with several events to
> arrive also in noninteractive mode, this should be changed.

See above: what you wrote is only true for D-Bus builds.  Also,
binding 'noninteractive' in Lisp creates schizophrenia between Lisp
and C, as explained above, so I'm not sure this is a good idea anyway.

So, where to go from here?

First, I think the above fragment, which makes D-Bus builds behave in
batch differently from any other build, is not a good idea.  It is
probably not a good idea even in D-Bus builds when they run an daemon
mode.  Does anyone understand why we need to call getchar in batch, on
any of the supported platforms and configurations?  Or why it is
needed in daemon mode?

I will ask this on emacs-devel.

If no reasons emerge, I would suggest to remove this code entirely,
except maybe for daemon.  But that probably cannot be done during
feature freeze, so what shall we do now in order to have non-D-Bus
builds pass the file-notify tests?

Also, the fact that read-event behaves differently in batch mode
should probably be documented.  Right now, neither the doc string nor
the ELisp manual mention that.

As for the need to call read-event in file-notify--wait-for-events on
w32, if no other build needs that, I guess we can condition that call
on windows-nt.  But given the above discussion, I think we should do
the opposite: call read-event directly on _all_ platforms.

Thanks.