Re: [PATCH 6/6] use print or printf or cat as $ECHO (really)
Hello again, * Paolo Bonzini wrote on Mon, Nov 10, 2008 at 02:16:03PM CET: * libltdl/m4/libtool.m4 (LT_INIT): Add _LT_SHELL_INIT to work around Autoconf 2.64 bug. (_LT_OUTPUT_LIBTOOL_COMMANDS_INIT): Add func_fallback_echo. Eliminate lt_ECHO requoting. (_LT_SHELL_INIT): Use a public M4sh diversion. (_LT_PROG_ECHO_BACKSLASH): Rewrite. (LT_CMD_MAX_LEN): Do not use --fallback-echo. * libltdl/config/ltmain.m4sh: Remove --no-reexec and --fallback-echo handling. (func_fallback_echo): New. (func_emit_wrapper_part1): Quote ECHO. Remove --no-reexec and --fallback-echo handling. (Execute mode): Do not set qecho. --- a/libltdl/m4/libtool.m4 +++ b/libltdl/m4/libtool.m4 [...] +m4_ifdef([_AS_DETECT_SUGGESTED], +[_AS_DETECT_SUGGESTED([ + ECHO='\\\' + ECHO=$ECHO$ECHO$ECHO$ECHO$ECHO + ECHO=$ECHO$ECHO$ECHO$ECHO$ECHO$ECHO + PATH=/tmp/nonexistent; export PATH; + FPATH=$PATH; export FPATH; + test X`printf %s $ECHO` = X$ECHO])]) IIUC then this code has two problems: first, it changes PATH and FPATH for any _AS_DETECT_SUGGESTED shell snippets expanded right after this one. Second, it uses an unsafe replacement path (this problem stems from before your patch). I propose two alternative patches to fix these issues. Both, as the code before the patch, assume that `test' is a built-in command. Yuck. The first patch further assumes that, since `test' is not a special built-in, the shell will take care to change the variables for the duration of the following command only. Do we have reason to believe this not to be portable? In case of doubt, I guess the second approach using a subshell should be feasible. Another idea would be to save and restore the variables, which incurs the obvious complications for treating unset variables. Thoughts? Thanks, Ralf * libltdl/m4/libtool.m4 (_LT_PROG_ECHO_BACKSLASH): Fix test to not influence further tests registered with _AS_DETECT_SUGGESTED and do not use a PATH below /tmp, to avoid influence by a third party. diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4 index 7fbf965..78370b1 100644 --- a/libltdl/m4/libtool.m4 +++ b/libltdl/m4/libtool.m4 @@ -1117,9 +1117,7 @@ m4_ifdef([_AS_DETECT_SUGGESTED], ECHO='\\\' ECHO=$ECHO$ECHO$ECHO$ECHO$ECHO ECHO=$ECHO$ECHO$ECHO$ECHO$ECHO$ECHO - PATH=/tmp/nonexistent; export PATH; - FPATH=$PATH; export FPATH; - test X`printf %s $ECHO` = X$ECHO])]) + PATH=/empty FPATH=/empty test X`printf %s $ECHO` = X$ECHO])]) _LT_DECL([], [SHELL], [1], [Shell to use when invoking shell scripts]) _LT_DECL([], [ECHO], [1], [An echo program that protects backslashes]) diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4 index 7fbf965..548863c 100644 --- a/libltdl/m4/libtool.m4 +++ b/libltdl/m4/libtool.m4 @@ -1117,9 +1117,8 @@ m4_ifdef([_AS_DETECT_SUGGESTED], ECHO='\\\' ECHO=$ECHO$ECHO$ECHO$ECHO$ECHO ECHO=$ECHO$ECHO$ECHO$ECHO$ECHO$ECHO - PATH=/tmp/nonexistent; export PATH; - FPATH=$PATH; export FPATH; - test X`printf %s $ECHO` = X$ECHO])]) + ( PATH=/empty FPATH=/empty; export PATH FPATH +test X`printf %s $ECHO` = X$ECHO )])]) _LT_DECL([], [SHELL], [1], [Shell to use when invoking shell scripts]) _LT_DECL([], [ECHO], [1], [An echo program that protects backslashes])
Re: [PATCH 6/6] use print or printf or cat as $ECHO (really)
IIUC then this code has two problems: first, it changes PATH and FPATH for any _AS_DETECT_SUGGESTED shell snippets expanded right after this one. Second, it uses an unsafe replacement path (this problem stems from before your patch). Oops, I remembered that each snippet expanded to a subshell, but that's only true of the entire suggested-tests. I propose two alternative patches to fix these issues. Both, as the code before the patch, assume that `test' is a built-in command. Yuck. The first patch further assumes that, since `test' is not a special built-in, the shell will take care to change the variables for the duration of the following command only. Do we have reason to believe this not to be portable? If it works on Solaris... Paolo
Re: [PATCH 6/6] use print or printf or cat as $ECHO (really)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Ralf Wildenhues on 11/22/2008 6:57 AM: I propose two alternative patches to fix these issues. Both, as the code before the patch, assume that `test' is a built-in command. Yuck. But reasonably safe; per http://www.in-ulm.de/~mascheck/bourne/, all System III shells provide this. + PATH=/empty FPATH=/empty test X`printf %s $ECHO` = X$ECHO])]) Your attempt to modify PATH in-place won't work. Even with bash 3.2: $ foo=bar echo `env | grep foo` $ In other words, although echo sees the altered $foo, the `` subshell does not. + ( PATH=/empty FPATH=/empty; export PATH FPATH +test X`printf %s $ECHO` = X$ECHO )])]) Yes, that looks more portable (even if it is slower). But do we even need to _AS_DETECT_SUGGEST for a working builtin printf? As long as we search for 'print -r' before 'printf', we are either guaranteed to find the working builtin variant, or we end up resorting to forking no matter which alternative we choose. The primary use case where this will help is with Solaris /bin/sh, which lacks both builtins, so we really _do_ want to suggest a shell that is more likely to provide a builtin. But on a Solaris machine, ksh fails the suggested test (it lacks builtin printf), so ksh is not determined to be better than /bin/sh, and you end up running with /bin/sh and forking for every $ECHO anyway. Maybe a better suggested test would be one that checks for either print or printf (that way, a Solaris machine will let ksh pass the suggested test). Since the overall test is running with stderr silenced, we don't even have to worry about messages about command not found. In other words, why not: _AS_DETECT_SUGGESTED([ ECHO='\\\' ECHO=$ECHO$ECHO$ECHO$ECHO$ECHO ECHO=$ECHO$ECHO$ECHO$ECHO$ECHO$ECHO test X`print -r -- $ECHO` = X$ECHO \ || test X`printf %s $ECHO` = X$ECHO]) - -- Don't work too hard, make some time for fun as well! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkkoF64ACgkQ84KuGfSFAYAGRgCeM3rJDiTmFgdSSOu74dwNqpd+ CmkAnR++b8sDGKRxrWW+ovGcgZqcCJoU =GuTz -END PGP SIGNATURE-
Re: [PATCH 6/6] use print or printf or cat as $ECHO (really)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 11/22/2008 7:31 AM: Maybe a better suggested test would be one that checks for either print or printf (that way, a Solaris machine will let ksh pass the suggested test). Since the overall test is running with stderr silenced, we don't even have to worry about messages about command not found. In other words, why not: test X`print -r -- $ECHO` = X$ECHO \ || test X`printf %s $ECHO` = X$ECHO Scratch that; Solaris /bin/sh passes that test, so we wouldn't favor ksh. It really boils down to finding a shell with either print or a builtin printf, so I think we have to play the PATH games, and expend the extra fork in looking :( At least we can hard-code the fact that ZSH_VERSION or BASH_VERSION implies a builtin printf, to skip the forks on those shells. - -- Don't work too hard, make some time for fun as well! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkkoGQ8ACgkQ84KuGfSFAYBUUACfSnEOeNxt190rKmNMRWpHbd2x 560AoK3c92s6Czz2tCIyyUH3oQAywo5m =8rWl -END PGP SIGNATURE-
