Re: [PATCH, take 4][cygwin|mingw] Control where win32 DLLs get installed.

[Ralf Wildenhues]

Hi Dave,

sorry for making you go through another round.

Except for the sed issues the following nits are all mechanical, and it
would suffice if you re-checked only your new tests and only on one of
the affected systems.

I consider the patch ready after this round, but I will probably leave a
couple of days for the w32 experts to comment on it, before applying it.

* Dave Korn wrote on Sun, Aug 16, 2009 at 02:55:22AM CEST:
>  
>  @item -R @var{libdir}
>  If @var{output-file} is a program, add @var{libdir} to its run-time
> diff --git a/libltdl/config/general.m4sh b/libltdl/config/general.m4sh
> index 4bc304c..9954057 100644
> --- a/libltdl/config/general.m4sh
> +++ b/libltdl/config/general.m4sh
> @@ -100,6 +100,149 @@ func_dirname_and_basename ()
>  
>  # Generated shell functions inserted here.
>  
> +pathcar="s,^/\([^/]*\).*$,\1,"
> +pathcdr="s,^/[^/]*,,"
> +removedotparts="s,/\(\.\(/\|$\)\)\+,/,g"
> +collapseslashes="s,/\+,/,g"

\+ is a GNU sed extension, \{1,\} is Posix (two instances).
Nested \( \) are not portabled sed, neither are alternations \|.
Anchors $ inside groups are not portable.
The nesting probably works on most systems we care about, but if we can
avoid it then I'd prefer to do so.

(Sorry btw for completely overlooking this the last time.)

> +func_relative_path ()
> +{

> +      *)
> +        func_dirname $func_relative_path_tlibdir
> +        func_relative_path_tlibdir=${func_dirname_result}
> +        if test x$func_relative_path_tlibdir = x ; then

Please double-quote $func_relative_path_tlibdir.  The rest of your patch
seems to cope with with spaces in file name components (even if the rest
of Libtool may not).

> --- /dev/null
> +++ b/tests/bindir.at

> +# These routines save the PATH before a test and restore it after,
> +# prepending a chosen directory to the path on the platforms where
> +# -bindir is needed after saving.
> +#
> +
> +func_save_and_prepend_path ()
> +{
> +  save_PATH=$PATH
> +  if $bindirneeded ; then
> +    PATH=$1:$PATH
> +  fi
> +  export PATH
> +}
> +
> +func_restore_path ()
> +{
> +  PATH=$save_PATH

No 'export PATH' here?  (two instances)

> +}

> +# Check both static and shared versions run.  We don't install them
> +# here, that will be covered by the later tests; we've rpath'd things
> +# so that they can all be run in situ.
> +
> +AT_CHECK([$LIBTOOL --mode=execute ./main$EXEEXT], [], [stdout])

Please use LT_AT_NOINST_EXEC_CHECK here, to avoid error when cross
compiling.  Similar for the other executions of uninstalled programs.
(This macro is defined in tests/testsuite.at.)

> +# 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/.libs
> +AT_CHECK([$LIBTOOL --mode=execute ./.libs/main$EXEEXT], [], [stdout])

eval "`$LIBTOOL --config | grep '^objdir='`"
func_save_and_prepend_path $curdir/$objdir
LT_AT_NOINST_EXEC_CHECK(... $objdir/...)

I'm afraid the direct execution of programs below .libs may not work
everywhere.  I'd have to check to be sure though.  It's fine with me
if you run this particular test only on systems of interest to you.

> +#  In fact, prepending the PATH as above is superfluous on the windows
> +# platforms that this feature is primarily aimed at, as the DLL search
> +# path always includes the directory from which the app was launched.
> +# To make sure it still works even when not side-by-side, we'll install
> +# the main executable and execute it from there while the PATH still
> +# points to the shared libs in the .libs subdir.  On other platforms,
> +# the rpaths we set at link time will guarantee it runs from the bindir.
> +
> +mkdir $curdir/bin
> +AT_CHECK([$LIBTOOL --mode=install $lt_INSTALL main$EXEEXT 
> $curdir/bin/main$EXEEXT], [], [ignore], [ignore])
> +AT_CHECK([$LIBTOOL --mode=execute $curdir/bin/main$EXEEXT], [], [stdout])

This one should work without "$LIBTOOL --mode=execute" prepended, no?
In that case, you could use LT_AT_EXEC_CHECK here, again, to avoid
spurious failures when cross compile testing.

