[PATCH] test: Use env to set TMPDIR when running emacs in screen.

[PATCH] test: Use env to set TMPDIR when running emacs in screen.

[Jameson Graef Rollins]

This is to get around a bug where screen unsets TMPDIR.  This causes
problems for users who set TMPDIR to something other than it's default
(/tmp).
---
 test/test-lib.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index ff5183f..108537e 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -842,7 +842,9 @@ test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
 		# start a detached screen session with an emacs server
-		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
+		screen -S "$EMACS_SERVER" -d -m \
+		    env TMPDIR="$TMPDIR" \
+		    "$TMP_DIRECTORY/run_emacs" \
 			--no-window-system \
 			--eval "(setq server-name \"$EMACS_SERVER\")" \
 			--eval '(server-start)' \
-- 
1.7.7.1

Re: [PATCH] test: Use env to set TMPDIR when running emacs in screen.

[Jameson Graef Rollins]

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

On Thu, 10 Nov 2011 08:59:35 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> This is to get around a bug where screen unsets TMPDIR.  This causes
> problems for users who set TMPDIR to something other than it's default
> (/tmp).

Arg.  This was supposed to be sent to the previous thread about this.
Ironically, I'm using the patch that removes id: from the message-id
kill-ring, but then I manually added it back to git send-email!  HA!

So there is now discussion about use detach instead of screen, which is
much more minimal and doesn't use setuid and setgid and therefore
doesn't purge TMPDIR.

In any event, I think we need to push this patch until we have a patch
to use detach in hand.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] test: Use env to set TMPDIR when running emacs in screen.

[Austin Clements]

This looks good to me, though it would be good to add a comment
explaining why this dance is necessary.  Perhaps something along the
lines of

# The emacs server places its socket in TMPDIR, but ld.so unsets
# TMPDIR when loading setgid binaries like screen, so we must
# explicitly pass TMPDIR through to emacs.

Quoth Jameson Graef Rollins on Nov 10 at  8:59 am:
> This is to get around a bug where screen unsets TMPDIR.  This causes
> problems for users who set TMPDIR to something other than it's default
> (/tmp).
> ---
>  test/test-lib.sh |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index ff5183f..108537e 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -842,7 +842,9 @@ test_emacs () {
>  	if [ -z "$EMACS_SERVER" ]; then
>  		EMACS_SERVER="notmuch-test-suite-$$"
>  		# start a detached screen session with an emacs server
> -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> +		screen -S "$EMACS_SERVER" -d -m \
> +		    env TMPDIR="$TMPDIR" \
> +		    "$TMP_DIRECTORY/run_emacs" \
>  			--no-window-system \
>  			--eval "(setq server-name \"$EMACS_SERVER\")" \
>  			--eval '(server-start)' \

[PATCH] test: Use env to set TMPDIR when running emacs in screen.

[Jameson Graef Rollins]

This is to get around a bug where screen unsets TMPDIR.  This causes
problems for users who set TMPDIR to something other than it's default
(/tmp).
---
This just adds some comments over the previous patch.
Thanks to Austin Clements for the suggestion.

 test/test-lib.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index c81c709..5ed6a96 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -844,7 +844,12 @@ test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
 		# start a detached screen session with an emacs server
-		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
+		# The emacs server places its socket in TMPDIR, but ld.so unsets
+		# TMPDIR when loading setgid binaries like screen, so we must
+		# explicitly pass TMPDIR through to emacs.
+		screen -S "$EMACS_SERVER" -d -m \
+		    env TMPDIR="$TMPDIR" \
+		    "$TMP_DIRECTORY/run_emacs" \
 			--no-window-system \
 			--eval "(setq server-name \"$EMACS_SERVER\")" \
 			--eval '(server-start)' \
-- 
1.7.7.1

[PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Tomi Ollila]

dtach is lighter than screen and is not setuid/setgid program so
TMPDIR does not get reset by dynamic loader when executed.
---
 test/test-lib.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index c81c709..af723ad 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -844,8 +844,8 @@ test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
 		# start a detached screen session with an emacs server
-		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
-			--no-window-system \
+		TERM=xterm dtach -n "$TMP_DIRECTORY/dtach-emacs-socket-$$" \
+			"$TMP_DIRECTORY/run_emacs" --no-window-system \
 			--eval "(setq server-name \"$EMACS_SERVER\")" \
 			--eval '(server-start)' \
 			--eval "(orphan-watchdog $$)" || return
-- 
1.5.6.5

[PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Jameson Graef Rollins]

From: Tomi Ollila <tomi.ollila@iki.fi>

dtach is lighter than screen and is not setuid/setgid program so
TMPDIR does not get reset by dynamic loader when executed.

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
This tweaks the original patch slightly by removing the no-longer needed
screenrc variables.

 test/test-lib.sh |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index c81c709..c0fe037 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -50,8 +50,6 @@ TZ=UTC
 TERM=dumb
 export LANG LC_ALL PAGER TERM TZ
 GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
-export SCREENRC=/dev/null
-export SYSSCREENRC=/dev/null
 
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
@@ -844,7 +842,8 @@ test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
 		# start a detached screen session with an emacs server
-		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
+		TERM=xterm dtach -n "$TMP_DIRECTORY/dtach-emacs-socket-$$" \
+			"$TMP_DIRECTORY/run_emacs" \
 			--no-window-system \
 			--eval "(setq server-name \"$EMACS_SERVER\")" \
 			--eval '(server-start)' \
-- 
1.7.7.1

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Pieter Praet]

