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