another bug fix release: 0.15.2, in progress.

another bug fix release: 0.15.2, in progress.

[david]

I plan to do another quick bug fix release, with aidecoe's parallel
build fix and a patch originally from Tomi to make the Emacs tests not
commit suicide right away on certain OSes.

I didn't include the parallel building patch here since it's already
in master.

[PATCH 1/2] test: delay watchdog checks in emacs.

[david]

From: David Bremner <bremner@debian.org>

Instead of checking immediately for the watched process, delay a
minute, or in the case that process-attributes returns nil, for two
minutes.  This is intended to cope with the case that
process-attributes is unimplimented, and returns always returns nil.
In this case, the watchdog check is the same as the two minute limit
imposed by timeout.
---
 test/test-lib.el |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index dece811..d26b49f 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -77,12 +77,19 @@ invisible text."
 	(setq start next-pos)))
     str))
 
-(defun orphan-watchdog (pid)
+(defun orphan-watchdog-check (pid)
   "Periodically check that the process with id PID is still
 running, quit if it terminated."
   (if (not (process-attributes pid))
-      (kill-emacs)
-    (run-at-time "1 min" nil 'orphan-watchdog pid)))
+      (kill-emacs)))
+
+(defun orphan-watchdog (pid)
+  "Initiate orphan watchdog check."
+  ; If process-attributes returns nil right away, that probably means
+  ; it is unimplimented. So we delay two minutes before killing emacs.
+  (if (process-attributes pid)
+      (run-at-time 60 60 'orphan-watchdog-check pid)
+    (run-at-time 120 60 'orphan-watchdog-check pid)))
 
 (defun hook-counter (hook)
   "Count how many times a hook is called.  Increments
-- 
1.7.10.4

[PATCH 2/2] NEWS: News for 0.15.2

[david]

From: David Bremner <bremner@debian.org>

Another couple of build fixes.
---
 NEWS |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/NEWS b/NEWS
index 97f2305..3ba5a50 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,17 @@
+Notmuch 0.15.2 (2013-01-29)
+===========================
+
+Build fixes
+-----------
+
+Update dependencies to avoid problems when building in parallel.
+
+Internal test framework changes
+-------------------------------
+
+Adjust Emacs test watchdog mechanism to cope with `process-attributes`
+being unimplimented.
+
 Notmuch 0.15.1 (2013-01-24)
 =========================
 
-- 
1.7.10.4

Re: another bug fix release: 0.15.2, in progress.

[Tomi Ollila]

On Wed, Jan 30 2013, david@tethera.net wrote:

> I plan to do another quick bug fix release, with aidecoe's parallel
> build fix and a patch originally from Tomi to make the Emacs tests not
> commit suicide right away on certain OSes.

LGTM.

> I didn't include the parallel building patch here since it's already
> in master.

(moved lgtm up for obvious reasons ;)

Tomi

Re: another bug fix release: 0.15.2, in progress.

[David Bremner]

david@tethera.net writes:

> I plan to do another quick bug fix release, with aidecoe's parallel
> build fix and a patch originally from Tomi to make the Emacs tests not
> commit suicide right away on certain OSes.
>
> I didn't include the parallel building patch here since it's already
> in master.

Apparently not so quick.  I've been busy, and it turned out the bug I
fixed in the Hurd build process revealed another bug, I think in Emacs,
namely that emacs on Hurd segfaults running the notmuch test suite.
Anyway, I guess we may as well push these fixes out; I'm a bit curious
if Robert is going to revise
id:1359917491-17178-1-git-send-email-beheerder@tekenbeetziekten.nl in
the near future; in that case I might include that fix as well.

d

RE: [Spam-verdenking][english 100%] Re: another bug fix release: 0.15.2, in progress.

[Robert Mast]

David/Austin,

I sent it as a patch to my previous patch. Is that correct, or should I first 'revert' the file to 15.1?

Robert

-----Oorspronkelijk bericht-----
Van: David Bremner [mailto:david@tethera.net] 
Verzonden: zondag 10 februari 2013 2:17
Aan: notmuch@notmuchmail.org
CC: Robert Mast
Onderwerp: [Spam-verdenking][english 100%] Re: another bug fix release: 0.15.2, in progress.

david@tethera.net writes:

> I plan to do another quick bug fix release, with aidecoe's parallel 
> build fix and a patch originally from Tomi to make the Emacs tests not 
> commit suicide right away on certain OSes.
>
> I didn't include the parallel building patch here since it's already 
> in master.

Apparently not so quick.  I've been busy, and it turned out the bug I fixed in the Hurd build process revealed another bug, I think in Emacs, namely that emacs on Hurd segfaults running the notmuch test suite.
Anyway, I guess we may as well push these fixes out; I'm a bit curious if Robert is going to revise id:1359917491-17178-1-git-send-email-beheerder@tekenbeetziekten.nl in the near future; in that case I might include that fix as well.

d


.

RE: [Spam-verdenking][english 100%] Re: another bug fix release: 0.15.2, in progress.

[David Bremner]

Robert Mast <beheerder@tekenbeetziekten.nl> writes:

> David/Austin,
>
> I sent it as a patch to my previous patch. Is that correct, or should I first 'revert' the file to 15.1?
>
> Robert

Hi Robert;

I didn't see any followup patch from you on the list; can you tell me the
message-id?

In any case, in general you should send complete patches or or patch series
against git master, since we care both about the commit message, and what
changes are part of each commit.  Most people use the git rebase command
to update patches before re-sending them.

d

Re: [PATCH 1/2] test: delay watchdog checks in emacs.

[David Bremner]

david@tethera.net writes:

> From: David Bremner <bremner@debian.org>
>
> Instead of checking immediately for the watched process, delay a
> minute, or in the case that process-attributes returns nil, for two
> minutes.  This is intended to cope with the case that
> process-attributes is unimplimented, and returns always returns nil.
> In this case, the watchdog check is the same as the two minute limit
> imposed by timeout.
> ---

It seems there are still lingering problems with this.

On a Debian Hurd system, the following consistently dies after running
for two minutes. It seems like our test harness can't cope with the
emacs server dying as well as it ought to.  I'm not sure what the right
answer is. A crude workaround would be to increase the hardcoded
timeout. Alternatively, maybe test_emacs should make some effort to
restart the server if it is no longer running.

#!/usr/bin/env bash
test_description="emacs fcc"
. ./test-lib.sh

for ((count=1; count<=1000; count++)); do
    id=fub.$count
    subject=meh
    echo attempting $count
    emacs_fcc_message \
	"$subject" \
	'This is a test message with inline attachment with a filename' \
	"(mml-attach-file \"$TEST_DIRECTORY/README\" nil nil \"inline\")
         (message-goto-eoh)
         (insert \"Message-ID: <$id>\n\")"
    delivered=$(find $MAIL_DIR/sent -type f | wc -l)
    if [[ $delivered != $count ]]; then
	GIT_EXIT_OK=1
	echo FATAL: $count attempted $delivered delivered
	exit 1;
    fi
done

test_done

[PATCH] test/emacs: replace the use of process-attributes with kill(1)

[David Bremner]

In some environments (at least Hurd), process-attributes is
unimplimented and always returns nil.  This ends up causing test
failures (see e.g. id:87a9ffofsc.fsf@zancas.localnet).

According to POSIX 1003.1-2001, a signal of 0 can be used to check the
validity of a pid. This seems less heinous than parsing the output of ps(1).
---

I'm not sure if it would be worthwhile, but a point release could
finally get notmuch compiling on Hurd again.  At least the test in the parent message does pass on Hurd with this patch

 test/test-lib.el | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index d26b49f..93f5f72 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -77,19 +77,22 @@ invisible text."
 	(setq start next-pos)))
     str))
 
+;; process-attributes is not defined everywhere, so define an
+;; alternate way to test if a process still exists.
+
+(defun test-process-running (pid)
+  (= 0
+   (call-process "kill" nil nil nil "-0" (int-to-string pid))))
+
 (defun orphan-watchdog-check (pid)
   "Periodically check that the process with id PID is still
 running, quit if it terminated."
-  (if (not (process-attributes pid))
+  (if (not (test-process-running pid))
       (kill-emacs)))
 
 (defun orphan-watchdog (pid)
   "Initiate orphan watchdog check."
-  ; If process-attributes returns nil right away, that probably means
-  ; it is unimplimented. So we delay two minutes before killing emacs.
-  (if (process-attributes pid)
-      (run-at-time 60 60 'orphan-watchdog-check pid)
-    (run-at-time 120 60 'orphan-watchdog-check pid)))
+  (run-at-time 60 60 'orphan-watchdog-check pid))
 
 (defun hook-counter (hook)
   "Count how many times a hook is called.  Increments
-- 
1.8.5.2

Re: [PATCH] test/emacs: replace the use of process-attributes with kill(1)

[Tomi Ollila]

On Sat, Jan 11 2014, David Bremner <david@tethera.net> wrote:

> In some environments (at least Hurd), process-attributes is
> unimplimented and always returns nil.  This ends up causing test
> failures (see e.g. id:87a9ffofsc.fsf@zancas.localnet).
>
> According to POSIX 1003.1-2001, a signal of 0 can be used to check the
> validity of a pid. This seems less heinous than parsing the output of ps(1).
> ---

This has been pretty portable method long before 2001 :D

You could replace the  call-process with

(signal-process pid 0)

Tomi

>
> I'm not sure if it would be worthwhile, but a point release could
> finally get notmuch compiling on Hurd again.  At least the test in the parent message does pass on Hurd with this patch
>
>  test/test-lib.el | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/test/test-lib.el b/test/test-lib.el
> index d26b49f..93f5f72 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -77,19 +77,22 @@ invisible text."
>  	(setq start next-pos)))
>      str))
>  
> +;; process-attributes is not defined everywhere, so define an
> +;; alternate way to test if a process still exists.
> +
> +(defun test-process-running (pid)
> +  (= 0
> +   (call-process "kill" nil nil nil "-0" (int-to-string pid))))
> +
>  (defun orphan-watchdog-check (pid)
>    "Periodically check that the process with id PID is still
>  running, quit if it terminated."
> -  (if (not (process-attributes pid))
> +  (if (not (test-process-running pid))
>        (kill-emacs)))
>  
>  (defun orphan-watchdog (pid)
>    "Initiate orphan watchdog check."
> -  ; If process-attributes returns nil right away, that probably means
> -  ; it is unimplimented. So we delay two minutes before killing emacs.
> -  (if (process-attributes pid)
> -      (run-at-time 60 60 'orphan-watchdog-check pid)
> -    (run-at-time 120 60 'orphan-watchdog-check pid)))
> +  (run-at-time 60 60 'orphan-watchdog-check pid))
>  
>  (defun hook-counter (hook)
>    "Count how many times a hook is called.  Increments
> -- 
> 1.8.5.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] test/emacs: replace the use of process-attributes with signal-process

[David Bremner]

In some environments (at least Hurd), process-attributes is
unimplimented and always returns nil.  This ends up causing test
failures (see e.g. id:87a9ffofsc.fsf@zancas.localnet).

Historically and according to POSIX 1003.1-2001, a signal of 0 can be
used to check the validity of a pid. This seems less heinous than
parsing the output of ps(1).
---

Thanks to Domo for the simpler solution. Now that I think about it,
perhaps this makes sense as a debian only update, since it currently
fails only on Debian/Hurd, which according to the GNU project is the
only "working distribution

 test/test-lib.el | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index d26b49f..37fcb3d 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -77,19 +77,22 @@ invisible text."
 	(setq start next-pos)))
     str))
 
+;; process-attributes is not defined everywhere, so define an
+;; alternate way to test if a process still exists.
+
+(defun test-process-running (pid)
+  (= 0
+   (signal-process pid 0)))
+
 (defun orphan-watchdog-check (pid)
   "Periodically check that the process with id PID is still
 running, quit if it terminated."
-  (if (not (process-attributes pid))
+  (if (not (test-process-running pid))
       (kill-emacs)))
 
 (defun orphan-watchdog (pid)
   "Initiate orphan watchdog check."
-  ; If process-attributes returns nil right away, that probably means
-  ; it is unimplimented. So we delay two minutes before killing emacs.
-  (if (process-attributes pid)
-      (run-at-time 60 60 'orphan-watchdog-check pid)
-    (run-at-time 120 60 'orphan-watchdog-check pid)))
+  (run-at-time 60 60 'orphan-watchdog-check pid))
 
 (defun hook-counter (hook)
   "Count how many times a hook is called.  Increments
-- 
1.8.5.2

Re: [PATCH] test/emacs: replace the use of process-attributes with signal-process

[Tomi Ollila]

On Sat, Jan 11 2014, David Bremner <david@tethera.net> wrote:

> In some environments (at least Hurd), process-attributes is
> unimplimented and always returns nil.  This ends up causing test
> failures (see e.g. id:87a9ffofsc.fsf@zancas.localnet).
>
> Historically and according to POSIX 1003.1-2001, a signal of 0 can be
> used to check the validity of a pid. This seems less heinous than
> parsing the output of ps(1).
> ---
>
> Thanks to Domo for the simpler solution. Now that I think about it,
> perhaps this makes sense as a debian only update, since it currently
> fails only on Debian/Hurd, which according to the GNU project is the
> only "working distribution

Would an ultimate(tm) solution be:

  (defun orphan-watchdog (pid)
    "Initiate orphan watchdog check."
      (run-at-time 60 60 
        (if (process-attributes pid)
      	    (lambda (pid) (if (not (process-attributes pid) (kill-emacs))))
      	  (lambda (pid) (if (/= 0 (signal-process pid 0)) (kill-emacs)))) 
        pid))

of course the lambdas can be replaced w/ symbols to functions, just
compressed for this example -- also w/ appropriate commit messages...

This could work on unices, hurd, windows, whatnot...


Anyway, the current patch also LGTM.

Tomi

>
>  test/test-lib.el | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/test/test-lib.el b/test/test-lib.el
> index d26b49f..37fcb3d 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -77,19 +77,22 @@ invisible text."
>  	(setq start next-pos)))
>      str))
>  
> +;; process-attributes is not defined everywhere, so define an
> +;; alternate way to test if a process still exists.
> +
> +(defun test-process-running (pid)
> +  (= 0
> +   (signal-process pid 0)))
> +
>  (defun orphan-watchdog-check (pid)
>    "Periodically check that the process with id PID is still
>  running, quit if it terminated."
> -  (if (not (process-attributes pid))
> +  (if (not (test-process-running pid))
>        (kill-emacs)))
>  
>  (defun orphan-watchdog (pid)
>    "Initiate orphan watchdog check."
> -  ; If process-attributes returns nil right away, that probably means
> -  ; it is unimplimented. So we delay two minutes before killing emacs.

Btw: who let this typo above pass through review ? ;)

> -  (if (process-attributes pid)
> -      (run-at-time 60 60 'orphan-watchdog-check pid)
> -    (run-at-time 120 60 'orphan-watchdog-check pid)))
> +  (run-at-time 60 60 'orphan-watchdog-check pid))
>  
>  (defun hook-counter (hook)
>    "Count how many times a hook is called.  Increments
> -- 
> 1.8.5.2

Re: [PATCH] test/emacs: replace the use of process-attributes with signal-process

[David Bremner]

David Bremner <david@tethera.net> writes:

> In some environments (at least Hurd), process-attributes is
> unimplimented and always returns nil.  This ends up causing test
> failures (see e.g. id:87a9ffofsc.fsf@zancas.localnet).
>
> Historically and according to POSIX 1003.1-2001, a signal of 0 can be
> used to check the validity of a pid. This seems less heinous than
> parsing the output of ps(1).

pushed, along with some fiddling with debian emacs packaging, as
debian/0.17-3

d