If you care about the fact that an installed program does not
accidentally require, or search for, a library in an uninstalled
location, then you can, after the mode=install but before the execution,
clean up the files in the build tree, or even put a poisoned library
into the build tree.  I don't think either is necessary to do in this
test (at least the latter seems overkill, as we do it in other tests
already), but please decide for yourself.

> +AT_SETUP([bindir install tests])

> +curdir=`pwd`
> +for libdir in \
> +     $curdir/usr/lib/gcc/i686-pc-cygwin/4.5.0 \
> +     $curdir/usr/lib/gcc/../gcc/.//i686-pc-cygwin/4.5.0/../../././//. \
> +     $curdir/usr/lib/ \
> +     $curdir/usr/lib \
> +     $curdir/baz \
> +     $curdir/baz/lib/ ;

Mini nit: this ";" is not necessary.  :-)

> +do
> +
> +  # Do a basic install with no -bindir option for reference.  We use the 
> sbin/
> +  # dir for the main exe to avoid the potential "this only works because it's
> +  # side-by-side with the libs" default DLL search path problem mentioned 
> above.
> +  rm -rf $libdir $curdir/bin $curdir/sbin $curdir/baz $curdir/usr 2>/dev/null
> +  AS_MKDIR_P($libdir)
> +  AS_MKDIR_P($curdir/sbin)
> +  AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC -no-undefined -shared -o 
> libfoo.la $CPPFLAGS $CFLAGS $LDFLAGS foo.lo bar.lo baz.lo -rpath 
> $libdir],[0],[ignore],[ignore])

Please drop -shared, it should be unnecessary.

> +  AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC -o main$EXEEXT $CPPFLAGS 
> $CFLAGS $LDFLAGS main.lo libfoo.la -rpath $libdir],[0],[ignore],[ignore])
> +  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.
> +  AT_CHECK($bindirneeded && ( test -f $libdir/../bin/???foo-0.dll || test -f 
> $libdir/../bin/libfoo.so.0 ) || test -f $libdir/libfoo.so.0)

Please M4-quote "[...]" the argument to AT_CHECK.

You can replace the inner subshell "( ... )" with "{ ... ; }" to avoid a
subshell.

Out of curiosity, which of the systems of interest creates a
libfoo.so.0 file?  What if that is a symlink rather than a plain file
("test -f" only tests for plain files)?

> +    # Clear any old stuff out before we install.  Because bindir
> +    # may be in /tmp, we have to take care to create it securely
> +    # and not to delete and recreate it if we do.
> +    rm -rf $libdir $curdir/bin $curdir/sbin $curdir/baz $curdir/usr 
> 2>/dev/null

I'd drop the 2>/dev/null; let's see errors in the log.

> +    # Relink with new rpaths.
> +    AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC -no-undefined -bindir 
> $bindir -shared -o libfoo.la $CPPFLAGS $CFLAGS $LDFLAGS foo.lo bar.lo baz.lo 
> -rpath $libdir],[0],[ignore],[ignore])
> +    AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC -o main$EXEEXT $CPPFLAGS 
> $CFLAGS $LDFLAGS main.lo libfoo.la],[0],[ignore],[ignore])
> +
> +    # Recreate directories (bindir already done) and install.
> +    AS_MKDIR_P($libdir)
> +    AS_MKDIR_P($curdir/sbin)
> +    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])
> +
> +    # Ensure it went to bindir rather than default dir this time.
> +    AT_CHECK($bindirneeded && ( test -f $bindir/???foo-0.dll || test -f 
> $bindir/libfoo.so.0 ) || test -f $libdir/libfoo.so.0)
> +
> +    # And that it can be executed.
> +    extrapath=
> +    $bindirneeded && extrapath=$bindir
> +    func_save_and_prepend_path $extrapath
> +    AT_CHECK([$LIBTOOL --mode=execute $curdir/sbin/main$EXEEXT], [], 
> [stdout])
> +    func_restore_path
> +
> +    # Clean up if we made a temp dir.  Subdirs under our testdir get rm'd
> +    # and recreated at the top of the loop.  Securely created subdirs under
> +    # /tmp get created precisely once and rm'd when we're done with them.
> +    if test ! -z "$tmp" ; then
> +      rm -rf "$tmp"
> +    fi

For extra credit you could also throw in
      $LIBTOOL --mode=uninstall rm -f $libdir/libfoo.la $curdir/sbin/main$EXEEXT
      $LIBTOOL --mode=clean rm -f libfoo.la main$EXEEXT

here, if you like.

> +  done
> +done
> +
> +AT_CLEANUP

Thanks,
Ralf