On Saturday 21 August 2010, Ralf Wildenhues wrote:
> Hi Stefano,
>
> * Stefano Lattarini wrote on Thu, Aug 19, 2010 at 02:57:04PM CEST:
> > * tests/missing.test: Build and use our own `autoconf' script, to
> > avoid spurious failures due to configure-time value of $AUTOCONF
> > being an absolute path. Prefer passing overrides to ./configure
> > on the command line rather than in the environment. Some other
> > related and unrelated improvements.
> > * tests/missing2.test: Likewise, and keep more in sync with
> > `missing.test' by adding checks with version number suffixes.
> >
> > --- a/tests/missing.test
> > +++ b/tests/missing.test
> >
> > @@ -16,6 +16,7 @@
> >
> > # along with this program. If not, see
> > <http://www.gnu.org/licenses/>.
> >
> > # Test missing with version mismatches.
> >
> > +# Keep this in sync with sister test `missing.test'.
>
> Referring to self.
Oops, should be `missing2.test'. Fixed.
> > @@ -31,36 +32,48 @@ $ACLOCAL
> >
> > $AUTOCONF
> > $AUTOMAKE --add-missing
> >
> > +# Avoid problems with configure-time $AUTOCONF that is absolute
> > values.
>
> "Avoid problems when $AUTOCONF has an absolute value at configure
> time."
Better (my formulation also had the typo `values' for `value')
> > +# Such problems already occurred in practice.
>
> I'm wary of comments that go out of date. Is this such a comment?
> Do we have a bug report to this end? Are you the reporter?
Well, I experinced the problem, but didn't report it (I fixed it
instead, with this patch).
> What is the actual problem by the way?
Basically, `missing --run /abs/path/to/autoconf' does not recognize
the program being run as an "autoconf":
$ sed -n '147,166p' lib/missing
# If it does not exist, or fails to run (possibly an outdated version),
# try to emulate it.
case $program in
aclocal*)
echo 1>&2 "\
WARNING: \`$1' is $msg. You should only need it if
you modified \`acinclude.m4' or \`${configure_ac}'. You might want
to install the \`Automake' and \`Perl' packages. Grab them from
any GNU archive site."
touch aclocal.m4
;;
autoconf*)
echo 1>&2 "\
WARNING: \`$1' is $msg. You should only need it if
you modified \`${configure_ac}'. You might want to install the
\`Autoconf' and \`GNU m4' packages. Grab them from any GNU
archive site."
touch configure
;;
> This should be answered in the comment.
Right. I've went for this formulation:
# Avoid problems when $AUTOCONF has an absolute value at configure time.
# Such problems already occurred in practice.
# They are due the way the `missing' script matches the program it has
# to wrap:
# case $program in
# aclocal*) ...;;
# autconf*) ...;;
# ...
# esac
# This does not account for absolute paths. Which makes perfect sense,
# given the way missing is used in practie, but would break a hack like:
# ./configure AUTOCONF="./missing --run /bas/path/to/autoconf"
> > +mkdir xbin
>
> Why not s/xbin/bin/g throughout? That seems to be more
> conventional.
OK (`xbin' was for "eXtra BINaries" or `eXtended BINaries",
but a simple `bin' is just fine).
> > +cat > xbin/autoconf <<END
> > +#!/bin/sh
> > +exec $AUTOCONF \${1+"\$@"}
> > +END
> > +chmod a+x xbin/autoconf
> > +cat xbin/autoconf # for debugging
> > +cp xbin/autoconf xbin/autoconf6789
> > +
> > +PATH=`pwd`/xbin:$PATH; export PATH
>
> Oh, we keep forgetting PATH_SEPARATOR in our tests. Could (and
> should) be addressed in an independent followup patch and probably
> checked for by maintainer-check.
Yes, I'm planning to to that, sooner or later. I forgot to add a
proper "FIXME" comment here, though. Now added.
> Do you think it is such a good idea to have a script named
> 'autoconf' early in $PATH that itself calls $AUTOCONF, when
> $AUTOCONF could expand to 'autoconf'?
No, now that I think about it, it's horrible. I should really
save the older $PATH, and restore it in the fake autoconf before
calling $AUTOCONF. Done in the amended patch.
> Looks like a potential fork bomb to me;
Luckily, since I use `exec' in the fake autoconf, it would be just
and endless loop, hanging the testsuite. But is bad nonetheless.
> I mean, in the case there is a bug and something calls
> 'autoconf' when it should have called $MYAUTOCONF below ...
> At this point, I'm stopping review of this patch series for the
> moment. Not because I think it is bad, just to await feedback from
> you for the remark I gave for 4/6 (dunno if it will let you make
> bigger changes), and guessing that you might have more than just
> above place to fix for potential fork bombs. I'll continue (or
> review an updated series) after feedback.
Good idea. I will post amended patches where it seems necessary.
> Cheers, and thanks for working on this!
> Ralf
Thanks for the review,
Stefano
From 72667faed237987607030d688cfead7b8b7d7a6d Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <stefano.lattar...@gmail.com>
Date: Wed, 18 Aug 2010 19:38:53 +0200
Subject: [PATCH] Testsuite: fix missing*.test for non-default autotools.
* tests/missing.test: Build and use our own `autoconf' script, to
avoid spurious failures due to configure-time value of $AUTOCONF
being an absolute path. Prefer passing overrides to ./configure
on the command line rather than in the environment. Some other
related and unrelated improvements.
* tests/missing2.test: Likewise, and keep more in sync with
`missing.test' by adding checks with version number suffixes.
---
ChangeLog | 9 ++++++++
tests/missing.test | 58 ++++++++++++++++++++++++++++++++++++--------------
tests/missing2.test | 55 ++++++++++++++++++++++++++++++++++++++++--------
3 files changed, 97 insertions(+), 25 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 92f2063..7f817e0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
2010-08-19 Stefano Lattarini <stefano.lattar...@gmail.com>
+ Testsuite: fix missing*.test for non-default autotools.
+ * tests/missing.test: Build and use our own `autoconf' script, to
+ avoid spurious failures due to configure-time value of $AUTOCONF
+ being an absolute path. Prefer passing overrides to ./configure
+ on the command line rather than in the environment. Some other
+ related and unrelated improvements.
+ * tests/missing2.test: Likewise, and keep more in sync with
+ `missing.test' by adding checks with version number suffixes.
+
Test that autoconf(s) used by the testsuite and by aclocal match.
* tests/remake0.test: New test.
* tests/Makefile.am (TESTS): Updated.
diff --git a/tests/missing.test b/tests/missing.test
index eaf6f54..861d7bf 100755
--- a/tests/missing.test
+++ b/tests/missing.test
@@ -1,6 +1,6 @@
#! /bin/sh
-# Copyright (C) 2003, 2004, 2006, 2007, 2008 Free Software Foundation,
-# Inc.
+# Copyright (C) 2003, 2004, 2006, 2007, 2008, 2010 Free Software
+# Foundation, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -16,6 +16,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Test missing with version mismatches.
+# Keep this in sync with sister test `missing2.test'.
. ./defs || Exit 1
@@ -31,36 +32,61 @@ $ACLOCAL
$AUTOCONF
$AUTOMAKE --add-missing
+# Avoid problems when $AUTOCONF has an absolute value at configure time.
+# Such problems already occurred in practice.
+# They are due the way the `missing' script matches the program it has
+# to wrap:
+# case $program in
+# aclocal*) ...;;
+# autconf*) ...;;
+# ...
+# esac
+# This does not account for absolute paths. Which makes perfect sense,
+# given the way missing is used in practice, but would break a hack like:
+# ./configure AUTOCONF="./missing --run /bas/path/to/autoconf"
+mkdir bin
+cat > bin/autoconf <<END
+#!/bin/sh
+# Restoring PATH avoids "fork bombs" in case \$AUTOCONF is "autoconf".
+PATH='$PATH'; export PATH;
+exec $AUTOCONF \${1+"\$@"}
+END
+chmod a+x bin/autoconf
+cat bin/autoconf # for debugging
+cp bin/autoconf bin/autoconf6789
+
+# FIXME: this is not portable! Use @path_separa...@.
+PATH=`pwd`/bin:$PATH; export PATH
+
# Make sure we do use missing, even if the user exported AUTOCONF.
# (We cannot export this new value, because it would be used by Automake
# when tracing, and missing is no good for this.)
-MYAUTOCONF="./missing --run $AUTOCONF"
unset AUTOCONF
+MYAUTOCONF="./missing --run autoconf"
./configure AUTOCONF="$MYAUTOCONF"
+
$MAKE
$sleep
# Hopefully the install version of Autoconf cannot compete with this one...
-echo 'AC_PREREQ(9999)' >> aclocal.m4
+echo 'AC_PREREQ([9999])' >> aclocal.m4
$MAKE distdir
-# Try version number suffixes if we can add them safely.
-case $MYAUTOCONF in *autoconf)
- ./configure AUTOCONF="${MYAUTOCONF}6789"
- $MAKE
- $sleep
- # Hopefully the install version of Autoconf cannot compete with this one...
- echo 'AC_PREREQ(9999)' >> aclocal.m4
- $MAKE distdir
-esac
+# Try version number suffixes.
+./configure AUTOCONF="${MYAUTOCONF}6789"
+$MAKE
+$sleep
+touch aclocal.m4
+$MAKE distdir
-# Run again, but without missing, to ensure that timestamps were updated.
-export AUTOMAKE ACLOCAL
-./configure AUTOCONF="$MYAUTOCONF"
+# Run again, but without missing to wrap aclocal/automake, to ensure that
+# timestamps were updated.
+./configure AUTOCONF="$MYAUTOCONF" AUTOMAKE="$AUTOMAKE" ACLOCAL="$ACLOCAL"
$MAKE
# Make sure $MAKE fails when timestamps aren't updated and missing is not used.
$sleep
touch aclocal.m4
$MAKE && Exit 1
+
:
diff --git a/tests/missing2.test b/tests/missing2.test
index 2629198..9ffd1f2 100755
--- a/tests/missing2.test
+++ b/tests/missing2.test
@@ -1,5 +1,6 @@
#! /bin/sh
-# Copyright (C) 2003, 2004, 2006, 2007 Free Software Foundation, Inc.
+# Copyright (C) 2003, 2004, 2006, 2007, 2010 Free Software Foundation,
+# Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -15,6 +16,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Test missing with version mismatches.
+# Keep this in sync with sister test `missing.test'.
. ./defs || Exit 1
@@ -26,32 +28,67 @@ AC_OUTPUT
EOF
: > v.m4
-
: > Makefile.am
$ACLOCAL
$AUTOCONF
$AUTOMAKE --add-missing
-# See missing.test for explanations about this.
-MYAUTOCONF="./missing --run $AUTOCONF"
+# Avoid problems when $AUTOCONF has an absolute value at configure time.
+# Such problems already occurred in practice.
+# They are due the way the `missing' script matches the program it has
+# to wrap:
+# case $program in
+# aclocal*) ...;;
+# autconf*) ...;;
+# ...
+# esac
+# This does not account for absolute paths. Which makes perfect sense,
+# given the way missing is used in practice, but would break a hack like:
+# ./configure AUTOCONF="./missing --run /bas/path/to/autoconf"
+mkdir bin
+cat > bin/autoconf <<END
+#!/bin/sh
+# Restoring PATH avoids "fork bombs" in case \$AUTOCONF is "autoconf".
+PATH='$PATH'; export PATH;
+exec $AUTOCONF \${1+"\$@"}
+END
+chmod a+x bin/autoconf
+cat bin/autoconf # for debugging
+cp bin/autoconf bin/autoconf6789
+
+# FIXME: this is not portable! Use @path_separa...@.
+PATH=`pwd`/bin:$PATH; export PATH
+
+# Make sure we do use missing, even if the user exported AUTOCONF.
+# (We cannot export this new value, because it would be used by Automake
+# when tracing, and missing is no good for this.)
unset AUTOCONF
+MYAUTOCONF="./missing --run autoconf"
./configure AUTOCONF="$MYAUTOCONF"
$MAKE
$sleep
# Hopefully the install version of Autoconf cannot compete with this one...
-echo 'AC_PREREQ(9999)' > v.m4
+echo 'AC_PREREQ([9999])' > v.m4
$MAKE distdir
-# Run again, but without missing, to ensure that timestamps were updated.
-export AUTOMAKE ACLOCAL
-./configure AUTOCONF="$MYAUTOCONF"
+# Try version number suffixes.
+./configure AUTOCONF="${MYAUTOCONF}6789"
+$MAKE
+$sleep
+touch v.m4
+$MAKE distdir
+
+# Run again, but without missing to wrap aclocal/automake, to ensure that
+# timestamps were updated.
+./configure AUTOCONF="$MYAUTOCONF" AUTOMAKE="$AUTOMAKE" ACLOCAL="$ACLOCAL"
$MAKE
-# Make sure $MAKE fail when timestamps aren't updated and missing is not used.
+# Make sure $MAKE fails when timestamps aren't updated and missing is not used.
$sleep
touch v.m4
$MAKE && Exit 1
+
:
--
1.7.1
