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

From: Jeff King <peff@peff.net>
Date: Dec 10, 2017, 14:23 UTC
Message-ID: <20171210142309.GA19453@sigill.intra.peff.net>
In-Reply-To: <alpine.DEB.2.21.1.1712091443560.4318@virtualbox>

On Sat, Dec 09, 2017 at 02:44:44PM +0100, Johannes Schindelin wrote:

Show 17 quoted lines

> > > ... and we could simply see whether the environment variable
> > > TEST_SHELL_PATH (which we would set in t/Makefile from the passed-in
> > > SHELL_PATH) is set, and override it again.
> > > 
> > > I still think we can do without recording test-phase details in the
> > > build-phase (which may, or may not, know what the test-phase wants to do).
> > > 
> > > In other words, I believe that we can make the invocation you mentioned
> > > above work, by touching only t/Makefile (to pass SHELL_PATH as
> > > TEST_SHELL_PATH) and t/test-lib.sh (to override the SHELL_PATH from
> > > GIT-BUILD-OPTIONS with TEST_SHELL_PATH, if set).
> > 
> > We could do that, but it makes TEST_SHELL_PATH inconsistent with all of
> > the other config.mak variables.
> 
> It is already inconsistent with the other variables because its scope is
> the "test" phase, not the "build" phase.

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.

-Peff