On Mon, Dec 13, 2021 at 02:38:36AM +0100, Ævar Arnfjörð Bjarmason wrote: > Stop setting "BASH_XTRACEFD=4" to direct "-x" output to file > descriptor 4 under bash. > > When it was added in d88785e424a (test-lib: set BASH_XTRACEFD > automatically, 2016-05-11) it was needed as a workaround for various > tests that didn't pass cleanly under "-x". > > Most of those were later fixed in 71e472dc43 (Merge branch > 'sg/test-x', 2018-03-14), and in the preceding commits we've fixed the > final remaining and removed the "test_untraceable" facility. Those commits made the test suite pass with '-x' without BASH_XTRACEFD only when all went well, but during development that's often not the case. So let's not forget about c5c39f4e34 (test-lib: fix interrupt handling with 'dash' and '--verbose-log -x', 2019-03-13), before which dash was not really suitable to run tests involving daemon processes with '-x' during development. If dash were to announce redirections in its '-x' trace, like many not as quite as popular shells do, then the workaround in that commit wouldn't work at all. In general, between POSIX leaving a lot of things explicitly unspecified, or, worse, silently unspecified, shells not conforming to POSIX, being buggy, and/or implementing their own extensions, I am actually quite surprised that it works as well as it does with so many shell. At least as far as we know it, and I wouldn't at all be surprised if there were unknown issues lurking in some corner cases and/or with some more exotic shells. > We could retain "BASH_XTRACEFD=4" anyway, but doing so is bad because: > > 1) Since we're caring about "-x" passing in CI under "dash" on Ubuntu > using "BASH_XTRACEFD=4" will amount to hiding an error we'll run > into eventually. Tests will pass locally with "bash", but fail in > CI with "dash" (or under other non-"bash" shells). This is not "bad", this is exactly what we use CI for. This is the smae case as when the test suite passes on a developer's Linux box, but breaks on OSX or Windows in CI. Furthermore, while I fully agree that keeping the whole test suite passing with '-x' without BASH_XTRACEFD is desirable, I do think it's a bad idea to forbid developers from using it while hacking away to scratch their itches. I for one sometimes deliberately use various bashisms in my tests, including 'test_cmp'-ing the stderr of loops and functions, because they make writing tests then and there easier, when at that point I'd rather focus my attention on getting the C changes right, and clean them up eventually when I deem the changes worthy for submission. Overall I consider this patch as a cleanup solely for cleanup's sake, without any benefits at all. I'm kind of low on time myself as well, at least to argue about this any further. Therefore, as the one who did the vast majority of work to make '-x' work even without BASH_XTRACEFD, I leave here my firm: Not-Acked-By: SZEDER Gábor <szeder.dev@xxxxxxxxx> to any patch that attempts to remove support for BASH_XTRACEFD.
