Stefano Lattarini skrev 2011-10-20 16:48:
> Hi Peter.
>
> I will only comment the parts that still have unresolved issues ...
>
>> diff --git a/tests/ar-lib3.test b/tests/ar-lib3.test
>> new file mode 100755
>> index 0000000..be0d6df
>> --- /dev/null
>> +++ b/tests/ar-lib3.test
>> @@ -0,0 +1,45 @@
>> +#! /bin/sh
>> +# Copyright (C) 2011 Free Software Foundation, Inc.
>> + #
>> + ... [SNIP]
>> +
>> +$ACLOCAL
>> +AUTOMAKE_fails
>> +
>> +grep '/library.am:.*requires.*AM_PROG_AR' stderr
>>
> Please remove `/library.am' from here. Sorry for the confusion.
No problem.
>> diff --git a/tests/ar-lib4.test b/tests/ar-lib4.test
>> new file mode 100755
>> index 0000000..0232d45
>> --- /dev/null
>> +++ b/tests/ar-lib4.test
>> @@ -0,0 +1,57 @@
>> +#! /bin/sh
>> +# Copyright (C) 2011 Free Software Foundation, Inc.
>> +#
>> + ... [SNIP]
>> +
>> +grep '/library.am:.*requires.*AM_PROG_AR' stderr
>>
> Ditto.
>
>> diff --git a/tests/ar-lib6.test b/tests/ar-lib6.test
>> new file mode 100755
>> index 0000000..af3cb2e
>> --- /dev/null
>> +++ b/tests/ar-lib6.test
>> @@ -0,0 +1,38 @@
>> +#! /bin/sh
>> +# Copyright (C) 2011 Free Software Foundation, Inc.
>> +#
>> +# Test AM_PROG_AR ordering requirements
>> +
>> +required=libtoolize
>> +. ./defs || Exit 1
>> +
>> +set -e
>> +
>> +cat >> configure.in << 'END'
>> +AC_PROG_CC
>> +AC_PROG_RANLIB
>> +m4_ifdef([LT_INIT], [LT_INIT], [AC_PROG_LIBTOOL])
>> +AM_PROG_AR
>> +END
>> +
>> +libtoolize
>> +$ACLOCAL
>> +$AUTOCONF 2>stderr || { cat stderr >&2; Exit 1; }
>> +cat stderr >&2
>> +
>> +$EGREP '(AC_PROG_LIBTOOL|LT_INIT).*before.*AM_PROG_AR' stderr
>> +
> I think it would be better to do two separate checks here, one
> for AC_PROG_LIBTOOL and one for LT_INIT. This can be done in
> a follow-up patch, though, so no need to re-roll this test again.
The problem is that older Libtools do not have LT_INIT, so the
test which checks LT_INIT ordering would have to be skipped in
that case. That seemed like a lot of trouble compared to an
m4_ifdef.
I could keep the existing test and then do another test which
uses AC_PROG_LIBTOOL unconditionally. I assume you want to
guarantee coverage for AC_PROG_LIBTOOL in a world were LT_INIT
is the norm?
>> diff --git a/tests/ar4.test b/tests/ar4.test
>> new file mode 100755
>> index 0000000..018722d
>> --- /dev/null
>> +++ b/tests/ar4.test
>> @@ -0,0 +1,35 @@
>> +#! /bin/sh
>> +# Copyright (C) 2011 Free Software Foundation, Inc.
>> +#
>> +# Test if configure bails out if $AR does not work and AM_PROG_AR is used.
>> +
>> +. ./defs || Exit 1
>> +
>> +set -e
>> +
>> +cat >> configure.in << 'END'
>> +AM_PROG_AR
>> +END
>> +
>> +$ACLOCAL
>> +$AUTOCONF
>> +
>> +./configure AR=/bin/false 2>stderr && Exit 1
>> +cat stderr >&2
>>
> You should always display the captured stderr before exiting:
>
> ./configure AR=/bin/false 2>stderr && { cat stderr >&2; Exit 1; }
> cat stderr >&2
Consider it done.
>> +
>> +grep 'configure: error: could not determine /bin/false interface' stderr
>> +
> Nice test for the rest.
>
>> diff --git a/tests/ar5.test b/tests/ar5.test
>> new file mode 100755
>> index 0000000..60d1015
>> --- /dev/null
>> +++ b/tests/ar5.test
>> @@ -0,0 +1,38 @@
>> +#! /bin/sh
>> +# Copyright (C) 2011 Free Software Foundation, Inc.
>> +
>> +# Test the optional argument of AM_PROG_AR.
>> +
>> +. ./defs || Exit 1
>> +
>> +set -e
>> +
>> +cat >> configure.in << 'END'
>> +spy_archiver=
>> +AM_PROG_AR([spy_archiver=nope])
>> +test "$spy_archiver" = nope && echo "archiver trigger" >&2
>> +:
>> +END
>> +
>> +$ACLOCAL
>> +$AUTOCONF
>> +
>> +./configure AR=/bin/false 2>stderr
>> +cat stderr
>> +
> (Again, you should always display the captured stderr before exiting.
> But see below).
dito.
>> +grep 'archiver trigger' stderr
>> +
> Hmmm... what about simplifying the test as follows?
>
> cat >> configure.in << 'END'
> AM_PROG_AR([echo ok > bad-archiver-interface-detected])
> END
> $ACLOCAL
> $AUTOCONF
> ./configure AR=/bin/false
> test -f bad-archiver-interface-detected
Yes, nice, consider it done.
>> diff --git a/tests/defs.in b/tests/defs.in
>> index 2959f8b..5046a40 100644
>> --- a/tests/defs.in
>> +++ b/tests/defs.in
>> @@ -278,6 +278,14 @@ do
>> echo "$me: running javac -version -help"
>> javac -version -help || exit 77
>> ;;
>> + lib)
>> + AR=lib
>> + export AR
>> + # Attempting to create an empty archive will actually not
>> + # create the archive, but lib will output its version.
>> + echo "$me: running $AR -out:defstest.lib"
>> + ( $AR -out:defstest.lib ) || skip_ "Microsoft \`lib' utility not
>> available"
>> + ;;
> Micro-nit: the subshell here shouldn't be needed.
I'll remove the brackets.
>> makedepend)
>> echo "$me: running makedepend -f-"
>> ( makedepend -f- ) || exit 77
>
> Thanks,
> Stefano
Cheers,
Peter
