Re: testsuite: fixed spurious failure in vala4.test (w.r.t. Solaris sh)

[Stefano Lattarini]

At Sunday 11 October 2009, Ralf Wildenhues <address@hidden> 
wrote:
> * Stefano Lattarini wrote on Sun, Oct 11, 2009 at 05:49:20PM CEST:
> > While trying to run the automake test scripts with the "Heirloom
> > sh" (http://heirloom.sourceforge.net/sh.html, which should be
> > quite similar to the Solaris/OpenSolaris shell), I ran into a
> > spurious failure in test `vala4.test'.  I still don't understand
> > the real cause of the failure (whose log is allegated), but I
> > managed to fix it with a quick and dirty patch, which is
> > attached.
> 
> Thanks for looking into this.  I prefer not to paper over any bugs
>  that we don't understand, though.
> 
> If you are willing to look into this more, it's got to do with 'set
>  -e'.
> 
I think I found the bug in heirloom-sh which was triggering the 
spurious failure (it's related with `set -e', as you guessed).  I also 
found a simple workaround for it (caching the result of `pwd` command 
substitution in a shell variable $cwd), which by the way also provided 
a tiny optimization (two fewer forks).

I also added a comment in the test script, containing a quite extended 
description of the bug (so that a future developer will know that the 
$cwd "optimization" isn't just an optimization).

A patch is attached.

Regards,
     Stefano

From 780ca4e4346c57916bd497f12aa7e58a33e7c5e8 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Fri, 4 Dec 2009 15:47:23 +0100
Subject: [PATCH] Intertwined optimization and bugfix in vala4.test

* tests/vala4.test: New variable $cwd, holding the full path of the
current directory.  Use it instead of `pwd` command substitution.
Document how a bug of Heirloom/Solaris Sh caused a spurious failure
of this test script, and how our use of "$cwd" rather than "`pwd`"
offers a workaround.
---
 ChangeLog        |    9 +++++++++
 tests/vala4.test |   36 +++++++++++++++++++++++++++++++++---
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2fe0566..3199d03 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-12-04  Stefano Lattarini  <address@hidden>
+
+       Intertwined optimization and bugfix in testcase vala4.test.
+       * tests/vala4.test: New variable $cwd, holding the full path of the
+       current directory.  Use it instead of `pwd` command substitution.
+       Document how a bug of Heirloom/Solaris Sh caused a spurious failure
+       of this test script, and how our use of "$cwd" rather than "`pwd`"
+       offers a workaround.
+
 2009-10-18  Ralf Wildenhues  <address@hidden>
 
        Simplify Variable::_check_ambiguous_condition.
diff --git a/tests/vala4.test b/tests/vala4.test
index 4301d45..d8e58eb 100755
--- a/tests/vala4.test
+++ b/tests/vala4.test
@@ -43,19 +43,49 @@ exit 0
 END
 chmod +x valac
 
+cwd=`pwd`
+
+#
+# Note that the previous assignment doesn't provide just an optimization,
+# but helps us to prevent a bug in the way Solaris/Heirloom Sh handles
+# `set -e'.
+#
+# The bug can be reproduced as follows:
+#
+#  $ cat >foo.sh <<__EOF__
+#  set -e
+#  false && exit 0
+#  echo ALIVE
+#  false `true` && exit 0
+#  echo STILL ALIVE
+#  __EOF__
+#  $ sh foo.sh; echo $? # sh can be e.g. bash, ksh, zsh
+#  ALIVE
+#  STILL ALIVE
+#  0
+#  $ heirloom-sh foo.sh; echo $?
+#  ALIVE
+#  1
+#
+# That bug had already bitten us in practice, when (below) we used:
+#   ./configure "VALAC=`pwd`/valac" && Exit 1
+# instead of the current:
+#   ./configure "VALAC=$cwd/valac" && Exit 1
+#
+
 libtoolize
 $ACLOCAL
 $AUTOMAKE -a
 $AUTOCONF
-./configure "VALAC=`pwd`/valac"
+./configure "VALAC=$cwd/valac"
 
 sed 's/AM_PROG_VALAC.*/AM_PROG_VALAC([9999.9])/' < configure.in >t
 mv -f t configure.in
 $AUTOCONF --force
-./configure "VALAC=`pwd`/valac" && Exit 1
+./configure "VALAC=$cwd/valac" && Exit 1
 
 sed 's/AM_PROG_VALAC.*/AM_PROG_VALAC([1.2.3])/' < configure.in >t
 mv -f t configure.in
 $AUTOCONF --force
-./configure "VALAC=`pwd`/valac"
+./configure "VALAC=$cwd/valac"
 :
-- 
1.6.5