all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#13662: 24.3.50; inotify-add-watch fails in batch mode
@ 2013-02-09 14:35 Chong Yidong
  2014-01-17 11:56 ` Michael Albinus
  0 siblings, 1 reply; 17+ messages in thread
From: Chong Yidong @ 2013-02-09 14:35 UTC (permalink / raw)
  To: 13662

On latest trunk, `make check' fails with
2 unexpected results:
   FAILED  ert-test-record-backtrace
   FAILED  inotify-file-watch-simple

The first of this is Bug#13064 (still not fixed; I couldn't disentangle
the mess of CL-isms and closures involved in that bug).  The second
failure is relatively new: apparently, inotify-file-watch-simple works
when Emacs is interactive but fails in batch mode.

Another way to see this is to create a file with the contents

(let* ((temp-file (make-temp-file "inotify-simple"))
       (events 0)
       (wd
	(inotify-add-watch temp-file t (lambda (ev)
					 (setq events (1+ events))))))
  (unwind-protect
      (progn
	(with-temp-file temp-file
	  (insert "Foo\n"))
	(sit-for 5)
	(message ">> %d <<" events))
    (inotify-rm-watch wd)))

and run `emacs -batch -l foo.el'.  This prints ">> 0 <<", indicating
that the inotify watcher failed to run.  If you repeat omitting -batch,
">> 4 <<" is printed, as expected.


In GNU Emacs 24.3.50.2 (x86_64-unknown-linux-gnu, GTK+ Version 3.6.4)
 of 2013-02-09 on tsparkle
Bzr revision: 111704 cyd@gnu.org-20130209050902-vge73m9xbyx96t2q
Windowing system distributor `The X.Org Foundation', version 11.0.11301000
Configured using:
 `configure --with-x-toolkit=gtk3 CFLAGS=-g --no-create --no-recursion'





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#13662: 24.3.50; inotify-add-watch fails in batch mode
  2013-02-09 14:35 bug#13662: 24.3.50; inotify-add-watch fails in batch mode Chong Yidong
@ 2014-01-17 11:56 ` Michael Albinus
  2014-01-25 14:16   ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Albinus @ 2014-01-17 11:56 UTC (permalink / raw)
  To: Chong Yidong; +Cc: 13662-done

Chong Yidong <cyd@gnu.org> writes:

> Another way to see this is to create a file with the contents
>
> (let* ((temp-file (make-temp-file "inotify-simple"))
>        (events 0)
>        (wd
> 	(inotify-add-watch temp-file t (lambda (ev)
> 					 (setq events (1+ events))))))
>   (unwind-protect
>       (progn
> 	(with-temp-file temp-file
> 	  (insert "Foo\n"))
> 	(sit-for 5)
> 	(message ">> %d <<" events))
>     (inotify-rm-watch wd)))
>
> and run `emacs -batch -l foo.el'.  This prints ">> 0 <<", indicating
> that the inotify watcher failed to run.  If you repeat omitting -batch,
> ">> 4 <<" is printed, as expected.

Should be fixed with r116052. Instead of `sit-for', one needs to apply
`read-event'.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#13662: 24.3.50; inotify-add-watch fails in batch mode
  2014-01-17 11:56 ` Michael Albinus
@ 2014-01-25 14:16   ` Eli Zaretskii
  2014-01-26 15:55     ` Michael Albinus
  2014-01-26 16:09     ` bug#16519: 24.3.50; gfile notifications not received " Michael Albinus
  0 siblings, 2 replies; 17+ messages in thread
From: Eli Zaretskii @ 2014-01-25 14:16 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 13662

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Fri, 17 Jan 2014 12:56:51 +0100
> Cc: 13662-done@debbugs.gnu.org
> 
> Chong Yidong <cyd@gnu.org> writes:
> 
> > Another way to see this is to create a file with the contents
> >
> > (let* ((temp-file (make-temp-file "inotify-simple"))
> >        (events 0)
> >        (wd
> > 	(inotify-add-watch temp-file t (lambda (ev)
> > 					 (setq events (1+ events))))))
> >   (unwind-protect
> >       (progn
> > 	(with-temp-file temp-file
> > 	  (insert "Foo\n"))
> > 	(sit-for 5)
> > 	(message ">> %d <<" events))
> >     (inotify-rm-watch wd)))
> >
> > and run `emacs -batch -l foo.el'.  This prints ">> 0 <<", indicating
> > that the inotify watcher failed to run.  If you repeat omitting -batch,
> > ">> 4 <<" is printed, as expected.
> 
> Should be fixed with r116052. Instead of `sit-for', one needs to apply
> `read-event'.

