Re: [PATCH] [mingw|cygwin] Modify cwrapper to invoke target directly.
Hi Charles, About following comment: /* execv doesn't actually work on mingw as expected on unix */ Actually execXXX on microsoft windows create a new process and it is not mingw problem. What about to substitute mingw with windows ? Roumen Charles Wilson wrote: Charles Wilson wrote: 2008-05-05 Charles Wilson ... * 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. (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. The attached patch (to be applied over the previous one) addresses the comments received so far. I can post the merged patch if desired -- and of course, I'll squash into a single patch before I push. Cygwin: passes 115 (9 skip) on old test suite only two unexpected failures on new test suite -- but 25 and 72 are actually expected on cygwin. Mingw (msys): no regressions over previous results: old test suite: fails demo-exec after demo-shared (helldl) fails the fortran tests, but that's a peculiarity of my system new test suite: fails 25 and 72 (expected), and 58-60 (a problem with the autotool wrappers on my system) OK for push? -- Chuck
Re: [PATCH] [mingw|cygwin] Modify cwrapper to invoke target directly.
Charles Wilson wrote: Charles Wilson wrote: 2008-05-05 Charles Wilson ... * 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. (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. The attached patch (to be applied over the previous one) addresses the comments received so far. I can post the merged patch if desired -- and of course, I'll squash into a single patch before I push. Cygwin: passes 115 (9 skip) on old test suite only two unexpected failures on new test suite -- but 25 and 72 are actually expected on cygwin. Mingw (msys): no regressions over previous results: old test suite: fails demo-exec after demo-shared (helldl) fails the fortran tests, but that's a peculiarity of my system new test suite: fails 25 and 72 (expected), and 58-60 (a problem with the autotool wrappers on my system) OK for push? -- Chuck libtool 2.2.4 patched with both patches still fail: ... (lt_setenv) setting 'PATH' to ':/usr/local/src//lt-2.2.4-mingw-mlib/lib2/.libs:/usr/local/src//lt-2.2.4-mingw-mlib/lib1/.libs:/tmp/test/pkg/lt-2.2.4-mingw-mlib/lib:/tmp/test/pkg/lt-2.2.4-mingw-mlib/bin:/usr/local/src//lt-2.2.4-mingw-mlib/lib2/.libs:/usr/local/src//lt-2.2.4-mingw-mlib/lib1/.libs:c:\windows\system32;c:\windows;z:\opt\mingw\bin' (main) lt_argv_zero : Z://lt-2.2.4-mingw-mlib/appl/.libs/foo.exe (main) newargz[0] : Z://lt-2.2.4-mingw-mlib/appl/.libs/foo.exe err:module:import_dll Library libfoo2.dll (which is needed by LZ:\\\\lt-2.2.4-mingw-mlib\\appl\\.libs\\foo.exe) not found ... The PATH contain unix instead dos path-separator. Roumen
Re: [PATCH] [mingw|cygwin] Modify cwrapper to invoke target directly.
Roumen Petrov wrote: libtool 2.2.4 patched with both patches still fail: ... (lt_setenv) setting 'PATH' to ':/usr/local/src//lt-2.2.4-mingw-mlib/lib2/.libs:/usr/local/src//lt-2.2.4-mingw-mlib/lib1/.libs:/tmp/test/pkg/lt-2.2.4-mingw-mlib/lib:/tmp/test/pkg/lt-2.2.4-mingw-mlib/bin:/usr/local/src//lt-2.2.4-mingw-mlib/lib2/.libs:/usr/local/src//lt-2.2.4-mingw-mlib/lib1/.libs:c:\windows\system32;c:\windows;z:\opt\mingw\bin' (main) lt_argv_zero : Z://lt-2.2.4-mingw-mlib/appl/.libs/foo.exe (main) newargz[0] : Z://lt-2.2.4-mingw-mlib/appl/.libs/foo.exe err:module:import_dll Library libfoo2.dll (which is needed by LZ:\\\\lt-2.2.4-mingw-mlib\\appl\\.libs\\foo.exe) not found ... The PATH contain unix instead dos path-separator. Right. With *this* patch, I do not expect improvement with cross-compiles. This is just the first step. To get cross-compiles working, I need to *extend* this change using some of the techniques demonstrated in the 'convert_mingw_paths_with_wine.sh' script -- which is the #3 attachment to this message: http://lists.gnu.org/archive/html/libtool-patches/2008-04/msg00164.html One step at a time -- Chuck
Re: [PATCH] [mingw|cygwin] Modify cwrapper to invoke target directly.
Roumen Petrov wrote: Hi Charles, About following comment: /* execv doesn't actually work on mingw as expected on unix */ Actually execXXX on microsoft windows create a new process and it is not mingw problem. What about to substitute mingw with windows ? Disagree. The FSF discourages to use the term 'windows' alone to refer to the microsoft operating system (What? GNU-Linux/KDE doesn't have any windows?) execv doesn't actually work on win32 as expected implies that cygwin also suffers from this problem, but it doesn't. Ditto ...work on Microsoft Windows as expected... While ...work as expected when using the native w32api... Is too wordy, and is probably confusing to many users -- even though it is the most accurate. (I don't understand this native w32api thing -- I'm using mingw!) ...work as expected when using the native w32api (that is, when $host is mingw)... is marginal, but even more wordy -- and will need changing AGAIN when MSVC support is added (either directly, or via one of the various wrapper script implementations being developed). Finally, a request: can we please not criticize patches for pre-existing issues in the comments or code, that are not bugs? This comment has been present over a year. With regards to my other, -std=c99 patch, Ralf made similar comments about pre-existing comments, and Eric complained about printf vs. puts -- when the original code also used printf. It's a little difficult to separate the wheat from the chaff, when many of the review comments are not specifically about the patch itself... -- Chuck
Re: [PATCH] [mingw|cygwin] Modify cwrapper to invoke target directly.
Charles Wilson wrote: Roumen Petrov wrote: libtool 2.2.4 patched with both patches still fail: ... (lt_setenv) setting 'PATH' to ':/usr/local/src//lt-2.2.4-mingw-mlib/lib2/.libs:/usr/local/src//lt-2.2.4-mingw-mlib/lib1/.libs:/tmp/test/pkg/lt-2.2.4-mingw-mlib/lib:/tmp/test/pkg/lt-2.2.4-mingw-mlib/bin:/usr/local/src//lt-2.2.4-mingw-mlib/lib2/.libs:/usr/local/src//lt-2.2.4-mingw-mlib/lib1/.libs:c:\windows\system32;c:\windows;z:\opt\mingw\bin' (main) lt_argv_zero : Z://lt-2.2.4-mingw-mlib/appl/.libs/foo.exe (main) newargz[0] : Z://lt-2.2.4-mingw-mlib/appl/.libs/foo.exe err:module:import_dll Library libfoo2.dll (which is needed by LZ:\\\\lt-2.2.4-mingw-mlib\\appl\\.libs\\foo.exe) not found ... The PATH contain unix instead dos path-separator. Right. With *this* patch, I do not expect improvement with cross-compiles. This is just the first step. To get cross-compiles working, I need to *extend* this change using some of the techniques demonstrated in the 'convert_mingw_paths_with_wine.sh' script -- which is the #3 attachment to this message: http://lists.gnu.org/archive/html/libtool-patches/2008-04/msg00164.html One step at a time -- Chuck I confirm that in cross environment (wine emulation) is enough to replace all ':' (unix) with ';' (dos) in LIB_PATH_VALUE and LIB_PATH_VALUE to get cwrapper to work. Roumen
Re: [PATCH] Ensure cwrapper compiles without warnings under -std=c99.
Hi Chuck, On 6 May 2008, at 22:47, Charles Wilson wrote: Please refer to arguments in all caps: ARG (as is done elsewhere). See above (.libs). # func_emit_wrapper [ARG=no] Somewhat confusingly, we took our inspiration from the text rendering of a gnu info document, so the line above would still be: # func_emit_wrapper [arg=no] But when *referring* to arg in the docucomment, the convention is to capitalize the reference as you have below: # ARG is the value that the WRAPPER_SCRIPT_BELONGS_IN_OBJDIR # variable will take. If 'yes', then the emitted script # will assume that the directory in which it is stored is # the $objdir directory. This is a cygwin/mingw-specific # behavior. Cheers, Gary -- ())_. Email me: [EMAIL PROTECTED] ( '/ Read my blog: http://blog.azazil.net / )= ...and my book: http://sources.redhat.com/autobook `(_~)_ PGP.sig Description: This is a digitally signed message part
Re: [PATCH] Ensure cwrapper compiles without warnings under -std=c99.
Hi Chuck, On 6 May 2008, at 22:47, Charles Wilson wrote: Unresolved: (1) whether func_emit_wrapper_part1 should even TAKE an argument (2) whether the cwrapper src, when printing a const char*, should use puts() in preference to printf(%s,...) The quality of your patches has always been exemplary, and I trust you to make a sensible decision. To prevent us all from getting caught up in a democratic committee meeting to approve the colour of the bike shed, just pick a nice colour and go ahead and paint it! I've long been of the opinion that any patch that doesn't receive negative criticism within 72 hours of posting is fair game to be applied to the repository. :-) Please go ahead and choose however you think best and then push the patch. (For the record, I personally believe that maintainability is king, and always strive to make my patches look like the code that surrounds them) Cheers, Gary -- ())_. Email me: [EMAIL PROTECTED] ( '/ Read my blog: http://blog.azazil.net / )= ...and my book: http://sources.redhat.com/autobook `(_~)_ PGP.sig Description: This is a digitally signed message part
