Subject: Re: [PATCH] t0000: fix test if run with TEST_OUTPUT_DIRECTORY

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Jul 2021 09:21:10 -0700

Jeff King <[email-masked]> writes:

> On Tue, Jul 20, 2021 at 08:32:26AM +0200, Patrick Steinhardt wrote:
>
>> Fix the issue by adding a new TEST_OUTPUT_DIRECTORY_OVERRIDE variable.
>> If set, then we'll always override the TEST_OUTPUT_DIRECTORY with its
>> value after sourcing GIT-BUILD-OPTIONS.
>
> Thanks, I like this approach much better than removing
> TEST_OUTPUT_DIRECTORY entirely (and I confirmed that it fixes the
> problem).

Yup, and it combines with your subtests-skip fix rather nicely to
solve both problems.  Thanks for working well together.

> I do wish we had a more generic way of overriding stuff in
> GIT-BUILD-OPTIONS, rather than introducing manual _OVERRIDE variables
> for each. But there's not an easy solution here (see the earlier thread
> for some discussion), so this seems like a good immediate step to take.
>
> One small note on the commit message:
>
>> While this works as expected in the general case, it falls apart when
>> the developer has TEST_OUTPUT_DIRECTORY explicitly defined either via
>> the environment or via config.mak.
>
> The mention of the environment confused me for a moment, since:
>
>   TEST_OUTPUT_DIRECTORY=/tmp/foo ./t0000-basic.sh
>
> is already OK. But you probably meant that:
>
>   TEST_OUTPUT_DIRECTORY=/tmp/foo make test
>
> would fail, since "make" would pick up the variable and then write it
> into GIT-BUILD-OPTIONS (just as it would if you put it in config.mak, or
> on the command-line of make).
>
> I don't think it's sufficiently confusing to rewrite the commit message,
> but just something I noted while reading it.

I'll just insert "and runs 'make test'" after "via config.mak" while
queuing.

Again, thanks, both.