[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add test case for 69e77671 (cwrapper PATH manipulation order
From: |
Ralf Wildenhues |
Subject: |
Re: [PATCH] Add test case for 69e77671 (cwrapper PATH manipulation order) |
Date: |
Mon, 4 Oct 2010 07:14:56 +0200 |
User-agent: |
Mutt/1.5.20 (2010-08-04) |
Hi Charles,
* address@hidden wrote on Sun, Oct 03, 2010 at 11:15:17PM CEST:
> * tests/cwrapper.at: Add new test 'cwrapper and installed shared
> libraries.'
OK with nits addressed. You may want to use a ChangeLog and/or --author
entry that suitably documents the main author of the patch.
Thanks,
Ralf
> --- a/tests/cwrapper.at
> +++ b/tests/cwrapper.at
> @@ -134,3 +134,73 @@ done
>
> AT_CLEANUP
>
> +
> +AT_SETUP([cwrapper and installed shared libraries])
> +AT_KEYWORDS([libtool])
> +
> +# make sure existing libtool is configured for shared libraries
> +AT_CHECK([$LIBTOOL --features | grep 'disable shared libraries' && exit 77],
> + [1], [ignore])
Let's be positive:
AT_CHECK([$LIBTOOL --features | grep 'enable shared libraries' || exit 77],
[], [ignore])
> +# FIXME with shared_fails for this test on AIX
> +# copy from link-order2.at:
> +eval `$LIBTOOL --config | $EGREP '^(shlibpath_var|allow_undefined_flag)='`
> +
> +undefined_setting=-no-undefined
> +shared_fails=no
> +case $host_os,$LDFLAGS,$allow_undefined_flag in
> +aix*,*-brtl*,*) ;;
> +aix*) shared_fails=yes ;;
> +darwin*,*,*-flat_namespace*) undefined_setting= ;;
> +darwin*,*,*) shared_fails=yes ;;
> +esac
> +# end of copy from link-order2.at
> +
> +LDFLAGS="$LDFLAGS $undefined_setting"
Let's replace these 15 lines with
LDFLAGS="$LDFLAGS -no-undefined"
because I don't see how this test should need to depend on them at all.
Since the test is explicitly about the cwrapper, I'd probably prefer to
skip it on systems that have a problem with the test but don't use the
cwrapper anyway. If you agree then I can just test this later and we
keep the simple code for now.
> +inst=`pwd`/inst
> +libdir=$inst/lib
> +bindir=$inst/bin
> +mkdir $inst $libdir $bindir
> +
> +# we must build foo library in a separate directory to avoid on some
> +# platforms shared library to be loaded from "current" directory
I have trouble parsing this sentence, and it is lacking punctuation and
capitalization. How about this?
# Build the library in a separate directory to avoid the special case
# of loading from the current directory.
> +mkdir foo
> +cd foo
> +# build and install "old" library version
> +AT_DATA([a.c], [[
> +int liba_ver (void) { return(1); }
Please no parens for argument to return, that is not a function.
Three instances.
> +]])
> +$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c
> +$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -version-info=0.0.0 -o liba.la
> -rpath $libdir a.lo
> +$LIBTOOL --mode=install $lt_INSTALL liba.la $libdir
Is there any, however unlikely, chance that these $LIBTOOL commands fail
on some system or setup? Then they should be wrapped in
AT_CHECK([...], [], [ignore], [ignore])
> +# build a new library version
> +AT_DATA([a.c], [[
> +int liba_ver (void) { return(2); }
> +]])
> +$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c
> +$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -version-info=0.0.0 -o liba.la
> -rpath $libdir a.lo
Ditto.
> +cd ..
> +
> +# build and run test application
> +AT_DATA([m.c], [[
> +extern int liba_ver (void);
> +int main (void)
> +{
> + int r = (liba_ver () == 2) ? 0 : 1;
> + return(r);
> +}
> +]])
> +
> +$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c m.c
> +
> +$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o m1$EXEEXT m.$OBJEXT foo/liba.la
> +LT_AT_EXEC_CHECK([./m1], [0], [ignore], [ignore], [])
> +
> +$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o m2$EXEEXT m.$OBJEXT foo/liba.la
> -L$inst/lib
> +LT_AT_EXEC_CHECK([./m2], [0], [ignore], [ignore], [])
> +
> +AT_CLEANUP