Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 11, 2017, 20:37 UTC
Message-ID: <xmqq4lox57c9.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171210142309.GA19453@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 26 quoted lines

> I'm not sure that's true. Look at what already goes into
> GIT-BUILD-OPTIONS: TEST_OUTPUT_DIRECTORY, GIT_TEST_CMP, GIT_PERF_*, etc.
>
> Interestingly, many of those do something like this in the Makefile:
>
>   ifdef GIT_TEST_CMP
> 	@echo GIT_TEST_OPTS=... >>$@+
>   endif
>
> which seems utterly confusing to me. Because it means that if you build
> with those options set, then they will override anything in the
> environment. But if you don't, then you _may_ override them in the
> environment. In other words:
>
>   make
>   cd t
>   GIT_TEST_CMP=foo ./t0000-*
>
> will respect that variable. But:
>
>   make GIT_TEST_CMP=foo
>   cd t
>   GIT_TEST_CMP=bar ./t0000-*
>
> will not. Which seems weird.  But I guess we could follow that pattern
> with TEST_SHELL_PATH.

Or perhaps we can start setting a better example with the new variable, and migrate those weird existing ones over to the new way of not forbidding run-time overriding?

There is a long outstanding NEEDSWORK comment in help.c that wonders if we want to embed contents from GIT-BUILD-OPTIONS in the resulting binary, and the distinction Dscho brought up between "build" and "test" phases would start to matter even more once we go in that direction.