Re: [PATCH 02/12] t: prepare `test_must_fail ()` for `set -e`

[Patrick Steinhardt <ps@pks.im>]

On Tue, Apr 14, 2026 at 10:41:29AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > On Mon, Apr 13, 2026 at 11:49:23AM +0200, Patrick Steinhardt wrote:
> >
> >> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> >> index f3af10fb7e..f8bc77619b 100644
> >> --- a/t/test-lib-functions.sh
> >> +++ b/t/test-lib-functions.sh
> >> @@ -1195,8 +1195,12 @@ test_must_fail () {
> >>  		echo >&7 "test_must_fail: only 'git' is allowed: $*"
> >>  		return 1
> >>  	fi
> >> -	"$@" 2>&7
> >> -	exit_code=$?
> >> +	if "$@" 2>&7
> >> +	then
> >> +		exit_code=0
> >> +	else
> >> +		exit_code=$?
> >> +	fi
> >
> > One subtle interaction here is that the command in "$@" will be run with
> > "set -e" suppressed if it's a shell function (even if the body
> > explicitly says "set -e").
> >
> > I think it's mostly academic since we don't tend to use "set -e" in the
> > tests at all, but it may be an eventual gotcha.
> 
> Yeah, ugly, but this is tricky that they felt the need to explain it
> with an example in the informative section X-<.
> 
> "set" is described in
> 
>   https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_26
> 
> and the description for "-e" does not even talk about any function.
> The application usage notes associated with the "set" gives this
> 
>     Application writers should avoid relying on set -e within
>     functions. For example, in the following script:
> 
>     set -e
>     start() {
>         some_server
>         echo some_server started successfully
>     }
>     start || echo >&2 some_server failed
> 
>     the -e setting is ignored within the function body (because the
>     function is a command in an AND-OR list other than the
>     last). Therefore, if some_server fails, the function carries on to
>     echo "some_server started successfully", and the exit status of the
>     function is zero (which means "some_server failed" is not output).
> 
> which greatly helps.

The funny thing is that I discovered shells where this is not the case,
and `set -e` _does_ extend into functions. Anyway, I think I'll follow
Peff's suggestion and just enable `set -e` for Bash 5 and newer. This
should ensure that the behaviour is a lot more uniform without having
negative fallout on other platforms that don't use Bash, but we'd still
get a failing CI as we use Bash in some of the jobs.

Patrick