This breaks the file-notify-test02-events test on w32.  It looks like
read-event never returns there.  If I replace that with sit-for, it
does return, but the test still fails because
file-notify--test-results remains nil, something that wasn't being
tested when I last ran the test (in early December).

Frankly, given the inordinate amount of work it took to try to get
file notifications work in batch mode, I'd rather we declared they are
not expected to work in batch, and moved on.  If you, for some reason,
still want to stick with that test, please mark
file-notify-test02-events to be skipped for w32, as I'm tired of
fixing the breakage there time and again.  Thanks.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#13662: 24.3.50; inotify-add-watch fails in batch mode
  2014-01-25 14:16   ` Eli Zaretskii
@ 2014-01-26 15:55     ` Michael Albinus
  2014-01-26 16:09     ` bug#16519: 24.3.50; gfile notifications not received " Michael Albinus
  1 sibling, 0 replies; 17+ messages in thread
From: Michael Albinus @ 2014-01-26 15:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 13662

Eli Zaretskii <eliz@gnu.org> writes:

>> Should be fixed with r116052. Instead of `sit-for', one needs to apply
>> `read-event'.
>
> This breaks the file-notify-test02-events test on w32.

Well, *this* bug report is about inotify-file-watch-simple, which
should be skipped on w32.

You are speaking about bug#16519. I will comment in another message.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-25 14:16   ` Eli Zaretskii
  2014-01-26 15:55     ` Michael Albinus
@ 2014-01-26 16:09     ` Michael Albinus
  2014-01-26 17:43       ` Michael Albinus
  2014-01-27 16:08       ` Michael Albinus
  1 sibling, 2 replies; 17+ messages in thread
From: Michael Albinus @ 2014-01-26 16:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 16519

Eli Zaretskii <eliz@gnu.org> writes:

> This breaks the file-notify-test02-events test on w32.  It looks like
> read-event never returns there.

Strange. There is the call (read-event nil nil 0.1) - shouldn't it
return under any circumstances?

> If I replace that with sit-for, it does return, but the test still
> fails because file-notify--test-results remains nil, something that
> wasn't being tested when I last ran the test (in early December).

sit-for seems to work for inotify based file notifications only. If
there is gio in place, one needs to perform accept-process-output. This
is unfortune, because the user cannot know in advance how to check for
events. Fortnately, both read-event and accept-process-output are called
for other reasons in interactive mode, letting the events arrive. Not
the best situation, but I don't know (yet) how to harmonize.

I don't know what it needs on w32 to get the file notification
events. Is it sit-for?

> Frankly, given the inordinate amount of work it took to try to get
> file notifications work in batch mode, I'd rather we declared they are
> not expected to work in batch, and moved on.  If you, for some reason,
> still want to stick with that test, please mark
> file-notify-test02-events to be skipped for w32, as I'm tired of
> fixing the breakage there time and again.

Well, maybe there is no use case which requires file notification events
in batch mode. BUT I like to be informed by hydra, that a whatever
change has broken Emacs. Often, this are collateral damages by unrelated
changes.

Given, that I have no possibility to develop on w32, I'll go as proposed
by you and mark file-notify-test02-events (and maybe also
file-notify-test03-autorevert) as expected to fail on w32 in batch
mode. The same is currently true for gio based file notifications, but
here I'm eager to find a solution, which works on hydra.

