Re: bindir.at takes forever.

2010-10-14 Thread Ralf Wildenhues
* Peter Rosin wrote on Thu, Oct 14, 2010 at 11:32:20PM CEST:
> Den 2010-10-14 21:48 skrev Ralf Wildenhues:
> > Changes over the previous patch version:
> > - removed some loop iterations in the inner test, for efficiency, to
> >   address Peter's report,
> > - correctly SKIP the test if tempdir creation fails.
> > 
> > OK to commit both patches?
> 
> Thanks for doing this!  I have one minor nit with these patches which
> I have included inline.  Other than that, the patches seem to cut the
> test time in about half.  Still long, but this shaves off many minutes.
> 
> BTW, the bindir tests still pass on MSYS/MSVC.

Cool, thanks for the review.  I have squashed in this incremental diff
before pushing.

Cheers,
Ralf

diff --git a/ChangeLog b/ChangeLog
index 379e609..b071b92 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,7 +9,7 @@
require a major version number in the $libdir file name, for AIX
without runtimelinking.  If tmpdir creation fails, skip the
test.  Use fewer bindir directory names for testing, to speed
-   up the test.
+   up the test.  Also mention MSVC style DLL name in comment.
Report by Peter Rosin.
 
tests: remove unneeded 'bindir compile check' test.
diff --git a/tests/bindir.at b/tests/bindir.at
index 3fa185c..4e2fecc 100644
--- a/tests/bindir.at
+++ b/tests/bindir.at
@@ -271,10 +271,10 @@ do
   AT_CHECK([$LIBTOOL --mode=install $lt_INSTALL libfoo.la $libdir], [], 
[ignore], [ignore])
   AT_CHECK([$LIBTOOL --mode=install $lt_INSTALL main$EXEEXT 
$curdir/sbin/main$EXEEXT], [], [ignore], [ignore])
 
