unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
@ 2015-11-01 15:51 Ken Brown
  2015-11-01 16:40 ` Eli Zaretskii
  2015-11-01 17:34 ` Michael Albinus
  0 siblings, 2 replies; 32+ messages in thread
From: Ken Brown @ 2015-11-01 15:51 UTC (permalink / raw)
  To: 21804; +Cc: Michael Albinus

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

The following tests are failing on Cygwin:

file-notify-test02-events
file-notify-test02-events-remote
file-notify-test04-file-validity
file-notify-test04-file-validity-remote
file-notify-test05-dir-validity

The first two have been failing for as long as I can remember, but I 
never got around to reporting it.

The next two failed immediately after they were introduced in the 
following commit:

commit 7a3f3183cd8faff8901ead547711e1c90ea02efe
Author: Tassilo Horn <tsdh@gnu.org>
Date: Mon Sep 14 08:03:11 2015 +0200

Test file-notify-valid-p.

* test/automated/file-notify-tests.el
(file-notify-test04-file-validity, file-notify-test05-dir-validity): New
tests.

The last one started failing shortly after that, but I haven’t pinned 
down the exact commit.

I’m attaching the log based on a build from git 1500667.  Please let me 
know what I can do to help track this down.
Ken




[-- Attachment #2: file-notify-tests.log --]
[-- Type: application/octet-stream, Size: 5703 bytes --]

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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-01 15:51 bug#21804: 25.0.50; file-notify-tests failure on Cygwin Ken Brown
@ 2015-11-01 16:40 ` Eli Zaretskii
  2015-11-01 17:25   ` Ken Brown
  2015-11-01 17:34 ` Michael Albinus
  1 sibling, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2015-11-01 16:40 UTC (permalink / raw)
  To: Ken Brown; +Cc: michael.albinus, 21804

> From: Ken Brown <kbrown@cornell.edu>
> Date: Sun, 1 Nov 2015 10:51:19 -0500
> Cc: Michael Albinus <michael.albinus@gmx.de>
> 
> The following tests are failing on Cygwin:
> 
> file-notify-test02-events
> file-notify-test02-events-remote
> file-notify-test04-file-validity
> file-notify-test04-file-validity-remote
> file-notify-test05-dir-validity

What back-end do you use? gfilenotify or something else?





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-01 16:40 ` Eli Zaretskii
@ 2015-11-01 17:25   ` Ken Brown
  0 siblings, 0 replies; 32+ messages in thread
From: Ken Brown @ 2015-11-01 17:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: michael.albinus, 21804

On 11/1/2015 11:40 AM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown@cornell.edu>
>> Date: Sun, 1 Nov 2015 10:51:19 -0500
>> Cc: Michael Albinus <michael.albinus@gmx.de>
>>
>> The following tests are failing on Cygwin:
>>
>> file-notify-test02-events
>> file-notify-test02-events-remote
>> file-notify-test04-file-validity
>> file-notify-test04-file-validity-remote
>> file-notify-test05-dir-validity
>
> What back-end do you use? gfilenotify or something else?

gfilenotify






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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-01 15:51 bug#21804: 25.0.50; file-notify-tests failure on Cygwin Ken Brown
  2015-11-01 16:40 ` Eli Zaretskii
@ 2015-11-01 17:34 ` Michael Albinus
  2015-11-03 17:20   ` Michael Albinus
  1 sibling, 1 reply; 32+ messages in thread
From: Michael Albinus @ 2015-11-01 17:34 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

Ken Brown <kbrown@cornell.edu> writes:

Hi Ken,

> The following tests are failing on Cygwin:
>
> file-notify-test02-events
> file-notify-test02-events-remote
> file-notify-test04-file-validity
> file-notify-test04-file-validity-remote
> file-notify-test05-dir-validity
>
> I’m attaching the log based on a build from git 1500667.  Please let
> me know what I can do to help track this down.

You are using

> Local library: `gfilenotify'
>    passed   1/12  file-notify-test00-availability
> Remote command: `gvfs-monitor-dir'
>    passed   2/12  file-notify-test00-availability-remote

With the same settings, for me it fails for

file-notify-test02-events
file-notify-test04-file-validity

With local gfilenotify, there is a problem to get the `stopped' event
once a watched file or directory has been deleted. Notably, it works for
the remote gvfs-monitor-dir, which shall behave similar.

I will try to fix these cases first, because I could reproduce them
locally. Let's see how it behaves afterwards.

> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-01 17:34 ` Michael Albinus
@ 2015-11-03 17:20   ` Michael Albinus
  2015-11-04  3:54     ` Ken Brown
  0 siblings, 1 reply; 32+ messages in thread
From: Michael Albinus @ 2015-11-03 17:20 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

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

Hi Ken,

> With the same settings, for me it fails for
>
> file-notify-test02-events
> file-notify-test04-file-validity
>
> With local gfilenotify, there is a problem to get the `stopped' event
> once a watched file or directory has been deleted. Notably, it works for
> the remote gvfs-monitor-dir, which shall behave similar.
>
> I will try to fix these cases first, because I could reproduce them
> locally. Let's see how it behaves afterwards.

I've fixed this with commit 436ed2399ade5c41b8ed3cffe177fb5210eff574.
Could you, please, check, whether it improves your tests?

>> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-03 17:20   ` Michael Albinus
@ 2015-11-04  3:54     ` Ken Brown
  2015-11-04  8:20       ` Michael Albinus
  0 siblings, 1 reply; 32+ messages in thread
From: Ken Brown @ 2015-11-04  3:54 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

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

On 11/3/2015 12:20 PM, Michael Albinus wrote:
> Michael Albinus <michael.albinus@gmx.de> writes:
>
> Hi Ken,
>
>> With the same settings, for me it fails for
>>
>> file-notify-test02-events
>> file-notify-test04-file-validity
>>
>> With local gfilenotify, there is a problem to get the `stopped' event
>> once a watched file or directory has been deleted. Notably, it works for
>> the remote gvfs-monitor-dir, which shall behave similar.
>>
>> I will try to fix these cases first, because I could reproduce them
>> locally. Let's see how it behaves afterwards.
>
> I've fixed this with commit 436ed2399ade5c41b8ed3cffe177fb5210eff574.
> Could you, please, check, whether it improves your tests?

Hi Michael,

Yes, it improves the tests slightly; file-notify-test05-dir-validity now 
passes.  But the other four still fail.  The new log is attached.

Ken



[-- Attachment #2: file-notify-tests.log --]
[-- Type: application/octet-stream, Size: 4880 bytes --]

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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-04  3:54     ` Ken Brown
@ 2015-11-04  8:20       ` Michael Albinus
  2015-11-04 16:55         ` Ken Brown
  0 siblings, 1 reply; 32+ messages in thread
From: Michael Albinus @ 2015-11-04  8:20 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

> Yes, it improves the tests slightly; file-notify-test05-dir-validity
> now passes.  But the other four still fail.  The new log is attached.

In all four cases, `file-notify--test-with-events' returned nil, where
it was supposed to show the incoming events. Strange.

In lisp/filenotify.el and test/automated/file-notify-tests.el, I've
prepared trace messages. You'll find them searching for ";;(message".
Could you, please, enable these traces and show the resulting log?

Another idea is to increase the timeouts in `file-notify--test-timeout'.

Btw (and likely you know it already), you can run just this test with

# make -C test/automated file-notify-tests

or

# rm test/automated/file-notify-tests.log
# make -C test/automated file-notify-tests.log

This might save time for your tests.

> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-04  8:20       ` Michael Albinus
@ 2015-11-04 16:55         ` Ken Brown
  2015-11-04 18:44           ` Michael Albinus
  0 siblings, 1 reply; 32+ messages in thread
From: Ken Brown @ 2015-11-04 16:55 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

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

On 11/4/2015 3:20 AM, Michael Albinus wrote:
> In all four cases, `file-notify--test-with-events' returned nil, where
> it was supposed to show the incoming events. Strange.
>
> In lisp/filenotify.el and test/automated/file-notify-tests.el, I've
> prepared trace messages. You'll find them searching for ";;(message".
> Could you, please, enable these traces and show the resulting log?

Attached.

> Another idea is to increase the timeouts in `file-notify--test-timeout'.

This didn't help.

Ken

[-- Attachment #2: file-notify-tests.log --]
[-- Type: application/octet-stream, Size: 6373 bytes --]

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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-04 16:55         ` Ken Brown
@ 2015-11-04 18:44           ` Michael Albinus
  2015-11-05  3:21             ` Ken Brown
  0 siblings, 1 reply; 32+ messages in thread
From: Michael Albinus @ 2015-11-04 18:44 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

Ken Brown <kbrown@cornell.edu> writes:

>> In lisp/filenotify.el and test/automated/file-notify-tests.el, I've
>> prepared trace messages. You'll find them searching for ";;(message".
>> Could you, please, enable these traces and show the resulting log?
>
> Attached.
>
> Running 12 tests (2015-11-04 11:51:58-0500)
> Local library: `gfilenotify'
>    passed   1/12  file-notify-test00-availability
> Remote command: `gvfs-monitor-dir'
>    passed   2/12  file-notify-test00-availability-remote
>    passed   3/12  file-notify-test01-add-watch
>    passed   4/12  file-notify-test01-add-watch-remote
> file-notify--test-event-handler (6443046528 stopped "/tmp/file-notify-test552448U")
> Test file-notify-test02-events backtrace:

It is obvious, that `file-notify--test-with-events' isn't kosher. No
event arrived inside this macro. The stopped event arrived later, in one
of the unwindforms of the surrounding unwind-protect.

Are there special problems with cygwin Emacs related to the
`with-timeout' macro?

Hard to debug, because we are faced with timing issues. So I would need
to do this myself ...

Well, I'll try to find a machine where I could install cygwin. Where
could I find a very recent Emacs, including the patch of gfilenotify.c
I've committed yesterday?

> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-04 18:44           ` Michael Albinus
@ 2015-11-05  3:21             ` Ken Brown
  2015-11-05 19:58               ` Michael Albinus
  0 siblings, 1 reply; 32+ messages in thread
From: Ken Brown @ 2015-11-05  3:21 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

Hi Michael,

On 11/4/2015 1:44 PM, Michael Albinus wrote:
> Ken Brown <kbrown@cornell.edu> writes:
>> Running 12 tests (2015-11-04 11:51:58-0500)
>> Local library: `gfilenotify'
>>     passed   1/12  file-notify-test00-availability
>> Remote command: `gvfs-monitor-dir'
>>     passed   2/12  file-notify-test00-availability-remote
>>     passed   3/12  file-notify-test01-add-watch
>>     passed   4/12  file-notify-test01-add-watch-remote
>> file-notify--test-event-handler (6443046528 stopped "/tmp/file-notify-test552448U")
>> Test file-notify-test02-events backtrace:
>
> It is obvious, that `file-notify--test-with-events' isn't kosher. No
> event arrived inside this macro. The stopped event arrived later, in one
> of the unwindforms of the surrounding unwind-protect.
>
> Are there special problems with cygwin Emacs related to the
> `with-timeout' macro?

Not that I know of.  There are other tests that use it, and those aren't 
failing.

> Hard to debug, because we are faced with timing issues. So I would need
> to do this myself ...
>
> Well, I'll try to find a machine where I could install cygwin. Where
> could I find a very recent Emacs, including the patch of gfilenotify.c
> I've committed yesterday?

I've just built one and put it on my private Cygwin repository at

   http://sanibeltranquility.com/cygwin/index.html

There are instructions at that site.  Let me know if anything is 
unclear.  Thanks for your help with this.

Ken






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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-05  3:21             ` Ken Brown
@ 2015-11-05 19:58               ` Michael Albinus
  2015-11-05 20:14                 ` Ken Brown
                                   ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Michael Albinus @ 2015-11-05 19:58 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

>> Hard to debug, because we are faced with timing issues. So I would need
>> to do this myself ...
>>
>> Well, I'll try to find a machine where I could install cygwin. Where
>> could I find a very recent Emacs, including the patch of gfilenotify.c
>> I've committed yesterday?
>
> I've just built one and put it on my private Cygwin repository at

I've played with this the whole afternoon. Looks like there is no error
in gfilenotify for Cygwin. But it is a hard job to trigger the file
notification events to appear such a way they could be checked for
correctness in the test cases.

Increasing timeouts was necessary. But even this does not make the
events to appear reliably. One would need to write additional code to
get every single event one after the other. Waiting for a series of
events, as the test cases do expect, does not seem to work.

Since I have no further idea how to get those events reliably, I tend to
skip both test cases for cygwin. What do you think?

> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-05 19:58               ` Michael Albinus
@ 2015-11-05 20:14                 ` Ken Brown
  2015-11-06  6:35                   ` Michael Albinus
  2016-12-26 17:08                 ` Ken Brown
       [not found]                 ` <cea29d3c-30d7-45b1-6d8c-1a8b3511791c@cornell.edu>
  2 siblings, 1 reply; 32+ messages in thread
From: Ken Brown @ 2015-11-05 20:14 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

On 11/5/2015 2:58 PM, Michael Albinus wrote:
> Ken Brown <kbrown@cornell.edu> writes:
>
>> Hi Michael,
>
> Hi Ken,
>
>>> Hard to debug, because we are faced with timing issues. So I would need
>>> to do this myself ...
>>>
>>> Well, I'll try to find a machine where I could install cygwin. Where
>>> could I find a very recent Emacs, including the patch of gfilenotify.c
>>> I've committed yesterday?
>>
>> I've just built one and put it on my private Cygwin repository at
>
> I've played with this the whole afternoon. Looks like there is no error
> in gfilenotify for Cygwin. But it is a hard job to trigger the file
> notification events to appear such a way they could be checked for
> correctness in the test cases.
>
> Increasing timeouts was necessary. But even this does not make the
> events to appear reliably. One would need to write additional code to
> get every single event one after the other. Waiting for a series of
> events, as the test cases do expect, does not seem to work.
>
> Since I have no further idea how to get those events reliably, I tend to
> skip both test cases for cygwin. What do you think?

That makes sense to me.  Thanks for your efforts on this.

Ken






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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-05 20:14                 ` Ken Brown
@ 2015-11-06  6:35                   ` Michael Albinus
  0 siblings, 0 replies; 32+ messages in thread
From: Michael Albinus @ 2015-11-06  6:35 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804-done

Ken Brown <kbrown@cornell.edu> writes:

>> Since I have no further idea how to get those events reliably, I tend to
>> skip both test cases for cygwin. What do you think?
>
> That makes sense to me.  Thanks for your efforts on this.

Done. Closing the bug.

> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2015-11-05 19:58               ` Michael Albinus
  2015-11-05 20:14                 ` Ken Brown
@ 2016-12-26 17:08                 ` Ken Brown
       [not found]                 ` <cea29d3c-30d7-45b1-6d8c-1a8b3511791c@cornell.edu>
  2 siblings, 0 replies; 32+ messages in thread
From: Ken Brown @ 2016-12-26 17:08 UTC (permalink / raw)
  To: 21804

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

[Resending after unarchiving the bug.]


On 11/5/2015 2:58 PM, Michael Albinus wrote:
> I've played with this the whole afternoon. Looks like there is no error
> in gfilenotify for Cygwin. But it is a hard job to trigger the file
> notification events to appear such a way they could be checked for
> correctness in the test cases.
>
> Increasing timeouts was necessary. But even this does not make the
> events to appear reliably. One would need to write additional code to
> get every single event one after the other. Waiting for a series of
> events, as the test cases do expect, does not seem to work.

Hi Michael,

I've made some progress on understanding why some file notifications 
aren't triggered in the tests on Cygwin.  One issue is that 
file-notify--test-read-event-timeout has to be increased drastically in 
order for `created' and `changed' events to be received.  These can take 
5 seconds or more.

A second issue is that a small delay is needed between the creation of a 
file watch and the beginning of file activity.  This is not normally a 
problem in interactive sessions.  But it can be observed in an 
interactive session by evaluating the following:

(require 'filenotify)
filenotify

(defun my-notify-callback (event)
    (message "Event %S" event))

(progn
    (file-notify-add-watch
     "/tmp/foo" '(change) 'my-notify-callback)
    (write-region "" nil "/tmp/foo"))

(delete-file "/tmp/foo")

;; The above yields `deleted' and `stopped' events, but no `created' 
event.  Now we introduce a delay after adding the watch.

(progn
    (file-notify-add-watch
     "/tmp/foo" '(change) 'my-notify-callback)
    (sleep-for 1)
    (write-region "" nil "/tmp/foo"))

(delete-file "/tmp/foo")

;; This time we get the expected `created' event.

The attached patch takes care of both timing issues.  With this patch, 
all the inexpensive file-notify tests pass on all my Cygwin systems.

Best regards,

Ken



[-- Attachment #2: filenotify-tests.patch --]
[-- Type: text/plain, Size: 7789 bytes --]

diff --git a/test/lisp/filenotify-tests.el b/test/lisp/filenotify-tests.el
index 0e6e58e..1ed764f 100644
--- a/test/lisp/filenotify-tests.el
+++ b/test/lisp/filenotify-tests.el
@@ -64,7 +64,11 @@ file-notify--test-results
 (defvar file-notify--test-event nil)
 (defvar file-notify--test-events nil)
 
-(defconst file-notify--test-read-event-timeout 0.01
+(defconst file-notify--test-read-event-timeout
+  (cond
+   ;; Some events take several seconds to arrive on cygwin.
+   ((eq system-type 'cygwin) 7)
+   (t 0.01))
   "Timeout for `read-event' calls.
 It is different for local and remote file notification libraries.")
 
@@ -388,6 +392,10 @@ file-notify--test-with-events
       (not (input-pending-p)))
      (setq file-notify--test-events nil
            file-notify--test-results nil)
+     ;; cygwin needs a delay between setting a watch and beginning
+     ;; file activity, or else the first event is not sent.
+     (if (eq system-type 'cygwin)
+         (sleep-for 1))
      ,@body
      (file-notify--wait-for-events
       ;; More events need more time.  Use some fudge factor.
@@ -409,10 +417,9 @@ file-notify--test-with-events
   (unwind-protect
       (progn
         ;; Check file creation, change and deletion.  It doesn't work
-        ;; for cygwin and kqueue, because we don't use an implicit
-        ;; directory monitor (kqueue), or the timings are too bad (cygwin).
-        (unless (or (eq system-type 'cygwin)
-		    (string-equal (file-notify--test-library) "kqueue"))
+        ;; for kqueue, because we don't use an implicit directory
+        ;; monitor.
+        (unless (string-equal (file-notify--test-library) "kqueue")
           (setq file-notify--test-tmpfile (file-notify--test-make-temp-name))
           (should
            (setq file-notify--test-desc
@@ -421,9 +428,9 @@ file-notify--test-with-events
                   '(change) #'file-notify--test-event-handler)))
           (file-notify--test-with-events
               (cond
-               ;; cygwin recognizes only `deleted' and `stopped' events.
+               ;; cygwin does not raise a `changed' event.
                ((eq system-type 'cygwin)
-                '(deleted stopped))
+                '(created deleted stopped))
                (t '(created changed deleted stopped)))
             (write-region
              "another text" nil file-notify--test-tmpfile nil 'no-message)
@@ -440,13 +447,9 @@ file-notify--test-with-events
 		file-notify--test-tmpfile
 		'(change) #'file-notify--test-event-handler)))
         (file-notify--test-with-events
-	    (cond
-	     ;; cygwin recognizes only `deleted' and `stopped' events.
-	     ((eq system-type 'cygwin)
-	      '(deleted stopped))
-             ;; There could be one or two `changed' events.
-             (t '((changed deleted stopped)
-                  (changed changed deleted stopped))))
+            ;; There could be one or two `changed' events.
+            '((changed deleted stopped)
+              (changed changed deleted stopped))
           (write-region
            "another text" nil file-notify--test-tmpfile nil 'no-message)
           (read-event nil nil file-notify--test-read-event-timeout)
@@ -470,11 +473,11 @@ file-notify--test-with-events
 	       ;; events for the watched directory.
 	       ((string-equal (file-notify--test-library) "w32notify")
 		'(created changed deleted))
-	       ;; cygwin recognizes only `deleted' and `stopped' events.
-	       ((eq system-type 'cygwin)
-		'(deleted stopped))
 	       ;; There are two `deleted' events, for the file and for
-	       ;; the directory.  Except for kqueue.
+	       ;; the directory.  Except for cygwin and kqueue.  And
+	       ;; cygwin does not raise a `changed' event.
+	       ((eq system-type 'cygwin)
+		'(created deleted stopped))
 	       ((string-equal (file-notify--test-library) "kqueue")
 		'(created changed deleted stopped))
 	       (t '(created changed deleted deleted stopped)))
@@ -503,11 +506,10 @@ file-notify--test-with-events
 		'(created changed created changed
 		  changed changed changed
 		  deleted deleted))
-	       ;; cygwin recognizes only `deleted' and `stopped' events.
-	       ((eq system-type 'cygwin)
-		'(deleted stopped))
 	       ;; There are three `deleted' events, for two files and
-	       ;; for the directory.  Except for kqueue.
+	       ;; for the directory.  Except for cygwin and kqueue.
+	       ((eq system-type 'cygwin)
+		'(created created changed changed deleted stopped))
 	       ((string-equal (file-notify--test-library) "kqueue")
 		'(created changed created changed deleted stopped))
 	       (t '(created changed created changed
@@ -541,11 +543,12 @@ file-notify--test-with-events
 	       ;; events for the watched directory.
 	       ((string-equal (file-notify--test-library) "w32notify")
 		'(created changed renamed deleted))
-	       ;; cygwin recognizes only `deleted' and `stopped' events.
-	       ((eq system-type 'cygwin)
-		'(deleted stopped))
 	       ;; There are two `deleted' events, for the file and for
-	       ;; the directory.  Except for kqueue.
+	       ;; the directory.  Except for cygwin and kqueue.  And
+	       ;; cygwin raises `created' and `deleted' events instead
+	       ;; of a `renamed' event.
+	       ((eq system-type 'cygwin)
+		'(created created deleted deleted stopped))
 	       ((string-equal (file-notify--test-library) "kqueue")
 		'(created changed renamed deleted stopped))
 	       (t '(created changed renamed deleted deleted stopped)))
@@ -728,13 +731,9 @@ file-notify--test-with-events
 		'(change) #'file-notify--test-event-handler)))
 	(should (file-notify-valid-p file-notify--test-desc))
         (file-notify--test-with-events
-            (cond
-             ;; cygwin recognizes only `deleted' and `stopped' events.
-	     ((eq system-type 'cygwin)
-	      '(deleted stopped))
              ;; There could be one or two `changed' events.
-             (t '((changed deleted stopped)
-                  (changed changed deleted stopped))))
+             '((changed deleted stopped)
+               (changed changed deleted stopped))
           (write-region
            "another text" nil file-notify--test-tmpfile nil 'no-message)
 	  (read-event nil nil file-notify--test-read-event-timeout)
@@ -765,11 +764,11 @@ file-notify--test-with-events
 	  ;; for the watched directory.
 	  ((string-equal (file-notify--test-library) "w32notify")
 	   '(created changed deleted))
-	  ;; cygwin recognizes only `deleted' and `stopped' events.
-	  ((eq system-type 'cygwin)
-	   '(deleted stopped))
 	  ;; There are two `deleted' events, for the file and for the
-	  ;; directory.  Except for kqueue.
+	  ;; directory.  Except for cygwin and kqueue.  And cygwin
+	  ;; does not raise a `changed' event.
+	  ((eq system-type 'cygwin)
+	   '(created deleted stopped))
 	  ((string-equal (file-notify--test-library) "kqueue")
 	   '(created changed deleted stopped))
 	  (t '(created changed deleted deleted stopped)))
@@ -1172,9 +1171,10 @@ file-notify-test-all
 ;;   the missing directory monitor.
 ;; * For w32notify, no `deleted' and `stopped' events arrive when a
 ;;   directory is removed.
-;; * For w32notify, no `attribute-changed' events arrive.  Its sends
-;;   `changed' events instead.
-;; * Check, why cygwin recognizes only `deleted' and `stopped' events.
+;; * For cygwin and w32notify, no `attribute-changed' events arrive.
+;;   They send `changed' events instead.
+;; * cygwin does not send all expected `changed' and `deleted' events.
+;;   Probably due to timing issues.
 
 (provide 'file-notify-tests)
 ;;; filenotify-tests.el ends here


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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
       [not found]                 ` <cea29d3c-30d7-45b1-6d8c-1a8b3511791c@cornell.edu>
@ 2016-12-27 12:06                   ` Michael Albinus
  2016-12-27 16:28                     ` Ken Brown
  2016-12-27 17:33                     ` Ken Brown
  0 siblings, 2 replies; 32+ messages in thread
From: Michael Albinus @ 2016-12-27 12:06 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

> I've made some progress on understanding why some file notifications
> aren't triggered in the tests on Cygwin.  One issue is that
> file-notify--test-read-event-timeout has to be increased drastically
> in order for `created' and `changed' events to be received.  These can
> take 5 seconds or more.

Well, gfilenotify uses a native library like inotify or kqueue when
available. If not, it does polling.

I guess the latter happens in the cygwin case. Maybe we need to
determine the polling interval, and to set
file-notify--test-read-event-timeout larger than that. Will check,
whether information this could be retrieved from gfilenotify.

> A second issue is that a small delay is needed between the creation of
> a file watch and the beginning of file activity.  This is not normally
> a problem in interactive sessions.

I see. I will check further, whether I could modify Fgfile_add_watch
such a way that it returns only when it is able to create events.

> The attached patch takes care of both timing issues.  With this patch,
> all the inexpensive file-notify tests pass on all my Cygwin systems.

LGTM, please install.

I would also like to modify the expensive tests accordingly. Do you know
where I could retrieve a compiled Emacs 26 for cygwin?

> Best regards,
>
> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-27 12:06                   ` Michael Albinus
@ 2016-12-27 16:28                     ` Ken Brown
  2016-12-27 17:33                     ` Ken Brown
  1 sibling, 0 replies; 32+ messages in thread
From: Ken Brown @ 2016-12-27 16:28 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

Hi Michael,

On 12/27/2016 7:06 AM, Michael Albinus wrote:
> Well, gfilenotify uses a native library like inotify or kqueue when
> available. If not, it does polling.
>
> I guess the latter happens in the cygwin case. Maybe we need to
> determine the polling interval, and to set
> file-notify--test-read-event-timeout larger than that. Will check,
> whether information this could be retrieved from gfilenotify.

That would be helpful.

> I see. I will check further, whether I could modify Fgfile_add_watch
> such a way that it returns only when it is able to create events.

And that.

> LGTM, please install.

Done.

> I would also like to modify the expensive tests accordingly. Do you know
> where I could retrieve a compiled Emacs 26 for cygwin?

I've just built one and put it on my private Cygwin repository at

   http://sanibeltranquility.com/cygwin/index.html

There are instructions at that site.  Let me know if anything is unclear.

Best regards,

Ken





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-27 12:06                   ` Michael Albinus
  2016-12-27 16:28                     ` Ken Brown
@ 2016-12-27 17:33                     ` Ken Brown
  2016-12-27 18:35                       ` Michael Albinus
  1 sibling, 1 reply; 32+ messages in thread
From: Ken Brown @ 2016-12-27 17:33 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

Hi again Michael,

On 12/27/2016 7:06 AM, Michael Albinus wrote:
> Maybe we need to
> determine the polling interval, and to set
> file-notify--test-read-event-timeout larger than that. Will check,
> whether information this could be retrieved from gfilenotify.

Isn't the polling interval set to 100 msec by the following line in 
gfilenotify.c?

   g_file_monitor_set_rate_limit (monitor, 100);

By the way, I tried removing that line and using the default polling 
interval, and it seemed (subjectively) to work better, i.e., the wait 
time for a `created' event in an interactive session appeared to be 2-3 
seconds instead of 5-6 seconds.  But this doesn't seem to be consistent 
enough to allow me to substantially reduce 
file-notify--test-read-event-timeout.

Best regards,

Ken





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-27 17:33                     ` Ken Brown
@ 2016-12-27 18:35                       ` Michael Albinus
  2016-12-27 22:48                         ` Ken Brown
  2016-12-28 23:16                         ` Ken Brown
  0 siblings, 2 replies; 32+ messages in thread
From: Michael Albinus @ 2016-12-27 18:35 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

Ken Brown <kbrown@cornell.edu> writes:

> Hi again Michael,

Hi Ken,

>> Maybe we need to determine the polling interval, and to set
>> file-notify--test-read-event-timeout larger than that. Will check,
>> whether information this could be retrieved from gfilenotify.
>
> Isn't the polling interval set to 100 msec by the following line in
> gfilenotify.c?
>
>   g_file_monitor_set_rate_limit (monitor, 100);

Yes. IIRC, I did that, because there were problems with gfilenotify on
GNU/Linux otherwise.

> By the way, I tried removing that line and using the default polling
> interval, and it seemed (subjectively) to work better, i.e., the wait
> time for a `created' event in an interactive session appeared to be
> 2-3 
> seconds instead of 5-6 seconds.  But this doesn't seem to be
> consistent enough to allow me to substantially reduce
> file-notify--test-read-event-timeout.

Good to know, thanks. I'm reading glib's sources now, in order to
understand how the polling works exactly.

Btw, which version of glib do you use for your tests?

> Best regards,
>
> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-27 18:35                       ` Michael Albinus
@ 2016-12-27 22:48                         ` Ken Brown
  2016-12-28 23:16                         ` Ken Brown
  1 sibling, 0 replies; 32+ messages in thread
From: Ken Brown @ 2016-12-27 22:48 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

On 12/27/2016 1:35 PM, Michael Albinus wrote:
> Btw, which version of glib do you use for your tests?

glib2.0-2.46.2





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-27 18:35                       ` Michael Albinus
  2016-12-27 22:48                         ` Ken Brown
@ 2016-12-28 23:16                         ` Ken Brown
  2016-12-29 19:06                           ` Michael Albinus
  1 sibling, 1 reply; 32+ messages in thread
From: Ken Brown @ 2016-12-28 23:16 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

On 12/27/2016 1:35 PM, Michael Albinus wrote:
> I'm reading glib's sources now, in order to
> understand how the polling works exactly.

Hi Michael,

I just looked at the sources also, and the reason for the roughly 
5-second delays I've been observing is clear.  In gio/gpollfilemonitor.c 
we have the following:

#define POLL_TIME_SECS 5
[...]
schedule_poll_timeout (GPollFileMonitor* poll_monitor)
{
   poll_monitor->timeout = g_timeout_source_new_seconds (POLL_TIME_SECS);
[...]
}

So I guess there's no way to avoid using a value of 
file-notify--test-read-event-timeout bigger than 5.

Best regards,

Ken





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-28 23:16                         ` Ken Brown
@ 2016-12-29 19:06                           ` Michael Albinus
  2016-12-29 20:33                             ` Ken Brown
  0 siblings, 1 reply; 32+ messages in thread
From: Michael Albinus @ 2016-12-29 19:06 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

> I just looked at the sources also, and the reason for the roughly
> 5-second delays I've been observing is clear.  In
> gio/gpollfilemonitor.c we have the following:
>
> #define POLL_TIME_SECS 5
> [...]
> schedule_poll_timeout (GPollFileMonitor* poll_monitor)
> {
>   poll_monitor->timeout = g_timeout_source_new_seconds (POLL_TIME_SECS);
> [...]
> }
>
> So I guess there's no way to avoid using a value of
> file-notify--test-read-event-timeout bigger than 5.

Yes, for a polling gfilenotify.

I've added the new function Fgfile_monitor_name to gfilenotify.c. It
tells us the name of the used monitor. In my case (GNU/Linux), it is
GInotifyFileMonitor. In your case it is GPollFileMonitor.

Based on this, we could detect better which timeout to use in
filenotify-tests.el. I worked on this, and the non-remote cases seem to
be OK now (although sometimes not all events do arrive). You could play
also with this, just call from Emacs root directory

# make -C test filenotify-tests

I recommend to change the value of n to 10 temporarily in both
file-notify-test06-many-events and
file-notify-test08-watched-file-in-watched-dir; otherwise the tests last
too long. If you want to run just one test case, call (for example)

# make -C test filenotify-tests SELECTOR='\"file-notify-test06-many-events$$\"'

SELECTOR is a regexp over test case names.

Installing the cygwin package "gvfs" brings you gvfs-monitor-dir.exe. If
available, the remote test cases are not skipped anymore.

> Best regards,
>
> Ken

Best regards, Michael.

PS: Could you pls provide on your site a recompiled Emacs? I would like
to use the changed gfilenotify.c. Thanks!





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-29 19:06                           ` Michael Albinus
@ 2016-12-29 20:33                             ` Ken Brown
  2016-12-30  8:50                               ` Eli Zaretskii
  2016-12-30 19:16                               ` Michael Albinus
  0 siblings, 2 replies; 32+ messages in thread
From: Ken Brown @ 2016-12-29 20:33 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

Hi Michael,

On 12/29/2016 2:06 PM, Michael Albinus wrote:
> Ken Brown <kbrown@cornell.edu> writes:
> I've added the new function Fgfile_monitor_name to gfilenotify.c. It
> tells us the name of the used monitor. In my case (GNU/Linux), it is
> GInotifyFileMonitor. In your case it is GPollFileMonitor.

Actually it turns out to be GFamFileMonitor on my system, presumably because I have the gamin package installed.  I'm going to see if I can figure out how GFamFileMonitor works.  As a quick check, however, I applied the following patch...

--- a/test/lisp/filenotify-tests.el
+++ b/test/lisp/filenotify-tests.el
@@ -73,7 +73,8 @@ file-notify--test-read-event
     ;; gio/gpollfilemonitor.c declares POLL_TIME_SECS 5. So we must
     ;; wait at least this time.
     ((and (string-equal (file-notify--test-library) "gfilenotify")
-         (string-equal (file-notify--test-monitor) "GPollFileMonitor"))
+          (or (string-equal (file-notify--test-monitor) "GPollFileMonitor")
+              (string-equal (file-notify--test-monitor) "GFamFileMonitor")))
      7)
     ((file-remote-p temporary-file-directory) 0.1)
     (t 0.01))))

...and all the inexpensive tests passed.  I'll keep playing with this.

> PS: Could you pls provide on your site a recompiled Emacs? I would like
> to use the changed gfilenotify.c. Thanks!

It's building right now and should be available shortly.

Best regards,

Ken





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-29 20:33                             ` Ken Brown
@ 2016-12-30  8:50                               ` Eli Zaretskii
  2016-12-30 19:07                                 ` Michael Albinus
  2016-12-30 19:16                               ` Michael Albinus
  1 sibling, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2016-12-30  8:50 UTC (permalink / raw)
  To: Ken Brown; +Cc: michael.albinus, 21804

> From: Ken Brown <kbrown@cornell.edu>
> Date: Thu, 29 Dec 2016 15:33:15 -0500
> Cc: 21804@debbugs.gnu.org
> 
> -         (string-equal (file-notify--test-monitor) "GPollFileMonitor"))
> +          (or (string-equal (file-notify--test-monitor) "GPollFileMonitor")
> +              (string-equal (file-notify--test-monitor) "GFamFileMonitor")))

Wouldn't it be better if gfile-monitor-name returned a symbol instead
of a string?  That's our general convention with functions, I think.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-30  8:50                               ` Eli Zaretskii
@ 2016-12-30 19:07                                 ` Michael Albinus
  2016-12-30 22:19                                   ` Ken Brown
  0 siblings, 1 reply; 32+ messages in thread
From: Michael Albinus @ 2016-12-30 19:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 21804

Eli Zaretskii <eliz@gnu.org> writes:

>> -         (string-equal (file-notify--test-monitor) "GPollFileMonitor"))
>> +          (or (string-equal (file-notify--test-monitor) "GPollFileMonitor")
>> +              (string-equal (file-notify--test-monitor) "GFamFileMonitor")))
>
> Wouldn't it be better if gfile-monitor-name returned a symbol instead
> of a string?  That's our general convention with functions, I think.

I've changed it accordingly.

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-29 20:33                             ` Ken Brown
  2016-12-30  8:50                               ` Eli Zaretskii
@ 2016-12-30 19:16                               ` Michael Albinus
  2016-12-30 23:15                                 ` Ken Brown
  1 sibling, 1 reply; 32+ messages in thread
From: Michael Albinus @ 2016-12-30 19:16 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

>> Ken Brown <kbrown@cornell.edu> writes:
>> I've added the new function Fgfile_monitor_name to gfilenotify.c. It
>> tells us the name of the used monitor. In my case (GNU/Linux), it is
>> GInotifyFileMonitor. In your case it is GPollFileMonitor.
>
> Actually it turns out to be GFamFileMonitor on my system, presumably
> because I have the gamin package installed.  I'm going to see if I can
> figure out how GFamFileMonitor works.

Next time I have access to a cygwin machine I'll check. However, I
wonder whether we need to respect the polling period then. According to
<http://oss.sgi.com/projects/fam/faq.html#what_is_fam>, FAM does not poll.

> As a quick check, however, I applied the following patch...
>
> --- a/test/lisp/filenotify-tests.el
> +++ b/test/lisp/filenotify-tests.el
> @@ -73,7 +73,8 @@ file-notify--test-read-event
>      ;; gio/gpollfilemonitor.c declares POLL_TIME_SECS 5. So we must
>      ;; wait at least this time.
>      ((and (string-equal (file-notify--test-library) "gfilenotify")
> -         (string-equal (file-notify--test-monitor) "GPollFileMonitor"))
> +          (or (string-equal (file-notify--test-monitor) "GPollFileMonitor")
> +              (string-equal (file-notify--test-monitor) "GFamFileMonitor")))
>       7)
>      ((file-remote-p temporary-file-directory) 0.1)
>      (t 0.01))))
>
> ...and all the inexpensive tests passed.  I'll keep playing with this.

It's OK for me if you commit this patch. You know cygwin behaviour much
better then I do.

> Best regards,
>
> Ken

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-30 19:07                                 ` Michael Albinus
@ 2016-12-30 22:19                                   ` Ken Brown
  2016-12-31  8:26                                     ` Eli Zaretskii
  0 siblings, 1 reply; 32+ messages in thread
From: Ken Brown @ 2016-12-30 22:19 UTC (permalink / raw)
  To: Michael Albinus, Eli Zaretskii; +Cc: 21804

On 12/30/2016 2:07 PM, Michael Albinus wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> -         (string-equal (file-notify--test-monitor) "GPollFileMonitor"))
>>> +          (or (string-equal (file-notify--test-monitor) "GPollFileMonitor")
>>> +              (string-equal (file-notify--test-monitor) "GFamFileMonitor")))
>>
>> Wouldn't it be better if gfile-monitor-name returned a symbol instead
>> of a string?  That's our general convention with functions, I think.
>
> I've changed it accordingly.

But since it returns an uninterned symbol, I think I still need to use 
string-equal above.  Or am I misunderstanding something?

Ken






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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-30 19:16                               ` Michael Albinus
@ 2016-12-30 23:15                                 ` Ken Brown
  2017-01-02 18:47                                   ` Michael Albinus
  0 siblings, 1 reply; 32+ messages in thread
From: Ken Brown @ 2016-12-30 23:15 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

Hi Michael,

On 12/30/2016 2:16 PM, Michael Albinus wrote:
> Ken Brown <kbrown@cornell.edu> writes:
>> Actually it turns out to be GFamFileMonitor on my system, presumably
>> because I have the gamin package installed.

It turns out that this isn't special to my system.  The emacs package on 
Cygwin requires libglib2.0_0, which requires gamin.  So the monitor on 
Cygwin will always be of type GFamFileMonitor.

> Next time I have access to a cygwin machine I'll check. However, I
> wonder whether we need to respect the polling period then. According to
> <http://oss.sgi.com/projects/fam/faq.html#what_is_fam>, FAM does not poll.

It says on that page, "If FAM is running on a system that doesn't have a 
supported kernel monitor, it polls."  And I've checked the gamin sources 
(since Cygwin uses gamin for its implementation of libfam), and the same 
seems to be true there.

On the other hand, the default polling interval seems to be 1 second, so 
I'm not sure why the tests need such a big timeout.  One factor might be 
the following in gio/glocalfilemonitor.c:

   #define VIRTUAL_CHANGES_DONE_DELAY  2 * G_TIME_SPAN_SECOND

If I understand the code correctly, this causes a 2-second delay before 
a CHANGES_DONE_HINT event is sent after a CHANGED event.

>> As a quick check, however, I applied the following patch...
>>
>> --- a/test/lisp/filenotify-tests.el
>> +++ b/test/lisp/filenotify-tests.el
>> @@ -73,7 +73,8 @@ file-notify--test-read-event
>>      ;; gio/gpollfilemonitor.c declares POLL_TIME_SECS 5. So we must
>>      ;; wait at least this time.
>>      ((and (string-equal (file-notify--test-library) "gfilenotify")
>> -         (string-equal (file-notify--test-monitor) "GPollFileMonitor"))
>> +          (or (string-equal (file-notify--test-monitor) "GPollFileMonitor")
>> +              (string-equal (file-notify--test-monitor) "GFamFileMonitor")))
>>       7)
>>      ((file-remote-p temporary-file-directory) 0.1)
>>      (t 0.01))))
>>
>> ...and all the inexpensive tests passed.  I'll keep playing with this.
>
> It's OK for me if you commit this patch. You know cygwin behaviour much
> better then I do.

Let me play with this a little more first.

Best regards,

Ken






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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-30 22:19                                   ` Ken Brown
@ 2016-12-31  8:26                                     ` Eli Zaretskii
  2016-12-31  9:42                                       ` Michael Albinus
  0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2016-12-31  8:26 UTC (permalink / raw)
  To: Ken Brown; +Cc: michael.albinus, 21804

> Cc: 21804@debbugs.gnu.org
> From: Ken Brown <kbrown@cornell.edu>
> Date: Fri, 30 Dec 2016 17:19:03 -0500
> 
> On 12/30/2016 2:07 PM, Michael Albinus wrote:
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> -         (string-equal (file-notify--test-monitor) "GPollFileMonitor"))
> >>> +          (or (string-equal (file-notify--test-monitor) "GPollFileMonitor")
> >>> +              (string-equal (file-notify--test-monitor) "GFamFileMonitor")))
> >>
> >> Wouldn't it be better if gfile-monitor-name returned a symbol instead
> >> of a string?  That's our general convention with functions, I think.
> >
> > I've changed it accordingly.
> 
> But since it returns an uninterned symbol, I think I still need to use 
> string-equal above.  Or am I misunderstanding something?

I actually don't understand why Michael decided to return an
uninterned symbol here.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-31  8:26                                     ` Eli Zaretskii
@ 2016-12-31  9:42                                       ` Michael Albinus
  0 siblings, 0 replies; 32+ messages in thread
From: Michael Albinus @ 2016-12-31  9:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 21804

Eli Zaretskii <eliz@gnu.org> writes:

> I actually don't understand why Michael decided to return an
> uninterned symbol here.

I also don't understand :-(

Fixed.

Best regards, Michael.





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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2016-12-30 23:15                                 ` Ken Brown
@ 2017-01-02 18:47                                   ` Michael Albinus
  2017-01-02 23:15                                     ` Ken Brown
  0 siblings, 1 reply; 32+ messages in thread
From: Michael Albinus @ 2017-01-02 18:47 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804

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

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

> It turns out that this isn't special to my system.  The emacs package
> on Cygwin requires libglib2.0_0, which requires gamin.  So the monitor
> on Cygwin will always be of type GFamFileMonitor.

Indeed, it happens also for me.

> Let me play with this a little more first.

I've tested in parallel today. The applied patch let all non-remote test
cases pass successfully for cygwin. There were some special timing
issues (for example, when a watch was removed), which should be solved
now.

There are other problems in the remote case. Mainly, I don't know a way
to determine, which backend is linked to
gvfs-directory-monitor. According to my tests, it behaves different when
running on a (remote) GNU/Linux machine, or a (remote) cygwin machine.

I've committed a patch to Tramp, which let us detect this program as
"gvfs-monitor-dir.exe" for cygwin. This is hackish, of course; but I
don't know it better. The appended patch let pass all remote test cases
for cygwin too, except file-notify-test06-many-events-remote and
file-notify-test08-watched-file-in-watched-dir-remote. For these two
test cases, there's always a loss of some events. I see no chance to fix
this also.

If you agree, I would commit the patch to the master branch, and I would
close bug#21804 then.

> Best regards,
>
> Ken

Best regards, Michael.


[-- Attachment #2: Type: text/plain, Size: 10081 bytes --]

*** /home/albinus/src/emacs/test/lisp/filenotify-tests.el.~367dadf5541f3cc10ba992efb885bd259246ca66~	2017-01-02 19:41:12.906115611 +0100
--- /home/albinus/src/emacs/test/lisp/filenotify-tests.el	2017-01-02 10:15:32.173810090 +0100
***************
*** 63,68 ****
--- 63,69 ----
  (defvar file-notify--test-results nil)
  (defvar file-notify--test-event nil)
  (defvar file-notify--test-events nil)
+ (defvar file-notify--test-monitors nil)
  
  (defun file-notify--test-read-event ()
    "Read one event.
***************
*** 78,83 ****
--- 79,85 ----
            (memq (file-notify--test-monitor)
                  '(GFamFileMonitor GPollFileMonitor)))
       7)
+     ((string-equal (file-notify--test-library) "gvfs-monitor-dir.exe") 1)
      ((file-remote-p temporary-file-directory) 0.1)
      (t 0.01))))
  
***************
*** 153,159 ****
          file-notify--test-desc1 nil
          file-notify--test-desc2 nil
          file-notify--test-results nil
!         file-notify--test-events nil))
  
  (setq password-cache-expiry nil
        tramp-verbose 0
--- 155,162 ----
          file-notify--test-desc1 nil
          file-notify--test-desc2 nil
          file-notify--test-results nil
!         file-notify--test-events nil
!         file-notify--test-monitors nil))
  
  (setq password-cache-expiry nil
        tramp-verbose 0
***************
*** 210,219 ****
    "The used monitor for the test, as a symbol.
  This returns only for the local case and gfilenotify; otherwise it is nil.
  `file-notify--test-desc' must be a valid watch descriptor."
!   (and file-notify--test-desc
!        (null (file-remote-p temporary-file-directory))
!        (functionp 'gfile-monitor-name)
!        (gfile-monitor-name file-notify--test-desc)))
  
  (defmacro file-notify--deftest-remote (test docstring)
    "Define ert `TEST-remote' for remote files."
--- 213,228 ----
    "The used monitor for the test, as a symbol.
  This returns only for the local case and gfilenotify; otherwise it is nil.
  `file-notify--test-desc' must be a valid watch descriptor."
!   ;; We cache the result, because after `file-notify-rm-watch',
!   ;; `gfile-monitor-name' does not return a proper result anymore.
!   ;; But we still need this information.
!   (unless (file-remote-p temporary-file-directory)
!     (or (cdr (assq file-notify--test-desc file-notify--test-monitors))
!         (when (functionp 'gfile-monitor-name)
!           (add-to-list 'file-notify--test-monitors
!                        (cons file-notify--test-desc
!                              (gfile-monitor-name file-notify--test-desc)))
!           (cdr (assq file-notify--test-desc file-notify--test-monitors))))))
  
  (defmacro file-notify--deftest-remote (test docstring)
    "Define ert `TEST-remote' for remote files."
***************
*** 444,449 ****
--- 453,464 ----
                    '(change) #'file-notify--test-event-handler)))
            (file-notify--test-with-events
                (cond
+                ;; gvfs-monitor-dir on cygwin does not detect the
+                ;; `created' event reliably.
+ 	       ((string-equal
+ 		 (file-notify--test-library) "gvfs-monitor-dir.exe")
+ 		'((deleted stopped)
+ 		  (created deleted stopped)))
                 ;; cygwin does not raise a `changed' event.
                 ((eq system-type 'cygwin)
                  '(created deleted stopped))
***************
*** 463,471 ****
  		file-notify--test-tmpfile
  		'(change) #'file-notify--test-event-handler)))
          (file-notify--test-with-events
!             ;; There could be one or two `changed' events.
!             '((changed deleted stopped)
!               (changed changed deleted stopped))
            (write-region
             "another text" nil file-notify--test-tmpfile nil 'no-message)
            (file-notify--test-read-event)
--- 478,492 ----
  		file-notify--test-tmpfile
  		'(change) #'file-notify--test-event-handler)))
          (file-notify--test-with-events
! 	    (cond
!              ;; gvfs-monitor-dir on cygwin does not detect the
!              ;; `changed' event reliably.
! 	     ((string-equal (file-notify--test-library) "gvfs-monitor-dir.exe")
! 	      '((deleted stopped)
! 		(changed deleted stopped)))
! 	     ;; There could be one or two `changed' events.
! 	     (t '((changed deleted stopped)
! 		  (changed changed deleted stopped))))
            (write-region
             "another text" nil file-notify--test-tmpfile nil 'no-message)
            (file-notify--test-read-event)
***************
*** 489,494 ****
--- 510,521 ----
  	       ;; events for the watched directory.
  	       ((string-equal (file-notify--test-library) "w32notify")
  		'(created changed deleted))
+                ;; gvfs-monitor-dir on cygwin does not detect the
+                ;; `created' event reliably.
+ 	       ((string-equal
+ 		 (file-notify--test-library) "gvfs-monitor-dir.exe")
+ 		'((deleted stopped)
+ 		  (created deleted stopped)))
  	       ;; There are two `deleted' events, for the file and for
  	       ;; the directory.  Except for cygwin and kqueue.  And
  	       ;; cygwin does not raise a `changed' event.
***************
*** 522,527 ****
--- 549,560 ----
  		'(created changed created changed
  		  changed changed changed
  		  deleted deleted))
+                ;; gvfs-monitor-dir on cygwin does not detect the
+                ;; `created' event reliably.
+ 	       ((string-equal
+ 		 (file-notify--test-library) "gvfs-monitor-dir.exe")
+ 		'((deleted stopped)
+ 		  (created created deleted stopped)))
  	       ;; There are three `deleted' events, for two files and
  	       ;; for the directory.  Except for cygwin and kqueue.
  	       ((eq system-type 'cygwin)
***************
*** 559,564 ****
--- 592,603 ----
  	       ;; events for the watched directory.
  	       ((string-equal (file-notify--test-library) "w32notify")
  		'(created changed renamed deleted))
+                ;; gvfs-monitor-dir on cygwin does not detect the
+                ;; `created' event reliably.
+ 	       ((string-equal
+ 		 (file-notify--test-library) "gvfs-monitor-dir.exe")
+ 		'((deleted stopped)
+ 		  (created deleted stopped)))
  	       ;; There are two `deleted' events, for the file and for
  	       ;; the directory.  Except for cygwin and kqueue.  And
  	       ;; cygwin raises `created' and `deleted' events instead
***************
*** 578,585 ****
            (file-notify-rm-watch file-notify--test-desc))
  
          ;; Check attribute change.  Does not work for cygwin.
! 	(unless (and (eq system-type 'cygwin)
! 		     (not (file-remote-p temporary-file-directory)))
  	  (setq file-notify--test-tmpfile (file-notify--test-make-temp-name))
  	  (write-region
  	   "any text" nil file-notify--test-tmpfile nil 'no-message)
--- 617,623 ----
            (file-notify-rm-watch file-notify--test-desc))
  
          ;; Check attribute change.  Does not work for cygwin.
! 	(unless (eq system-type 'cygwin)
  	  (setq file-notify--test-tmpfile (file-notify--test-make-temp-name))
  	  (write-region
  	   "any text" nil file-notify--test-tmpfile nil 'no-message)
***************
*** 654,659 ****
--- 692,702 ----
  	      (while (null auto-revert-notify-watch-descriptor)
  		(sleep-for 1)))
  
+             ;; `file-notify--test-monitor' needs to know
+             ;; `file-notify--test-desc' in order to compute proper
+             ;; timeouts.
+             (setq file-notify--test-desc auto-revert-notify-watch-descriptor)
+ 
  	    ;; Check, that file notification has been used.
  	    (should auto-revert-mode)
  	    (should auto-revert-use-notify)
***************
*** 748,756 ****
  		'(change) #'file-notify--test-event-handler)))
  	(should (file-notify-valid-p file-notify--test-desc))
          (file-notify--test-with-events
!              ;; There could be one or two `changed' events.
!              '((changed deleted stopped)
!                (changed changed deleted stopped))
            (write-region
             "another text" nil file-notify--test-tmpfile nil 'no-message)
  	  (file-notify--test-read-event)
--- 791,805 ----
  		'(change) #'file-notify--test-event-handler)))
  	(should (file-notify-valid-p file-notify--test-desc))
          (file-notify--test-with-events
! 	    (cond
!              ;; gvfs-monitor-dir on cygwin does not detect the
!              ;; `changed' event reliably.
! 	     ((string-equal (file-notify--test-library) "gvfs-monitor-dir.exe")
! 	      '((deleted stopped)
! 		(changed deleted stopped)))
! 	     ;; There could be one or two `changed' events.
! 	     (t '((changed deleted stopped)
! 		  (changed changed deleted stopped))))
            (write-region
             "another text" nil file-notify--test-tmpfile nil 'no-message)
  	  (file-notify--test-read-event)
***************
*** 781,786 ****
--- 830,840 ----
  	  ;; for the watched directory.
  	  ((string-equal (file-notify--test-library) "w32notify")
  	   '(created changed deleted))
+           ;; gvfs-monitor-dir on cygwin does not detect the `created'
+           ;; event reliably.
+ 	  ((string-equal (file-notify--test-library) "gvfs-monitor-dir.exe")
+ 	   '((deleted stopped)
+ 	     (created deleted stopped)))
  	  ;; There are two `deleted' events, for the file and for the
  	  ;; directory.  Except for cygwin and kqueue.  And cygwin
  	  ;; does not raise a `changed' event.
***************
*** 1043,1049 ****
           (setq file-notify--test-desc1
                 (file-notify-add-watch
                  file-notify--test-tmpfile
!                 '(change) #'dir-callback)))
          (should
           (setq file-notify--test-desc2
                 (file-notify-add-watch
--- 1097,1105 ----
           (setq file-notify--test-desc1
                 (file-notify-add-watch
                  file-notify--test-tmpfile
!                 '(change) #'dir-callback)
!                ;; This is needed for `file-notify--test-monitor'.
!                file-notify--test-desc file-notify--test-desc1))
          (should
           (setq file-notify--test-desc2
                 (file-notify-add-watch

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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2017-01-02 18:47                                   ` Michael Albinus
@ 2017-01-02 23:15                                     ` Ken Brown
  2017-01-03  9:11                                       ` Michael Albinus
  0 siblings, 1 reply; 32+ messages in thread
From: Ken Brown @ 2017-01-02 23:15 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 21804

Hi Michael,

On 1/2/2017 1:47 PM, Michael Albinus wrote:
> If you agree, I would commit the patch to the master branch, and I would
> close bug#21804 then.

Yes, please go ahead.  And thanks for all of your work on this.

Best regards,

Ken






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

* bug#21804: 25.0.50; file-notify-tests failure on Cygwin
  2017-01-02 23:15                                     ` Ken Brown
@ 2017-01-03  9:11                                       ` Michael Albinus
  0 siblings, 0 replies; 32+ messages in thread
From: Michael Albinus @ 2017-01-03  9:11 UTC (permalink / raw)
  To: Ken Brown; +Cc: 21804-done

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

>> If you agree, I would commit the patch to the master branch, and I would
>> close bug#21804 then.
>
> Yes, please go ahead.  And thanks for all of your work on this.

I've committed it to master, closing the bug.

> Best regards,
>
> Ken

Best regards, Michael.





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

end of thread, other threads:[~2017-01-03  9:11 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-01 15:51 bug#21804: 25.0.50; file-notify-tests failure on Cygwin Ken Brown
2015-11-01 16:40 ` Eli Zaretskii
2015-11-01 17:25   ` Ken Brown
2015-11-01 17:34 ` Michael Albinus
2015-11-03 17:20   ` Michael Albinus
2015-11-04  3:54     ` Ken Brown
2015-11-04  8:20       ` Michael Albinus
2015-11-04 16:55         ` Ken Brown
2015-11-04 18:44           ` Michael Albinus
2015-11-05  3:21             ` Ken Brown
2015-11-05 19:58               ` Michael Albinus
2015-11-05 20:14                 ` Ken Brown
2015-11-06  6:35                   ` Michael Albinus
2016-12-26 17:08                 ` Ken Brown
     [not found]                 ` <cea29d3c-30d7-45b1-6d8c-1a8b3511791c@cornell.edu>
2016-12-27 12:06                   ` Michael Albinus
2016-12-27 16:28                     ` Ken Brown
2016-12-27 17:33                     ` Ken Brown
2016-12-27 18:35                       ` Michael Albinus
2016-12-27 22:48                         ` Ken Brown
2016-12-28 23:16                         ` Ken Brown
2016-12-29 19:06                           ` Michael Albinus
2016-12-29 20:33                             ` Ken Brown
2016-12-30  8:50                               ` Eli Zaretskii
2016-12-30 19:07                                 ` Michael Albinus
2016-12-30 22:19                                   ` Ken Brown
2016-12-31  8:26                                     ` Eli Zaretskii
2016-12-31  9:42                                       ` Michael Albinus
2016-12-30 19:16                               ` Michael Albinus
2016-12-30 23:15                                 ` Ken Brown
2017-01-02 18:47                                   ` Michael Albinus
2017-01-02 23:15                                     ` Ken Brown
2017-01-03  9:11                                       ` Michael Albinus

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).