On Tue, Nov 30, 2021 at 04:08:48PM -0500, Jeff King wrote:
> On Mon, Nov 29, 2021 at 09:13:23PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> > Once that's been removed we can dig deeper and see that we only have
> > "BASH_XTRACEFD" due to an earlier attempt to work around the same
> > issue. See d88785e424a (test-lib: set BASH_XTRACEFD automatically,
> > 2016-05-11) and the 90c8a1db9d6 (test-lib: silence "-x" cleanup under
> > bash, 2017-12-08) follow-up.
> >
> > I.e. we had redirection in "test_eval_" to get more relevant trace
> > output under bash, which in turn was only needed because
> > BASH_XTRACEFD=1 was set, which in turn was trying to work around test
> > failures under "set -x".
> >
> > It's better if our test suite works the same way on all shells, rather
> > than getting a passing run under "bash", only to have it fail with
> > "-x" under say "dash". As the deleted code shows this is much simpler
> > to implement across our supported POSIX shells.
>
> I'm mildly negative on dropping BASH_XTRACEFD.
I agree, using BASH_XTRACEFD is the most robust (and least hacky) way
to get reliable trace from our test scripts, and we should definitely
keep it.
> IMHO it is not worth the
> maintenance headache to have to remain vigilant against any shell
> function's stderr being examined, when there is single-line solution
> that fixes everything. Yes, the cost of using bash is high on some
> platforms, but "-x" is an optional feature (though I am sympathetic to
> people who are _surprised_ when "-x" breaks things, because it really is
> a subtle thing, and knowing "you should try using bash" is not
> immediately obvious).
>
> Some folks (like Gábor) disagree and have done the work so far to make
> sure that we pass even with "-x". But it feels like this is committing
> the whole project to that maintenance. I dunno.
It's not that I disagree but rather it's in our best interest to keep
'-x' working with non-Bash shells as well, because:
- We run our tests with '-x' in CI, because we want to get as much
information out of test failures as possible.
- We run our tests with dash in the Ubuntu jobs not only because
that's the default /bin/sh, but more importantly because we want
to avoid bashisms in our test suite.
