Re: [PATCH] Fix order of PATH manipulation in cwrapper and shwrapper

[Ralf Wildenhues]

* Charles Wilson wrote on Fri, Sep 17, 2010 at 06:23:28PM CEST:
> * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src:main): Call
> lt_update_exe_path before lt_update_lib_path, to ensure that the
> temporary rpath values (which include the OBJDIRs of uninstalled
> libtool libraries) precede installation and final -rpath directories.
> (func_emit_wrapper): Prepend $dllsearchpath to PATH before prepending
> $temp_rpath to $shlibpath_var; similar rationale as above.
> Reported by Jon Turney <address@hidden>

This is OK.  I mention a nit below.

Thanks,
Ralf

> --- a/libltdl/config/ltmain.m4sh
> +++ b/libltdl/config/ltmain.m4sh
> @@ -3293,6 +3293,22 @@ func_exec_program ()
>  
>    if test -f \"\$progdir/\$program\"; then"
>  
> +     # fixup the dll searchpath if we need to.
> +     #
> +     # For Windows, this must occur prior to any manipulation of
> +     # $shlibpath (which, ON Windows, is PATH).  That way, we ensure
> +     # that the $dllsearchpath value is prepended to $PATH first, and
> +     # that the temporary rpath values (which contain the actual
> +     # location of uninstalled DLLs, in their respective OBJDIR
> +     # directories) are prepended second.  This ensures that just-built
> +     # uninstalled libraries supersede installed ones.

I'd much rather have a short comment here,

  # Fix the DLL searchpath if we need to.  Do this before prepending to
  # $shlibpath, because on Windows, both are PATH and uninstalled
  # libraries must come first.

and have testsuite exposure that guarantees us that we won't regress,
and that, almost like an image, says more than thousand words.  ;-)

It's ok with me if the test patch comes later (due to release schedule).
If you like, I can write a test, it ought to be fairly simple.

> +     if test -n "$dllsearchpath"; then
> +       $ECHO "\
> +    # Add the dll search path components to the executable PATH
> +    PATH=$dllsearchpath:\$PATH
> +"
> +     fi
> +
>       # Export our shlibpath_var if we have one.
>       if test "$shlibpath_overrides_runpath" = yes && test -n 
> "$shlibpath_var" && test -n "$temp_rpath"; then
>         $ECHO "\
> @@ -3307,14 +3323,6 @@ func_exec_program ()
>  "
>       fi
>  
> -     # fixup the dll searchpath if we need to.
> -     if test -n "$dllsearchpath"; then
> -       $ECHO "\
> -    # Add the dll search path components to the executable PATH
> -    PATH=$dllsearchpath:\$PATH
> -"
> -     fi
> -
>       $ECHO "\
>      if test \"\$libtool_execute_magic\" != \"$magic\"; then
>        # Run the actual program with our arguments.
> @@ -3703,8 +3711,13 @@ EOF
>  
>    lt_setenv ("BIN_SH", "xpg4"); /* for Tru64 */
>    lt_setenv ("DUALCASE", "1");  /* for MSK sh */
> -  lt_update_lib_path (LIB_PATH_VARNAME, LIB_PATH_VALUE);
> +  /* For Windows, this order is important: it ensures that the $dllsearchpath
> +     value is prepended first, and that the temporary rpath values (which
> +     contain the actual location of uninstalled DLLs, in their respective
> +     OBJDIR directories) are prepended second.  This ensures that just-built
> +     uninstalled libraries supersede installed ones. */

See above.

>    lt_update_exe_path (EXE_PATH_VARNAME, EXE_PATH_VALUE);
> +  lt_update_lib_path (LIB_PATH_VARNAME, LIB_PATH_VALUE);
>  
>    lt_debugprintf (__FILE__, __LINE__, "(main) lt_argv_zero: %s\n",
>                 nonnull (lt_argv_zero));