Re: [PATCH] perf: fix test_export with SHELL=zsh

From: Johannes Altmanninger <aclopte@gmail.com>
Date: Oct 2, 2021, 11:26 UTC
Message-ID: <20211002112640.hrn2ojndhoa2dd4c@gmail.com>
In-Reply-To: <8b70d04f-0ad1-6e68-f5a2-2d8ec3bb98ea@web.de>

On Sat, Oct 02, 2021 at 11:40:02AM +0200, René Scharfe wrote:

> Pass the value through an unquoted command substitution to force
> word-splitting even in zsh.

This seems like the wrong direction. There are probably other places where we rely on such POSIX features. Also it doesn't fix the case where users use other non-POSIX shells, say

	SHELL=/bin/fish t/perf/run p0000-perf-lib-sanity.sh

We already have solutions in place - our top-level Makefile has

	# Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
	...
	ifndef SHELL_PATH
		SHELL_PATH = /bin/sh
	endif
	...
	SHELL = $(SHELL_PATH)

Meanwhile t/Makefile uses

	SHELL_PATH ?= $(SHELL)
	TEST_SHELL_PATH ?= $(SHELL_PATH)

A related fix is to make those two consistent. I would prefer the version in the top-level Makefile, because t/Makefile gives the false illusion that $SHELL is honored:

	SHELL=garbage make -C t t4211-line-log.sh # succeeds because make sets SHELL=/bin/sh
	make SHELL=garbage -C t t4211-line-log.sh # fails

---

Anyway, for this issue we should just have t/perf/perf-lib.sh follow our convention of using ${SHELL_PATH:-/bin/sh} instead of $SHELL.

Looks like t/interop/Makefile has the same problem.

I'll prepare the patches later unless someone beats me to it :)