Thread (7 messages) 7 messages, 4 authors, 2021-10-08

Re: [PATCH] t/perf: do not run tests in user's $SHELL

flat view

From: Jeff King <hidden>
Date: 2021-10-08 03:07:45

On Thu, Oct 07, 2021 at 08:47:16PM +0200, Johannes Altmanninger wrote:
The environment variable $SHELL is usually set to the user's
interactive shell. We never use that shell for build and test scripts
because it might not be a POSIX shell.

Perf tests are run inside $SHELL via a wrapper defined in
t/perf/perf-lib.sh. Use $TEST_SHELL_PATH like elsewhere.
Yes, I think this is the right thing to do. We didn't have
$TEST_SHELL_PATH back when this code was added, but I think it should
have been $SHELL_PATH from the start.

I wondered if we would always have TEST_SHELL_PATH set, but we should:
it is put unconditionally into GIT-BUILD-OPTIONS, and we will always
load that via test-lib.sh, even if the test is run outside of "make".
---

Regarding the inconsistency around $(SHELL) in Makefiles: we could do
something like

	-SHELL_PATH ?= $(SHELL)
	+SHELL_PATH ?= /bin/sh
	+SHELL = $(SHELL_PATH)

in some Makefiles. Though the upside (consistency & slightly easier to build
with broken /bin/sh) seems fairly low, so I'd leave it be.
In general assuming that $SHELL is a valid /bin/sh replacement is
questionable (e.g., if your login shell is zsh or god forbid tcsh). But
I think GNU make will set SHELL=/bin/sh, rather than pick it up from the
environment (probably for this exact reason).

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help