Re: [PATCH 2/3] t5615: avoid re-using descriptor 4
- From
Stefan Beller <sbeller@google.com>
- Date
- Oct 20, 2017, 21:27 UTC
- Message-ID
- <CAGZ79kaSu+nHHntFPWbkXk1TC4ys9htesRg9HHXcpK+Y6MwZeg@mail.gmail.com>
- In-Reply-To
- <20171020055058.s5dgxyfjnqtlxv4d@sigill.intra.peff.net>
On Thu, Oct 19, 2017 at 10:50 PM, Jeff King <peff@peff.net> wrote:
Show 65 quoted lines
> On Thu, Oct 19, 2017 at 07:23:37PM -0400, Jeff King wrote:
>
>> So one trick is that we can't just set it to a higher number. We have to
>> also open and manage that descriptor. It might be enough to do:
>>
>> if test -n "$BASH_VERSION"
>> then
>> exec 999>&4
>> BASH_XTRACEFD=999
>> fi
>> [...]
>> I think it might be workable, but I'm worried we're opening a can of
>> worms. Or continuing to dig into the can of worms from the original
>> BASH_XTRACEFD, if you prefer. :)
>
> So this is the best I came up with (on top of my other patches for
> ease of reading, though I'd re-work them if this is the route we're
> going to go):
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 14fac6d6f2..833ceaefd2 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -377,7 +377,12 @@ fi
> #
> # Note also that we don't need or want to export it. The tracing is local to
> # this shell, and we would not want to influence any shells we exec.
> -BASH_XTRACEFD=4
> +if test -n "$BASH_VERSION"
> +then
> + exec 999>&4
> + BASH_XTRACEFD=999
> + silence_trace="999>/dev/null"
> +fi
>
> test_failure=0
> test_count=0
> @@ -627,14 +632,16 @@ test_eval_ () {
> # be _inside_ the block to avoid polluting the "set -x" output
> #
>
> - test_eval_inner_ "$@" </dev/null >&3 2>&4
> - {
> - test_eval_ret_=$?
> - if want_trace
> - then
> - set +x
> - fi
> - } 2>/dev/null 4>&2
> + eval '
> + test_eval_inner_ "$@" </dev/null >&3 2>&4
> + {
> + test_eval_ret_=$?
> + if want_trace
> + then
> + set +x
> + fi
> + } 2>/dev/null '"$silence_trace"'
> + '
>
> if test "$test_eval_ret_" != 0 && want_trace
> then
>
> I really hate that extra eval, since it adds an extra layer of "+" to
> the tracing output in bash...
> So I dunno. It does solve the problem in a way that the individual test > scripts wouldn't have to care about. But it's a lot of eval trickery. >
I agree. Maybe just stick with the original patch?
Thanks, Stefan