> Thanks.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-26 16:09     ` bug#16519: 24.3.50; gfile notifications not received " Michael Albinus
@ 2014-01-26 17:43       ` Michael Albinus
  2014-01-27 16:08       ` Michael Albinus
  1 sibling, 0 replies; 17+ messages in thread
From: Michael Albinus @ 2014-01-26 17:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 16519

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

> sit-for seems to work for inotify based file notifications only.

s/sit-for/read-event/





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-26 16:09     ` bug#16519: 24.3.50; gfile notifications not received " Michael Albinus
  2014-01-26 17:43       ` Michael Albinus
@ 2014-01-27 16:08       ` Michael Albinus
  2014-01-29 18:14         ` Eli Zaretskii
  1 sibling, 1 reply; 17+ messages in thread
From: Michael Albinus @ 2014-01-27 16:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 16519

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

> I don't know what it needs on w32 to get the file notification
> events. Is it sit-for?

Finally, the following seems to work for all three file notification
libraries in interactive mode, and for inotify in batch mode:

(defmacro file-notify--wait-for-events (timeout until)
  "Wait for file notification events until form UNTIL is true.
TIMEOUT is the maximum time to wait for, in seconds."
  `(with-timeout (,timeout (ignore))
     (while (null ,until)
       (let (noninteractive)
        (sit-for 0.1 'nodisplay)))))

I am not able to test w32notify in batch mode. The following call from a
CMD terminal works fine for me:

C:\>"C:\Program Files\emacs\bin\runemacs.exe" -Q -l Y:\file-notify-tests.el -f ert

However, the batch-mode equivalent returns immediately without running
the test:

C:\>"C:\Program Files\emacs\bin\runemacs.exe" -batch -Q -l Y:\file-notify-tests.el -f ert-run-tests-batch-and-exit

What do I miss?

Best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-27 16:08       ` Michael Albinus
@ 2014-01-29 18:14         ` Eli Zaretskii
  2014-01-30 10:03           ` Michael Albinus
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2014-01-29 18:14 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 16519

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: 16519@debbugs.gnu.org
> Date: Mon, 27 Jan 2014 17:08:35 +0100
> 
> Michael Albinus <michael.albinus@gmx.de> writes:
> 
> > I don't know what it needs on w32 to get the file notification
> > events. Is it sit-for?

No, it seems to be read-event, see below.

> Finally, the following seems to work for all three file notification
> libraries in interactive mode, and for inotify in batch mode:
> 
> (defmacro file-notify--wait-for-events (timeout until)
>   "Wait for file notification events until form UNTIL is true.
> TIMEOUT is the maximum time to wait for, in seconds."
>   `(with-timeout (,timeout (ignore))
>      (while (null ,until)
>        (let (noninteractive)
>         (sit-for 0.1 'nodisplay)))))
> 
> I am not able to test w32notify in batch mode. The following call from a
> CMD terminal works fine for me:
> 
> C:\>"C:\Program Files\emacs\bin\runemacs.exe" -Q -l Y:\file-notify-tests.el -f ert

How do you mean "works for me"?  The 02 test fails, doesn't it?  If I
type "2" to the prompt, I get 1 unexpected failure and 1 skipped test.

I need this change to file-notify-tests.el to get it to work with w32
in interactive mode (both -nw and GUI sessions work):

=== modified file 'test/automated/file-notify-tests.el'
--- test/automated/file-notify-tests.el	2014-01-27 19:10:02 +0000
+++ test/automated/file-notify-tests.el	2014-01-29 18:05:42 +0000
@@ -188,7 +188,8 @@ TIMEOUT is the maximum time to wait for,
   `(with-timeout (,timeout (ignore))
      (while (null ,until)
        (let (noninteractive)
-	 (sit-for 0.1 'nodisplay)))))
+	 (sit-for 0.1 'nodisplay)
+	 (read-event)))))
 
 (ert-deftest file-notify-test02-events ()
   "Check file creation/removal notifications."

If we don't call read-event, the notifications are not read, since
evidently the read_socket_hook isn't called.

> However, the batch-mode equivalent returns immediately without running
> the test:
> 
> C:\>"C:\Program Files\emacs\bin\runemacs.exe" -batch -Q -l Y:\file-notify-tests.el -f ert-run-tests-batch-and-exit
> 
> What do I miss?

Don't run runemacs.exe, run emacs.exe instead.  I have no idea what
will runemacs do in batch mode.

(I also run emacs.exe in the interactive invocation.  runemacs.exe has
only one purpose: to invoke Emacs from a desktop icon without opening
a console window.)

Anyway, the read-event thing doesn't solve the -batch operation:
notifications don't come in and Emacs is stuck until I type something
on the keyboard.  I will need more debugging to understand what is
going on.  One thing is clear: the way the notifications get to the
event queue is different for every back-end, so naive assumptions,
like just let it sit-for for a while, are usually wrong.

Btw, why did you need to bind noninteractive to nil?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-29 18:14         ` Eli Zaretskii
@ 2014-01-30 10:03           ` Michael Albinus
  2014-01-30 17:03             ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Albinus @ 2014-01-30 10:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 16519

Eli Zaretskii <eliz@gnu.org> writes:

>> > I don't know what it needs on w32 to get the file notification
>> > events. Is it sit-for?
>
> 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.

>> Finally, the following seems to work for all three file notification
>> libraries in interactive mode, and for inotify in batch mode:
>> 
>> (defmacro file-notify--wait-for-events (timeout until)
>>   "Wait for file notification events until form UNTIL is true.
>> TIMEOUT is the maximum time to wait for, in seconds."
>>   `(with-timeout (,timeout (ignore))
>>      (while (null ,until)
>>        (let (noninteractive)
>>         (sit-for 0.1 'nodisplay)))))
>> 
>> I am not able to test w32notify in batch mode. The following call from a
>> CMD terminal works fine for me:
>> 
>> C:\>"C:\Program Files\emacs\bin\runemacs.exe" -Q -l Y:\file-notify-tests.el -f ert
>
> How do you mean "works for me"?  The 02 test fails, doesn't it?  If I
> type "2" to the prompt, I get 1 unexpected failure and 1 skipped test.

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.

I'm still working on the noninteractive gfilenotify case. The
integration of glib seems to be proper. According to some trace
messages I have added to dir_monitor_callback in gfilenotify.c, events
are received in test case `file-notify-test03-autorevert', but not in
`file-notify-test02-events'.

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. I'll debug
further.

> I need this change to file-notify-tests.el to get it to work with w32
> in interactive mode (both -nw and GUI sessions work):
>
> === modified file 'test/automated/file-notify-tests.el'
> --- test/automated/file-notify-tests.el	2014-01-27 19:10:02 +0000
> +++ test/automated/file-notify-tests.el	2014-01-29 18:05:42 +0000
> @@ -188,7 +188,8 @@ TIMEOUT is the maximum time to wait for,
>    `(with-timeout (,timeout (ignore))
>       (while (null ,until)
>         (let (noninteractive)
> -	 (sit-for 0.1 'nodisplay)))))
> +	 (sit-for 0.1 'nodisplay)
> +	 (read-event)))))
>  
>  (ert-deftest file-notify-test02-events ()
>    "Check file creation/removal notifications."
>
> 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 haven't
tried -nw, 'tho.

>> However, the batch-mode equivalent returns immediately without running
>> the test:
>> 
>> C:\>"C:\Program Files\emacs\bin\runemacs.exe" -batch -Q -l
>> Y:\file-notify-tests.el -f ert-run-tests-batch-and-exit
>> 
>> What do I miss?
>
> Don't run runemacs.exe, run emacs.exe instead.  I have no idea what
> will runemacs do in batch mode.

Ah, thanks. Will use for testing.

> Anyway, the read-event thing doesn't solve the -batch operation:
> notifications don't come in and Emacs is stuck until I type something
> on the keyboard.  I will need more debugging to understand what is
> going on.  One thing is clear: the way the notifications get to the
> event queue is different for every back-end, so naive assumptions,
> like just let it sit-for for a while, are usually wrong.

Again, at least for me it works with all 3 backends in interactive mode,
and it works for inotify in batch mode. And it works half way for
gfilenotify in batch mode for auto reverting files. And it works for
remote files, in both interactive and batch mode.

> 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'. That was proper in the past, but these days with
several events to arrive also in noninteractive mode, this should be
changed. Not something for the feature freeze time, that's why I have
masked it with binding noninteractive to nil in the test code. Maybe I
should add a respective comment.

A while ago, Stefan has proposed to add a new mechanism for handling
such events. Maybe this shall go there.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-30 10:03           ` Michael Albinus
@ 2014-01-30 17:03             ` Eli Zaretskii
  2014-01-31 14:11               ` Michael Albinus
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2014-01-30 17:03 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 16519

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-30 17:03             ` Eli Zaretskii
@ 2014-01-31 14:11               ` Michael Albinus
  2014-01-31 15:17                 ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Albinus @ 2014-01-31 14:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 16519

Eli Zaretskii <eliz@gnu.org> writes:

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

Out of curiosity: why? We are in batch mode.

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

The reason that I have preferred sit-for over read-event is, that
sit-for puts the events back to unread-command-events. Likely, we don't
need this in the test cases we're speaking about, but the code shall
give also how-to-use examples to people. sit-for seems to be cleaner to
me in this respect.

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

Perfect crystal ball. When I reconfigure Emacs not to use D-Bus, also
the inotify test case fails in batch mode.

> 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  */

I remember. I've added the !HAVE_DBUS prepocessor statetements due to
Bug#11415. It was a similar situation, D-Bus events were not read in
batch mode. So we really need a changed handling of special events in
the main loop, instead of adding more preprocessor lines.

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

This one I will continue to debug, although changes should be applied
only after the release.

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

I'll continue to check if time allows. I have only limited access to
that Windows XP machine.

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

Guaranteed not :-)

I cannot and I do not compile w32 versions of Emacs. The only changes
are in file-notify-tests.el, where I add traces etc pp.

> 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?

As said above: Bug#11415.

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

I will run some tests for that. Then we can switch to use read-event.

I vaguely remember there was also a problem with file notifications of
remote files; I had to call accept-process-output. But maybe it was
superfluous, I will check again.

These tests may take some days (I have some private trouble). But since
we don't want to change Emacs proper but only the test code and the
documentation, it might not be a serious problem.

> Thanks.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-31 14:11               ` Michael Albinus
@ 2014-01-31 15:17                 ` Eli Zaretskii
  2014-01-31 16:00                   ` Michael Albinus
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2014-01-31 15:17 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 16519

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: 16519@debbugs.gnu.org
> Date: Fri, 31 Jan 2014 15:11:20 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> `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.
> 
> Out of curiosity: why? We are in batch mode.

I didn't really debug that part, so what's below might be my
imagination (though the fact that input-pending-p returns non-nil is
verified).  That said, it doesn't make much difference to Emacs on
Windows whether we are in batch mode or in console (i.e. -nw) mode.
We still read the keyboard using the same Windows APIs (see
w32inevt.c), and so we still see the console events that Windows sends
us.  Those events are strange, and not all of them are documented, but
they do arrive.

More generally, you can never rely on what kind of input events are
sent to Emacs in any mode.  This is inherently system-dependent,
especially with today's systems that have any number of channels to
send events to a program.

> > 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.
> 
> The reason that I have preferred sit-for over read-event is, that
> sit-for puts the events back to unread-command-events.

If we need this part, we can simply copy the relevant portion of
sit-for into file-notify-tests.el.

> Likely, we don't need this in the test cases we're speaking about,
> but the code shall give also how-to-use examples to people. sit-for
> seems to be cleaner to me in this respect.

Maybe we should have ert-sit-for, then, which does everything like
sit-for, except relying on input-pending-p?

> >   #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  */
> 
> I remember. I've added the !HAVE_DBUS prepocessor statetements due to
> Bug#11415. It was a similar situation, D-Bus events were not read in
> batch mode. So we really need a changed handling of special events in
> the main loop, instead of adding more preprocessor lines.

I don't see why we would need a separate queue for special events.
What we need, perhaps, is a way to peek at stdin to see whether
there's some input ready to be gobbled.  Shouldn't select/pselect
already provide that?  IOW, there should be no need to call getchar at
all at this point.

> > If you still don't see the failure, is it possible that you have some
> > local uncommitted changes?
> 
> Guaranteed not :-)
> 
> I cannot and I do not compile w32 versions of Emacs. The only changes
> are in file-notify-tests.el, where I add traces etc pp.

I meant uncommitted changes in file-notify-tests.el.

> > 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.
> 
> I will run some tests for that. Then we can switch to use read-event.

Thanks.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-31 15:17                 ` Eli Zaretskii
@ 2014-01-31 16:00                   ` Michael Albinus
  2014-01-31 16:53                     ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Albinus @ 2014-01-31 16:00 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 16519

Eli Zaretskii <eliz@gnu.org> writes:

> Maybe we should have ert-sit-for, then, which does everything like
> sit-for, except relying on input-pending-p?

There is ert-async.el <https://github.com/rejeep/ert-async.el>. I like
this idea, although the timer loop applies only
accept-process-output. read-event shall be added, at least.

Maybe we should contact the author, whether he agrees to merge this code
into ert.el?

>> I remember. I've added the !HAVE_DBUS prepocessor statetements due to
>> Bug#11415. It was a similar situation, D-Bus events were not read in
>> batch mode. So we really need a changed handling of special events in
>> the main loop, instead of adding more preprocessor lines.
>
> I don't see why we would need a separate queue for special events.
> What we need, perhaps, is a way to peek at stdin to see whether
> there's some input ready to be gobbled.  Shouldn't select/pselect
> already provide that?  IOW, there should be no need to call getchar at
> all at this point.

Reading file notifications via gio does not use file descriptors, which
are handled via xg_select. The same will happen, if we switch to gdbus
or kdbus later on. That's why we might need another kind of mainloop integration.

I'm not too familiar with kbd_buffer_store_event and the mechanisms
behind. If we still could use them - OK.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-31 16:00                   ` Michael Albinus
@ 2014-01-31 16:53                     ` Eli Zaretskii
  2014-02-03 13:31                       ` Michael Albinus
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2014-01-31 16:53 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 16519

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: 16519@debbugs.gnu.org
> Date: Fri, 31 Jan 2014 17:00:18 +0100
> 
> > I don't see why we would need a separate queue for special events.
> > What we need, perhaps, is a way to peek at stdin to see whether
> > there's some input ready to be gobbled.  Shouldn't select/pselect
> > already provide that?  IOW, there should be no need to call getchar at
> > all at this point.
> 
> Reading file notifications via gio does not use file descriptors, which
> are handled via xg_select. The same will happen, if we switch to gdbus
> or kdbus later on. That's why we might need another kind of mainloop integration.

That just means we should call xg_select etc. in addition to calling
pselect.  The events that come that way should still be put on the
same single queue, IMO.

> I'm not too familiar with kbd_buffer_store_event and the mechanisms
> behind.

Basically, all the events that come in are put on that queue, and then
when we need to read input we look at that queue and take events off
it to process.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-01-31 16:53                     ` Eli Zaretskii
@ 2014-02-03 13:31                       ` Michael Albinus
  2014-02-03 16:13                         ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Albinus @ 2014-02-03 13:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 16519

Eli Zaretskii <eliz@gnu.org> writes:

>> Reading file notifications via gio does not use file descriptors, which
>> are handled via xg_select. The same will happen, if we switch to gdbus
>> or kdbus later on. That's why we might need another kind of mainloop integration.
>
> That just means we should call xg_select etc. in addition to calling
> pselect.  The events that come that way should still be put on the
> same single queue, IMO.

Maybe I'm too stupid, but how shall we call xg_select w/o a file descriptor?

Best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-02-03 13:31                       ` Michael Albinus
@ 2014-02-03 16:13                         ` Eli Zaretskii
  2014-02-04 11:43                           ` Michael Albinus
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2014-02-03 16:13 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 16519

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: 16519@debbugs.gnu.org
> Date: Mon, 03 Feb 2014 14:31:47 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Reading file notifications via gio does not use file descriptors, which
> >> are handled via xg_select. The same will happen, if we switch to gdbus
> >> or kdbus later on. That's why we might need another kind of mainloop integration.
> >
> > That just means we should call xg_select etc. in addition to calling
> > pselect.  The events that come that way should still be put on the
> > same single queue, IMO.
> 
> Maybe I'm too stupid, but how shall we call xg_select w/o a file descriptor?

No, it's my fault: I misunderstood your comment about xg_select,
sorry.

What I meant to say was that the same method we use to read file
notifications via gio should be used, directly or indirectly, where we
call pselect, so that these events enter the event queue like any
others.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16519: 24.3.50; gfile notifications not received in batch mode
  2014-02-03 16:13                         ` Eli Zaretskii
@ 2014-02-04 11:43                           ` Michael Albinus
  0 siblings, 0 replies; 17+ messages in thread
From: Michael Albinus @ 2014-02-04 11:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 16519-done

Eli Zaretskii <eliz@gnu.org> writes:

> What I meant to say was that the same method we use to read file
> notifications via gio should be used, directly or indirectly, where we
> call pselect, so that these events enter the event queue like any
> others.

Finally, I could fix this with a simple change in xg_select. Closing the bug.

Thanks for pointing me into the right direction, and best regards, Michael.





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-02-04 11:43 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-09 14:35 bug#13662: 24.3.50; inotify-add-watch fails in batch mode Chong Yidong
2014-01-17 11:56 ` Michael Albinus
2014-01-25 14:16   ` Eli Zaretskii
2014-01-26 15:55     ` Michael Albinus
2014-01-26 16:09     ` bug#16519: 24.3.50; gfile notifications not received " Michael Albinus
2014-01-26 17:43       ` Michael Albinus
2014-01-27 16:08       ` Michael Albinus
2014-01-29 18:14         ` Eli Zaretskii
2014-01-30 10:03           ` Michael Albinus
2014-01-30 17:03             ` Eli Zaretskii
2014-01-31 14:11               ` Michael Albinus
2014-01-31 15:17                 ` Eli Zaretskii
2014-01-31 16:00                   ` Michael Albinus
2014-01-31 16:53                     ` Eli Zaretskii
2014-02-03 13:31                       ` Michael Albinus
2014-02-03 16:13                         ` Eli Zaretskii
2014-02-04 11:43                           ` Michael Albinus

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.