git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] t0005: ksh93 portability workaround
@ 2016-05-31 22:47 Junio C Hamano
  2016-05-31 22:49 ` Eric Sunshine
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Junio C Hamano @ 2016-05-31 22:47 UTC (permalink / raw)
  To: git

The test has two things ksh93 does not happy about:

 * It thinks "(( command1; command2 ) | command3)" is a perfectly
   sane way to write a pipeline.  ksh93, unlike other POSIX shells,
   does not like the two open parentheses next to each other for
   whatever reason it has.

 * It adds 256, unlike 128 that are used by other POSIX shells, to
   the signal number that caused the process to die when coming up
   with the exit status.

What is interesting is that we knew about the latter issue and had a
workaround in the test-sigchain test when verifying that SIGTERM
works OK, but we didn't have corresponding workaround for SIGPIPE.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0005-signals.sh | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh
index e7f27eb..12b4efb 100755
--- a/t/t0005-signals.sh
+++ b/t/t0005-signals.sh
@@ -9,6 +9,16 @@ two
 one
 EOF
 
+died_with_sigpipe () {
+	case "$1" in
+	141 | 269)
+		# POSIX w/ SIGPIPE=13 gives 141
+		# ksh w/ SIGPIPE=13 gives 269
+		true ;;
+	*)	false ;;
+	esac
+}
+
 test_expect_success 'sigchain works' '
 	{ test-sigchain >actual; ret=$?; } &&
 	case "$ret" in
@@ -40,13 +50,13 @@ test_expect_success 'create blob' '
 '
 
 test_expect_success !MINGW 'a constipated git dies with SIGPIPE' '
-	OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&
-	test "$OUT" -eq 141
+	OUT=$( ( (large_git; echo $? 1>&3) | :) 3>&1 ) &&
+	died_with_sigpipe "$OUT"
 '
 
 test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' '
-	OUT=$( ((trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) &&
-	test "$OUT" -eq 141
+	OUT=$( ( (trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) &&
+	died_with_sigpipe "$OUT"
 '
 
 test_done

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] t0005: ksh93 portability workaround
  2016-05-31 22:47 [PATCH] t0005: ksh93 portability workaround Junio C Hamano
@ 2016-05-31 22:49 ` Eric Sunshine
  2016-05-31 23:03 ` Jeff King
  2016-05-31 23:04 ` [PATCH] t/lib-git-daemon: ksh " Junio C Hamano
  2 siblings, 0 replies; 6+ messages in thread
From: Eric Sunshine @ 2016-05-31 22:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List

On Tue, May 31, 2016 at 6:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The test has two things ksh93 does not happy about:

s/does/is/

>  * It thinks "(( command1; command2 ) | command3)" is a perfectly
>    sane way to write a pipeline.  ksh93, unlike other POSIX shells,
>    does not like the two open parentheses next to each other for
>    whatever reason it has.
>
>  * It adds 256, unlike 128 that are used by other POSIX shells, to
>    the signal number that caused the process to die when coming up
>    with the exit status.
>
> What is interesting is that we knew about the latter issue and had a
> workaround in the test-sigchain test when verifying that SIGTERM
> works OK, but we didn't have corresponding workaround for SIGPIPE.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] t0005: ksh93 portability workaround
  2016-05-31 22:47 [PATCH] t0005: ksh93 portability workaround Junio C Hamano
  2016-05-31 22:49 ` Eric Sunshine
@ 2016-05-31 23:03 ` Jeff King
  2016-05-31 23:17   ` Junio C Hamano
  2016-05-31 23:04 ` [PATCH] t/lib-git-daemon: ksh " Junio C Hamano
  2 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2016-05-31 23:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Tue, May 31, 2016 at 03:47:01PM -0700, Junio C Hamano wrote:

> The test has two things ksh93 does not happy about:
> 
>  * It thinks "(( command1; command2 ) | command3)" is a perfectly
>    sane way to write a pipeline.  ksh93, unlike other POSIX shells,
>    does not like the two open parentheses next to each other for
>    whatever reason it has.
> 
>  * It adds 256, unlike 128 that are used by other POSIX shells, to
>    the signal number that caused the process to die when coming up
>    with the exit status.
> 
> What is interesting is that we knew about the latter issue and had a
> workaround in the test-sigchain test when verifying that SIGTERM
> works OK, but we didn't have corresponding workaround for SIGPIPE.

