Re: bindir.at takes forever.
* 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.
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.
[ 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