[PATCH 0/2] Test fixes when running with zsh in sh mode

[PATCH 0/2] Test fixes when running with zsh in sh mode

[brian m. carlson]

zsh is a common shell on a variety of systems, primarily for interactive
use.  However, it can be used as a POSIX sh on Debian, macOS, and on other
systems where /bin/sh is not POSIX compliant or doesn't provide the
local keyword as we require from our shell.

This series fixes two test failures with zsh when run in sh mode; that
is, the mode that occurs when the shell is invoked as "sh".  sh mode
provides the most POSIX-compliant behavior and avoids a lot of
incompatibilities that the default zsh mode provides.

Additionally, as of zsh 5.9, thanks to a patch I sent, zsh also runs all
parts of a pipeline in a subshell when in sh mode, which fixes a
substantial portion of our testsuite.  POSIX specifies this behavior,
but allows shells to run any or all portions in the main shell instead
as an extension, and the default zsh and AT&T ksh behavior[0] is to run the
final element in a pipeline in the main shell, which breaks lots of our
testsuite.  This, and the fact that every default /bin/sh implementation
_does_ run all the elements in a subshell, is the reason I sent that
patch.

One of the two test failures is due to a zsh bug already fixed upstream
but not yet released, and the other is due to zsh complying with the
standard more strictly than competing shells.  With this series, we can
do this[1]:

----
#!/bin/sh

mkdir -p "$TMPDIR/shell"
ln -s /usr/bin/zsh "$TMPDIR/shell/sh"
make -j8 SHELL_PATH="$TMPDIR/shell/sh" && (cd t && make prove SHELL_PATH=$TMPDIR/shell/sh GIT_PROVE_OPTS=-j8)
----

and the testsuite passes.

Note that this series doesn't make any attempt to make zsh pass the
testsuite in any mode other than sh mode and I have not even tested such
a configuration (which I am sure is horribly broken and would probably
require invasive patches to fix).

These issues are long-standing and probably don't need to be merged down
to maint unless you really feel like it.

[0] Note that AT&T ksh does not support local, so its behavior is
    already very broken with regard to our scripts and testsuite.  Other
    ksh implementations, such as mksh and lksh (MirBSD ksh), pdksh, and
    OpenBSD's ksh/sh _do_ support local and _do_ run all elements of a
    pipeline in a subshell, so they are fully functional with our
    testsuite already.

[1] Please note that I am using user-private temporary directories and
    you should otherwise choose a different directory than `$TMPDIR`.

brian m. carlson (2):
  t4046: avoid continue in &&-chain for zsh
  vimdiff: make script and tests work with zsh

 mergetools/vimdiff       |  3 +--
 t/t4046-diff-unmerged.sh | 16 +++++++++-------
 2 files changed, 10 insertions(+), 9 deletions(-)

[PATCH 1/2] t4046: avoid continue in &&-chain for zsh

[brian m. carlson]

zsh has a bug in which the keyword "continue" within an &&-chain is not
effective and the code following it is executed nonetheless.
Fortunately, this bug has been fixed upstream in 12e5db145 ("51608:
Don't execute commands after "continue &&"", 2023-03-29).  However, zsh
releases very infrequently, so it is not present in a stable release
yet.

That, combined with the fact that almost all zsh users get their shell
from their OS vendor, means that it will likely be a long time before
this problem is fixed for most users.  We have other workarounds in
place for FreeBSD ash and dash, so it shouldn't be too difficult to add
one here, either.

Replace the existing code with a test and if-block, which comes only at
the cost of an additional indentation, and leaves the code a little more
idiomatic anyway.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t4046-diff-unmerged.sh | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh
index ffaf69335f..fb8c51746e 100755
--- a/t/t4046-diff-unmerged.sh
+++ b/t/t4046-diff-unmerged.sh
@@ -20,13 +20,15 @@ test_expect_success setup '
 			for t in o x
 			do
 				path="$b$o$t" &&
-				case "$path" in ooo) continue ;; esac &&
-				paths="$paths$path " &&
-				p="	$path" &&
-				case "$b" in x) echo "$m1$p" ;; esac &&
-				case "$o" in x) echo "$m2$p" ;; esac &&
-				case "$t" in x) echo "$m3$p" ;; esac ||
-				return 1
+				if test "$path" != ooo
+				then
+					paths="$paths$path " &&
+					p="	$path" &&
+					case "$b" in x) echo "$m1$p" ;; esac &&
+					case "$o" in x) echo "$m2$p" ;; esac &&
+					case "$t" in x) echo "$m3$p" ;; esac ||
+					return 1
+				fi
 			done
 		done
 	done >ls-files-s.expect &&

[PATCH 2/2] vimdiff: make script and tests work with zsh

[brian m. carlson]

When we process the $LAYOUT variable through sed, the result will end
with the character "#".  We then split it at the shell using IFS so that
we can process it a character at a time.

POSIX specifies that only "IFS white space shall be ignored at the
beginning and end of the input".  The hash mark is not a white space
character, so it is not ignored at the beginning and end of the input.

POSIX then specifies that "[e]ach occurrence in the input of an IFS
character that is not IFS white space, along with any adjacent IFS white
space, shall delimit a field, as described previously."  Thus, the final
hash mark delimits a field, and the final field is the empty string.

zsh implements this behavior strictly in compliance with POSIX (and
differently from most other shells), such that we end up with a trailing
empty field.  We don't want this empty field and processing it in the
normal way causes us to fail to parse properly and fail the tests with
"ERROR" entries, so let's just ignore it instead.  This is the behavior
of bash and dash anyway and what was clearly intended, so this is a
reasonable thing to do.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 mergetools/vimdiff | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 97e376329b..734d15a03b 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -72,7 +72,6 @@ gen_cmd_aux () {
 	nested=0
 	nested_min=100
 
-
 	# Step 1:
 	#
 	# Increase/decrease "start"/"end" indices respectively to get rid of
@@ -87,7 +86,7 @@ gen_cmd_aux () {
 	IFS=#
 	for c in $(echo "$LAYOUT" | sed 's:.:&#:g')
 	do
-		if test "$c" = " "
+		if test -z "$c" || test "$c" = " "
 		then
 			continue
 		fi

Re: [PATCH 0/2] Test fixes when running with zsh in sh mode

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> This series fixes two test failures with zsh when run in sh mode; that
> is, the mode that occurs when the shell is invoked as "sh".  sh mode
> provides the most POSIX-compliant behavior and avoids a lot of
> incompatibilities that the default zsh mode provides.

Thanks, will queue.