Re: [PATCH 2/6] Testsuite: fix missing*.test for non-default autotools.

[Ralf Wildenhues]

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.

> @@ -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."

> +# 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?

What is the actual problem by the way?  This should be answered in the
comment.

> +mkdir xbin

Why not  s/xbin/bin/g  throughout?  That seems to be more conventional.

> +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.

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'?  Looks like a potential fork bomb to me; 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.

Cheers, and thanks for working on this!
Ralf