{"thread":{"id":"59489","subject":"[PATCH 0/6] test: make the test suite work with zsh","startedAt":"2023-03-28T17:39:50Z","lastAt":"2023-04-01T02:40:09Z","messageCount":29,"participants":["Felipe Contreras","brian m. carlson","Ævar Arnfjörð Bjarmason","Junio C Hamano","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"474328","messageId":"20230328173932.3614601-1-felipe.contreras@gmail.com","threadId":"59489","inReplyTo":null,"subject":"[PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-28T17:39:26Z","receivedAt":"2023-03-28T17:39:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It's not difficult to make the testing library work for zsh, so I did that in\nthe first patch.\n\nThe rest of the patches are basically to deal with some variables that are\nspecial in zsh, workaround a bug, and a minor discrepancy.\n\nFelipe Contreras (6):\n  test: fix build for zsh\n  test: avoid `stat` variable\n  test: avoid `options` variable\n  test: avoid `path` variable\n  test: hack for zsh\n  mergetools: vimdiff: check for empty fields\n\n mergetools/vimdiff               |  4 +--\n t/annotate-tests.sh              | 10 +++----\n t/lib-bash.sh                    |  2 +-\n t/t0001-init.sh                  |  4 +--\n t/t0003-attributes.sh            | 16 +++++------\n t/t1450-fsck.sh                  | 16 +++++------\n t/t3305-notes-fanout.sh          | 12 ++++----\n t/t3432-rebase-fast-forward.sh   |  4 +--\n t/t4013-diff-various.sh          |  6 ++--\n t/t4046-diff-unmerged.sh         | 49 ++++++++++++++++----------------\n t/t4051-diff-function-context.sh |  4 +--\n t/t5329-pack-objects-cruft.sh    |  8 +++---\n t/t5512-ls-remote.sh             |  4 +--\n t/t5516-fetch-push.sh            |  8 +++---\n t/t9300-fast-import.sh           |  4 +--\n t/test-lib.sh                    | 24 ++++++++++------\n 16 files changed, 92 insertions(+), 83 deletions(-)\n\n-- \n2.39.2.13.g1fb56cf030\n\n"},{"id":"474329","messageId":"20230328173932.3614601-4-felipe.contreras@gmail.com","threadId":"59489","inReplyTo":"20230328173932.3614601-1-felipe.contreras@gmail.com","subject":"[PATCH 3/6] test: avoid `options` variable","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-28T17:39:29Z","receivedAt":"2023-03-28T17:40:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It has a special meaning in zsh.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/annotate-tests.sh              | 10 +++++-----\n t/t4051-diff-function-context.sh |  4 ++--\n t/t9300-fast-import.sh           |  4 ++--\n 3 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex f1b9a6ce4d..3b57d4dbb5 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -15,19 +15,19 @@ fi\n check_count () {\n \thead= &&\n \tfile='file' &&\n-\toptions= &&\n+\targs= &&\n \twhile :\n \tdo\n \t\tcase \"$1\" in\n \t\t-h) head=\"$2\"; shift; shift ;;\n \t\t-f) file=\"$2\"; shift; shift ;;\n-\t\t-L*) options=\"$options $(sanitize_L \"$1\")\"; shift ;;\n-\t\t-*) options=\"$options $1\"; shift ;;\n+\t\t-L*) args=\"$args $(sanitize_L \"$1\")\"; shift ;;\n+\t\t-*) args=\"$args $1\"; shift ;;\n \t\t*) break ;;\n \t\tesac\n \tdone &&\n-\techo \"$PROG $options $file $head\" >&4 &&\n-\t$PROG $options $file $head >actual &&\n+\techo \"$PROG $args $file $head\" >&4 &&\n+\t$PROG $args $file $head >actual &&\n \tperl -e '\n \t\tmy %expect = (@ARGV);\n \t\tmy %count = map { $_ => 0 } keys %expect;\ndiff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh\nindex 725278ad19..3bf4f0b6b0 100755\n--- a/t/t4051-diff-function-context.sh\n+++ b/t/t4051-diff-function-context.sh\n@@ -30,10 +30,10 @@ last_context_line () {\n check_diff () {\n \tname=$1\n \tdesc=$2\n-\toptions=\"-W $3\"\n+\targs=\"-W $3\"\n \n \ttest_expect_success \"$desc\" '\n-\t\tgit diff $options \"$name^\" \"$name\" >\"$name.diff\"\n+\t\tgit diff $args \"$name^\" \"$name\" >\"$name.diff\"\n \t'\n \n \ttest_expect_success ' diff applies' '\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex aa55b41b9a..c30503870f 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -3222,7 +3222,7 @@ test_expect_success 'U: validate root delete result' '\n # fast-import process is still running using background_import_still_running\n # *after* evaluating the test conditions.\n background_import_then_checkpoint () {\n-\toptions=$1\n+\targs=$1\n \tinput_file=$2\n \n \tmkfifo V.input\n@@ -3234,7 +3234,7 @@ background_import_then_checkpoint () {\n \trm V.output\n \n \t(\n-\t\tgit fast-import $options <&8 >&9 &\n+\t\tgit fast-import $args <&8 >&9 &\n \t\techo $! >&9\n \t\twait $!\n \t\techo >&2 \"background fast-import terminated too early with exit code $?\"\n-- \n2.39.2.13.g1fb56cf030\n\n"},{"id":"474330","messageId":"20230328173932.3614601-2-felipe.contreras@gmail.com","threadId":"59489","inReplyTo":"20230328173932.3614601-1-felipe.contreras@gmail.com","subject":"[PATCH 1/6] test: fix build for zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-28T17:39:27Z","receivedAt":"2023-03-28T17:40:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/lib-bash.sh |  2 +-\n t/test-lib.sh | 20 ++++++++++++++------\n 2 files changed, 15 insertions(+), 7 deletions(-)\n\ndiff --git a/t/lib-bash.sh b/t/lib-bash.sh\nindex b0b6060929..798b6d0265 100644\n--- a/t/lib-bash.sh\n+++ b/t/lib-bash.sh\n@@ -10,7 +10,7 @@ elif type bash >/dev/null 2>&1\n then\n \t# execute in full-on bash mode\n \tunset POSIXLY_CORRECT\n-\texec bash \"$0\" \"$@\"\n+\texec bash \"${ZSH_ARGZERO-$0}\" \"$@\"\n else\n \techo '1..0 #SKIP skipping bash completion tests; bash not available'\n \texit 0\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 62136caee5..cc705df981 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -15,6 +15,14 @@\n # You should have received a copy of the GNU General Public License\n # along with this program.  If not, see http://www.gnu.org/licenses/ .\n \n+if test -n \"${ZSH_VERSION-}\"\n+then\n+\temulate sh\n+\tARGZERO=\"$ZSH_ARGZERO\"\n+else\n+\tARGZERO=\"$0\"\n+fi\n+\n # Test the binaries we have just built.  The tests are kept in\n # t/ subdirectory and are run in 'trash directory' subdirectory.\n if test -z \"$TEST_DIRECTORY\"\n@@ -310,7 +318,7 @@ then\n fi\n \n TEST_STRESS_JOB_SFX=\"${GIT_TEST_STRESS_JOB_NR:+.stress-$GIT_TEST_STRESS_JOB_NR}\"\n-TEST_NAME=\"$(basename \"$0\" .sh)\"\n+TEST_NAME=\"$(basename \"$ARGZERO\" .sh)\"\n TEST_NUMBER=\"${TEST_NAME%%-*}\"\n TEST_NUMBER=\"${TEST_NUMBER#t}\"\n TEST_RESULTS_DIR=\"$TEST_OUTPUT_DIRECTORY/test-results\"\n@@ -388,7 +396,7 @@ then\n \t\t\t      { test -z \"$stress_limit\" ||\n \t\t\t\ttest $cnt -le $stress_limit ; }\n \t\t\tdo\n-\t\t\t\t$TEST_SHELL_PATH \"$0\" \"$@\" >\"$TEST_RESULTS_BASE.stress-$job_nr.out\" 2>&1 &\n+\t\t\t\t$TEST_SHELL_PATH \"$ARGZERO\" \"$@\" >\"$TEST_RESULTS_BASE.stress-$job_nr.out\" 2>&1 &\n \t\t\t\ttest_pid=$!\n \n \t\t\t\tif wait $test_pid\n@@ -442,7 +450,7 @@ then\n \t# from any previous runs.\n \t>\"$GIT_TEST_TEE_OUTPUT_FILE\"\n \n-\t(GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} \"$0\" \"$@\" 2>&1;\n+\t(GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} \"$ARGZERO\" \"$@\" 2>&1;\n \t echo $? >\"$TEST_RESULTS_BASE.exit\") | tee -a \"$GIT_TEST_TEE_OUTPUT_FILE\"\n \ttest \"$(cat \"$TEST_RESULTS_BASE.exit\")\" = 0\n \texit\n@@ -467,7 +475,7 @@ then\n \tthen\n \t\t: Executed by a Bash version supporting BASH_XTRACEFD.  Good.\n \telse\n-\t\techo >&2 \"warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD\"\n+\t\techo >&2 \"warning: ignoring -x; '$ARGZERO' is untraceable without BASH_XTRACEFD\"\n \t\ttrace=\n \tfi\n fi\n@@ -1595,7 +1603,7 @@ fi\n \n if test \"${GIT_TEST_CHAIN_LINT:-1}\" != 0\n then\n-\t\"$PERL_PATH\" \"$TEST_DIRECTORY/chainlint.pl\" \"$0\" ||\n+\t\"$PERL_PATH\" \"$TEST_DIRECTORY/chainlint.pl\" \"$ARGZERO\" ||\n \t\tBUG \"lint error (see '?!...!? annotations above)\"\n fi\n \n@@ -1637,7 +1645,7 @@ fi\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$TRASH_DIRECTORY\" || BAIL_OUT \"cannot cd -P to \\\"$TRASH_DIRECTORY\\\"\"\n \n-start_test_output \"$0\"\n+start_test_output \"$ARGZERO\"\n \n # Convenience\n # A regexp to match 5 and 35 hexdigits\n-- \n2.39.2.13.g1fb56cf030\n\n"},{"id":"474331","messageId":"20230328173932.3614601-6-felipe.contreras@gmail.com","threadId":"59489","inReplyTo":"20230328173932.3614601-1-felipe.contreras@gmail.com","subject":"[PATCH 5/6] test: hack for zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-28T17:39:31Z","receivedAt":"2023-03-28T17:40:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"See: https://www.zsh.org/mla/workers/2023/msg00336.html\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t4046-diff-unmerged.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh\nindex 4998a1ce27..ca31771070 100755\n--- a/t/t4046-diff-unmerged.sh\n+++ b/t/t4046-diff-unmerged.sh\n@@ -21,7 +21,8 @@ test_expect_success setup '\n \t\t\tdo\n \t\t\t\tfpath=\"$b$o$t\" &&\n \t\t\t\tcase \"$fpath\" in ooo) continue ;; esac &&\n-\t\t\t\tpaths=\"$paths$fpath \" &&\n+\t\t\t\t: hack for zsh &&\n+\t\t\t\t{ paths=\"$paths$fpath \"; } &&\n \t\t\t\tp=\"\t$fpath\" &&\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-- \n2.39.2.13.g1fb56cf030\n\n"},{"id":"474332","messageId":"20230328173932.3614601-7-felipe.contreras@gmail.com","threadId":"59489","inReplyTo":"20230328173932.3614601-1-felipe.contreras@gmail.com","subject":"[PATCH 6/6] mergetools: vimdiff: check for empty fields","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-28T17:39:32Z","receivedAt":"2023-03-28T17:40:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"In zsh 'foo#bar#' produces an empty field at the end.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n mergetools/vimdiff | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex 06937acbf5..27d1644faf 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -87,7 +87,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 \"$c\" = \" \" || test -z \"$c\"\n \t\tthen\n \t\t\tcontinue\n \t\tfi\n@@ -155,7 +155,7 @@ gen_cmd_aux () {\n \tdo\n \t\ti=$(( i + 1 ))\n \n-\t\tif test \"$c\" = \" \"\n+\t\tif test \"$c\" = \" \" || test -z \"$c\"\n \t\tthen\n \t\t\tcontinue\n \t\tfi\n-- \n2.39.2.13.g1fb56cf030\n\n"},{"id":"474333","messageId":"20230328173932.3614601-3-felipe.contreras@gmail.com","threadId":"59489","inReplyTo":"20230328173932.3614601-1-felipe.contreras@gmail.com","subject":"[PATCH 2/6] test: avoid `stat` variable","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-28T17:39:28Z","receivedAt":"2023-03-28T17:40:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It has special meaning in zsh.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t0001-init.sh                | 4 ++--\n t/t3432-rebase-fast-forward.sh | 4 ++--\n t/t4013-diff-various.sh        | 6 +++---\n t/t5512-ls-remote.sh           | 4 ++--\n t/t5516-fetch-push.sh          | 8 ++++----\n t/test-lib.sh                  | 4 ++--\n 6 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 30a6edca1d..88c9de8f53 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -362,12 +362,12 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '\n \t\tcd $base/dir &&\n \t\ttest-tool getcwd\n \t)\n-\tstatus=$?\n+\tstat=$?\n \n \tchmod 700 $base &&\n \trm -rf $base ||\n \tBUG \"cannot clean $base\"\n-\treturn $status\n+\treturn $stat\n '\n \n check_long_base_path () {\ndiff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh\nindex 7f1a5dd3de..49ecf8f18b 100755\n--- a/t/t3432-rebase-fast-forward.sh\n+++ b/t/t3432-rebase-fast-forward.sh\n@@ -41,7 +41,7 @@ test_rebase_same_head () {\n }\n \n test_rebase_same_head_ () {\n-\tstatus=\"$1\" &&\n+\tstat=\"$1\" &&\n \tshift &&\n \twhat=\"$1\" &&\n \tshift &&\n@@ -57,7 +57,7 @@ test_rebase_same_head_ () {\n \telse\n \t\tmsg=\"git rebase$flag $* with $changes is $what with $cmp HEAD\"\n \tfi &&\n-\ttest_expect_$status \"$msg\" \"\n+\ttest_expect_$stat \"$msg\" \"\n \t\tif test $abbreviate -eq 1\n \t\tthen\n \t\t\ttest_config rebase.abbreviateCommands true\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 5de1d19075..8f82946c2f 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -178,7 +178,7 @@ process_diffs () {\n V=$(git version | sed -e 's/^git version //' -e 's/\\./\\\\./g')\n while read magic cmd\n do\n-\tstatus=success\n+\tstat=success\n \tcase \"$magic\" in\n \t'' | '#'*)\n \t\tcontinue ;;\n@@ -188,7 +188,7 @@ do\n \t\tcase \"$magic\" in\n \t\tnoellipses) ;;\n \t\tfailure)\n-\t\t\tstatus=failure\n+\t\t\tstat=failure\n \t\t\tmagic=\n \t\t\tlabel=\"$cmd\" ;;\n \t\t*)\n@@ -203,7 +203,7 @@ do\n \texpect=\"$TEST_DIRECTORY/t4013/diff.$test\"\n \tactual=\"$pfx-diff.$test\"\n \n-\ttest_expect_$status \"git $cmd # magic is ${magic:-(not used)}\" '\n+\ttest_expect_$stat \"git $cmd # magic is ${magic:-(not used)}\" '\n \t\t{\n \t\t\techo \"$ git $cmd\"\n \t\t\tcase \"$magic\" in\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 20d063fb9a..88f0f81baf 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -167,9 +167,9 @@ test_expect_success 'confuses pattern as remote when no remote specified' '\n test_expect_success 'die with non-2 for wrong repository even with --exit-code' '\n \t{\n \t\tgit ls-remote --exit-code ./no-such-repository\n-\t\tstatus=$?\n+\t\tstat=$?\n \t} &&\n-\ttest $status != 2 && test $status != 0\n+\ttest $stat != 2 && test $stat != 0\n '\n \n test_expect_success 'Report success even when nothing matches' '\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 19ebefa5ac..ed8b676156 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1732,9 +1732,9 @@ test_expect_success 'updateInstead with push-to-checkout hook' '\n \n \tgit update-index -q --refresh &&\n \tgit read-tree -u -m HEAD \"$1\" || {\n-\t\tstatus=$?\n+\t\tstat=$?\n \t\techo >&2 read-tree failed\n-\t\texit $status\n+\t\texit $stat\n \t}\n \tEOF\n \n@@ -1800,9 +1800,9 @@ test_expect_success 'updateInstead with push-to-checkout hook' '\n \tno)\n \t\tgit read-tree -u -m \"$1\" ;;\n \tesac || {\n-\t\tstatus=$?\n+\t\tstat=$?\n \t\techo >&2 read-tree failed\n-\t\texit $status\n+\t\texit $stat\n \t}\n \tEOF\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex cc705df981..621a10941d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1866,12 +1866,12 @@ test_lazy_prereq SANITY '\n \n \t! test -r SANETESTD.1/x &&\n \t! rm SANETESTD.1/x && ! test -f SANETESTD.2/x\n-\tstatus=$?\n+\tstat=$?\n \n \tchmod +rwx SANETESTD.1 SANETESTD.2 &&\n \trm -rf SANETESTD.1 SANETESTD.2 ||\n \tBUG \"cannot clean SANETESTD\"\n-\treturn $status\n+\treturn $stat\n '\n \n test FreeBSD != $uname_s || GIT_UNZIP=${GIT_UNZIP:-/usr/local/bin/unzip}\n-- \n2.39.2.13.g1fb56cf030\n\n"},{"id":"474334","messageId":"20230328173932.3614601-5-felipe.contreras@gmail.com","threadId":"59489","inReplyTo":"20230328173932.3614601-1-felipe.contreras@gmail.com","subject":"[PATCH 4/6] test: avoid `path` variable","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-28T17:39:30Z","receivedAt":"2023-03-28T17:40:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"In zsh it's linked to `PATH`.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t0003-attributes.sh         | 16 ++++++------\n t/t1450-fsck.sh               | 16 ++++++------\n t/t3305-notes-fanout.sh       | 12 ++++-----\n t/t4046-diff-unmerged.sh      | 48 +++++++++++++++++------------------\n t/t5329-pack-objects-cruft.sh |  8 +++---\n 5 files changed, 50 insertions(+), 50 deletions(-)\n\ndiff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\nindex 89b306cb11..b956b2fc29 100755\n--- a/t/t0003-attributes.sh\n+++ b/t/t0003-attributes.sh\n@@ -7,10 +7,10 @@ TEST_CREATE_REPO_NO_TEMPLATE=1\n . ./test-lib.sh\n \n attr_check_basic () {\n-\tpath=\"$1\" expect=\"$2\" git_opts=\"$3\" &&\n+\tfpath=\"$1\" expect=\"$2\" git_opts=\"$3\" &&\n \n-\tgit $git_opts check-attr test -- \"$path\" >actual 2>err &&\n-\techo \"$path: test: $expect\" >expect &&\n+\tgit $git_opts check-attr test -- \"$fpath\" >actual 2>err &&\n+\techo \"$fpath: test: $expect\" >expect &&\n \ttest_cmp expect actual\n }\n \n@@ -20,18 +20,18 @@ attr_check () {\n }\n \n attr_check_quote () {\n-\tpath=\"$1\" quoted_path=\"$2\" expect=\"$3\" &&\n+\tfpath=\"$1\" quoted_path=\"$2\" expect=\"$3\" &&\n \n-\tgit check-attr test -- \"$path\" >actual &&\n+\tgit check-attr test -- \"$fpath\" >actual &&\n \techo \"\\\"$quoted_path\\\": test: $expect\" >expect &&\n \ttest_cmp expect actual\n }\n \n attr_check_source () {\n-\tpath=\"$1\" expect=\"$2\" source=\"$3\" git_opts=\"$4\" &&\n+\tfpath=\"$1\" expect=\"$2\" source=\"$3\" git_opts=\"$4\" &&\n \n-\tgit $git_opts check-attr --source $source test -- \"$path\" >actual 2>err &&\n-\techo \"$path: test: $expect\" >expect &&\n+\tgit $git_opts check-attr --source $source test -- \"$fpath\" >actual 2>err &&\n+\techo \"$fpath: test: $expect\" >expect &&\n \ttest_cmp expect actual &&\n \ttest_must_be_empty err\n }\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex bca46378b2..489e03865c 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -589,9 +589,9 @@ test_expect_success 'fsck notices submodule entry pointing to null sha1' '\n \t)\n '\n \n-while read name path pretty; do\n+while read name fpath pretty; do\n \twhile read mode type; do\n-\t\t: ${pretty:=$path}\n+\t\t: ${pretty:=$fpath}\n \t\ttest_expect_success \"fsck notices $pretty as $type\" '\n \t\t(\n \t\t\tgit init $name-$type &&\n@@ -603,7 +603,7 @@ while read name path pretty; do\n \t\t\tblob=$(git rev-parse :file) &&\n \t\t\ttree=$(git rev-parse HEAD^{tree}) &&\n \t\t\tvalue=$(eval \"echo \\$$type\") &&\n-\t\t\tprintf \"$mode $type %s\\t%s\" \"$value\" \"$path\" >bad &&\n+\t\t\tprintf \"$mode $type %s\\t%s\" \"$value\" \"$fpath\" >bad &&\n \t\t\tbad_tree=$(git mktree <bad) &&\n \t\t\tgit fsck 2>out &&\n \t\t\ttest_i18ngrep \"warning.*tree $bad_tree\" out\n@@ -674,8 +674,8 @@ create_repo_missing () {\n \t\tunrelated=$(echo unrelated | git hash-object --stdin -w) &&\n \t\tgit tag -m foo tag $unrelated &&\n \t\tsha1=$(git rev-parse --verify \"$1\") &&\n-\t\tpath=$(echo $sha1 | sed 's|..|&/|') &&\n-\t\trm .git/objects/$path\n+\t\tfpath=$(echo $sha1 | sed 's|..|&/|') &&\n+\t\trm .git/objects/$fpath\n \t)\n }\n \n@@ -781,11 +781,11 @@ test_expect_success 'fsck --name-objects' '\n test_expect_success 'alternate objects are correctly blamed' '\n \ttest_when_finished \"rm -rf alt.git .git/objects/info/alternates\" &&\n \tname=$(test_oid numeric) &&\n-\tpath=$(test_oid_to_path \"$name\") &&\n+\tfpath=$(test_oid_to_path \"$name\") &&\n \tgit init --bare alt.git &&\n \techo \"../../alt.git/objects\" >.git/objects/info/alternates &&\n-\tmkdir alt.git/objects/$(dirname $path) &&\n-\t>alt.git/objects/$(dirname $path)/$(basename $path) &&\n+\tmkdir alt.git/objects/$(dirname $fpath) &&\n+\t>alt.git/objects/$(dirname $fpath)/$(basename $fpath) &&\n \ttest_must_fail git fsck >out 2>&1 &&\n \ttest_i18ngrep alt.git out\n '\ndiff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh\nindex 1ec1fb6715..a0d8ec85a1 100755\n--- a/t/t3305-notes-fanout.sh\n+++ b/t/t3305-notes-fanout.sh\n@@ -6,10 +6,10 @@ TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n path_has_fanout() {\n-\tpath=$1 &&\n+\tfpath=$1 &&\n \tfanout=$2 &&\n \tafter_last_slash=$(($(test_oid hexsz) - $fanout * 2)) &&\n-\techo $path | grep -q -E \"^([0-9a-f]{2}/){$fanout}[0-9a-f]{$after_last_slash}$\"\n+\techo $fpath | grep -q -E \"^([0-9a-f]{2}/){$fanout}[0-9a-f]{$after_last_slash}$\"\n }\n \n touched_one_note_with_fanout() {\n@@ -17,17 +17,17 @@ touched_one_note_with_fanout() {\n \tmodification=$2 &&  # 'A' for addition, 'D' for deletion\n \tfanout=$3 &&\n \tdiff=$(git diff-tree --no-commit-id --name-status --root -r $notes_commit) &&\n-\tpath=$(echo $diff | sed -e \"s/^$modification[\\t ]//\") &&\n-\tpath_has_fanout \"$path\" $fanout;\n+\tfpath=$(echo $diff | sed -e \"s/^$modification[\\t ]//\") &&\n+\tpath_has_fanout \"$fpath\" $fanout;\n }\n \n all_notes_have_fanout() {\n \tnotes_commit=$1 &&\n \tfanout=$2 &&\n \tgit ls-tree -r --name-only $notes_commit |\n-\twhile read path\n+\twhile read fpath\n \tdo\n-\t\tpath_has_fanout $path $fanout || return 1\n+\t\tpath_has_fanout $fpath $fanout || return 1\n \tdone\n }\n \ndiff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh\nindex ffaf69335f..4998a1ce27 100755\n--- a/t/t4046-diff-unmerged.sh\n+++ b/t/t4046-diff-unmerged.sh\n@@ -19,10 +19,10 @@ test_expect_success setup '\n \t\tdo\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\tfpath=\"$b$o$t\" &&\n+\t\t\t\tcase \"$fpath\" in ooo) continue ;; esac &&\n+\t\t\t\tpaths=\"$paths$fpath \" &&\n+\t\t\t\tp=\"\t$fpath\" &&\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@@ -36,22 +36,22 @@ test_expect_success setup '\n '\n \n test_expect_success 'diff-files -0' '\n-\tfor path in $paths\n+\tfor fpath in $paths\n \tdo\n-\t\t>\"$path\" &&\n-\t\techo \":000000 100644 $ZERO_OID $ZERO_OID U\t$path\" || return 1\n+\t\t>\"$fpath\" &&\n+\t\techo \":000000 100644 $ZERO_OID $ZERO_OID U\t$fpath\" || return 1\n \tdone >diff-files-0.expect &&\n \tgit diff-files -0 >diff-files-0.actual &&\n \ttest_cmp diff-files-0.expect diff-files-0.actual\n '\n \n test_expect_success 'diff-files -1' '\n-\tfor path in $paths\n+\tfor fpath in $paths\n \tdo\n-\t\t>\"$path\" &&\n-\t\techo \":000000 100644 $ZERO_OID $ZERO_OID U\t$path\" &&\n-\t\tcase \"$path\" in\n-\t\tx??) echo \":100644 100644 $blob1 $ZERO_OID M\t$path\"\n+\t\t>\"$fpath\" &&\n+\t\techo \":000000 100644 $ZERO_OID $ZERO_OID U\t$fpath\" &&\n+\t\tcase \"$fpath\" in\n+\t\tx??) echo \":100644 100644 $blob1 $ZERO_OID M\t$fpath\"\n \t\tesac || return 1\n \tdone >diff-files-1.expect &&\n \tgit diff-files -1 >diff-files-1.actual &&\n@@ -59,12 +59,12 @@ test_expect_success 'diff-files -1' '\n '\n \n test_expect_success 'diff-files -2' '\n-\tfor path in $paths\n+\tfor fpath in $paths\n \tdo\n-\t\t>\"$path\" &&\n-\t\techo \":000000 100644 $ZERO_OID $ZERO_OID U\t$path\" &&\n-\t\tcase \"$path\" in\n-\t\t?x?) echo \":100644 100644 $blob2 $ZERO_OID M\t$path\"\n+\t\t>\"$fpath\" &&\n+\t\techo \":000000 100644 $ZERO_OID $ZERO_OID U\t$fpath\" &&\n+\t\tcase \"$fpath\" in\n+\t\t?x?) echo \":100644 100644 $blob2 $ZERO_OID M\t$fpath\"\n \t\tesac || return 1\n \tdone >diff-files-2.expect &&\n \tgit diff-files -2 >diff-files-2.actual &&\n@@ -74,12 +74,12 @@ test_expect_success 'diff-files -2' '\n '\n \n test_expect_success 'diff-files -3' '\n-\tfor path in $paths\n+\tfor fpath in $paths\n \tdo\n-\t\t>\"$path\" &&\n-\t\techo \":000000 100644 $ZERO_OID $ZERO_OID U\t$path\" &&\n-\t\tcase \"$path\" in\n-\t\t??x) echo \":100644 100644 $blob3 $ZERO_OID M\t$path\"\n+\t\t>\"$fpath\" &&\n+\t\techo \":000000 100644 $ZERO_OID $ZERO_OID U\t$fpath\" &&\n+\t\tcase \"$fpath\" in\n+\t\t??x) echo \":100644 100644 $blob3 $ZERO_OID M\t$fpath\"\n \t\tesac || return 1\n \tdone >diff-files-3.expect &&\n \tgit diff-files -3 >diff-files-3.actual &&\n@@ -87,9 +87,9 @@ test_expect_success 'diff-files -3' '\n '\n \n test_expect_success 'diff --stat' '\n-\tfor path in $paths\n+\tfor fpath in $paths\n \tdo\n-\t\techo \" $path | Unmerged\" || return 1\n+\t\techo \" $fpath | Unmerged\" || return 1\n \tdone >diff-stat.expect &&\n \techo \" 0 files changed\" >>diff-stat.expect &&\n \tgit diff --cached --stat >diff-stat.actual &&\ndiff --git a/t/t5329-pack-objects-cruft.sh b/t/t5329-pack-objects-cruft.sh\nindex 303f7a5d84..86838d2b0f 100755\n--- a/t/t5329-pack-objects-cruft.sh\n+++ b/t/t5329-pack-objects-cruft.sh\n@@ -28,8 +28,8 @@ basic_cruft_pack_tests () {\n \t\t\t\tgit rev-list --objects --no-object-names base..loose |\n \t\t\t\twhile read oid\n \t\t\t\tdo\n-\t\t\t\t\tpath=\"$objdir/$(test_oid_to_path \"$oid\")\" &&\n-\t\t\t\t\tprintf \"%s %d\\n\" \"$oid\" \"$(test-tool chmtime --get \"$path\")\" ||\n+\t\t\t\t\tfpath=\"$objdir/$(test_oid_to_path \"$oid\")\" &&\n+\t\t\t\t\tprintf \"%s %d\\n\" \"$oid\" \"$(test-tool chmtime --get \"$fpath\")\" ||\n \t\t\t\t\techo \"object list generation failed for $oid\"\n \t\t\t\tdone |\n \t\t\t\tsort -k1\n@@ -413,8 +413,8 @@ test_expect_success 'loose objects mtimes upsert others' '\n \t\tgit repack &&\n \n \t\ttip=\"$(git rev-parse cruft)\" &&\n-\t\tpath=\"$objdir/$(test_oid_to_path \"$tip\")\" &&\n-\t\ttest-tool chmtime --get +1000 \"$path\" >expect &&\n+\t\tfpath=\"$objdir/$(test_oid_to_path \"$tip\")\" &&\n+\t\ttest-tool chmtime --get +1000 \"$fpath\" >expect &&\n \n \t\tgit checkout main &&\n \t\tgit branch -D other &&\n-- \n2.39.2.13.g1fb56cf030\n\n"},{"id":"474385","messageId":"ZCONCVC2ITBJWoBA@tapette.crustytoothpaste.net","threadId":"59489","inReplyTo":"20230328173932.3614601-1-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-03-29T00:57:45Z","receivedAt":"2023-03-29T00:57:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-03-28 at 17:39:26, Felipe Contreras wrote:\n> It's not difficult to make the testing library work for zsh, so I did that in\n> the first patch.\n> \n> The rest of the patches are basically to deal with some variables that are\n> special in zsh, workaround a bug, and a minor discrepancy.\n\nThere was a point at which the tests worked entirely in sh mode with\nzsh.  I know because I fixed a handful of tests there, ending with\nc64368e3a2a47, and I patched zsh to run all commands in a pipeline in a\nsubshell in sh mode to fix the remaining tests.\n\nIf I symlink zsh (zsh 5.9 (x86_64-debian-linux-gnu)) to sh in a\ntemporary directory and use it in SHELL_PATH, I get only the following\nfailures:\n\n  t4046-diff-unmerged.sh                           (Wstat: 256 (exited 1) Tests: 6 Failed: 5)\n    Failed tests:  2-6\n    Non-zero exit status: 1\n  t7609-mergetool--lib.sh                          (Wstat: 256 (exited 1) Tests: 1 Failed: 1)\n    Failed test:  1\n    Non-zero exit status: 1\n\nI haven't investigated further.\n\nI don't care a lot of other folks want to make zsh run the testsuite in\nzsh mode, but I'd think that using sh mode would be simpler and less\nlikely to break in general, and would avoid us needing to carry a lot of\npatches to work around various variables that are special in zsh mode.\nIt would also be easier to potentially test in the testsuite as well.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"474386","messageId":"CAMP44s07W6SzJZnmpMMejM8cTNvwrKnNiD+1U=JWMBwUX8JbMA@mail.gmail.com","threadId":"59489","inReplyTo":"ZCONCVC2ITBJWoBA@tapette.crustytoothpaste.net","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-29T01:57:57Z","receivedAt":"2023-03-29T01:58:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Mar 28, 2023 at 6:57 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2023-03-28 at 17:39:26, Felipe Contreras wrote:\n> > It's not difficult to make the testing library work for zsh, so I did that in\n> > the first patch.\n> >\n> > The rest of the patches are basically to deal with some variables that are\n> > special in zsh, workaround a bug, and a minor discrepancy.\n>\n> There was a point at which the tests worked entirely in sh mode with\n> zsh.  I know because I fixed a handful of tests there, ending with\n> c64368e3a2a47, and I patched zsh to run all commands in a pipeline in a\n> subshell in sh mode to fix the remaining tests.\n>\n> If I symlink zsh (zsh 5.9 (x86_64-debian-linux-gnu)) to sh in a\n> temporary directory and use it in SHELL_PATH, I get only the following\n> failures:\n\nThat would defeat my motivation behind the patches, which is to be\nable to run one test file in zsh.\n\nOnly the first patch is needed for that, the rest were in case anyone\ncared to run all the tests.\n\n> I don't care a lot of other folks want to make zsh run the testsuite in\n> zsh mode, but I'd think that using sh mode would be simpler and less\n> likely to break in general, and would avoid us needing to carry a lot of\n> patches to work around various variables that are special in zsh mode.\n\nWe don't need to carry the patches if the patches are applied.\n\n-- \nFelipe Contreras\n"},{"id":"474399","messageId":"230329.865yajx4xv.gmgdl@evledraar.gmail.com","threadId":"59489","inReplyTo":"20230328173932.3614601-3-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/6] test: avoid `stat` variable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-03-29T09:48:42Z","receivedAt":"2023-03-29T09:49:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 28 2023, Felipe Contreras wrote:\n\n> It has special meaning in zsh.\n\nNit: If we are going to rename these I'd prefer (just from grepping\nexisting examples) $exit_code, $code, $ret etc. a \"$stat\" at a glance\nwould seem to refer to stat() somehow, which is confusing.\n"},{"id":"474400","messageId":"230329.861ql7x4k4.gmgdl@evledraar.gmail.com","threadId":"59489","inReplyTo":"CAMP44s07W6SzJZnmpMMejM8cTNvwrKnNiD+1U=JWMBwUX8JbMA@mail.gmail.com","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-03-29T09:51:08Z","receivedAt":"2023-03-29T09:58:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 28 2023, Felipe Contreras wrote:\n\n> On Tue, Mar 28, 2023 at 6:57 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n>>\n>> On 2023-03-28 at 17:39:26, Felipe Contreras wrote:\n>> > It's not difficult to make the testing library work for zsh, so I did that in\n>> > the first patch.\n>> >\n>> > The rest of the patches are basically to deal with some variables that are\n>> > special in zsh, workaround a bug, and a minor discrepancy.\n>>\n>> There was a point at which the tests worked entirely in sh mode with\n>> zsh.  I know because I fixed a handful of tests there, ending with\n>> c64368e3a2a47, and I patched zsh to run all commands in a pipeline in a\n>> subshell in sh mode to fix the remaining tests.\n>>\n>> If I symlink zsh (zsh 5.9 (x86_64-debian-linux-gnu)) to sh in a\n>> temporary directory and use it in SHELL_PATH, I get only the following\n>> failures:\n>\n> That would defeat my motivation behind the patches, which is to be\n> able to run one test file in zsh.\n\n\"One\" as in one specific file you have in mind, or a \"one-off run\"? The\n1/6 here looks like it fixes most of the issues, but e.g. the\ntest-lib.sh fix in 2/6 would be needed by any test that reached that\ncode, wouldn't it?\n\nIf it's the latter, I don't really see the point of making just some of\nit work with zsh's native mode (if I'm getting this correctly).\n\nBut for that case, wouldn't:\n\n\tzsh --some-options t0001-init.sh\n\nOr whatever work, which just from skimming the help might be some of the\n--posix options? But I can see that being more of a hassle, presumably\nyou want to use it as /bin/sh, and to have it pick up the script and\nhave it Just Work.\n\nSome details on all of this in an updated commit message would be most\nwelcome...\n\n> Only the first patch is needed for that, the rest were in case anyone\n> cared to run all the tests.\n>\n>> I don't care a lot of other folks want to make zsh run the testsuite in\n>> zsh mode, but I'd think that using sh mode would be simpler and less\n>> likely to break in general, and would avoid us needing to carry a lot of\n>> patches to work around various variables that are special in zsh mode.\n>\n> We don't need to carry the patches if the patches are applied.\n\nBut we do need to carry some hacks going forward, some of it seems\npretty isolated & easy to spot, but e.g. the 6/6 fix of:\n\n-\t\tif test \"$c\" = \" \"\n+\t\tif test \"$c\" = \" \" || test -z \"$c\"\n\nIs quite subtle, you might look at that and be convinced that the RHS is\nredundant, and be right, but only because you assume POSIX semantics.\n\nIf we are going to include this I think the relevant t/README and\nDocumentation/CodingGuidelines parts should be updated to note that\nwe're not targeting POSIX shellscripts anymore, but the subset of it\nthat zsh is happy with.\n\nAnd, to avoid the inevitable re-breakage have this tested in CI,\nusurping 1/2 OSX jobs for that might be a good target (or one of the\nother linux jobs)...\n"},{"id":"474402","messageId":"CAMP44s1z6PBS8whv6+nhbzQr-H2+QYTDw=WHf0AkY2mbJDZfMA@mail.gmail.com","threadId":"59489","inReplyTo":"230329.861ql7x4k4.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-29T11:19:27Z","receivedAt":"2023-03-29T11:19:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Mar 29, 2023 at 3:58 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>\n> On Tue, Mar 28 2023, Felipe Contreras wrote:\n>\n> > On Tue, Mar 28, 2023 at 6:57 PM brian m. carlson\n> > <sandals@crustytoothpaste.net> wrote:\n> >>\n> >> On 2023-03-28 at 17:39:26, Felipe Contreras wrote:\n> >> > It's not difficult to make the testing library work for zsh, so I did that in\n> >> > the first patch.\n> >> >\n> >> > The rest of the patches are basically to deal with some variables that are\n> >> > special in zsh, workaround a bug, and a minor discrepancy.\n> >>\n> >> There was a point at which the tests worked entirely in sh mode with\n> >> zsh.  I know because I fixed a handful of tests there, ending with\n> >> c64368e3a2a47, and I patched zsh to run all commands in a pipeline in a\n> >> subshell in sh mode to fix the remaining tests.\n> >>\n> >> If I symlink zsh (zsh 5.9 (x86_64-debian-linux-gnu)) to sh in a\n> >> temporary directory and use it in SHELL_PATH, I get only the following\n> >> failures:\n> >\n> > That would defeat my motivation behind the patches, which is to be\n> > able to run one test file in zsh.\n>\n> \"One\" as in one specific file you have in mind, or a \"one-off run\"?\n\nOne specific file.\n\nI just did a quick porting of the code in my fork, and this should\ngive you a good idea (it's preliminary):\n\nhttps://github.com/felipec/git/blob/zsh/tests/t/t9904-completion-zsh.sh\n\nI had already sent versions of this test file that run in bash, but\nit's much better to fix test-lib.sh so that it works for zsh and they\ncan be run directly.\n\n> The\n> 1/6 here looks like it fixes most of the issues, but e.g. the\n> test-lib.sh fix in 2/6 would be needed by any test that reached that\n> code, wouldn't it?\n\nYes, but my test file doesn't reach that code.\n\n> Some details on all of this in an updated commit message would be most\n> welcome...\n\nWell, those details depended on the response.\n\nSince nobody seems to be interested in the tests running with vanilla\nzsh, I can just drop them and the explanation.\n\n> > Only the first patch is needed for that, the rest were in case anyone\n> > cared to run all the tests.\n> >\n> >> I don't care a lot of other folks want to make zsh run the testsuite in\n> >> zsh mode, but I'd think that using sh mode would be simpler and less\n> >> likely to break in general, and would avoid us needing to carry a lot of\n> >> patches to work around various variables that are special in zsh mode.\n> >\n> > We don't need to carry the patches if the patches are applied.\n>\n> But we do need to carry some hacks going forward, some of it seems\n> pretty isolated & easy to spot, but e.g. the 6/6 fix of:\n>\n> -               if test \"$c\" = \" \"\n> +               if test \"$c\" = \" \" || test -z \"$c\"\n>\n> Is quite subtle, you might look at that and be convinced that the RHS is\n> redundant, and be right, but only because you assume POSIX semantics.\n>\n> If we are going to include this I think the relevant t/README and\n> Documentation/CodingGuidelines parts should be updated to note that\n> we're not targeting POSIX shellscripts anymore, but the subset of it\n> that zsh is happy with.\n\nThere's no point in that. I consider it a bug in zsh, along with 5/6,\nso presumably at some point it's going to be fixed.\n\nAnd if nobody cares about running these tests in zsh, it doesn't matter anyway.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"474410","messageId":"xmqqh6u3d119.fsf@gitster.g","threadId":"59489","inReplyTo":"ZCONCVC2ITBJWoBA@tapette.crustytoothpaste.net","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-29T15:34:26Z","receivedAt":"2023-03-29T15:35:27Z","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> I don't care a lot of other folks want to make zsh run the\n> testsuite in zsh mode, but I'd think that using sh mode would be\n> simpler and less likely to break in general, and would avoid us\n> needing to carry a lot of patches to work around various variables\n> that are special in zsh mode.  It would also be easier to\n> potentially test in the testsuite as well.\n\nWhile these patches may make it \"work\" with zsh in its native mode,\nbecause zsh that is running in its native mode is sufficiently\ndistant from the more POSIXy portable variants of Bourne shells like\ndash and bash, I find it hard to justify the cost of maintaining the\nresulting codebase to \"work\" that way.\n\nWe need to force our developers to pay attention to rules set by\nzsh, like \"$0 is not the way to spell the name of the script\",\n\"$status is not usable by end-users\", etc.  Making the problem\nworse, the rules are enforced only by zsh.  The situation is similar\nto the \"when to use $(pwd) and when to use $PWD?\" rules, which folks\nlike me without Windows have to spend extra minutes to remember\nevery time they use one of these constructs, but the number of rules\nseem to be much larger as these patches seem to show.\n\nThanks.\n\n"},{"id":"474426","messageId":"CAMP44s0eSDBXZEN9fRPdxV4GKLqckKtujMLDPORDjHfsdypDBQ@mail.gmail.com","threadId":"59489","inReplyTo":"xmqqh6u3d119.fsf@gitster.g","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-29T21:54:01Z","receivedAt":"2023-03-29T21:54:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Mar 29, 2023 at 9:34 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n>\n> > I don't care a lot of other folks want to make zsh run the\n> > testsuite in zsh mode, but I'd think that using sh mode would be\n> > simpler and less likely to break in general, and would avoid us\n> > needing to carry a lot of patches to work around various variables\n> > that are special in zsh mode.  It would also be easier to\n> > potentially test in the testsuite as well.\n>\n> While these patches may make it \"work\" with zsh in its native mode,\n> because zsh that is running in its native mode is sufficiently\n> distant from the more POSIXy portable variants of Bourne shells like\n> dash and bash, I find it hard to justify the cost of maintaining the\n> resulting codebase to \"work\" that way.\n\nWhy do we follow POSIX anyway?\n\nIs it a) because following POSIX is just good? Or b) because POSIX is\na standard that many shells attempt to follow and thus by following\nPOSIX we maximize the amount of code that works on most shells?\n\nA quick non-exhaustive search of the history of Git, shows these:\n\n74ec8cf674 Conform to POSIX (works in mksh/lksh)\nc5c39f4e34 Fix for dash\n7d661e5ed1 Conform with POSIX (works with dash and mksh)\nddf3a1152d Remove a warning in bash\n9b67c9942e Workaround for ksh\n5c63920190 Consideration for some shells (e.g. ksh93)\ne9980419cb Fails in ksh88\n268ef4d3d0 Aligns with POSIX\n8e98b35f87 ksh88 on AIX considers it a syntax error\n8f92c7755e Code would break under mksh\nb082687cba Not allowed in mksh\n661bfd13b4 Fails in xpdg4/sh and ksh\nf5799e05c0 Public domain ksh and ksh on IRIX 6.5 have a problem\n365c2aaafc Doesn't work in pdksh, POSIX unclear\n482ce70e14 Doesn't work in dash and ash\n77e572653b Doesn't work in dash\n0e418e568f Workaround for ksh\n49a43f5468 ksh doesn't like this\nc289c315c2 Doesn't work in xpg4/sh\n933e4f090d Workaround possible dash bug\nc7053aa88f Doesn't seem to be supported by dash\n\nIt does seem like in only around 15% of these patches the\nconsideration was \"this aligns with POSIX\", in most cases the patch\nwas to make the code work for a particular shell.\n\nIn most cases the mention of POSIX is used as a justification for the\nproposed code, but it was clear the intention was to make it work for\na specific shell (usually ksh).\n\nIn the few cases where POSIX was the main rationale, the shells in\nwhich the current code doesn't work are mentioned. Not a single patch\nsays \"this aligns with POSIX\" without mentioning any shell.\n\n> We need to force our developers to pay attention to rules set by\n> zsh, like \"$0 is not the way to spell the name of the script\",\n> \"$status is not usable by end-users\", etc.  Making the problem\n> worse, the rules are enforced only by zsh.\n\nNo, we don't. $0 is not something a test script needs. In fact, there\ndoesn't seem to be a single instance in which $0 is used except test\ninfrastructure (e.g. test-lib.sh)\n\nThis sounds like a rationalization post-decision rather than a reason\nfor the decision.\n\nLooking at the history it seems the test code has been more than happy\nto make accommodations for xpg4/sh, pdksh, ash, and even mksh, even\nwhen the code was already POSIX-compliant.\n\nIn truth all the patches regarding shell portability have been along\nthe lines of: \"this code makes $x shell work, doesn't break other\nshells, and isn't against POSIX\". In some cases even when the Austin\ngroup disagreed on what POSIX actually said, we did whatever worked in\nmost shells.\n\nIs there some sort of predisposition against zsh?\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"474429","messageId":"ZCS4SXlkqnvjxidA@tapette.crustytoothpaste.net","threadId":"59489","inReplyTo":"xmqqh6u3d119.fsf@gitster.g","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-03-29T22:14:33Z","receivedAt":"2023-03-29T22:14:38Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-03-29 at 15:34:26, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> > I don't care a lot of other folks want to make zsh run the\n> > testsuite in zsh mode, but I'd think that using sh mode would be\n> > simpler and less likely to break in general, and would avoid us\n> > needing to carry a lot of patches to work around various variables\n> > that are special in zsh mode.  It would also be easier to\n> > potentially test in the testsuite as well.\n> \n> While these patches may make it \"work\" with zsh in its native mode,\n> because zsh that is running in its native mode is sufficiently\n> distant from the more POSIXy portable variants of Bourne shells like\n> dash and bash, I find it hard to justify the cost of maintaining the\n> resulting codebase to \"work\" that way.\n\nI think that's a fine position to take.  We have found some fun bugs in\nour testsuite via zsh's sh mode, and zsh (in sh mode) is a valid shell\naccording to Debian's /bin/sh rules, so I think it's fair to address\nshell fixes for it in sh mode where we can, even though it sounds like\nwe'd prefer to skip the zsh mode.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"474440","messageId":"xmqqjzyz3p5n.fsf@gitster.g","threadId":"59489","inReplyTo":"ZCS4SXlkqnvjxidA@tapette.crustytoothpaste.net","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-30T03:15:48Z","receivedAt":"2023-03-30T03:16:04Z","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>> While these patches may make it \"work\" with zsh in its native mode,\n>> because zsh that is running in its native mode is sufficiently\n>> distant from the more POSIXy portable variants of Bourne shells like\n>> dash and bash, I find it hard to justify the cost of maintaining the\n>> resulting codebase to \"work\" that way.\n>\n> I think that's a fine position to take.  We have found some fun bugs in\n> our testsuite via zsh's sh mode, and zsh (in sh mode) is a valid shell\n> according to Debian's /bin/sh rules, so I think it's fair to address\n> shell fixes for it in sh mode where we can, even though it sounds like\n> we'd prefer to skip the zsh mode.\n\nYeah, as long as the sh mode is sufficiently compatible, I would be\nhappy to see the shell supported as one of the /bin/sh we can use,\nand I agree that it is fair to address shell portability fixes for\nzsh running in the sh compatible mode.\n\nIf the native mode didn't impose too much burden on our developers'\nto maintain, it would have been nicer, but judging from the contents\nof these patches, I am afraid that it falls on the other side of the\nline.\n\nThanks.\n"},{"id":"474447","messageId":"CAMP44s1Amx4raO2rzTHxCgfEXywZwPCJwJ8edzJO_+_pY8PAkg@mail.gmail.com","threadId":"59489","inReplyTo":"xmqqjzyz3p5n.fsf@gitster.g","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-30T07:47:09Z","receivedAt":"2023-03-30T07:47:25Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Mar 29, 2023 at 9:15 PM Junio C Hamano <gitster@pobox.com> wrote:\n\n> If the native mode didn't impose too much burden on our developers'\n> to maintain, it would have been nicer, but judging from the contents\n> of these patches, I am afraid that it falls on the other side of the\n> line.\n\nThere is zero burden to maintain.\n\nWhat burden did this fix specific for ksh93 cause to \"our developers\"\n0e418e568f (t0005: work around strange $? in ksh when program\nterminated by a signal, 2010-07-09)?\n\n--- a/t/t0005-signals.sh\n+++ b/t/t0005-signals.sh\n@@ -13,6 +13,7 @@ test_expect_success 'sigchain works' '\n        test-sigchain >actual\n        case \"$?\" in\n        143) true ;; # POSIX w/ SIGTERM=15\n+       271) true ;; # ksh w/ SIGTERM=15\n          3) true ;; # Windows\n          *) false ;;\n        esac &&\n\nThis patch was applied once and forgotten about for 6 six years until\none developer refactored the code in 9b67c9942e (tests: factor\nportable signal check out of t0005, 2016-06-30).\n\n\"Our developers\" did not have to care about code that affected only\none shell. Not since 2010, not since 2016, not even today.\n\nEven the original author admitted he didn't understand how this code\ntruly interacted in other situations [1]:\n\n> Frankly, I don't know how bash's 'kill -TERM' and a Windows program\n> interact.  I've avoided this topic like the plague so far.\n\nIt's not true that applying patches to fix a situation for a specific\nshell causes a hypothetical burden to our developers to magically\nappear.\n\nIn reality the patch is applied once and forgotten about it until a)\nanother developer stumbles upon a similar problem, or b) another\ndeveloper stumbles upon that specific code.\n\nNo developer cared about 0e418e568f being applied. It fixed the\nsituation for one shell, it didn't affect other shells negatively, and\nthat's all anyone needed to know.\n\n[1] https://lore.kernel.org/git/576DA6FB.1050108@kdbg.org\n\n-- \nFelipe Contreras\n"},{"id":"474449","messageId":"CAMP44s3m=gYhtH+h-Gsy9F+YNPsA5MXB5wuvLDEO+svA=FhATA@mail.gmail.com","threadId":"59489","inReplyTo":"20230328173932.3614601-6-felipe.contreras@gmail.com","subject":"Re: [PATCH 5/6] test: hack for zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-30T08:15:08Z","receivedAt":"2023-03-30T08:15:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Mar 28, 2023 at 11:39 AM Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n>\n> See: https://www.zsh.org/mla/workers/2023/msg00336.html\n\nIn case anyone is interested this has just been fixed:\n\nhttps://github.com/zsh-users/zsh/commit/12e5db145b098a62ff11b88eea26f473ea2ecdcf\n\n-- \nFelipe Contreras\n"},{"id":"474450","messageId":"xmqqa5zu4ka3.fsf@gitster.g","threadId":"59489","inReplyTo":"CAMP44s0eSDBXZEN9fRPdxV4GKLqckKtujMLDPORDjHfsdypDBQ@mail.gmail.com","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-30T10:15:48Z","receivedAt":"2023-03-30T10:16:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> While these patches may make it \"work\" with zsh in its native mode,\n>> because zsh that is running in its native mode is sufficiently\n>> distant from the more POSIXy portable variants of Bourne shells like\n>> dash and bash, I find it hard to justify the cost of maintaining the\n>> resulting codebase to \"work\" that way.\n>\n> Why do we follow POSIX anyway?\n\nIt is not what we follow that is at the primary issue.\n\nThe criteria is more about what our developers are expected to be\nfamiliar with, and what is reasonable to force our developers to\nbecome sufficiently familiar with.  Even among POSIXy Bourne\nvariants, dash being stricter than bash already gives some new\nfolks, who only know bash, things to learn to avoid.  But at least\nwhat they learn through such an effort would help them write more\nportable scripts.  It is my impression, however, that zsh in its\nnative mode is even further out and away, pushing it on the other\nside of the line of being reasonable to force our develoerps to\nadjust to.  What they will learn through such an effort would be\nmore of \"what to do when you are forced to use zsh\" than \"how to\nwrite your shell script portably\".\n\n> In truth all the patches regarding shell portability have been along\n> the lines of: \"this code makes $x shell work, doesn't break other\n> shells, and isn't against POSIX\". In some cases even when the Austin\n> group disagreed on what POSIX actually said, we did whatever worked in\n> most shells.\n\nOne aspect that is missing in the above is the extra burden on our\ndevelopers.\n\n> Is there some sort of predisposition against zsh?\n\nThere isn't.  If somebody makes the tests to also work with csh or\n/bin/sh on SunOS, I would pretty much say the same thing.  The\nend-result may work perfectly well, but the cost to maintain it\nworking over the long haul may be too high for our developers.\n"},{"id":"474456","messageId":"CAMP44s1MuQK1uWTnWLv9AJ4X6P_Xd3XSH1KGJOG0bn0aaBNMqQ@mail.gmail.com","threadId":"59489","inReplyTo":"CAMP44s1z6PBS8whv6+nhbzQr-H2+QYTDw=WHf0AkY2mbJDZfMA@mail.gmail.com","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-30T13:00:24Z","receivedAt":"2023-03-30T13:00:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Mar 29, 2023 at 5:19 AM Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> On Wed, Mar 29, 2023 at 3:58 AM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n\n> > But we do need to carry some hacks going forward, some of it seems\n> > pretty isolated & easy to spot, but e.g. the 6/6 fix of:\n> >\n> > -               if test \"$c\" = \" \"\n> > +               if test \"$c\" = \" \" || test -z \"$c\"\n> >\n> > Is quite subtle, you might look at that and be convinced that the RHS is\n> > redundant, and be right, but only because you assume POSIX semantics.\n\nActually, that isn't even true (see below).\n\n> > If we are going to include this I think the relevant t/README and\n> > Documentation/CodingGuidelines parts should be updated to note that\n> > we're not targeting POSIX shellscripts anymore, but the subset of it\n> > that zsh is happy with.\n\nBut in this particular case the exact opposite is true: the script is\n*not* POSIX, it just happens to work on bash and other shells.\n\nYou *assume* it's POSIX because it works on bash and it doesn't work\non zsh, but in this particular case bash is the non-POSIX one, zsh is\nfollowing POSIX correctly.\n\n> There's no point in that. I consider it a bug in zsh, along with 5/6,\n> so presumably at some point it's going to be fixed.\n\nActually, no. I've changed my mind.\n\nI was going to report to the zsh dev mailing list the fact that this\ncreated an extra empty field at the end (in sh mode):\n\n  IFS=, ; str='foo,bar,,roo,'; printf '\"%s\"\\n' $str\n\nBut then I read the POSIX specification, and the section 2.6.5 Field\nSplitting [1] is very clear on what should happen.\n\nWhat muddles the waters is the distinction between `IFS white space`\ncharacters (newline, space and tab), and non-`IFS white space`\ncharacters (all the other).\n\nIf we ignore all the shite space stuff and concentrate on the rules\nfor non-`IFS white space` characters (as comma is), then we arrive at\nthis subitem:\n\n3.b. \"Each occurrence in the input of an IFS character that is not IFS\nwhite space, along with any adjacent IFS white space, shall delimit a\nfield, as described previously.\"\n\nIn other words: each occurence of a non-`IFS white space` character\nshall delimit a field. Or: each occurence of a comma should delimit a\nfield.\n\nThe script only works if the last delimiter does *not* delimit a\nfield, and thus it's not following POSIX, it just happens to work on\nmost shells. My patch does make it align with POSIX.\n\nI've reported bash's non-compliance with POSIX to their mailing list [2].\n\nBut I bet nobody here will care, because POSIX is just an excuse to\nsegregate the shells the main developers want to make work, from the\nones they are not (Brian even used the language of certain shells\nbeing one of \"the good ones\").\n\nCheers.\n\n[1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_05\n[2] https://lists.gnu.org/archive/html/bug-bash/2023-03/msg00152.html\n\n--\nFelipe Contreras\n"},{"id":"474458","messageId":"CAMP44s3xVL0UHCHh2Ei=STTx=grkpvUTfj6o9roe3tL35GhG4Q@mail.gmail.com","threadId":"59489","inReplyTo":"xmqqa5zu4ka3.fsf@gitster.g","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-30T14:19:08Z","receivedAt":"2023-03-30T14:19:56Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Mar 30, 2023 at 4:15 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n> >> While these patches may make it \"work\" with zsh in its native mode,\n> >> because zsh that is running in its native mode is sufficiently\n> >> distant from the more POSIXy portable variants of Bourne shells like\n> >> dash and bash, I find it hard to justify the cost of maintaining the\n> >> resulting codebase to \"work\" that way.\n> >\n> > Why do we follow POSIX anyway?\n>\n> It is not what we follow that is at the primary issue.\n>\n> The criteria is more about what our developers are expected to be\n> familiar with, and what is reasonable to force our developers to\n> become sufficiently familiar with.\n\nThat is not true.\n\nThis patch 77e572653b (t0050: fix printf format strings for\nportability, 2010-12-21) fixed a problem that was specific with dash.\nDid our developers have to learn the details of such issue? No.\n\nHow about f5799e05c0 (git-submodule.sh: separate parens by a space to\navoid confusing some shells, 2011-05-26)? Did anybody have to learn\nthat `((foo` causes problems in ksh in order to apply the patch?\n\nNo, these patches fixed specific issues for specific shells, and did\nnot affect what our developers were expected to be familiar with.\n\n> It is my impression, however, that zsh in its native mode is even\n> further out and away, pushing it on the other side of the line of\n> being reasonable to force our develoerps to adjust to.\n\nJust because it's your impression doesn't mean it's true.\n\n> What they will learn through such an effort would be more of \"what to\n> do when you are forced to use zsh\" than \"how to write your shell\n> script portably\".\n\nNo, they don't have to learn anything, just like they don't have to\nlearn that ksh93 returns signals that are 256+n: 0e418e568f (t0005:\nwork around strange $? in ksh when program terminated by a signal,\n2010-07-09).\n\nYou just picked the patch that fixed a specific problem for a specific\nshell, and we all forgot about it.\n\n> > Is there some sort of predisposition against zsh?\n>\n> There isn't.  If somebody makes the tests to also work with csh or\n> /bin/sh on SunOS, I would pretty much say the same thing.\n\nExcept you did pick the patches that fixed specific issues for these\nshells when the existing code was already compliant with POSIX.\n\n--\nFelipe Contreras\n"},{"id":"474534","messageId":"ZCd0MyieHOZDQeVM@nand.local","threadId":"59489","inReplyTo":"xmqqa5zu4ka3.fsf@gitster.g","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-04-01T00:00:51Z","receivedAt":"2023-04-01T00:01:03Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Mar 30, 2023 at 03:15:48AM -0700, Junio C Hamano wrote:\n> > In truth all the patches regarding shell portability have been along\n> > the lines of: \"this code makes $x shell work, doesn't break other\n> > shells, and isn't against POSIX\". In some cases even when the Austin\n> > group disagreed on what POSIX actually said, we did whatever worked in\n> > most shells.\n>\n> One aspect that is missing in the above is the extra burden on our\n> developers.\n\nWell said.\n\nHaving to remember that we need to write \"$ARGZERO\" instead of \"$0\" is\nyet another burden to place on developers who want to make changes to\nthe test suite.\n\nIs that a big deal? Probably not. But it's a slippery slope, and a weird\ngotcha to remember when dealing with our otherwise POSIX-y test suite.\n\nSo I'm not convinced that it's a worthwhile price to pay for zsh support\nin our test suite.\n\nThanks,\nTaylor\n"},{"id":"474535","messageId":"ZCd0+MqwYkKBO1DK@nand.local","threadId":"59489","inReplyTo":"CAMP44s3xVL0UHCHh2Ei=STTx=grkpvUTfj6o9roe3tL35GhG4Q@mail.gmail.com","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-04-01T00:04:08Z","receivedAt":"2023-04-01T00:04:38Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Mar 30, 2023 at 08:19:08AM -0600, Felipe Contreras wrote:\n> > It is my impression, however, that zsh in its native mode is even\n> > further out and away, pushing it on the other side of the line of\n> > being reasonable to force our develoerps to adjust to.\n>\n> Just because it's your impression doesn't mean it's true.\n\nSure, but zsh's incompatibility with bash is clear evidence that it is\nfurther out from the POSIX standard. But I think the argument is missing\nthe point, anyway, which is that it's incompatible with POSIX.\n\nIs it worth adapting our explicitly POSIX-compatible test suite to work\nwith a non-POSIX shell? I think not.\n\nThanks,\nTaylor\n"},{"id":"474536","messageId":"ZCd1Vyr+l9w/NmZ+@nand.local","threadId":"59489","inReplyTo":"20230328173932.3614601-3-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/6] test: avoid `stat` variable","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-04-01T00:05:43Z","receivedAt":"2023-04-01T00:06:10Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Mar 28, 2023 at 11:39:28AM -0600, Felipe Contreras wrote:\n> ---\n>  t/t0001-init.sh                | 4 ++--\n>  t/t3432-rebase-fast-forward.sh | 4 ++--\n>  t/t4013-diff-various.sh        | 6 +++---\n>  t/t5512-ls-remote.sh           | 4 ++--\n>  t/t5516-fetch-push.sh          | 8 ++++----\n>  t/test-lib.sh                  | 4 ++--\n>  6 files changed, 15 insertions(+), 15 deletions(-)\n\nMaybe I'm misreading your patch or its subject line, but all of these\nhunks convert from something (usually \"status\") _to_ \"stat\". Is the\npoint to avoid \"status\", not \"stat\"? Or the other way around?\n\nThanks,\nTaylor\n"},{"id":"474538","messageId":"CAMP44s0XQnDkRcR7bzztsiuPBXTBdECHLQ=7pZYzt4d3oBB8Sw@mail.gmail.com","threadId":"59489","inReplyTo":"ZCd1Vyr+l9w/NmZ+@nand.local","subject":"Re: [PATCH 2/6] test: avoid `stat` variable","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-01T00:25:05Z","receivedAt":"2023-04-01T00:25:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Mar 31, 2023 at 6:05 PM Taylor Blau <me@ttaylorr.com> wrote:\n>\n> On Tue, Mar 28, 2023 at 11:39:28AM -0600, Felipe Contreras wrote:\n> > ---\n> >  t/t0001-init.sh                | 4 ++--\n> >  t/t3432-rebase-fast-forward.sh | 4 ++--\n> >  t/t4013-diff-various.sh        | 6 +++---\n> >  t/t5512-ls-remote.sh           | 4 ++--\n> >  t/t5516-fetch-push.sh          | 8 ++++----\n> >  t/test-lib.sh                  | 4 ++--\n> >  6 files changed, 15 insertions(+), 15 deletions(-)\n>\n> Maybe I'm misreading your patch or its subject line, but all of these\n> hunks convert from something (usually \"status\") _to_ \"stat\".\n\nNo, the title is wrong; the purpose is to avoid `status`, which in zsh\nit's linked to $?\n\n-- \nFelipe Contreras\n"},{"id":"474539","messageId":"CAMP44s3MSSAC9dEDo23dJw=G1X31xJgNQJZEDYEoEheeEX9JSA@mail.gmail.com","threadId":"59489","inReplyTo":"ZCd0MyieHOZDQeVM@nand.local","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-01T00:50:17Z","receivedAt":"2023-04-01T00:52:08Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Mar 31, 2023 at 6:00 PM Taylor Blau <me@ttaylorr.com> wrote:\n>\n> On Thu, Mar 30, 2023 at 03:15:48AM -0700, Junio C Hamano wrote:\n> > > In truth all the patches regarding shell portability have been along\n> > > the lines of: \"this code makes $x shell work, doesn't break other\n> > > shells, and isn't against POSIX\". In some cases even when the Austin\n> > > group disagreed on what POSIX actually said, we did whatever worked in\n> > > most shells.\n> >\n> > One aspect that is missing in the above is the extra burden on our\n> > developers.\n>\n> Well said.\n>\n> Having to remember that we need to write \"$ARGZERO\" instead of \"$0\"\n\nWhen have you ever used \"$0\" in the test suite?\n\nA quick grep shows zero results (`git log --author=me@ttaylorr.com\n-S'$0'`), so I think you are talking about a hypothetical, not\nsomething that would actually happen in reality.\n\nSometimes preemptive optimization pays off in the future, but other\ntimes it doesn't, and it's just wasted mental effort.\n\nThis is one of those times when worrying about a future that will\nnever happen in reality does not pay off.\n\n> Is that a big deal? Probably not. But it's a slippery slope, and a weird\n> gotcha to remember when dealing with our otherwise POSIX-y test suite.\n\nYou won't have to remember that because you'll never use $0 in the\ntest suite. Nobody does use it, and nobody ever will.\n\n*If* for some weird reason somebody needs to use $0, we can worry\nabout it *then*.\n\n---\n\nBut this is a red herring. The reality is that developers do not have\nto worry about every little aspect of the test suite. When somebody\nuses `seq`, somebody else reminds them to use `test_seq` instead, and\nif for some reason a `seq` slips by and it breaks the test in some\nobscure platform, the test is updated to fix that. It's not a big\ndeal.\n\nWhy are many tests using `chmod` instead of `test_chmod`? Did the\nintroduction of `test_chmod` imply an extra burden to \"our developers\"\nto remember using that instead of `chmod`? No, *in reality* what\neveryone cares about is that the test runs on the platforms of the\nreal world.\n\nIf we could run the test suite on 100 hypothetical platforms that\ndon't exist in the real world, it would break all over the place.\n\nIn reality all the portability considerations of the test suite are\ngeared towards certain platforms that exist in the real world. Nobody\ncares that the test suite doesn't run on some hypothetical platforms.\n\nSo no, nobody needs to remember to use test_chmod, or test_seq, and\nnobody will ever have to remember to use $ARGZERO instead of $0, and\nif some hypothetical person does use that in some hypothetical test\nand that breaks the tests for zsh in native mode, nobody will care,\nexcept the person running those tests (likely me), which would\npromptly fix that single hypothetical instance of $0.\n\nNobody will die if a instance of $0 slipped by (which will never happen).\n\nThis is simply not a real consideration.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"474540","messageId":"CAMP44s0FMQhRCLnx0eLr-Xz3ajn_+83eCpdJMcda6wovtUHuwQ@mail.gmail.com","threadId":"59489","inReplyTo":"ZCd0+MqwYkKBO1DK@nand.local","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-01T00:59:43Z","receivedAt":"2023-04-01T01:00:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Mar 31, 2023 at 6:04 PM Taylor Blau <me@ttaylorr.com> wrote:\n>\n> On Thu, Mar 30, 2023 at 08:19:08AM -0600, Felipe Contreras wrote:\n> > > It is my impression, however, that zsh in its native mode is even\n> > > further out and away, pushing it on the other side of the line of\n> > > being reasonable to force our develoerps to adjust to.\n> >\n> > Just because it's your impression doesn't mean it's true.\n>\n> Sure, but zsh's incompatibility with bash is clear evidence that it is\n> further out from the POSIX standard.\n\nNo, the POSIX standard is not whatever bash does; bash does many\nthings that are not part of POSIX, and it even has a POSIX mode\nprecisely because it does not follow it to a tee: `bash --posix`.\n\nIf `ksh` does something that bash does differently, is that evidence\nthat `ksh` is further out from the POSIX standard?\n\nThis argument does not make sense.\n\n> But I think the argument is missing the point, anyway, which is that it's incompatible with POSIX.\n\nSo is bash.\n\nAll shells in the world are incompatible with the POSIX standard in\none way or another.\n\nPOSIX doesn't say anything about the $COLUMNS variable, and yet bash\nsets it (and many other variables). Does that mean bash is\n\"non-POSIX\"?\n\nOf course not; POSIX doesn't say \"thou shall not set the $COLUMNS\nenvironment variable\", so it's OK for bash to do that, it's still\ncompatible with posix (as long as you use `--posix`).\n\nThis's just not how standards work.\n\n> Is it worth adapting our explicitly POSIX-compatible test suite to work\n> with a non-POSIX shell? I think not.\n\nAll shells are non-POSIX.\n\n-- \nFelipe Contreras\n"},{"id":"474541","messageId":"xmqqv8igbd8e.fsf@gitster.g","threadId":"59489","inReplyTo":"CAMP44s3xVL0UHCHh2Ei=STTx=grkpvUTfj6o9roe3tL35GhG4Q@mail.gmail.com","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-01T01:30:41Z","receivedAt":"2023-04-01T01:34:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> The criteria is more about what our developers are expected to be\n>> familiar with, and what is reasonable to force our developers to\n>> become sufficiently familiar with.\n>\n> That is not true.\n\nWe probably should agree to disagree.  Let me respond by picking the\nfirst example from your message and then stop.\n\n> This patch 77e572653b (t0050: fix printf format strings for\n> portability, 2010-12-21) fixed a problem that was specific with dash.\n> Did our developers have to learn the details of such issue? No.\n\nThe code before that change was feeding \"\\xc3\\xa4\" to printf,\nexpecting that it would be an acceptable way to spell hexadecimal\nbyte values, which was wrong.  The commit improved portability by\nrewriting them to spell the same byte values in octal.\n\nYes, our developers have to learn to avoid hexadecimal byte values\nand the commit serves as a reminder for them to learn from.  When a\ndeveloper writes printf format with '\\xCC' hexadecimal, reviewers\nwould need to catch it as a mistake, and that commit makes a good\nreference why we insist on such a rule.\n\nBy learning that practice, our developers will be trained to write\nscripts that not just work with bash but also with dash, which is a\nsmall good thing.  But still, it *is* forcing our developers to\nlearn one more rule.\n\nThere is a trade off: is it worth supporting dash by forcing our\ndevelopers to stick to the rule to write octal and not hex?  dash is\nused as the default for some distros and considered one of the\nstandard ones, and is worth supporting even if we need to stay away\nfrom some stuff people may have picked up from other shells like\nbash.\n\nIf it were a different shell, the equation may have been different.\nIf industry standards like POSIX.1 required supporting hex literals,\nthe equation may also have been different.\n\n\tNote: though, we do not blindly say \"it is in POSIX, your\n        shell behaves differently and we won't support it\".\n\nIn any case, once we declare that \"we aim for our scripts and tests\nto work with dash and bash and these other shells\", our developers\nare forced to stick to intersection of these supported ones.\n\nIt takes a judgement call.  And \"don't write literals you feed\nprintf in hex, instead do so in octal, because printf built into\nsome shells do not like it\" is something reasonable to force our\ndevelopers to stick to (as it allows \"dash\" to be thrown into the\nset of supported shells).  I however do not see \"don't use path or\nstatus or options or 0 as shell variable names\" falls into the same\ncategory (even though it may allow \"zsh\" in native mode to be\nincluded into the supported shells).  Do benefits outweigh\ndownsides?\n\nAnd somebody has to draw that line.  Judgement calls are just that.\nThere are no absolutely right or wrong answers and they will not\nplease everybody.  Some folks may not agree with where the line gets\ndrawn.  Tough.\n\nBut the job of maintainer is not about being loved by everybody.\nJust drawing the line somewhere in order to allow folks to move on,\nwithout having everybody dragged into and getting stuck in endless\narguments, in which there is no absolute right or wrong.  That is\nwhat needs to be done, and that is what I just did.\n"},{"id":"474546","messageId":"CAMP44s0eQqvDTevs_tNqnc4Z_ZaOb4PoEXdSFmXxJn9Nyd5Emg@mail.gmail.com","threadId":"59489","inReplyTo":"xmqqv8igbd8e.fsf@gitster.g","subject":"Re: [PATCH 0/6] test: make the test suite work with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-01T02:39:06Z","receivedAt":"2023-04-01T02:40:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Mar 31, 2023 at 7:30 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n> >> The criteria is more about what our developers are expected to be\n> >> familiar with, and what is reasonable to force our developers to\n> >> become sufficiently familiar with.\n> >\n> > That is not true.\n>\n> We probably should agree to disagree.\n\nYou can decide to disagree with a fact, but it's still a fact.\n\nIt is a fact that portability fixes have been merged to the code with\nzero consideration to what \"our developers\" would have to be\n\"sufficiently familiar with\" (which is very little, if anything).\n\nI provided dozens of examples, and in this particular response chose\nto only pick two.\n\n>Let me respond by picking the first example from your message and then stop.\n\nSure, but that's just *one* example of many that prove what I'm saying\nis a fact, and I explain below it doesn't apply.\n\n> > This patch 77e572653b (t0050: fix printf format strings for\n> > portability, 2010-12-21) fixed a problem that was specific with dash.\n> > Did our developers have to learn the details of such issue? No.\n>\n> The code before that change was feeding \"\\xc3\\xa4\" to printf,\n> expecting that it would be an acceptable way to spell hexadecimal\n> byte values, which was wrong.  The commit improved portability by\n> rewriting them to spell the same byte values in octal.\n>\n> Yes, our developers have to learn to avoid hexadecimal byte values\n> and the commit serves as a reminder for them to learn from.\n\nEven if that were true (which it isn't), that has *absolutely nothing*\nto do with the patch in question.\n\nDevelopers would have to avoid that practice because it breaks dash,\nnot because you picked the patch.\n\nThis is a red herring: merging or not merging the patch makes no\ndifference to the practice.\n\nAnd this is a prescriptive claim from you. *You* say that our\ndevelopers have to learn that practice, and that's your opinion of\nwhat our developers should do. But that doesn't mean that our\ndevelopers did actually learn that.\n\n> When a developer writes printf format with '\\xCC' hexadecimal,\n> reviewers would need to catch it as a mistake, and that commit makes a\n> good reference why we insist on such a rule.\n\nYes, according to you, and regardless of whether or not you merge the patch.\n\n> But still, it *is* forcing our developers to learn one more rule.\n\nNot true. The fact that such code breaks dash is what is forcing our\ndevelopers to learn that rule, not you merging the patch.\n\nMost developers would not even notice that you merged the patch. And\nsuch practice would not be shared in a memo, notes from the\nmaintainer, or update of the CodingGuidelines.\n\nYou may be expecting such practice from developers after you merge\nsuch patch, but that's very different from developers actually knowing\nthat they have to adopt this practice.\n\n> There is a trade off: is it worth supporting dash by forcing our\n> developers to stick to the rule to write octal and not hex?  dash is\n> used as the default for some distros and considered one of the\n> standard ones, and is worth supporting even if we need to stay away\n> from some stuff people may have picked up from other shells like\n> bash.\n\nzsh is the default in macOS.\n\nIs macOS less important than some distros?\n\n> In any case, once we declare that \"we aim for our scripts and tests\n> to work with dash and bash and these other shells\", our developers\n> are forced to stick to intersection of these supported ones.\n\nOnce again: red herring. Merging a patch has absolutely nothing to do\nwith you declaring such an aim.\n\nAnd if that were true, there would be a declaration of \"Git now\nsupports dash in its test suite\".\n\nWhen did such a declaration take place?\n\nMoreover, the declaration has nothing to do with the patch.\n\n> It takes a judgement call.  And \"don't write literals you feed\n> printf in hex, instead do so in octal, because printf built into\n> some shells do not like it\" is something reasonable to force our\n> developers to stick to (as it allows \"dash\" to be thrown into the\n> set of supported shells).\n\nNothing to do with the patch.\n\n> I however do not see \"don't use path or status or options or 0 as\n> shell variable names\" falls into the same category\n\nAnother red herring: because this is not something that anybody is\nsuggesting to actually do.\n\n---\n\nThe fact is that $0 is used nowhere in the tests.\n\nYou can decide to disagree with what `git grep '$0' t/t????-*.sh`\nshows, but it's an unobjectionable fact.\n\nNot that it matters, because I just sent another patch with `emulate\nsh -o POSIX_ARGZERO`, which doesn't require changing $0, so you'll\nneed another rationale to not support zsh.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}