Re: bindir.at takes forever.

[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