Re: [PATCH] Add test case for 69e77671 (cwrapper PATH manipulation order)

2010-10-07 Thread Charles Wilson 
On 10/4/2010 1:14 AM, Ralf Wildenhues wrote:
 OK with nits addressed.  You may want to use a ChangeLog and/or --author
 entry that suitably documents the main author of the patch.

Updated and pushed as attached.

--
Chuck
diff --git a/ChangeLog b/ChangeLog
index c0492fe..9caba84 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2010-10-07  Roumen Petrov  ...
+
+   Add test case for 69e77671 (cwrapper PATH manipulation order)
+   * tests/cwrapper.at: Add new test 'cwrapper and installed shared
+   libraries.'
+
 2010-10-04  Peter Rosin  ...
 
cwrapper: split long lines when dumping the wrapper script.
diff --git a/tests/cwrapper.at b/tests/cwrapper.at
index cd618dc..6e8cf3c 100644
--- a/tests/cwrapper.at
+++ b/tests/cwrapper.at
@@ -194,4 +194,71 @@ AT_CHECK([grep ' *fputs' $objdir/lt-usea.c  /dev/null])
 # Check for no overly long fputs.
 AT_CHECK([grep ' *fputs.\{250\}' $objdir/lt-usea.c], [1])
 
+
+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 'enable shared libraries' || exit 77],
+[], [ignore])
+
+LDFLAGS=$LDFLAGS -no-undefined
+
+inst=`pwd`/inst
+libdir=$inst/lib
+bindir=$inst/bin
+mkdir $inst $libdir $bindir
+
+# 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; }
+]])
+AT_CHECK([$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c],
+ [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -version-info=0.0.0 -o 
liba.la -rpath $libdir a.lo],
+ [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=install $lt_INSTALL liba.la $libdir],
+ [], [ignore], [ignore])
+
+# build a new library version
+AT_DATA([a.c], [[
+int liba_ver (void) { return 2; }
+]])
+AT_CHECK([$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c],
+ [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -version-info=0.0.0 -o 
liba.la -rpath $libdir a.lo],
+ [], [ignore], [ignore])
+
+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;
+}
+]])
+
+AT_CHECK([$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c m.c],
+ [], [ignore], [ignore])
+
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o m1$EXEEXT m.$OBJEXT 
foo/liba.la],
+ [], [ignore], [ignore])
+LT_AT_EXEC_CHECK([./m1], [0], [ignore], [ignore], [])
+
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o m2$EXEEXT m.$OBJEXT 
foo/liba.la -L$inst/lib],
+ [], [ignore], [ignore])
+LT_AT_EXEC_CHECK([./m2], [0], [ignore], [ignore], [])
+
+
 AT_CLEANUP

Re: [PATCH] Add test case for 69e77671 (cwrapper PATH manipulation order)

2010-10-04 Thread Ralf Wildenhues 
Hi Charles,

* libt...@cwilson.fastmail.fm 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

[PATCH] Add test case for 69e77671 (cwrapper PATH manipulation order)

2010-10-03 Thread libtool 
* tests/cwrapper.at: Add new test 'cwrapper and installed shared
libraries.'
---
This patch was actually proposed by Roumen Petrov here:
http://lists.gnu.org/archive/html/bug-libtool/2009-12/msg00037.html

He mentioned here:
http://lists.gnu.org/archive/html/libtool-patches/2010-09/msg00222.html
that 69e77671 would actually fix the error exposed by this test. I
ran this test both with and without 69e77671; without 69e77671 this
test fails (cygwin), but with it the new test passes.  Examination
shows that Roumen's test is exactly what is needed to verify that
the problem fixed by 69e77671 does not regress; his test explicitly
verifies that a newly built DLL (more generally, shared library)
is used at runtime in preference to an installed version.


 tests/cwrapper.at |   70 +
 1 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/tests/cwrapper.at b/tests/cwrapper.at
index 248c0c0..ff3d04f 100644
--- 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])
+
+# 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
+
+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
+
+mkdir foo
+cd foo
+# build and install old library version
+AT_DATA([a.c], [[
+int liba_ver (void) { return(1); }
+]])
+$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
+
+# 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
+
+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
-- 
1.7.1