On Fri, Nov 19 2021, Fabian Stelzer wrote:

On 19.11.2021 15:26, Ævar Arnfjörð Bjarmason wrote:

quoted

On Fri, Nov 19 2021, Fabian Stelzer wrote:

quoted

On 19.11.2021 12:13, Ævar Arnfjörð Bjarmason wrote:

quoted

On Wed, Nov 17 2021, Fabian Stelzer wrote:

quoted

In certain environments or for specific test scenarios we might expect a
specific prerequisite check to succeed. Therefore we would like to
trigger an error when running our tests if this is not the case.

trigger an error but...

quoted

To remedy this we add the environment variable GIT_TEST_REQUIRE_PREREQ
which can be set to a comma separated list of prereqs. If one of these
prereq tests fail then the whole test run will abort.

..here it's "abort the whole test run". If that's what you want use
BAIL_OUT, not error. See: 234383cd401 (test-lib.sh: use "Bail out!"
syntax on bad SANITIZE=leak use, 2021-10-14)

Hm, while testing this change i noticed another problem that i really
have no idea how to fix.
When a test uses test_have_prereq then the error/BAIL_OUT message will only be printed
when run with '-v'. This is not the case when the prereq is specified
in the test header. The test run will abort, but no error will be
printed which can be quite confusing :/
I guess this has something to do with how tests are run in subshells and
their outputs only printed with -v. Maybe there should be some kind of
override for BAIL_OUT at least? Not sure if/how this could be done.

It has to do with how we juggle file descriptors around, see test_eval_
in test-lib.sh.

So the "real" stdout is fd 5, not 1 when you're in a prereq.

Just:

   BAIL_OUT "bad" >&5

Will work, maybe it's a good idea to have:

BAIL_OUT_PREREQ () {
	BAIL_OUT $@ >&5
}

Sorry, I forgot about that caveat when suggesting it.

Hm. Any reason to not do this in BAIL_OUT itself?  As far as i can see
the setup of the additional fd's would only need to move up a few lines.

That does look like a better solution, I've tried it just now locally &
it works for me. Perhaps there's some subtlety I'm missing, but that
should Just Work.

This is by far not the first time I've poked at something in test-lib.sh
only to discover that its pattern of doing setup A, setup C, setup B
etc. caused a problem solved by moving B & C around :(

It could really do with a change to move everything it's now doing to
functions, which we'd then call, so what setup we do in what order would
fit on a single screen, but that's a much larger change...