-  # And ensure it went where we expect.  Could be looking for any of 
'cygfoo-0.dll',
-  # 'libfoo-0.dll', or 'libfoo.so.0'.  We'll simplify this check by taking 
advantage
-  # of the fact that if it's a DLL, it has to go in bindir, so we'll not check 
for
-  # both forms in libdir.
+  # And ensure it went where we expect.  Could be looking for any of
+  # 'cygfoo-0.dll', 'libfoo-0.dll', 'foo-0.dll', or 'libfoo.so.0'.  We'll
+  # simplify this check by taking advantage of the fact that if it's a DLL,
+  # it has to go in bindir, so we'll not check for both forms in libdir.
   if $bindirneeded; then
 AT_CHECK([test -f $libdir/../bin/???foo-0.dll || ls 
$libdir/../bin/*foo*0*], [], [ignore], [ignore])
   else



Re: bindir.at takes forever.

2010-10-14 Thread Peter Rosin
Den 2010-10-14 21:48 skrev Ralf Wildenhues:
> [ moving from libtool@ to -patches ]
> 
> Hi Dave,
> 
> * Dave Korn wrote on Sun, Oct 03, 2010 at 01:15:46PM CEST:
>> On 28/09/2010 21:36, Ralf Wildenhues wrote:
>>> * Peter Rosin wrote on Tue, Sep 28, 2010 at 02:28:48PM CEST:
 I have been looking at the loops in tests/bindir.at and I see
 this:
>>>
>>> bindir.at has several problems.
>>
>>   Argh.  Sorry for the mess, it's the first and only libtool test I've ever
>> written and I didn't have much to go on except copying from the others that
>> seemed nearest what I wanted to do.  It certainly wouldn't hurt to cut down
>> that inner loop down to a half or third of what it currently tests, I was 
>> just
>> erring on the side of thoroughness.
> 
> Cool.  And no need to apologize, those are all things we should have
> caught during review, or we can just fix now.  It's great that you have
> provided a test case at all!
> 
> Anyway, here we go with bindir cleanup.  I have tested the following
> couple of patches on native MinGW, Cygwin, GNU/Linux, and AIX (where it
> fixes a spurious testsuite failure), as well as a GNU/Linux -> MinGW
> cross.  All pass the bindir tests now.
> 
> Changes over the previous patch version:
> - removed some loop iterations in the inner test, for efficiency, to
>   address Peter's report,
> - correctly SKIP the test if tempdir creation fails.
> 
> OK to commit both patches?

Thanks for doing this!  I have one minor nit with these patches which
I have included inline.  Other than that, the patches seem to cut the
test time in about half.  Still long, but this shaves off many minutes.

BTW, the bindir tests still pass on MSYS/MSVC.

*snip*

> Fix bindir check logic, and relax non-bindir case for AIX.
> 
> * tests/bindir.at (bindir install tests): Rewrite checks for
> place of the installed shared library in two separate tests,
> depending on whether -bindir is supposed to have an effect or
> not.  In the positive case, make the test stricter so that we
> reject libraries in $libdir.  In the negative case, do not
> require a major version number in the $libdir file name, for AIX
> without runtimelinking.  If tmpdir creation fails, skip the
> test.  Use fewer bindir directory names for testing, to speed
> up the test.
> Report by Peter Rosin.
> 
> diff --git a/tests/bindir.at b/tests/bindir.at
> index ebe1baa..3fa185c 100644
> --- a/tests/bindir.at
> +++ b/tests/bindir.at
> @@ -138,14 +138,14 @@ AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC -o 
> main$EXEEXT $CPPFLAGS $CFLAGS $LD
>  # here, that will be covered by the later tests; we've rpath'd things
>  # so that they can all be run in situ.
>  
> -LT_AT_NOINST_EXEC_CHECK([$LIBTOOL], [], [0], [ignore], [ignore], 
> [--mode=execute ./main$EXEEXT])
> +LT_AT_NOINST_EXEC_CHECK([./main])
>  
>  # Ensure libraries can be found on PATH, if we are on one
>  # of the affected platforms, before testing the shared version.
>  
>  func_save_and_prepend_path $curdir/$objdir
>  $bindirneeded && {
> -  LT_AT_NOINST_EXEC_CHECK([$LIBTOOL], [], [0], [ignore], [ignore], 
> [--mode=execute $objdir/main$EXEEXT])
> +  LT_AT_NOINST_EXEC_CHECK([$objdir/main])
>  }
>  
>  #  In fact, prepending the PATH as above is superfluous on the windows
> @@ -182,7 +182,7 @@ case "$host_os" in
>  esac
>  
>  eval "`$LIBTOOL --config | grep '^build_libtool_libs='`"
> -AT_CHECK([test "$build_libtool_libs" = yes || (exit 77)])
> +AT_CHECK([test "$build_libtool_libs" = yes || exit 77])
>  
>  
>  # These routines save the PATH before a test and restore it after,
> @@ -275,7 +275,11 @@ do
># 'libfoo-0.dll', or 'libfoo.so.0'.  We'll simplify this check by taking 
> advantage

Can you please throw in 'foo-0.dll' in the sentence that ends in the
beginning of the above line?

># of the fact that if it's a DLL, it has to go in bindir, so we'll not 
> check for
># both forms in libdir.
> -  AT_CHECK([$bindirneeded && { test -f $libdir/../bin/???foo-0.dll || ls 
> $libdir/../bin/*foo*0* 2>/dev/null ; } || ls $libdir/*foo*0* 2>/dev/null], 
> [], [ignore], [ignore])
> +  if $bindirneeded; then
> +AT_CHECK([test -f $libdir/../bin/???foo-0.dll || ls 
> $libdir/../bin/*foo*0*], [], [ignore], [ignore])
> +  else
> +AT_CHECK([ls $libdir/*foo*], [], [ignore], [ignore])
> +  fi
>  

*snip*

Cheers,
Peter



Re: bindir.at takes forever.

2010-10-14 Thread Ralf Wildenhues
[ moving from libtool@ to -patches ]

Hi Dave,

* Dave Korn wrote on Sun, Oct 03, 2010 at 01:15:46PM CEST:
> On 28/09/2010 21:36, Ralf Wildenhues wrote:
> > * Peter Rosin wrote on Tue, Sep 28, 2010 at 02:28:48PM CEST:
> >> I have been looking at the loops in tests/bindir.at and I see
> >> this:
> > 
> > bindir.at has several problems.
> 
>   Argh.  Sorry for the mess, it's the first and only libtool test I've ever
> written and I didn't have much to go on except copying from the others that
> seemed nearest what I wanted to do.  It certainly wouldn't hurt to cut down
> that inner loop down to a half or third of what it currently tests, I was just
> erring on the side of thoroughness.

Cool.  And no need to apologize, those are all things we should have
caught during review, or we can just fix now.  It's great that you have
provided a test case at all!

Anyway, here we go with bindir cleanup.  I have tested the following
couple of patches on native MinGW, Cygwin, GNU/Linux, and AIX (where it
fixes a spurious testsuite failure), as well as a GNU/Linux -> MinGW
cross.  All pass the bindir tests now.

Changes over the previous patch version:
- removed some loop iterations in the inner test, for efficiency, to
  address Peter's report,
- correctly SKIP the test if tempdir creation fails.

OK to commit both patches?

Thanks,
Ralf

tests: remove unneeded 'bindir compile check' test.

* tests/bindir.at (bindir compile check): Remove.

diff --git a/tests/bindir.at b/tests/bindir.at
index 40e67cc..ebe1baa 100644
--- a/tests/bindir.at
+++ b/tests/bindir.at
@@ -1,6 +1,6 @@
 # bindir.at -  Test the -bindir option
 #
-#   Copyright (C) 2009 Free Software Foundation, Inc.
+#   Copyright (C) 2009, 2010 Free Software Foundation, Inc.
 #   Written by Dave Korn, 2009
 #
 #   This file is part of GNU Libtool.
@@ -58,25 +58,8 @@
 # statement in libtool.m4sh around where the 'tdlname' variable is set.
 
 
-# Verify compiling works, and skip remaining tests if not.
-#
-
-AT_SETUP([bindir compile check])
-
-AT_DATA([simple.c] ,[[
-int main() { return 0;}
-]])
-
-$CC $CPPFLAGS $CFLAGS -o simple simple.c || noskip=false
-rm -f simple
-
-AT_CHECK([$noskip || (exit 77)])
-
-AT_CLEANUP
-
-
-# Now run the tests themselves.  First a simple test that we can
-# build and run an executable with a couple of tiny libraries.
+# First a simple test that we can build and run an executable with a couple of
+# tiny libraries.
 
 AT_SETUP([bindir basic lib test])
 


Fix bindir check logic, and relax non-bindir case for AIX.

* tests/bindir.at (bindir install tests): Rewrite checks for
place of the installed shared library in two separate tests,
depending on whether -bindir is supposed to have an effect or
not.  In the positive case, make the test stricter so that we
reject libraries in $libdir.  In the negative case, do not
require a major version number in the $libdir file name, for AIX
without runtimelinking.  If tmpdir creation fails, skip the
test.  Use fewer bindir directory names for testing, to speed
up the test.
Report by Peter Rosin.

diff --git a/tests/bindir.at b/tests/bindir.at
index ebe1baa..3fa185c 100644
--- a/tests/bindir.at
+++ b/tests/bindir.at
@@ -138,14 +138,14 @@ AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC -o 
main$EXEEXT $CPPFLAGS $CFLAGS $LD
 # here, that will be covered by the later tests; we've rpath'd things
 # so that they can all be run in situ.
 
-LT_AT_NOINST_EXEC_CHECK([$LIBTOOL], [], [0], [ignore], [ignore], 
[--mode=execute ./main$EXEEXT])
+LT_AT_NOINST_EXEC_CHECK([./main])
 
 # Ensure libraries can be found on PATH, if we are on one
 # of the affected platforms, before testing the shared version.
 
 func_save_and_prepend_path $curdir/$objdir
 $bindirneeded && {
-  LT_AT_NOINST_EXEC_CHECK([$LIBTOOL], [], [0], [ignore], [ignore], 
[--mode=execute $objdir/main$EXEEXT])
+  LT_AT_NOINST_EXEC_CHECK([$objdir/main])
 }
 
 #  In fact, prepending the PATH as above is superfluous on the windows
@@ -182,7 +182,7 @@ case "$host_os" in
 esac
 
 eval "`$LIBTOOL --config | grep '^build_libtool_libs='`"
-AT_CHECK([test "$build_libtool_libs" = yes || (exit 77)])
+AT_CHECK([test "$build_libtool_libs" = yes || exit 77])
 
 
 # These routines save the PATH before a test and restore it after,
@@ -275,7 +275,11 @@ do
   # 'libfoo-0.dll', or 'libfoo.so.0'.  We'll simplify this check by taking 
advantage
   # of the fact that if it's a DLL, it has to go in bindir, so we'll not check 
for
   # both forms in libdir.
-  AT_CHECK([$bindirneeded && { test -f $libdir/../bin/???foo-0.dll || ls 
$libdir/../bin/*foo*0* 2>/dev/null ; } || ls $libdir/*foo*0* 2>/dev/null], [], 
[ignore], [ignore])
+  if $bindirneeded; then
+AT_CHECK([test -f $libdir/../bin/???foo-0.dll || ls 
$libdir/../bin/*foo*0*], [], [ignore], [ignore])
+  else
+AT_CHECK([ls $libdir/*foo*], [], [ignore], [ignore])
+  fi
 
   # And that it can be executed.
   extrapath=
@@ -287,18 +291,10 @@ do
   for