On Thu, 10 Nov 2011 14:22:17 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> From: Tomi Ollila <tomi.ollila@iki.fi>
> 
> dtach is lighter than screen and is not setuid/setgid program so
> TMPDIR does not get reset by dynamic loader when executed.
> 

`dtach' may be lighter than `screen', but contrary to my expectations,
it appears to be far less efficient:

|      | original [1] | w/ screen [2] | w/ dtach [3] |
|------+--------------+---------------+--------------|
| real |    0m17.570s |     0m20.132s |    0m28.567s |
| user |     0m2.453s |      0m2.593s |     0m2.753s |
| sys  |     0m1.700s |      0m1.577s |     0m1.753s |

> Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
> ---
> This tweaks the original patch slightly by removing the no-longer needed
> screenrc variables.
> 
>  test/test-lib.sh |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index c81c709..c0fe037 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -50,8 +50,6 @@ TZ=UTC
>  TERM=dumb
>  export LANG LC_ALL PAGER TERM TZ
>  GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
> -export SCREENRC=/dev/null
> -export SYSSCREENRC=/dev/null
>  
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
> @@ -844,7 +842,8 @@ test_emacs () {
>  	if [ -z "$EMACS_SERVER" ]; then
>  		EMACS_SERVER="notmuch-test-suite-$$"
>  		# start a detached screen session with an emacs server
> -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> +		TERM=xterm dtach -n "$TMP_DIRECTORY/dtach-emacs-socket-$$" \

It's worth noting that $TERM is only set to "xterm" to deceive `dtach',
so thankfully, xterm (and all its deps) aren't actually required to run
the tests.

> +			"$TMP_DIRECTORY/run_emacs" \
>  			--no-window-system \
>  			--eval "(setq server-name \"$EMACS_SERVER\")" \
>  			--eval '(server-start)' \
> -- 
> 1.7.7.1
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

[1] @ commit 749abb74
[2] @ commit 760b311b
[3] @ commit 760b311b + id:"1320963737-1666-1-git-send-email-jrollins@finestructure.net"

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Tomi Ollila]

On Fri, 11 Nov 2011 01:17:37 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Thu, 10 Nov 2011 14:22:17 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > From: Tomi Ollila <tomi.ollila@iki.fi>
> > 
> > dtach is lighter than screen and is not setuid/setgid program so
> > TMPDIR does not get reset by dynamic loader when executed.
> > 
> 
> `dtach' may be lighter than `screen', but contrary to my expectations,
> it appears to be far less efficient:
> 
> |      | original [1] | w/ screen [2] | w/ dtach [3] |
> |------+--------------+---------------+--------------|
> | real |    0m17.570s |     0m20.132s |    0m28.567s |
> | user |     0m2.453s |      0m2.593s |     0m2.753s |
> | sys  |     0m1.700s |      0m1.577s |     0m1.753s |

This sounds interesting; how were these runs measured?
Just using shell builtin time command ?

Does time measure the time of the process and it's subprocesses
or just the process. In any way 20 vs 28 second 'real' time difference
could indicate that there are more other processes running and
the context switch time affects user and system time.

Or, dtach does some stupid io things. Or screen does some advanced
optimizations. Or emacs works more effectively when TERM=screen.
Or the 'virtual' window size emacs sees is unsuitable...


> 
> > Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
> > ---
> > This tweaks the original patch slightly by removing the no-longer needed
> > screenrc variables.
> > 
> >  test/test-lib.sh |    5 ++---
> >  1 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index c81c709..c0fe037 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -50,8 +50,6 @@ TZ=UTC
> >  TERM=dumb
> >  export LANG LC_ALL PAGER TERM TZ
> >  GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
> > -export SCREENRC=/dev/null
> > -export SYSSCREENRC=/dev/null
> >  
> >  # Protect ourselves from common misconfiguration to export
> >  # CDPATH into the environment
> > @@ -844,7 +842,8 @@ test_emacs () {
> >  	if [ -z "$EMACS_SERVER" ]; then
> >  		EMACS_SERVER="notmuch-test-suite-$$"
> >  		# start a detached screen session with an emacs server
> > -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> > +		TERM=xterm dtach -n "$TMP_DIRECTORY/dtach-emacs-socket-$$" \
> 
> It's worth noting that $TERM is only set to "xterm" to deceive `dtach',
> so thankfully, xterm (and all its deps) aren't actually required to run
> the tests.

Actually, emacs SIGSEGVs if there is unsuitable TERM in use. I don't know
what it originally was.

> 
> > +			"$TMP_DIRECTORY/run_emacs" \
> >  			--no-window-system \
> >  			--eval "(setq server-name \"$EMACS_SERVER\")" \
> >  			--eval '(server-start)' \
> > -- 
> > 1.7.7.1
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> 
> 
> Peace
> 
> -- 
> Pieter
> 
> [1] @ commit 749abb74
> [2] @ commit 760b311b
> [3] @ commit 760b311b + id:"1320963737-1666-1-git-send-email-jrollins@finestructure.net"
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
> 

Tomi

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Pieter Praet]

