On Sat, Sep 05, 2015 at 10:36:29AM -0700, Junio C Hamano wrote: > On Sat, Sep 5, 2015 at 6:12 AM, John Keeping <john@xxxxxxxxxxxxx> wrote: > > > > I don't think it's worth trying to clear $BASH_SUBSHELL before the tests > > start because to do so we have to reliably detect that we're not running > > under Bash, and if we don't trust people not to set $BASH_SUBSHELL why > > do we trust them not to set $BASH? > > I am not worried about evil people who do funny things to deliberately break > other people's arrangement. I am more worried about stupid people (e.g. those > who export CDPATH). > > In bash a stupid person may attempt to export BASH_SUBSHELL and then > have a script that runs our test suite, setting SHELL_PATH to point at a > non-bash while building Git and running the tests under a non-bash shell. I > am hesitant to believe that we will know the variable will never leak through > to the test via environment. > > Isn't it just the matter of resetting the variable regardless of $BASH > (and ignoring > a possible refusal to do so under bash) at the beginning of the test, or do you > really have to rely on the value of $BASH and do things differently? Bash doesn't refuse to set it, it lets you update the value; I did think that it wouldn't update it if the user had overridden the value, but it looks like that was only because I had unset it first. It seems that the variable is magic (autoincrementing in subshells and can only be set to integer values) but if you unset it then it becomes a normal variable. I'm wary of relying on that behaviour being unchanged across all versions of bash, so I'd prefer to avoid writing the variable if running under bash. We do already have t/lib-bash.sh which has an optimization to avoid exec'ing bash if "$BASH" is set, which will break in the same way if someone exports BASH and then runs t9902 or t9903 under non-bash. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html