On 09/22/2014 08:39 PM, KO Myung-Hun wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hi/2. > > Eric Blake wrote: >> On 09/22/2014 12:59 AM, KO Myung-Hun wrote: >>> \ may be recognized as an escape character on some shells such >>> as pdksh. And the executables on OS/2 have .exe as an extension. >> >> Umm, \ is an escape character on ALL sh-related shells. And .exe >> handling on OS/2 should behave as it does on mingw. >> > > Sorry, my change log was not enough somewhat. I meant 'echo'. At > least, echo of pdksh treats \ as an escape char without -E.
Yes, passing \ through echo is non-portable, and you must use printf instead of echo if the string to be echoed is likely to contain a backslash (in addition to the shell quoting rules that backslash has outside of echo). > > How does mingw handle .exe ? Configure probes early on if .exe is produced when compiling an executable, and sets $EXEEXT accordingly. But in many cases, executing '/bin/sh' is automatically translated into '/bin/sh.exe' without the user having to explicitly request .exe as part of the executable name. Maybe I'm missing a subtlety of OS/2 and what is automated vs. manually required, but giving more details will only make your case for this patch stronger. > > >>> >>> * lib/autoconf/general.m4 (AC_SITE_LOAD): Convert \ in PATH to >>> /. Add .exe to ac_executable_extensions. >> >> This says what you changed, but not why. A good commit message >> gives rationale on WHY the change is important, such as a >> demonstration of what goes wrong without the patch. >> > > I thought I explained WHY above message. But you never mentioned that 'echo' was at fault. > >>> --- lib/autoconf/general.m4 | 28 ++++++++++++++++++++++++++++ 1 >>> files changed, 28 insertions(+), 0 deletions(-) >>> >>> diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4 >>> index 77f71d2..5a87d5e 100644 --- a/lib/autoconf/general.m4 +++ >>> b/lib/autoconf/general.m4 @@ -1951,6 +1951,34 @@ do || >>> AC_MSG_FAILURE([failed to load site script $ac_site_file]) fi >>> done + +if test -n "$OS2_SHELL"; then + # Backslashes into >>> forward slashes: + # The following OS/2 specific code is >>> performed AFTER config.site + # has been loaded to allow users >>> to change their environment there. + # This strange code is >>> necessary to deal with handling of backslashes by + # ksh. + >>> ac_save_IFS="$IFS" + IFS="\\" + ac_TEMP_PATH= + for ac_dir in >>> $PATH; do + IFS=$ac_save_IFS + if test -z "$ac_TEMP_PATH"; >>> then + ac_TEMP_PATH="$ac_dir" + else + >>> ac_TEMP_PATH="$ac_TEMP_PATH/$ac_dir" + fi + done + export >>> PATH="$ac_TEMP_PATH" + unset ac_TEMP_PATH Your email client is horribly botching quoting. >> >> It looks like this is an (overly-complex) way of converting all \ >> in $PATH into / before proceeding. But why is it necessary? >> > > As I said above, without this, echoing components of PATH may be > corrupted. For examples, x:\usr\bin will be x:\usin on pdksh. Only if you do something non-portable like 'echo "$PATH"'. If you do 'printf %s\\n "$PATH"', the problem goes away. > >>> + + # add .exe to ac_executable_extensions + if test -z >>> "$ac_executable_extensions"; then + >>> AC_MSG_WARN([ac_executable_extensions not set, assuming .exe]) + >>> fi + ac_executable_extensions="$ac_executable_extensions .exe" + >>> export ac_executable_extensions >> >> Why is the existing code that sets ac_executable_extensions not >> sufficient? > > What is the existing code ? Anyway without this, > ac_executable_extensions is not set at all. > >> And why do you have to export it into the environment of child >> processes? > > I just preserved the old codes from OS/2 fork if possible. If it is > not needed, I'll remove it. Well, just reposting an old patch calls into play other questions - are you the original author of the patch, and if not, are there any copyright restrictions that would prevent us from applying the patch? Also, just because a downstream fork wrote a patch does not mean it was the ideal patch; discussing the issues in the open can often lead to a better understanding of the real issue and a better patch. > > This might be better as two separate patches, since it >> is doing two unrelated changes. >> > > I thought both these were OS/2 init codes. Anyway, I'll split. They are both related to OS/2, but tackling different items. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature