{"thread":{"id":"42499","subject":"[PATCH] t0005: ksh93 portability workaround","startedAt":"2016-05-31T22:47:01Z","lastAt":"2016-05-31T23:17:01Z","messageCount":6,"participants":["Junio C Hamano","Eric Sunshine","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"287947","messageId":"xmqqinxt3kwq.fsf@gitster.mtv.corp.google.com","threadId":"42499","inReplyTo":null,"subject":"[PATCH] t0005: ksh93 portability workaround","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-31T22:47:01Z","receivedAt":"2016-05-31T22:47:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The test has two things ksh93 does not happy about:\n\n * It thinks \"(( command1; command2 ) | command3)\" is a perfectly\n sane way to write a pipeline. ksh93, unlike other POSIX shells,\n does not like the two open parentheses next to each other for\n whatever reason it has.\n\n * It adds 256, unlike 128 that are used by other POSIX shells, to\n the signal number that caused the process to die when coming up\n with the exit status.\n\nWhat is interesting is that we knew about the latter issue and had a\nworkaround in the test-sigchain test when verifying that SIGTERM\nworks OK, but we didn't have corresponding workaround for SIGPIPE.\n\nSigned-off-by: Junio C Hamano \n---\n t/t0005-signals.sh | 18 ++++++++++++++----\n 1 file changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t0005-signals.sh b/t/t0005-signals.sh\nindex e7f27eb..12b4efb 100755\n--- a/t/t0005-signals.sh\n+++ b/t/t0005-signals.sh\n@@ -9,6 +9,16 @@ two\n one\n EOF\n \n+died_with_sigpipe () {\n+\tcase \"$1\" in\n+\t141 | 269)\n+\t\t# POSIX w/ SIGPIPE=13 gives 141\n+\t\t# ksh w/ SIGPIPE=13 gives 269\n+\t\ttrue ;;\n+\t*)\tfalse ;;\n+\tesac\n+}\n+\n test_expect_success 'sigchain works' '\n \t{ test-sigchain >actual; ret=$?; } &&\n \tcase \"$ret\" in\n@@ -40,13 +50,13 @@ test_expect_success 'create blob' '\n '\n \n test_expect_success !MINGW 'a constipated git dies with SIGPIPE' '\n-\tOUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&\n-\ttest \"$OUT\" -eq 141\n+\tOUT=$( ( (large_git; echo $? 1>&3) | :) 3>&1 ) &&\n+\tdied_with_sigpipe \"$OUT\"\n '\n \n test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' '\n-\tOUT=$( ((trap \"\" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) &&\n-\ttest \"$OUT\" -eq 141\n+\tOUT=$( ( (trap \"\" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) &&\n+\tdied_with_sigpipe \"$OUT\"\n '\n \n test_done\n"},{"id":"287948","messageId":"CAPig+cRD8aTNJxvfkc-5Xjr6_pKRzAi3SpZ7JCZcRYMQFT_xMg@mail.gmail.com","threadId":"42499","inReplyTo":"xmqqinxt3kwq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0005: ksh93 portability workaround","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-31T22:49:41Z","receivedAt":"2016-05-31T22:49:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 31, 2016 at 6:47 PM, Junio C Hamano wrote:\n> The test has two things ksh93 does not happy about:\n\ns/does/is/\n\n> * It thinks \"(( command1; command2 ) | command3)\" is a perfectly\n> sane way to write a pipeline. ksh93, unlike other POSIX shells,\n> does not like the two open parentheses next to each other for\n> whatever reason it has.\n>\n> * It adds 256, unlike 128 that are used by other POSIX shells, to\n> the signal number that caused the process to die when coming up\n> with the exit status.\n>\n> What is interesting is that we knew about the latter issue and had a\n> workaround in the test-sigchain test when verifying that SIGTERM\n> works OK, but we didn't have corresponding workaround for SIGPIPE.\n>\n> Signed-off-by: Junio C Hamano \n"},{"id":"287951","messageId":"20160531230301.GB4585@sigill.intra.peff.net","threadId":"42499","inReplyTo":"xmqqinxt3kwq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0005: ksh93 portability workaround","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-31T23:03:01Z","receivedAt":"2016-05-31T23:03:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 31, 2016 at 03:47:01PM -0700, Junio C Hamano wrote:\n\n> The test has two things ksh93 does not happy about:\n> \n> * It thinks \"(( command1; command2 ) | command3)\" is a perfectly\n> sane way to write a pipeline. ksh93, unlike other POSIX shells,\n> does not like the two open parentheses next to each other for\n> whatever reason it has.\n> \n> * It adds 256, unlike 128 that are used by other POSIX shells, to\n> the signal number that caused the process to die when coming up\n> with the exit status.\n> \n> What is interesting is that we knew about the latter issue and had a\n> workaround in the test-sigchain test when verifying that SIGTERM\n> works OK, but we didn't have corresponding workaround for SIGPIPE.\n\nHmm. We discussed these back in:\n\n http://thread.gmane.org/gmane.comp.version-control.git/268657\n\nbut I thought we decided not to do anything about them (according to\nthat thread, I found a bunch of other ksh93 oddities, but maybe we've\nsince fixed them?).\n\n> +died_with_sigpipe () {\n> +\tcase \"$1\" in\n> +\t141 | 269)\n> +\t\t# POSIX w/ SIGPIPE=13 gives 141\n> +\t\t# ksh w/ SIGPIPE=13 gives 269\n> +\t\ttrue ;;\n> +\t*)\tfalse ;;\n> +\tesac\n> +}\n\nThis is OK, but I like the patch I posted in\n\n http://article.gmane.org/gmane.comp.version-control.git/268666\n\nbetter, as it contains the shell logic in a single place (though I am\nnot sure how these tests fare on Windows, where I think every signal\njust looks like \"3\").\n\n-Peff\n"},{"id":"287952","messageId":"xmqq60tt3k4f.fsf@gitster.mtv.corp.google.com","threadId":"42499","inReplyTo":"xmqqinxt3kwq.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] t/lib-git-daemon: ksh portability workaround","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-31T23:04:00Z","receivedAt":"2016-05-31T23:04:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"git daemon\" test checks with what status the daemon exits when\nwe terminate it, and we expect that we can observe death by SIGTERM.\n\nWe forgot that ksh adds 256, unlike 128 that are used by other POSIX\nshells, to the signal number that caused the process to die when\ncoming up with the exit status.\n\nSigned-off-by: Junio C Hamano \n---\ndiff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\nindex 340534c..623b3ae 100644\n--- a/t/lib-git-daemon.sh\n+++ b/t/lib-git-daemon.sh\n@@ -83,7 +83,8 @@ stop_git_daemon() {\n \twait \"$GIT_DAEMON_PID\" >&3 2>&4\n \tret=$?\n \t# expect exit with status 143 = 128+15 for signal TERM=15\n-\tif test $ret -ne 143\n+\t# or 271 = 256+15 on ksh\n+\tif test $ret -ne 143 && test $ret -ne 271\n \tthen\n \t\terror \"git daemon exited with status: $ret\"\n \tfi\n"},{"id":"287953","messageId":"20160531230547.GC4585@sigill.intra.peff.net","threadId":"42499","inReplyTo":"xmqq60tt3k4f.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t/lib-git-daemon: ksh portability workaround","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-31T23:05:47Z","receivedAt":"2016-05-31T23:05:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 31, 2016 at 04:04:00PM -0700, Junio C Hamano wrote:\n\n> The \"git daemon\" test checks with what status the daemon exits when\n> we terminate it, and we expect that we can observe death by SIGTERM.\n> \n> We forgot that ksh adds 256, unlike 128 that are used by other POSIX\n> shells, to the signal number that caused the process to die when\n> coming up with the exit status.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n> diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\n> index 340534c..623b3ae 100644\n> --- a/t/lib-git-daemon.sh\n> +++ b/t/lib-git-daemon.sh\n> @@ -83,7 +83,8 @@ stop_git_daemon() {\n> \twait \"$GIT_DAEMON_PID\" >&3 2>&4\n> \tret=$?\n> \t# expect exit with status 143 = 128+15 for signal TERM=15\n> -\tif test $ret -ne 143\n> +\t# or 271 = 256+15 on ksh\n> +\tif test $ret -ne 143 && test $ret -ne 271\n\nThe presence of this patch (on top of the other one) makes me think we\nshould go with something like:\n\n test_exit_code_is\n\nor something, and make it available via test-lib-functions.sh.\n\n-Peff\n"},{"id":"287957","messageId":"xmqqwpm924ya.fsf@gitster.mtv.corp.google.com","threadId":"42499","inReplyTo":"20160531230301.GB4585@sigill.intra.peff.net","subject":"Re: [PATCH] t0005: ksh93 portability workaround","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-31T23:17:01Z","receivedAt":"2016-05-31T23:17:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Hmm. We discussed these back in:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/268657\n>\n> but I thought we decided not to do anything about them (according to\n> that thread, I found a bunch of other ksh93 oddities, but maybe we've\n> since fixed them?).\n\nOK, I completely forgot about that topic.\n\nSorry for the noise. Let's drop it.\n\nThanks for reminding me of that thread; I would have been stumped\nwith the \"cd ../.git/objects\" thing and wasted a lot of time.\n"}]}