On 06 Jan 2004 12:34:42 -0800, Paul Eggert wrote: > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > > In the case when $SHELL does not support functions, > > AS_SHELL_SANITIZE() reports that it could not find an appropriate > > shell even if it actually locates such a shell (which it assigns to > > CONFIG_SHELL). > Hmm, how could it assign to CONFIG_SHELL and then report that it > couldn't find a shell? The code does this: > CONFIG_SHELL=$as_dir/$as_base > export CONFIG_SHELL > exec "$CONFIG_SHELL" "$as_myself" ${1+"$[@]"} > so if it assigns to CONFIG_SHELL, it execs $CONFIG_SHELL, which means > it shouldn't report that it can't find a shell. Indeed, that's the way it is supposed to work, but that's not the way it is presently coded up. If you trace through it, you will see the flaw. Here is what the code is actually doing (pseudo-code): if $SHELL supports functions # Yay! else case $CONFIG_SHELL in '') CONFIG_SHELL=try_to_find_suitable_shell() if CONFIG_SHELL is set exec $CONFIG_SHELL $this_script endif ;; *) echo "Failed to find suitable shell" <<-- WRONG! ;; esac endif Assuming the $SHELL test fails, the first time through CONFIG_SHELL is not set, so it tries to find a suitable shell. If it finds one, then it exports CONFIG_SHELL re-invokes the script. Now, the second time through, CONFIG_SHELL is set, so it falls down to the '*' case which incorrectly prints an error saying that the shell was not found (even though it was). One way to correct the logic is as follows: case $CONFIG_SHELL in CONFIG_SHELL=try_to_find_suitable_shell() if CONFIG_SHELL is set exec $CONFIG_SHELL $this_script else echo "Failed to find suitable shell" <<-- CORRECT endif ;; *) ;; esac This way, we emit the error message iff if we actually fail to find a suitable shell. > Under the proposed patch, 'configure' would sometimes ignore > CONFIG_SHELL and substitute its own CONFIG_SHELL. This doesn't seem > right to me, as the user shouldn't be second-guessed. I also came to this conclusion after sending out the patch and decided to send a simpler patch which still allows the user to shoot himself in the foot if he so desires. > Yes, this looks like a problem to me too. How about this patch instead? No, that patch does not work either. It also incorrectly complains that it could not find a shell even when it does find one. I have included below two separate super simple patches, each of which corrects the logic. The second of the two patches is lengthier, but it is also much simpler to understand since it doesn't suffer from the convolusions which gave rise to this bug in the first place. Choose the patch which you like best. > (I don't know what the "In the future" comment means, so I removed it.) I presume that Paolo could explain it since he wrote it. Eric 2004-01-06 Eric Sunshine <sunshine@xxxxxxxxxxxxxx> * lib/m4sugar/m4sh.m4 (AS_SHELL_SANITIZE): Fixed bogus error reporting logic. If $SHELL did not support shell functions, and if it found a shell which did support functions, it would complain that it failed to find such a shell (backward logic). Worse, it did not complain if it failed to find a suitable shell. Now it complains iff it fails to find a function-supporting shell. >>> option 1 <<< --- lib/m4sugar/m4sh.m4 Tue Jan 6 18:39:20 2004 +++ lib/m4sugar/m4sh.m4-fix Tue Jan 6 18:37:32 2004 @@ -264,9 +264,11 @@ exec "$CONFIG_SHELL" "$as_myself" ${1+"$[@]"} ]);; esac - done]);; - *) + done]) + # If we got this far, we failed to find a function-supporting shell. $1;; + *) + ;; esac ]) >>> option 1 <<< --- lib/m4sugar/m4sh.m4 Tue Jan 6 19:04:41 2004 +++ lib/m4sugar/m4sh.m4-fix2 Tue Jan 6 19:04:26 2004 @@ -249,9 +249,8 @@ fi dnl In the future, the `else' branch will be that in AS_INIT_WITH_SHELL_FN. -AS_IF([_AS_SHELL_FN_WORK([$SHELL])], [], [ - case $CONFIG_SHELL in - '') +AS_IF([test -z "$CONFIG_SHELL"], + [AS_IF([_AS_SHELL_FN_WORK([$SHELL])], [], [ _AS_PATH_WALK([/bin$PATH_SEPARATOR/usr/bin$PATH_SEPARATOR$PATH], [for as_base in sh bash ksh sh5; do case $as_dir in @@ -264,11 +263,9 @@ exec "$CONFIG_SHELL" "$as_myself" ${1+"$[@]"} ]);; esac - done]);; - *) - $1;; - esac -]) + done]) + $1 +])]) # Work around bugs in pre-3.0 UWIN ksh. $as_unset ENV MAIL MAILPATH