Re: [PATCH] [mingw|cygwin] Modify cwrapper to invoke target directly.

[Ralf Wildenhues]

Hi Charles,

In addition to Eric's review:

* Charles Wilson wrote on Tue, May 06, 2008 at 02:23:05AM CEST:
> 
>       * libltdl/config/ltmain.m4sh (func_to_native_path):
>       new function. If $host is mingw, and $build is mingw
>       or cygwin, convert path to mingw native format.
>       (func_to_native_pathlist): new function. Ditto, for
>       :-separated pathlists.
>       (func_emit_cwrapperexe_src) [__CYGWIN__ && __STRICT_ANSI__]:
>       ensure putenv and setenv are declared. Define HAVE_SETENV.
>       (func_emit_cwrapperexe_src) [main]: add new constants to
>       hold desired PATH settings; initialize and convert to native
>       mingw format using functions above. Add new command-line
>       options --lt-env-set, --lt-env-prepend, and --lt-env-append.
>       No longer emit wrapper script as integral part of launching
>       child. Remove support for (now) unnecessary $TARGETSHELL.
>       Exec actual target executable directly.

You can simplify the remaining part of the ChangeLog entry:

>       (func_emit_cwrapperexe_src) [lt_setenv]: new function.
>       (func_emit_cwrapperexe_src) [lt_extend_str]: new function.
>       (func_emit_cwrapperexe_src) [lt_split_name_value]: new function.
>       (func_emit_cwrapperexe_src) [lt_opt_process_env_set]: new function.
>       (func_emit_cwrapperexe_src) [lt_opt_process_env_prepend]: new function.
>       (func_emit_cwrapperexe_src) [lt_opt_process_env_append]: new function.
>       (func_emit_cwrapperexe_src) [lt_update_exe_path]: new function.
>       (func_emit_cwrapperexe_src) [lt_update_lib_path]: new function.

to this:
        (func_emit_cwrapperexe_src) [lt_setenv]: New function.
        [lt_extend_str]: New function.
        [lt_split_name_value]: New function.
        [lt_opt_process_env_set]: New function.
        [lt_opt_process_env_prepend]: New function.
        [lt_opt_process_env_append]: New function.
        [lt_update_exe_path]: New function.
        [lt_update_lib_path]: New function.

or even to this:
        (func_emit_cwrapperexe_src) [lt_setenv, lt_extend_str]
        [lt_split_name_value, lt_opt_process_env_set]
        [lt_opt_process_env_prepend, lt_opt_process_env_append]
        [lt_update_exe_path, lt_update_lib_path]: New functions.


More nits:

> +# func_to_native_path
> +#
> +# intended for use on "native" mingw (where libtool itself

Please capitalize, also please start the description with what the
function does, not what it is intended for:
  Convert paths to build format when used with build tools.
  [...]

> +# is running under the msys shell).  Paths need to be converted
> +# to native format when used with native tools. Ordinarily, the
> +# (msys) shell automatically converts such things for non-msys
> +# applications it launches, but that isn't available from inside
> +# the cwrapper. Similar accommodations are necessary for $host
> +# mingw and $build cygwin.  Calling this function does no harm
> +# on other $build or for other $host.
> +func_to_native_path ()
> +{

> +void
> +lt_update_exe_path (const char *name, const char *value)
> +{
> +  LTWRAPPER_DEBUGPRINTF (("(lt_update_exe_path) modifying '%s' by prepending 
> '%s'\n",
> +                          (name ? name : "<NULL>"),
> +                          (value ? value : "<NULL>")));
> +
> +  if (name && *name && value && *value)
> +    {
> +      char *new_value = lt_extend_str (getenv (name), value, 0);
> +      /* some systems can't cope with a ':'-terminated path #' */

What does that #' do here?

> +      int len = strlen (new_value);
> +      while (((len = strlen (new_value)) > 0) && IS_PATH_SEPARATOR 
> (new_value[len-1]))
> +        {
> +          new_value[len-1] = '\0';
> +        }

Sorry I haven't had the time to test your patches on a cross build yet.
(This isn't a veto against the patch.)

Cheers,
Ralf