On Tuesday 14 September 2010, Peter Rosin wrote: > Hi! Hello Peter. Hope you don't mind a quick partial review until we hear from Ralf...
> This is a second attempt to implement AM_PROG_AR. The previous > attempt was bundled with the addition of the 'ar-lib' script but > was left behind. I have now fleshed it out with tests and > portability warnings etc. > > I think I have everything in the thread covered: > http://lists.gnu.org/archive/html/automake-patches/2010-08/msg00116.html > This is going with case (1) in that message. > > However, I have only updated tests/ar.test to cope with the new > reality. So, a lot of tests (100?) are likely to fall over due to > the new portability warnings. The reason is that I don't know on > what branch I should base such an intrusive patch. You mean a patch adding AM_PROG_AR into all the tests which now requires it, right? I'd base that on msvc branch (in fact, I'd make it a squash-in for your attached patch). > I fear that such a patch, when merged elsewhere, will be incomplete No big deal IMHO, we could resort to a fake merge in such case (`git commit --amend' can also edit merge commits AFAIK), fixing all the tests that need to be fixed. And there will be few of them anyway IMO. > or otherwise difficult to merge due to other changes. I'm also not > sure if this is the desired route. Please advise. Done :-). But let's hear what Ralf has to say, he might know better. > This patch is on top of the msvc branch. > > AC_PROG_RANLIB is rendered obsolete by LT_INIT. Is it also rendered > obsolete by AC_PROG_LIBTOOL? Should I not care about libtool 1.5? > > Cheers, > Peter > > > From e4810ab05c41461c3b6c5638a41d8d415ebcc40f Mon Sep 17 00:00:00 > 2001 From: Peter Rosin <p...@lysator.liu.se> > Date: Tue, 14 Sep 2010 14:58:17 +0200 > Subject: [PATCH] Add new 'AM_PROG_AR' macro, triggering the > 'ar-lib' script. > diff --git a/ChangeLog b/ChangeLog > index 02f2fcd..b911686 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,36 @@ > +2010-09-14 Peter Rosin <p...@lysator.liu.se> > + Ralf Wildenhues <ralf.wildenh...@gmx.de> > + > + Add new 'AM_PROG_AR' macro, triggering the 'ar-lib' script. > + * m4/ar-lib.m4: New macro AM_PROG_AR, which locates an > + archiver and triggers the auxiliary 'ar-lib' script if needed. > + * m4/Makefile.am: Add above. > + * automake.in (seen_ar): New variable. > + (scan_autoconf_traces): Set it. > + (handle_libraries): Don't set default values for AR and ARFLAGS > + if AM_PROG_AR has been seen. > + (handle_libraries, handle_ltlibraries): Require AM_PROG_AR for > + portability. > + * doc/automake.texi (Public Macros): Mention the new > + 'AM_PROG_AR' macro. > + (Subpackages): Add AM_PROG_AR to the example. > + (A Library): Adjust recommendations for AR given the new > + AM_PROG_AR macro. > + * tests/ar.test: Adjust to avoid portability warnings. > + * tests/ar-lib2.test: New test. Test if AM_PROG_AR triggers > + install of ar-lib. I feel that something like "New test, checking that AM_PROG_AR triggers install of ar-lib" would sound more natural. More instances below. This is admittedly a bike-shedding issue, so feel free to ignore it. > + * tests/ar-lib3.test: New test. Test if lib_LIBRARIES requires > + AM_PROG_AR. > + * tests/ar-lib4.test: New test. Test if lib_LTLIBRARIES requires > + AM_PROG_AR. > + * tests/ar-lib5.test: New test. Test if AM_PROG_AR triggers > + use of ar-lib when the archiver is Microsoft lib. > + * tests/ar-lib6.test: New test. Test ordering of AM_PROG_AR and > + LT_INIT. > + * tests/defs.in: New required entry 'lib'. > + * tests/Makefile.am: Add new tests. > + * NEWS: Update. > + > 2010-09-02 Peter Rosin <p...@lysator.liu.se> > > Make ar-lib support backslashed files in archives. > --- /dev/null > +++ b/m4/ar-lib.m4 > @@ -0,0 +1,55 @@ > +## -*- > Autoconf -*- +# Copyright (C) 2010 > +# Free Software Foundation, Inc. > +# > +# This file is free software; the Free Software Foundation > +# gives unlimited permission to copy and/or distribute it, > +# with or without modifications, as long as this notice is preserved. > + > +# serial 1 > + > +# AM_PROG_AR > +# -------------- You should add explanation of what this macro does, and, if it takes arguments, list them in a sort of "macro prototype"; e.g. # AM_PROG_AR([ACT-IF-FAIL]) # ------------------------- # Try to determine the the archiver interface (FIXME: more details); # If unable to, run ACT-IF-FAIL (default: abort configure). I guess you can come up with a better explanation ;-) > +AC_DEFUN([AM_PROG_AR], > +[AC_BEFORE([$0], [LT_INIT])dnl > +AC_BEFORE([$0], [AC_PROG_LIBTOOL])dnl > +AC_REQUIRE([AM_AUX_DIR_EXPAND])dnl > +AC_REQUIRE_AUX_FILE([ar-lib])dnl > +AC_CHECK_TOOLS(AR, [ar lib "link -lib"], false) Why not use proper m4 quoting everywhere? E.g.: AC_CHECK_TOOLS([AR], [ar lib "link -lib"], [false]) > +: ${AR=ar} > + > +AC_CACHE_CHECK([the archiver ($AR) interface], [am_cv_ar_interface], > + [am_cv_ar_interface=ar > + AC_COMPILE_IFELSE([[int some_variable = 0;]], > + [am_ar_try='$AR cru libconftest.a conftest.$ac_objext > >&AS_MESSAGE_LOG_FD' What is the rationale for not redirecting stderr here, along with stdout? > + AC_TRY_EVAL([am_ar_try]) > + if test "$ac_status" -eq 0; then > + am_cv_ar_interface=ar > + else > + am_ar_try='$AR -NOLOGO -OUT:conftest.lib conftest.$ac_objext' What is the rationale for not redirecting stdout/stderr here, considered how AC_TRY_EVAL is used above (with redirection)? > + AC_TRY_EVAL([am_ar_try]) > + if test "$ac_status" -eq 0; then > + am_cv_ar_interface=lib > + else > + m4_default([$1], > + [AC_MSG_ERROR([could not determine $AR interface])]) > + fi > + fi > + rm -f conftest.lib libconftest.a > + ]) > + ]) > + > +case $am_cv_ar_interface in > +ar) > + ;; > +lib) > + # Microsoft lib, so override with the ar-lib wrapper script. > + # FIXME: It is wrong to rewrite AR. > + # But if we don't then we get into trouble of one sort or another. > + # A longer-term fix would be to have automake use am__AR in this case, > + # and then we could set am__AR="\$(top_srcdir)/ar-lib \$(AR)" Do you mean am__AR="$am_aux_dir/ar-lib $AR" ? > + AR="$am_aux_dir/ar-lib $AR" > + ;; > +esac > +AC_SUBST([AR])dnl > +]) > diff --git a/tests/ar-lib4.test b/tests/ar-lib4.test > new file mode 100755 > index 0000000..c389189 > --- /dev/null > +++ b/tests/ar-lib4.test > @@ -0,0 +1,63 @@ > +#! /bin/sh > +# Test if lib_LTLIBRARIES requests AM_PROG_AR. > + > +required=libtoolize > +. ./defs || Exit 1 > + > +set -e > + > +cp configure.in X > + > +cat >> configure.in << 'END' > +AC_PROG_CC > +AC_PROG_RANLIB > +AC_PROG_LIBTOOL > +AC_OUTPUT > +END > + > +cat > Makefile.am << 'END' > +lib_LTLIBRARIES = libfoo.la > +libfoo_la_SOURCES = foo.c > +END > + > +libtoolize > +$ACLOCAL > +$AUTOCONF useless autoconf call > +AUTOMAKE_fails > + > +grep 'requires.*AM_PROG_AR' stderr > + > +cp X configure.in > + > +cat >> configure.in << 'END' > +AC_PROG_CC > +AM_PROG_AR > +AC_PROG_RANLIB > +AC_PROG_LIBTOOL > +AC_OUTPUT > +END > + > +$ACLOCAL > +$AUTOCONF another useless autoconf call > +AUTOMAKE_fails > + > +grep 'ar-lib.*not found' stderr > + > +$AUTOMAKE --add-missing > + > +: This testcase checks for two distinct automake failures; I'd prefer that to be done in two different tests, one checking for missing `AM_PROG_AR' call, the other one for missing `ar-lib' auxfile. WDYT? > diff --git a/tests/ar-lib5.test b/tests/ar-lib5.test > new file mode 100755 > index 0000000..f6eb5b4 > --- /dev/null > +++ b/tests/ar-lib5.test > @@ -0,0 +1,56 @@ > +#! /bin/sh > +# Test if AM_PROG_AR triggers the use of the ar-lib script. > + > +requires=lib > +. ./defs || Exit 1 > + > +set -e > + > +cat >> configure.in << 'END' > +AC_PROG_CC > +AM_PROG_AR > +AC_PROG_RANLIB > +AC_OUTPUT > +END > + > +cat > Makefile.am << 'END' > +lib_LIBRARIES = libwish.a > +libwish_a_SOURCES = wish.c > +END > + > +cat > wish.c << 'END' > +int wish(void) { return 0; } > +END > + > +$ACLOCAL > +$AUTOCONF > +$AUTOMAKE --add-missing --copy > +./configure AR=lib RANLIB=: > + > +cat > ar-lib << 'END' > +# /bin/sh > +echo "It works" > +END > +chmod +x ar-lib > + > +$MAKE > stdout > +cat stdout > + > +grep 'It works' stdout > + > +: Hmm... grepping the output of make might be fragile... What about rewriting the test on the following lines, to make it even more "semantic"? 1. Add proper call to `AC_CONFIG_AUX_DIR' in configure.in (with argument, say, `arlib-auxdir') 2. Add to Makefile.am: check-local: test -f ar-lib-worked MOSTLYCLEANFILES = ar-lib-worked (the last line required for "make distcheck" below) 3. Put the fake `ar-lib' into `arlib-auxdir'; this way, the call to automake should ensure that the new code respects AC_CONFIG_AUX_DIR 4. Make the fake `ar-lib' create the file `ar-lib-worked' in the current builddir: $ cat arlib-auxdir/ar-lib #! /bin/sh : > ar-lib-worked 5. Run "$MAKE check" and "$MAKE distcheck" > diff --git a/tests/ar-lib6.test b/tests/ar-lib6.test > new file mode 100755 > index 0000000..45693dd > --- /dev/null > +++ b/tests/ar-lib6.test > @@ -0,0 +1,38 @@ > +#! /bin/sh > +# 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 Please ansure the stderr is displayed even if autoconf fails: $AUTOCONF 2>stderr || { cat stderr >&2; Exit 1; } cat stderr >&2 Yes, I admit this idiom is annoying and should be factored out somehow... patch in preparation ;-) > + > +$EGREP '(AC_PROG_LIBTOOL|LT_INIT).*before.*AM_PROG_AR' stderr > + > +: > diff --git a/tests/ar.test b/tests/ar.test > index d68fc54..00ea69e 100755 > --- a/tests/ar.test > +++ b/tests/ar.test > @@ -21,6 +21,7 @@ > set -e > > cat >> configure.in << 'END' > +AM_PROG_AR > AC_SUBST([AR], ['echo it works']) > AC_SUBST([ARFLAGS], ['>']) > AC_SUBST([RANLIB], ['echo really works >>']) > @@ -34,7 +35,7 @@ END > > $ACLOCAL > $AUTOCONF > -$AUTOMAKE > +$AUTOMAKE --add-missing I'd rather create a dummy (empty) ar-lib above, and do not change this line. > ./configure > $MAKE > grep 'it works' libfoo.a > diff --git a/tests/defs.in b/tests/defs.in > index af4a3cd..31003ff 100644 > --- a/tests/defs.in > +++ b/tests/defs.in > @@ -156,6 +156,16 @@ do > echo "$me: running $CC -V -help" > ( $CC -V -help ) || exit 77 > ;; > + lib) > + AR=lib > + export AR > + # There is no way to get any identifying output with > + # a zero exit status. So, remap exit status 76 to 0. > + echo "$me: running $AR -?" > + exit_status=0 > + ( $AR -? ) || exit_status=$? Nitpicking: why a subshell here? > + test $exit_status = 76 && exit 77 > + ;; > makedepend) > echo "$me: running makedepend -f-" > ( makedepend -f- ) || exit 77 Thanks, Stefano
