[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Generic mechanism to detect shell features
From: |
Eric Sunshine |
Subject: |
Re: [PATCH] Generic mechanism to detect shell features |
Date: |
Mon, 12 Jan 2004 12:13:16 -0500 |
Hello,
On Mon, 12 Jan 2004 14:21:37 +0100, bonzini wrote:
> This patch adds to m4sh a generic mechanism to detect required or
> desirable shell features, and when looping over the shells, avoid shells
> that do not require the former and prefer shells that have the latter.
> The idea is to pass tests to AS_DETECT_REQUIRED and AS_DETECT_SUGGESTED:
> for now both $LINENO tests and shell functions tests go into
> AS_DETECT_SUGGESTED, but as soon as they will be used functions will
> automatically be required.
The notion of a generic detection mechanism is a nice one. Unfortunately, I
see a number of problems with the patch, some of which are fatal.
> +m4_define([_AS_DETECT_REQUIRED_BODY], [])
> +m4_define([_AS_DETECT_REQUIRED_FEATURES],
> +[{ _AS_QUOTE([$1]) <<\_ASEOF
> +_AS_BOURNE_COMPATIBLE
> +_AS_DETECT_REQUIRED_BODY
> +_ASEOF
> +} >/dev/null 2>&1])
Problem 1: When there are no tests in _AS_DETECT_REQUIRED_BODY (which is the
case presently), then this returns the result of whatever the last command
in _AS_BOURNE_COMPATIBLE returns. This is highly dangerous since it creates
a very tight coupling between _AS_BOURNE_COMPATIBLE and
_AS_DETECT_REQUIRED_FEATURES. If someone makes a change to
_AS_BOURNE_COMPATIBLE which causes 'false' to be returned from its last
command, then _AS_DETECT_REQUIRED_FEATURES will start failing mysteriously.
To fix, you should ensure that _AS_DETECT_REQUIRED_FEATURES results in 'true'
when _AS_DETECT_REQUIRED_BODY contains no tests.
> +# _AS_DETECT_BETTER_SHELL
> + AS_IF([_AS_DETECT_REQUIRED_FEATURES([$SHELL])], [as_have_required=yes],
Problem 2: This code is still making the fundamentally flawed assumption
that SHELL is the shell running the script. This very frequently is not the
case. Worse, it is incorrectly applying the results of the SHELL tests to
the shell running the script.
Keep in mind that SHELL is imported from the environment, it is not
typically set automatically to the shell which is running the script. I gave
a couple examples in previous emails illustrating why you can not rely upon
SHELL being the shell running the script. For instance, in my environment,
SHELL is set to tcsh, yet when I run the configure script it runs via /bin/sh
on account of the #!/bin/sh. Another example is that someone might have
SHELL set to /usr/local/bin/bash, yet the shell running the script will still
(by default) be /bin/sh (because of #!/bin/sh). (Paul Jarc was nice enough
to do some testing, and he discovered that only bash and Solaris ksh will set
SHELL automatically to the running shell, but _only_ if SHELL is not already
present in the environment.)
The upshot is that you simply can not rely upon SHELL being the shell which
is presently running the script. Consequently, the feature detection code
needs to be reorganized (and possibly refactored) so that it first tests the
currently running shell, then SHELL, and finally the shells discovered by the
more exhaustive search. (This is the approach I took with the patch I
submitted.)
> AS_IF([test $as_have_required = yes &&
> _AS_DETECT_SUGGESTED_FEATURES([$SHELL])],
> + [CONFIG_SHELL="$SHELL"],
> + [_AS_PATH_WALK([...],
> + for as_base in sh sh5 ksh zsh bash; do
> + if test -f $as_dir/$as_base; then
> + AS_IF([_AS_DETECT_REQUIRED_FEATURES([$as_dir/$as_base])],
> + [CONFIG_SHELL=$as_dir/$as_base
Problem 3: Inconsistency. The code sets CONFIG_SHELL to SHELL if SHELL
passes _both_ required and suggested feature tests; yet it sets CONFIG_SHELL
to one of the other shells (sh, sh5, ksh, zsh, bash) if it passes only the
required feature test. Why the inconsistency? Why are the requirements
placed upon SHELL more strict than the requirements for the other shells?
Aside: I see that you changed the order of the shell tests to "sh sh5 ksh
zsh bash". Historically, it has been "sh bash ksh sh5". Is this
intentional? My own experience is that the historical search order gives
better results.
> + as_have_required_yes
Problem 4: Syntax error. I assume that you meant "as_have_required=yes".
> + if test $as_have_required = no; then
> + echo This script requires a shell more modern than all the
> + echo shells that I found on your system. Please install a
> + echo modern shell, or manually run the script under such a
> + echo shell if you do have one.
> + AS_EXIT(1)
> + fi
Problem 5: Because of the way this is nested inside the 'if' block, this
error message will be emitted only if none of the externally discoverd shells
(sh bash ksh sh5) support the required features. It will _fail_ to emit
this message for SHELL if SHELL does not implement the required features,
since the results of the SHELL test are incorrectly applied to the running
shell, as discussed for problem #2.
> + if test "$SHELL" != "$CONFIG_SHELL"; then
> + AS_UNSET([ENV])
> + AS_UNSET([BASH_ENV])
> + export CONFIG_SHELL
> + exec "$CONFIG_SHELL" "$as_myself" ${1+"address@hidden"}
> + fi])
Problem 6: This is invalid logic, as discussed for problem #2. SHELL is not
guaranteed to be the running shell, thus this test is incorrect. Worse, if
SHELL does support required and suggested features, but the running shell
does not, then the script will not re-exec with SHELL, but will instead
continue running under the crippled shell.
> # _AS_LINENO_PREPARE
> m4_define([_AS_LINENO_PREPARE],
> [AS_REQUIRE([_AS_CR_PREPARE])dnl
> +AS_DETECT_SUGGESTED([_AS_LINENO_WORKS])
Problem 7: I have not yet tracked down the exact cause of this problem, but
in the generated configure script, wherever _AS_DETECT_SUGGESTED_FEATURES()
has been invoked, the _AS_LINENO_WORKS test is emitted twice. For instance,
in the generated configure script, I see this:
{ {
as_lineno_1=$LINENO
...
} } || { { (exit 1); exit 1; } }
{ {
as_lineno_1=$LINENO
...
} } || { { (exit 1); exit 1; } }
_ASEOF
The _AS_SHELL_FN_WORK test, on the other hand, is only emitted once per
invocation of _AS_DETECT_SUGGESTED_FEATURES(), which is correct.
> +# AS_SHELL_FN_SPY
> +m4_define([AS_SHELL_FN_SPY],
> +{ $SHELL 2> /dev/null <<\_ASEOF
> +_AS_SHELL_FN_WORK
> +_ASEOF
> +} || {
> + echo No shell found that supports shell functions.
> +}])])
Problem 8: As with problem #2, this is making the invalid assumptiong that
SHELL is the running shell. This "spy" is supposed to be reporting that the
running shell does not support functions, but instead it is reporting that
SHELL does not support functions.
> # AS_INIT(WHAT-IF-SHELL-FUNCTIONS-DO-NOT-WORK)
> # --------------------------------------------
> # The parameter is temporary; it will go away and you
You forgot to update the prototype and comment after eliminating the
argument from AS_INIT.
-- ES
Re: [PATCH] Generic mechanism to detect shell features,
Eric Sunshine <=