Hmm. We discussed these back in:

  http://thread.gmane.org/gmane.comp.version-control.git/268657

but I thought we decided not to do anything about them (according to
that thread, I found a bunch of other ksh93 oddities, but maybe we've
since fixed them?).

> +died_with_sigpipe () {
> +	case "$1" in
> +	141 | 269)
> +		# POSIX w/ SIGPIPE=13 gives 141
> +		# ksh w/ SIGPIPE=13 gives 269
> +		true ;;
> +	*)	false ;;
> +	esac
> +}

This is OK, but I like the patch I posted in

  http://article.gmane.org/gmane.comp.version-control.git/268666

better, as it contains the shell logic in a single place (though I am
not sure how these tests fare on Windows, where I think every signal
just looks like "3").

-Peff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] t/lib-git-daemon: ksh portability workaround
  2016-05-31 22:47 [PATCH] t0005: ksh93 portability workaround Junio C Hamano
  2016-05-31 22:49 ` Eric Sunshine
  2016-05-31 23:03 ` Jeff King
@ 2016-05-31 23:04 ` Junio C Hamano
  2016-05-31 23:05   ` Jeff King
  2 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2016-05-31 23:04 UTC (permalink / raw)
  To: git

The "git daemon" test checks with what status the daemon exits when
we terminate it, and we expect that we can observe death by SIGTERM.

We forgot that ksh adds 256, unlike 128 that are used by other POSIX
shells, to the signal number that caused the process to die when
coming up with the exit status.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index 340534c..623b3ae 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -83,7 +83,8 @@ stop_git_daemon() {
 	wait "$GIT_DAEMON_PID" >&3 2>&4
 	ret=$?
 	# expect exit with status 143 = 128+15 for signal TERM=15
-	if test $ret -ne 143
+	# or 271 = 256+15 on ksh
+	if test $ret -ne 143 && test $ret -ne 271
 	then
 		error "git daemon exited with status: $ret"
 	fi

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] t/lib-git-daemon: ksh portability workaround
  2016-05-31 23:04 ` [PATCH] t/lib-git-daemon: ksh " Junio C Hamano
@ 2016-05-31 23:05   ` Jeff King
  0 siblings, 0 replies; 6+ messages in thread
From: Jeff King @ 2016-05-31 23:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Tue, May 31, 2016 at 04:04:00PM -0700, Junio C Hamano wrote:

> The "git daemon" test checks with what status the daemon exits when
> we terminate it, and we expect that we can observe death by SIGTERM.
> 
> We forgot that ksh adds 256, unlike 128 that are used by other POSIX
> shells, to the signal number that caused the process to die when
> coming up with the exit status.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
> index 340534c..623b3ae 100644
> --- a/t/lib-git-daemon.sh
> +++ b/t/lib-git-daemon.sh
> @@ -83,7 +83,8 @@ stop_git_daemon() {
>  	wait "$GIT_DAEMON_PID" >&3 2>&4
>  	ret=$?
>  	# expect exit with status 143 = 128+15 for signal TERM=15
> -	if test $ret -ne 143
> +	# or 271 = 256+15 on ksh
> +	if test $ret -ne 143 && test $ret -ne 271

The presence of this patch (on top of the other one) makes me think we
should go with something like:

  test_exit_code_is

or something, and make it available via test-lib-functions.sh.

-Peff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] t0005: ksh93 portability workaround
  2016-05-31 23:03 ` Jeff King
@ 2016-05-31 23:17   ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2016-05-31 23:17 UTC (permalink / raw)
  To: Jeff King; +Cc: git

Jeff King <peff@peff.net> writes:

> Hmm. We discussed these back in:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/268657
>
> but I thought we decided not to do anything about them (according to
> that thread, I found a bunch of other ksh93 oddities, but maybe we've
> since fixed them?).

OK, I completely forgot about that topic.

Sorry for the noise.  Let's drop it.

Thanks for reminding me of that thread; I would have been stumped
with the "cd ../.git/objects" thing and wasted a lot of time.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-05-31 23:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-31 22:47 [PATCH] t0005: ksh93 portability workaround Junio C Hamano
2016-05-31 22:49 ` Eric Sunshine
2016-05-31 23:03 ` Jeff King
2016-05-31 23:17   ` Junio C Hamano
2016-05-31 23:04 ` [PATCH] t/lib-git-daemon: ksh " Junio C Hamano
2016-05-31 23:05   ` Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).