On Fri, 11 Nov 2011 10:09:17 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Fri, 11 Nov 2011 01:17:37 +0100, Pieter Praet <pieter@praet.org> wrote:
> > On Thu, 10 Nov 2011 14:22:17 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > > From: Tomi Ollila <tomi.ollila@iki.fi>
> > > 
> > > dtach is lighter than screen and is not setuid/setgid program so
> > > TMPDIR does not get reset by dynamic loader when executed.
> > > 
> > 
> > `dtach' may be lighter than `screen', but contrary to my expectations,
> > it appears to be far less efficient:
> > 
> > |      | original [1] | w/ screen [2] | w/ dtach [3] |
> > |------+--------------+---------------+--------------|
> > | real |    0m17.570s |     0m20.132s |    0m28.567s |
> > | user |     0m2.453s |      0m2.593s |     0m2.753s |
> > | sys  |     0m1.700s |      0m1.577s |     0m1.753s |
> 
> This sounds interesting; how were these runs measured?
> Just using shell builtin time command ?
> 

Correct.

> Does time measure the time of the process and it's subprocesses
> or just the process. In any way 20 vs 28 second 'real' time difference
> could indicate that there are more other processes running and
> the context switch time affects user and system time.
> 

I just ran this rather crude ad-hoc script while off to make some coffee :)

  #+begin_src sh
    revs=(749abb74 760b311b tmp)
    loop=5
    cd ~/src/dev/notmuch
    until [ ${loop} == 0 ] ; do
        for i in ${revs[*]} ; do
            git co ${i}
            rm -rf /dev/shm/notmuch/*
            make clean >/dev/null 2>&1
            make >/dev/null 2>&1
            sync
            time make test OPTIONS="--root=/dev/shm/notmuch" | tail -
        done
        loop=$[${loop}-1]
    done
  #+end_src

So, I try to start with a blank slate and then simply time the entire
test suite.  This for each of the relevant commits ("tmp" is a branch on
top of 760b311b, containing only the dtach patch), five times in a row.

The results remained consistent between iterations (17s, 20s, 28s "real"
time, without exception), so they should be fairly accurate.

> Or, dtach does some stupid io things. Or screen does some advanced
> optimizations. Or emacs works more effectively when TERM=screen.
> Or the 'virtual' window size emacs sees is unsuitable...
> 
> 
> > 
> > > Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
> > > ---
> > > This tweaks the original patch slightly by removing the no-longer needed
> > > screenrc variables.
> > > 
> > >  test/test-lib.sh |    5 ++---
> > >  1 files changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > > index c81c709..c0fe037 100755
> > > --- a/test/test-lib.sh
> > > +++ b/test/test-lib.sh
> > > @@ -50,8 +50,6 @@ TZ=UTC
> > >  TERM=dumb
> > >  export LANG LC_ALL PAGER TERM TZ
> > >  GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
> > > -export SCREENRC=/dev/null
> > > -export SYSSCREENRC=/dev/null
> > >  
> > >  # Protect ourselves from common misconfiguration to export
> > >  # CDPATH into the environment
> > > @@ -844,7 +842,8 @@ test_emacs () {
> > >  	if [ -z "$EMACS_SERVER" ]; then
> > >  		EMACS_SERVER="notmuch-test-suite-$$"
> > >  		# start a detached screen session with an emacs server
> > > -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> > > +		TERM=xterm dtach -n "$TMP_DIRECTORY/dtach-emacs-socket-$$" \
> > 
> > It's worth noting that $TERM is only set to "xterm" to deceive `dtach',
> > so thankfully, xterm (and all its deps) aren't actually required to run
> > the tests.
> 
> Actually, emacs SIGSEGVs if there is unsuitable TERM in use. I don't know
> what it originally was.
> 

Hmm, I may have jumped to conclusions there.  I'm glad the part about
being spared from xterm and friends was right, though... :)

> > 
> > > +			"$TMP_DIRECTORY/run_emacs" \
> > >  			--no-window-system \
> > >  			--eval "(setq server-name \"$EMACS_SERVER\")" \
> > >  			--eval '(server-start)' \
> > > -- 
> > > 1.7.7.1
> > > 
> > > _______________________________________________
> > > notmuch mailing list
> > > notmuch@notmuchmail.org
> > > http://notmuchmail.org/mailman/listinfo/notmuch
> > 
> > 
> > Peace
> > 
> > -- 
> > Pieter
> > 
> > [1] @ commit 749abb74
> > [2] @ commit 760b311b
> > [3] @ commit 760b311b + id:"1320963737-1666-1-git-send-email-jrollins@finestructure.net"
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> > 
> 
> Tomi


Peace

-- 
Pieter

[PATCH 1/2] debian: clean up Uploaders and Build-Depends fields in debian/control

[Jameson Graef Rollins]

No functional change, but this will make for cleaner diffs down the
line.
---
 debian/control |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/debian/control b/debian/control
index 45fc2a1..bb7b29c 100644
--- a/debian/control
+++ b/debian/control
@@ -2,11 +2,21 @@ Source: notmuch
 Section: mail
 Priority: extra
 Maintainer: Carl Worth <cworth@debian.org>
-Uploaders: Jameson Graef Rollins <jrollins@finestructure.net>, martin f. krafft <madduck@debian.org>, 
-	   David Bremner <bremner@debian.org>
-Build-Depends: debhelper (>= 7.0.50~), pkg-config, libxapian-dev, 
- libgmime-2.4-dev, libtalloc-dev, libz-dev,  python-all (>= 2.6.6-3~),
- emacs23-nox | emacs23 (>=23~) | emacs23-lucid (>=23~), gdb, screen
+Uploaders:
+ Jameson Graef Rollins <jrollins@finestructure.net>,
+ martin f. krafft <madduck@debian.org>,
+ David Bremner <bremner@debian.org>
+Build-Depends:
+ debhelper (>= 7.0.50~),
+ pkg-config,
+ libxapian-dev,
+ libgmime-2.4-dev,
+ libtalloc-dev,
+ libz-dev,
+ python-all (>= 2.6.6-3~),
+ emacs23-nox | emacs23 (>=23~) | emacs23-lucid (>=23~),
+ gdb,
+ screen
 Standards-Version: 3.9.2
 Homepage: http://notmuchmail.org/
 Vcs-Git: git://notmuchmail.org/git/notmuch
-- 
1.7.7.1

[PATCH 2/2] debian: update build dependency on dtach instead of screen

[Jameson Graef Rollins]

This reflects a modification to the test suite to use dtach instead of
screen.
---
 debian/control |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/debian/control b/debian/control
index bb7b29c..3252f32 100644
--- a/debian/control
+++ b/debian/control
@@ -16,7 +16,7 @@ Build-Depends:
  python-all (>= 2.6.6-3~),
  emacs23-nox | emacs23 (>=23~) | emacs23-lucid (>=23~),
  gdb,
- screen
+ dtach
 Standards-Version: 3.9.2
 Homepage: http://notmuchmail.org/
 Vcs-Git: git://notmuchmail.org/git/notmuch
-- 
1.7.7.1

Re: [PATCH 2/2] debian: update build dependency on dtach instead of screen

[David Bremner]

On Thu, 10 Nov 2011 16:17:57 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> This reflects a modification to the test suite to use dtach instead of
> screen.

pushed

d

Re: [PATCH 1/2] debian: clean up Uploaders and Build-Depends fields in debian/control

[David Bremner]

On Thu, 10 Nov 2011 16:17:56 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> No functional change, but this will make for cleaner diffs down the
> line.

Pushed

d

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Dmitry Kurochkin]

Hi Tomi.

On Thu, 10 Nov 2011 14:22:17 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> From: Tomi Ollila <tomi.ollila@iki.fi>
> 
> dtach is lighter than screen and is not setuid/setgid program so
> TMPDIR does not get reset by dynamic loader when executed.
> 
> Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
> ---
> This tweaks the original patch slightly by removing the no-longer needed
> screenrc variables.
> 
>  test/test-lib.sh |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index c81c709..c0fe037 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -50,8 +50,6 @@ TZ=UTC
>  TERM=dumb
>  export LANG LC_ALL PAGER TERM TZ
>  GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
> -export SCREENRC=/dev/null
> -export SYSSCREENRC=/dev/null
>  
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
> @@ -844,7 +842,8 @@ test_emacs () {
>  	if [ -z "$EMACS_SERVER" ]; then
>  		EMACS_SERVER="notmuch-test-suite-$$"
>  		# start a detached screen session with an emacs server
> -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> +		TERM=xterm dtach -n "$TMP_DIRECTORY/dtach-emacs-socket-$$" \

Can you please give a more detailed explanation (and a comment) to
justify this workaround:

* Is it actually needed for dtach or emacs?  If it is the latter, then
  it should be "dtach ... env TERM=xterm emacs ...", right?

* Why should we care about systems that use terminals without
  corresponding terminfo installed?

* Why is it safe to use xterm?  Is there any standard that requires
  xterm info to be present on the system?

I have tried running dtach with emacs in rxvt-unicode and it worked fine
without overriding TERM.

I have also tried running dtach with invalid TERM:

  $ TERM=non-existing-term dtach -n s /bin/sh
  zsh: can't find terminal definition for non-existing-term

No segfault, shell is started fine.

But emacs does not work without a valid terminfo:

  $ TERM=non-existing-term emacs -nw
  emacs: Terminal type non-existing-term is not defined.
  If that is not the actual type of terminal you have,
  use the Bourne shell command `TERM=... export TERM' (C-shell:
  `setenv TERM ...') to specify the correct type.  It may be necessary
  to do `unset TERMINFO' (C-shell: `unsetenv TERMINFO') as well.

Regards,
  Dmitry

> +			"$TMP_DIRECTORY/run_emacs" \
>  			--no-window-system \
>  			--eval "(setq server-name \"$EMACS_SERVER\")" \
>  			--eval '(server-start)' \
> -- 
> 1.7.7.1
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Tomi Ollila]

On Fri, 11 Nov 2011 12:41:11 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Hi Tomi.
> 
> On Thu, 10 Nov 2011 14:22:17 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > From: Tomi Ollila <tomi.ollila@iki.fi>
> > 
> > dtach is lighter than screen and is not setuid/setgid program so
> > TMPDIR does not get reset by dynamic loader when executed.
> > 
> > Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
> > ---
> > This tweaks the original patch slightly by removing the no-longer needed
> > screenrc variables.
> > 
> >  test/test-lib.sh |    5 ++---
> >  1 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index c81c709..c0fe037 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -50,8 +50,6 @@ TZ=UTC
> >  TERM=dumb
> >  export LANG LC_ALL PAGER TERM TZ
> >  GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
> > -export SCREENRC=/dev/null
> > -export SYSSCREENRC=/dev/null
> >  
> >  # Protect ourselves from common misconfiguration to export
> >  # CDPATH into the environment
> > @@ -844,7 +842,8 @@ test_emacs () {
> >  	if [ -z "$EMACS_SERVER" ]; then
> >  		EMACS_SERVER="notmuch-test-suite-$$"
> >  		# start a detached screen session with an emacs server
> > -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> > +		TERM=xterm dtach -n "$TMP_DIRECTORY/dtach-emacs-socket-$$" \
> 
> Can you please give a more detailed explanation (and a comment) to
> justify this workaround:
> 
> * Is it actually needed for dtach or emacs?  If it is the latter, then
>   it should be "dtach ... env TERM=xterm emacs ...", right?

dtach doesn't care about TERM, it is emacs which does (as you noticed
below). But it is bood that the TERM is same in dtach's environment.

doing VAR=val command args instead of env VAR=val saves one execve(2)
as the shell does the environment setting (after fork before execve)
> 
> * Why should we care about systems that use terminals without
>   corresponding terminfo installed?

That's the issue I thought, armdragon on IRC thought etc. The 'xterm'
is part of ncurses-base on Debian, at least (thanks armdragon for this
info) and is it pretty much as base to every terminal emulator there is 
(I just launched gnome-terminal for testing, TERM was set to xterm --
it may have been initially in environment but if gnome-terminal changed
it before launching shell, my shell config will not change it)

> * Why is it safe to use xterm?  Is there any standard that requires
>   xterm info to be present on the system?

xterm is not needed, it is just to tell emacs it can use it's display
control sequences. more common alternative would have been vt100, just
that if one ever wants to attach to that emacs session you he gets
better experience (such as colors), using any terminal emulator, like rxvt.

> I have tried running dtach with emacs in rxvt-unicode and it worked fine
> without overriding TERM.

you had originally suitable TERM to be passed to emacs.

> 
> I have also tried running dtach with invalid TERM:
> 
>   $ TERM=non-existing-term dtach -n s /bin/sh
>   zsh: can't find terminal definition for non-existing-term
> 
> No segfault, shell is started fine.

Yes, dtach could not care less...

> 
> But emacs does not work without a valid terminfo:
> 
>   $ TERM=non-existing-term emacs -nw
>   emacs: Terminal type non-existing-term is not defined.
>   If that is not the actual type of terminal you have,
>   use the Bourne shell command `TERM=... export TERM' (C-shell:
>   `setenv TERM ...') to specify the correct type.  It may be necessary
>   to do `unset TERMINFO' (C-shell: `unsetenv TERMINFO') as well.

Yes, I got SIGSEGV --  strace -o oprfx -ff command args  is your friend :D

> Regards,
>   Dmitry

Thanks for your analysis. 

Tomi

> 
> > +			"$TMP_DIRECTORY/run_emacs" \
> >  			--no-window-system \
> >  			--eval "(setq server-name \"$EMACS_SERVER\")" \
> >  			--eval '(server-start)' \
> > -- 
> > 1.7.7.1
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
> 

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Dmitry Kurochkin]

On Fri, 11 Nov 2011 13:29:48 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Fri, 11 Nov 2011 12:41:11 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Hi Tomi.
> > 
> > On Thu, 10 Nov 2011 14:22:17 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > > From: Tomi Ollila <tomi.ollila@iki.fi>
> > > 
> > > dtach is lighter than screen and is not setuid/setgid program so
> > > TMPDIR does not get reset by dynamic loader when executed.
> > > 
> > > Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
> > > ---
> > > This tweaks the original patch slightly by removing the no-longer needed
> > > screenrc variables.
> > > 
> > >  test/test-lib.sh |    5 ++---
> > >  1 files changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > > index c81c709..c0fe037 100755
> > > --- a/test/test-lib.sh
> > > +++ b/test/test-lib.sh
> > > @@ -50,8 +50,6 @@ TZ=UTC
> > >  TERM=dumb
> > >  export LANG LC_ALL PAGER TERM TZ
> > >  GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
> > > -export SCREENRC=/dev/null
> > > -export SYSSCREENRC=/dev/null
> > >  
> > >  # Protect ourselves from common misconfiguration to export
> > >  # CDPATH into the environment
> > > @@ -844,7 +842,8 @@ test_emacs () {
> > >  	if [ -z "$EMACS_SERVER" ]; then
> > >  		EMACS_SERVER="notmuch-test-suite-$$"
> > >  		# start a detached screen session with an emacs server
> > > -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> > > +		TERM=xterm dtach -n "$TMP_DIRECTORY/dtach-emacs-socket-$$" \
> > 
> > Can you please give a more detailed explanation (and a comment) to
> > justify this workaround:
> > 
> > * Is it actually needed for dtach or emacs?  If it is the latter, then
> >   it should be "dtach ... env TERM=xterm emacs ...", right?
> 
> dtach doesn't care about TERM, it is emacs which does (as you noticed
> below). But it is bood that the TERM is same in dtach's environment.
> 
> doing VAR=val command args instead of env VAR=val saves one execve(2)
> as the shell does the environment setting (after fork before execve)

I do not think trying that saving one execve(2) call is what we are
after.  Especially in test suite.

IMO setting TERM inside dtach makes it clear that we the workaround is
for emacs, not dtach (a comment should also make this clear).  Besides,
it is always better to minimize the context affected by a hack.

> > 
> > * Why should we care about systems that use terminals without
> >   corresponding terminfo installed?
> 
> That's the issue I thought, armdragon on IRC thought etc. The 'xterm'
> is part of ncurses-base on Debian, at least (thanks armdragon for this
> info) and is it pretty much as base to every terminal emulator there is 
> (I just launched gnome-terminal for testing, TERM was set to xterm --
> it may have been initially in environment but if gnome-terminal changed
> it before launching shell, my shell config will not change it)
> 

Sorry, this does not answer my question.

My point is that if a system can not run "emacs -nw" because it lacks
corresponding terminfo, it is not notmuch who should work around these
issues.  If you are using such system by whatever reason, it is your
responsibility to run notmuch tests (and all other programs who need a
valid terminfo) with modified TERM value.

Perhaps others disagree, but I think it is reasonable for notmuch test
suite to expect that "emacs -nw" works.

> > * Why is it safe to use xterm?  Is there any standard that requires
> >   xterm info to be present on the system?
> 
> xterm is not needed, it is just to tell emacs it can use it's display
> control sequences. more common alternative would have been vt100, just
> that if one ever wants to attach to that emacs session you he gets
> better experience (such as colors), using any terminal emulator, like rxvt.
> 

Sorry, I was misleading.  I did not mean that we are using xterm itself.
I am worried that we introduce dependency on xterm terminfo.  I
understand that it is part of ncurses-base package in Debian.  But
Debian is not the only system out there.  I think it is perfectly fine
for a system without X to lack xterm terminfo.  And by setting TERM to
"xterm" we break such systems.

Using "vt100" seems like a better alternative.  But the question
remains: is there any standard requirement that a particular terminfo
must be available on a system?  If there is no such requirement, then we
can not override TERM value without breaking the test suite on some
weird (but perfectly fine) systems.

> > I have tried running dtach with emacs in rxvt-unicode and it worked fine
> > without overriding TERM.
> 
> you had originally suitable TERM to be passed to emacs.
> 

Sure. Why do not you have a valid TERM value with a corresponding
terminfo on your system?

Sorry for being picky here.  I am fine with your patch except for this.
But TERM override feels wrong to me so far.  (Though, it is not me who
decides on the patches, perhaps others are fine with it and you can just
ignore me.)

Regards,
  Dmitry

> > 
> > I have also tried running dtach with invalid TERM:
> > 
> >   $ TERM=non-existing-term dtach -n s /bin/sh
> >   zsh: can't find terminal definition for non-existing-term
> > 
> > No segfault, shell is started fine.
> 
> Yes, dtach could not care less...
> 
> > 
> > But emacs does not work without a valid terminfo:
> > 
> >   $ TERM=non-existing-term emacs -nw
> >   emacs: Terminal type non-existing-term is not defined.
> >   If that is not the actual type of terminal you have,
> >   use the Bourne shell command `TERM=... export TERM' (C-shell:
> >   `setenv TERM ...') to specify the correct type.  It may be necessary
> >   to do `unset TERMINFO' (C-shell: `unsetenv TERMINFO') as well.
> 
> Yes, I got SIGSEGV --  strace -o oprfx -ff command args  is your friend :D
> 
> > Regards,
> >   Dmitry
> 
> Thanks for your analysis. 
> 
> Tomi
> 
> > 
> > > +			"$TMP_DIRECTORY/run_emacs" \
> > >  			--no-window-system \
> > >  			--eval "(setq server-name \"$EMACS_SERVER\")" \
> > >  			--eval '(server-start)' \
> > > -- 
> > > 1.7.7.1
> > > 
> > > _______________________________________________
> > > notmuch mailing list
> > > notmuch@notmuchmail.org
> > > http://notmuchmail.org/mailman/listinfo/notmuch
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> > 

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests.

[Jameson Graef Rollins]

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

On Fri, 11 Nov 2011 20:02:52 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> My point is that if a system can not run "emacs -nw" because it lacks
> corresponding terminfo, it is not notmuch who should work around these
> issues.  If you are using such system by whatever reason, it is your
> responsibility to run notmuch tests (and all other programs who need a
> valid terminfo) with modified TERM value.
> 
> Perhaps others disagree, but I think it is reasonable for notmuch test
> suite to expect that "emacs -nw" works.

Hi, Dmitry.  I think I disagree with this point.  I think that ideally
the test suite should be able to operate in a very minimal system, even
without a terminal.  Think of automated build daemons.  If the test
suite can provide some sort of terminal emulation for the things that
need it, that would potentially eliminate the need for the runner of the
test to provide it.  It may not be possible to completely abstract
everything away, but to the extent that we can we should.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

[PATCH] test: use dtach(1) instead of screen(1) in emacs tests

[Tomi Ollila]

dtach is simpler than screen and is not setuid/setgid program so
TMPDIR does not get cleared by dynamic loader when executed.
---

Updated version after discussion with DmitryKurochkin and amdragon
on IRC. Thank you.

 test/test-lib.sh |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index c81c709..c232130 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -39,7 +39,7 @@ done,*)
 	;;
 esac
 
-# Keep the original TERM for say_color
+# Keep the original TERM for say_color and emacs tests
 ORIGINAL_TERM=$TERM
 
 # For repeatability, reset the environment to known value.
@@ -843,12 +841,15 @@ EOF
 test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
-		# start a detached screen session with an emacs server
-		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
-			--no-window-system \
-			--eval "(setq server-name \"$EMACS_SERVER\")" \
-			--eval '(server-start)' \
-			--eval "(orphan-watchdog $$)" || return
+		# start a detached session with an emacs server
+		# user's TERM is given to dtach which assumes a minimally
+		# VT100-compatible terminal -- and emacs inherits that
+		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
+		 	sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
+				--no-window-system \
+				--eval '(setq server-name \"$EMACS_SERVER\")' \
+				--eval '(server-start)' \
+				--eval '(orphan-watchdog $$)'" || return
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
-- 
1.7.6.1

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests

[Dmitry Kurochkin]

On Fri, 11 Nov 2011 22:48:13 +0200, Tomi Ollila <too@iki.fi> wrote:
> dtach is simpler than screen and is not setuid/setgid program so
> TMPDIR does not get cleared by dynamic loader when executed.
> ---
> 
> Updated version after discussion with DmitryKurochkin and amdragon
> on IRC. Thank you.
> 

Looks good to me (though I did not test it).

Few minor comments below.

>  test/test-lib.sh |   25 +++++++++++++------------
>  1 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index c81c709..c232130 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -39,7 +39,7 @@ done,*)
>  	;;
>  esac
>  
> -# Keep the original TERM for say_color
> +# Keep the original TERM for say_color and emacs tests

s/emacs tests/test_emacs/ for consistency with say_color?

>  ORIGINAL_TERM=$TERM
>  
>  # For repeatability, reset the environment to known value.
> @@ -843,12 +841,15 @@ EOF
>  test_emacs () {
>  	if [ -z "$EMACS_SERVER" ]; then
>  		EMACS_SERVER="notmuch-test-suite-$$"
> -		# start a detached screen session with an emacs server
> -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> -			--no-window-system \
> -			--eval "(setq server-name \"$EMACS_SERVER\")" \
> -			--eval '(server-start)' \
> -			--eval "(orphan-watchdog $$)" || return
> +		# start a detached session with an emacs server
> +		# user's TERM is given to dtach which assumes a minimally
> +		# VT100-compatible terminal -- and emacs inherits that
> +		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
> +		 	sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \

The above line has a space after two tabs.

Regards,
  Dmitry

> +				--no-window-system \
> +				--eval '(setq server-name \"$EMACS_SERVER\")' \
> +				--eval '(server-start)' \
> +				--eval '(orphan-watchdog $$)'" || return
>  		# wait until the emacs server is up
>  		until test_emacs '()' 2>/dev/null; do
>  			sleep 1
> -- 
> 1.7.6.1
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests

[Tomi Ollila]

On Sat, 12 Nov 2011 01:06:40 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Fri, 11 Nov 2011 22:48:13 +0200, Tomi Ollila <too@iki.fi> wrote:
> > dtach is simpler than screen and is not setuid/setgid program so
> > TMPDIR does not get cleared by dynamic loader when executed.
> > ---
> > 
> > Updated version after discussion with DmitryKurochkin and amdragon
> > on IRC. Thank you.
> > 
> 
> Looks good to me (though I did not test it).
> 
> Few minor comments below.
> 
> >  test/test-lib.sh |   25 +++++++++++++------------
> >  1 files changed, 13 insertions(+), 12 deletions(-)
> > 
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index c81c709..c232130 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -39,7 +39,7 @@ done,*)
> >  	;;
> >  esac
> >  
> > -# Keep the original TERM for say_color
> > +# Keep the original TERM for say_color and emacs tests
> 
> s/emacs tests/test_emacs/ for consistency with say_color?

yes. thanks...

> 
> >  ORIGINAL_TERM=$TERM
> >  
> >  # For repeatability, reset the environment to known value.
> > @@ -843,12 +841,15 @@ EOF
> >  test_emacs () {
> >  	if [ -z "$EMACS_SERVER" ]; then
> >  		EMACS_SERVER="notmuch-test-suite-$$"
> > -		# start a detached screen session with an emacs server
> > -		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> > -			--no-window-system \
> > -			--eval "(setq server-name \"$EMACS_SERVER\")" \
> > -			--eval '(server-start)' \
> > -			--eval "(orphan-watchdog $$)" || return
> > +		# start a detached session with an emacs server
> > +		# user's TERM is given to dtach which assumes a minimally
> > +		# VT100-compatible terminal -- and emacs inherits that
> > +		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
> > +		 	sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
> 
> The above line has a space after two tabs.

oops...

I also forgot 

-export SCREENRC=/dev/null
-export SYSSCREENRC=/dev/null

(dropped accidetally when cherry-picking non-whitespace changes)
Thanks for playing. Try once more...

> 
> Regards,
>   Dmitry

Tomi

> 
> > +				--no-window-system \
> > +				--eval '(setq server-name \"$EMACS_SERVER\")' \
> > +				--eval '(server-start)' \
> > +				--eval '(orphan-watchdog $$)'" || return
> >  		# wait until the emacs server is up
> >  		until test_emacs '()' 2>/dev/null; do
> >  			sleep 1
> > -- 
> > 1.7.6.1
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> 

[PATCH] test: use dtach(1) instead of screen(1) in emacs tests

[Tomi Ollila]

dtach is simpler than screen and is not setuid/setgid program so
TMPDIR does not get cleared by dynamic loader when executed
---

The 2 Fixes based Dmitry's comments + Jameson's one too.

 test/test-lib.sh |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index c81c709..d88548b 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -39,7 +39,7 @@ done,*)
 	;;
 esac
 
-# Keep the original TERM for say_color
+# Keep the original TERM for say_color and test_emacs
 ORIGINAL_TERM=$TERM
 
 # For repeatability, reset the environment to known value.
@@ -50,8 +50,6 @@ TZ=UTC
 TERM=dumb
 export LANG LC_ALL PAGER TERM TZ
 GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
-export SCREENRC=/dev/null
-export SYSSCREENRC=/dev/null
 
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
@@ -843,12 +841,15 @@ EOF
 test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
-		# start a detached screen session with an emacs server
-		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
-			--no-window-system \
-			--eval "(setq server-name \"$EMACS_SERVER\")" \
-			--eval '(server-start)' \
-			--eval "(orphan-watchdog $$)" || return
+		# start a detached session with an emacs server
+		# user's TERM is given to dtach which assumes a minimally
+		# VT100-compatible terminal -- and emacs inherits that
+		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
+			sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
+				--no-window-system \
+				--eval '(setq server-name \"$EMACS_SERVER\")' \
+				--eval '(server-start)' \
+				--eval '(orphan-watchdog $$)'" || return
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
-- 
1.7.7

Re: [PATCH] test: use dtach(1) instead of screen(1) in emacs tests

[David Bremner]

On Fri, 11 Nov 2011 23:33:58 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> dtach is simpler than screen and is not setuid/setgid program so
> TMPDIR does not get cleared by dynamic loader when executed

Pushed.

[PATCH] test: use dtach(1) instead of screen(1) in emacs tests

[Tomi Ollila]

dtach is simpler than screen and is not setuid/setgid program so
TMPDIR does not get cleared by dynamic loader when executed.
---

Updated version after discussion with DmitryKurochkin and amdragon
on IRC. Thank you.

 test/test-lib.sh |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index c81c709..c232130 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -39,7 +39,7 @@ done,*)
 	;;
 esac
 
-# Keep the original TERM for say_color
+# Keep the original TERM for say_color and emacs tests
 ORIGINAL_TERM=$TERM
 
 # For repeatability, reset the environment to known value.
@@ -843,12 +841,15 @@ EOF
 test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
-		# start a detached screen session with an emacs server
-		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
-			--no-window-system \
-			--eval "(setq server-name \"$EMACS_SERVER\")" \
-			--eval '(server-start)' \
-			--eval "(orphan-watchdog $$)" || return
+		# start a detached session with an emacs server
+		# user's TERM is given to dtach which assumes a minimally
+		# VT100-compatible terminal -- and emacs inherits that
+		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
+		 	sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
+				--no-window-system \
+				--eval '(setq server-name \"$EMACS_SERVER\")' \
+				--eval '(server-start)' \
+				--eval '(orphan-watchdog $$)'" || return
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
-- 
1.7.6.1