Re: [PATCH] Generic mechanism to detect shell features

[Eric Sunshine]

On Tue, 13 Jan 2004 00:41:51 +0100, Paolo Bonzini wrote:
> Eric, thanks for the thorough review; however the $SHELL problem is serious
> in that you cannot afford running the tests from within the current shell
> because of syntax errors. Maybe eval can help?  You need to make the parent
> shell parse the shell functions test without possibly failing.

This is easy to deal with.  If you look closely at _AS_SHELL_FN_SUPPORTED in  
my patch, you will see that I correctly handled this issue by invoking the  
shell-function test in a subshell of the currently running shell by using (  
and ).  In other words, you can do the following without crashing the  
currently running shell:

  if (run shell-function test) 2>/dev/null; then
    # success
  fi

I forgot the /dev/null redirect in my patch, but that is easily fixed.

> So my review which would simplify the code makes it incorrect. Sorry, I
> must have missed the discussion about $SHELL.

Oops.  :-)

> > 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?
> Because if $SHELL was the current shell, this algorithm and the reordering
> the shells adds an implicit "suggested" feature, i.e. that the shell be
> ksh, zsh or bash.

After putting more thought into the issue, I now understand the  
inconsistency noted above, however I am still unclear about the reordering of  
the shell list.  At least in my case, the order used by your patch will  
result in choosing an ancient zsh over a more modern bash.  It seems to me  
that the historic search order of attempting bash before ksh and zsh makes  
more sense, but perhaps I do not properly understand your reasoning for  
making this change.

-- ES