perf: fix test_export with SHELL=zsh

Commit Message

René Scharfe Oct. 2, 2021, 9:40 a.m. UTC

Unlike other shells, zsh doesn't do word-splitting on variables.  This
is documented in https://zsh.sourceforge.io/FAQ/zshfaq03.html#31.  That
breaks the perf function test_export because it uses a space-separated
variable as a poor man's array, and as a consequence p0000 fails with
"not ok 3 - test_export works".  Pass the value through an unquoted
command substitution to force word-splitting even in zsh.

Signed-off-by: René Scharfe <[e-mail]>
---
 t/perf/perf-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.33.0

Comments

Johannes Altmanninger Oct. 2, 2021, 11:26 a.m. UTC | #1

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 :)

brian m. carlson Oct. 2, 2021, 9:02 p.m. UTC | #2

On 2021-10-02 at 09:40:02, René Scharfe wrote:
> Unlike other shells, zsh doesn't do word-splitting on variables.  This
> is documented in https://zsh.sourceforge.io/FAQ/zshfaq03.html#31.  That
> breaks the perf function test_export because it uses a space-separated
> variable as a poor man's array, and as a consequence p0000 fails with
> "not ok 3 - test_export works".  Pass the value through an unquoted
> command substitution to force word-splitting even in zsh.

There are a variety of places in our testsuite where zsh is broken in
zsh mode.  I recently sent a patch to make it do the right thing for
subshells in sh mode which has not yet been released, and as far as I'm
aware, with that patch, our testsuite is fully functional with zsh when
it's run in sh mode.

Note that in sh mode, zsh enables the SH_WORD_SPLIT option, and this
should work just fine.  The easiest way to do that is to create a
symlink to zsh called "sh" and invoke that.

Patch

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index f5ed092ee5..f74cfd35d6 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -165,7 +165,7 @@  test_export () {
 '"$1"'
 ret=$?
 needles=
-for v in $test_export_
+for v in $(echo "$test_export_")
 do
 	needles="$needles;s/^$v=/export $v=/p"
 done