On 05/07/2012 05:37 PM, Eric Blake wrote:
> On 05/01/2012 10:04 AM, Stefano Lattarini wrote:
>> * configure.ac: Add code (partially inspired to checks in gnulib's
>> 'tests/init.sh') to search for a good-enough, not-buggy POSIX/XSI
>> shell to be used in our testsuite. Accordingly AC_SUBSTitute the
>> variable 'AM_TEST_RUNNER_SHELL'.
>> * NEWS: Update.
>>
>> Signed-off-by: Stefano Lattarini <stefano.lattar...@gmail.com>
>> ---
>> NEWS | 11 ++++
>> configure.ac | 190
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++------
>> 2 files changed, 184 insertions(+), 17 deletions(-)
>>
>
>>
>> -# Shell used to run our test scripts. The same as $SHELL/$CONFIG_SHELL
>> -# for the moment.
>> -AC_SUBST([AM_TEST_RUNNER_SHELL], [$SHELL])
>> +dnl FIXME: could we extract this in a simpler way through autoconf
>> +dnl FIXME: idioms or internals?
>> +AC_DEFUN(
>> + [_AM_INIT_BOURNE_COMPATIBLE_VAR],
>> + [am_bourne_compatible="AS_ESCAPE(_m4_expand([AS_BOURNE_COMPATIBLE]))"])
>
> As is, _m4_expand is already an autoconf internal.
>
Yes, but the way we are using it is not simple enough IMHO. When we abuse
Autoconf's internal (like here), we should ideally have a bigger gain ;-)
> Thankfully, since you recently moved to requiring 2.65,
>
Oh, but the above is only meant to be used by the Automake's own build system
(not by any Automake-provided public m4 macros), so that we can safely assume
the presence of Autoconf 2.69.
> it looks like this macro is safe enough for you to use.
>
And even if it's not, this is a problem that would bite only the Automake
developers (which regenerate configure), not the Automake users.
>> +
>> +dnl
>> +dnl Arguments to this macro:
>> +dnl
>> +dnl $1 - shell to test
>> +dnl $2 - description of the tested feature
>> +dnl $3 - shell code used to check the feature; to indicate success,
>> +dnl it can either exit with status 77, or have the last command
>> +dnl returning with exit status of zero
>> +dnl $4 - shell code to execute if the check on the shell is successful
>> +dnl (defaults to nothing)
>> +dnl $5 - shell code to execute if the check on the shell is not
>> +dnl successful (defaults to nothing)
>> +dnl
>> +AC_DEFUN([_AM_CHECK_SHELL_FEATURE],
>> + [AC_REQUIRE([_AM_INIT_BOURNE_COMPATIBLE_VAR])
>> + AC_MSG_CHECKING([whether $1 $2])
>> + if { $1 -c "$am_bourne_compatible
>> +AS_ESCAPE([$3])
>> +test \$? -eq 0 || exit 1
>> +# Use 77 to indicate success (rather than 0), in case some shell
>> +# acts like Solaris 10's /bin/sh, exiting successfully on some
>> +# syntax errors.
>> +exit 77" >&AS_MESSAGE_LOG_FD 2>&1; test $? -eq 77; }
>> + then
>> + AC_MSG_RESULT([yes])
>> + $4
>> + else
>> + AC_MSG_RESULT([no])
>> + $5
>> + fi])
>
> Looks like as good a probe as we can do without something publicly
> documented in autoconf for the same.
>
Well, I won't make mistery of the fact that I hope the probes I implemented
in this series will eventually find their way into Autoconf proper, freeing
the Automake developers from the need to maintain and test them ...
>
>> +#
>> +# Finally, we look for weird bugs and portability problems mentioned in
>> +# the Autoconf manual, and reject shells that suffers from them. (TODO)
>
> Any reason to leave the TODO comment here?
>
The fact that we haven't yet written any actual code to reject those shells? ;-)
>> + _AM_CHECK_SHELL_FEATURE([$1],
>> + [supports \${var@%:@glob}],
>> + [v=a/b/c; test ${v@%:@*/} = b/c],
>> + [], [am_score=1; break])
>> +
>> + _AM_CHECK_SHELL_FEATURE([$1],
>> + [supports \${var@%:@@%:@glob}],
>> + [v=a/b/c; test ${v@%:@@%:@*/} = c],
>> + [], [am_score=1; break])
>> +
>> + _AM_CHECK_SHELL_FEATURE([$1],
>> + [supports \${var%glob}],
>> + [v=a.b.c; test ${v%.*} = a.b],
>> + [], [am_score=1; break])
>> +
>> + _AM_CHECK_SHELL_FEATURE([$1],
>> + [supports \${var%%glob}],
>> + [v=a.b.c; test ${v%%.*} = a],
>> + [], [am_score=1; break])
>
> Probably overkill to test all four; I don't know of any shell that
> supports one of the XSI expansions, but not all four.
>
You are very likely correct, but I'd rather err on the side of safety for
now.
>
>> +AC_CACHE_VAL(
>> + [ac_cv_AM_TEST_RUNNER_SHELL],
>> + [if test "$AM_TEST_RUNNER_SHELL"; then
>> + # Let the user override it.
>> + ac_cv_AM_TEST_RUNNER_SHELL=$AM_TEST_RUNNER_SHELL
>> + else
>> + ac_cv_AM_TEST_RUNNER_SHELL=no
>> + am_candidate_shells=${CONFIG_SHELL-}
>> + # For the benefit of Solaris.
>> + am_PATH=$PATH$PATH_SEPARATOR/usr/xpg4/bin$PATH_SEPARATOR/usr/xpg6/bin
>
> This favors the user's PATH first, which seems reasonable (first
> candidate on the user's path that does what we want). But in the
> fallback mode, wouldn't it be better to list xpg6 before xpg4, for the
> additional fixes in xpg6?
>
Good point, consider this fixed (although this is just a theoretical issue,
since I don't know of any system that actually has a shell in /usr/xpg6/bin).
> Overall, looks reasonable for finding a decent shell.
>
Thanks. I've pushed the patch now.
Regards,
Stefano
