-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Eric Blake wrote: > 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). > I know, too. Using echo is not me but many projects using autotools. Frankly, I did not look into autotools itself whether or not it uses echo. But it seems that many projects using autotools assumes a path does not include '\'. To avoid this, this patch is reasonable. >> >> 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. > Ah, OK. Unfortunately, however, a compiler itself cannot be located without ac_executable_extensions. Therefore, $EXEEXT cannot be set as well. I know, '/bin/sh' to /bin/sh.exe' translation also depends on ac_executable_extensions or $EXEEXT. >> >> >>>> >>>> * 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. > I admit. ^^ >> >>>> --- 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. > Ooops... That's too bad in my point of view. Hmm... >>> >>> 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. > As I said above, it's not me who using echo. ^^ >> >>>> + + # 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? Frankly, I don't know who is the original author. I know, the last porter is Andreas Buening. So I just guess he would be the author of this patch. He opened the patches. I think, copyright restriction is none. But I'm not sure. > 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. > Yes, this is exactly one of the reasons why I submit these patches. >> >> 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. > Ok. - -- KO Myung-Hun Using Mozilla SeaMonkey 2.7.2 Under OS/2 Warp 4 for Korean with FixPak #15 In VirtualBox v4.1.32 on Intel Core i7-3615QM 2.30GHz with 8GB RAM Korean OS/2 User Community : http://www.ecomstation.co.kr -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.7 (OS/2) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iD8DBQFUIPewE9YstvghgroRAnzvAJ9Gjp8V7CQrbLNV9f11y7apJ4mDMACfR4XO 3hhxJ7Zcjr7BgzVwtsyfUbA= =gGgs -----END PGP SIGNATURE-----