Re: [PATCH 1/2] ar-lib: new 'AM_PROG_AR' macro, triggering the 'ar-lib' script

[Stefano Lattarini]

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.

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

> 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

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

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

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

>      makedepend)
>        echo "$me: running makedepend -f-"
>        ( makedepend -f- ) || exit 77

Thanks,
  Stefano