{"thread":{"id":"61373","subject":"[PATCH 2/2] vimdiff: make script and tests work with zsh","startedAt":"2024-04-26T22:12:05Z","lastAt":"2024-04-27T17:23:22Z","messageCount":4,"participants":["brian m. carlson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"493534","messageId":"20240426221154.2194139-3-sandals@crustytoothpaste.net","threadId":"61373","inReplyTo":"20240426221154.2194139-1-sandals@crustytoothpaste.net","subject":"[PATCH 2/2] vimdiff: make script and tests work with zsh","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-04-26T22:11:54Z","receivedAt":"2024-04-26T22:12:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we process the $LAYOUT variable through sed, the result will end\nwith the character \"#\".  We then split it at the shell using IFS so that\nwe can process it a character at a time.\n\nPOSIX specifies that only \"IFS white space shall be ignored at the\nbeginning and end of the input\".  The hash mark is not a white space\ncharacter, so it is not ignored at the beginning and end of the input.\n\nPOSIX then specifies that \"[e]ach occurrence in the input of an IFS\ncharacter that is not IFS white space, along with any adjacent IFS white\nspace, shall delimit a field, as described previously.\"  Thus, the final\nhash mark delimits a field, and the final field is the empty string.\n\nzsh implements this behavior strictly in compliance with POSIX (and\ndifferently from most other shells), such that we end up with a trailing\nempty field.  We don't want this empty field and processing it in the\nnormal way causes us to fail to parse properly and fail the tests with\n\"ERROR\" entries, so let's just ignore it instead.  This is the behavior\nof bash and dash anyway and what was clearly intended, so this is a\nreasonable thing to do.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n mergetools/vimdiff | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex 97e376329b..734d15a03b 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -72,7 +72,6 @@ gen_cmd_aux () {\n \tnested=0\n \tnested_min=100\n \n-\n \t# Step 1:\n \t#\n \t# Increase/decrease \"start\"/\"end\" indices respectively to get rid of\n@@ -87,7 +86,7 @@ gen_cmd_aux () {\n \tIFS=#\n \tfor c in $(echo \"$LAYOUT\" | sed 's:.:&#:g')\n \tdo\n-\t\tif test \"$c\" = \" \"\n+\t\tif test -z \"$c\" || test \"$c\" = \" \"\n \t\tthen\n \t\t\tcontinue\n \t\tfi\n"},{"id":"493535","messageId":"20240426221154.2194139-1-sandals@crustytoothpaste.net","threadId":"61373","inReplyTo":null,"subject":"[PATCH 0/2] Test fixes when running with zsh in sh mode","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-04-26T22:11:52Z","receivedAt":"2024-04-26T22:12:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"zsh is a common shell on a variety of systems, primarily for interactive\nuse.  However, it can be used as a POSIX sh on Debian, macOS, and on other\nsystems where /bin/sh is not POSIX compliant or doesn't provide the\nlocal keyword as we require from our shell.\n\nThis series fixes two test failures with zsh when run in sh mode; that\nis, the mode that occurs when the shell is invoked as \"sh\".  sh mode\nprovides the most POSIX-compliant behavior and avoids a lot of\nincompatibilities that the default zsh mode provides.\n\nAdditionally, as of zsh 5.9, thanks to a patch I sent, zsh also runs all\nparts of a pipeline in a subshell when in sh mode, which fixes a\nsubstantial portion of our testsuite.  POSIX specifies this behavior,\nbut allows shells to run any or all portions in the main shell instead\nas an extension, and the default zsh and AT&T ksh behavior[0] is to run the\nfinal element in a pipeline in the main shell, which breaks lots of our\ntestsuite.  This, and the fact that every default /bin/sh implementation\n_does_ run all the elements in a subshell, is the reason I sent that\npatch.\n\nOne of the two test failures is due to a zsh bug already fixed upstream\nbut not yet released, and the other is due to zsh complying with the\nstandard more strictly than competing shells.  With this series, we can\ndo this[1]:\n\n----\n#!/bin/sh\n\nmkdir -p \"$TMPDIR/shell\"\nln -s /usr/bin/zsh \"$TMPDIR/shell/sh\"\nmake -j8 SHELL_PATH=\"$TMPDIR/shell/sh\" && (cd t && make prove SHELL_PATH=$TMPDIR/shell/sh GIT_PROVE_OPTS=-j8)\n----\n\nand the testsuite passes.\n\nNote that this series doesn't make any attempt to make zsh pass the\ntestsuite in any mode other than sh mode and I have not even tested such\na configuration (which I am sure is horribly broken and would probably\nrequire invasive patches to fix).\n\nThese issues are long-standing and probably don't need to be merged down\nto maint unless you really feel like it.\n\n[0] Note that AT&T ksh does not support local, so its behavior is\n    already very broken with regard to our scripts and testsuite.  Other\n    ksh implementations, such as mksh and lksh (MirBSD ksh), pdksh, and\n    OpenBSD's ksh/sh _do_ support local and _do_ run all elements of a\n    pipeline in a subshell, so they are fully functional with our\n    testsuite already.\n\n[1] Please note that I am using user-private temporary directories and\n    you should otherwise choose a different directory than `$TMPDIR`.\n\nbrian m. carlson (2):\n  t4046: avoid continue in &&-chain for zsh\n  vimdiff: make script and tests work with zsh\n\n mergetools/vimdiff       |  3 +--\n t/t4046-diff-unmerged.sh | 16 +++++++++-------\n 2 files changed, 10 insertions(+), 9 deletions(-)\n\n"},{"id":"493536","messageId":"20240426221154.2194139-2-sandals@crustytoothpaste.net","threadId":"61373","inReplyTo":"20240426221154.2194139-1-sandals@crustytoothpaste.net","subject":"[PATCH 1/2] t4046: avoid continue in &&-chain for zsh","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-04-26T22:11:53Z","receivedAt":"2024-04-26T22:12:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"zsh has a bug in which the keyword \"continue\" within an &&-chain is not\neffective and the code following it is executed nonetheless.\nFortunately, this bug has been fixed upstream in 12e5db145 (\"51608:\nDon't execute commands after \"continue &&\"\", 2023-03-29).  However, zsh\nreleases very infrequently, so it is not present in a stable release\nyet.\n\nThat, combined with the fact that almost all zsh users get their shell\nfrom their OS vendor, means that it will likely be a long time before\nthis problem is fixed for most users.  We have other workarounds in\nplace for FreeBSD ash and dash, so it shouldn't be too difficult to add\none here, either.\n\nReplace the existing code with a test and if-block, which comes only at\nthe cost of an additional indentation, and leaves the code a little more\nidiomatic anyway.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t4046-diff-unmerged.sh | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh\nindex ffaf69335f..fb8c51746e 100755\n--- a/t/t4046-diff-unmerged.sh\n+++ b/t/t4046-diff-unmerged.sh\n@@ -20,13 +20,15 @@ test_expect_success setup '\n \t\t\tfor t in o x\n \t\t\tdo\n \t\t\t\tpath=\"$b$o$t\" &&\n-\t\t\t\tcase \"$path\" in ooo) continue ;; esac &&\n-\t\t\t\tpaths=\"$paths$path \" &&\n-\t\t\t\tp=\"\t$path\" &&\n-\t\t\t\tcase \"$b\" in x) echo \"$m1$p\" ;; esac &&\n-\t\t\t\tcase \"$o\" in x) echo \"$m2$p\" ;; esac &&\n-\t\t\t\tcase \"$t\" in x) echo \"$m3$p\" ;; esac ||\n-\t\t\t\treturn 1\n+\t\t\t\tif test \"$path\" != ooo\n+\t\t\t\tthen\n+\t\t\t\t\tpaths=\"$paths$path \" &&\n+\t\t\t\t\tp=\"\t$path\" &&\n+\t\t\t\t\tcase \"$b\" in x) echo \"$m1$p\" ;; esac &&\n+\t\t\t\t\tcase \"$o\" in x) echo \"$m2$p\" ;; esac &&\n+\t\t\t\t\tcase \"$t\" in x) echo \"$m3$p\" ;; esac ||\n+\t\t\t\t\treturn 1\n+\t\t\t\tfi\n \t\t\tdone\n \t\tdone\n \tdone >ls-files-s.expect &&\n"},{"id":"493545","messageId":"xmqqil0267p4.fsf@gitster.g","threadId":"61373","inReplyTo":"20240426221154.2194139-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH 0/2] Test fixes when running with zsh in sh mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-27T17:23:19Z","receivedAt":"2024-04-27T17:23:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> This series fixes two test failures with zsh when run in sh mode; that\n> is, the mode that occurs when the shell is invoked as \"sh\".  sh mode\n> provides the most POSIX-compliant behavior and avoids a lot of\n> incompatibilities that the default zsh mode provides.\n\nThanks, will queue.\n"}]}