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).
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.
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -57,6 +57,15 @@ fi
> . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
> export PERL_PATH SHELL_PATH
>
> +# In t0000, we need to override test directories of nested testcases. In case
> +# the developer has TEST_OUTPUT_DIRECTORY part of his build options, then we'd
> +# reset this value to instead contain what the developer has specified. We thus
> +# have this knob to allow overriding the directory.
> +if test -n "${TEST_OUTPUT_DIRECTORY_OVERRIDE}"
> +then
> + TEST_OUTPUT_DIRECTORY="${TEST_OUTPUT_DIRECTORY_OVERRIDE}"
> +fi
Thanks for this comment. Hopefully that will make it clear to anybody
that the override mechanism is not meant for general use by developers.
-Peff
