Re: t5570 trap use in start/stop_git_daemon
On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote: On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote: On the NonStop port, we found that trap was causing an issue with test success for t5570. When start_git_daemon completes, the shell (ksh,bash) on this platform is sending a signal 0 that is being caught and acted on by the trap command within the start_git_daemon and stop_git_daemon functions. I am taking this up with the operating system group, Yeah, that seems wrong. If it were a subshell, even, I could see some argument for it, but it seems odd to trap 0 when a function returns (bash does have a RETURN trap, which AFAIK is bash-specific, but it should not trigger a 0-trap). Hmm, today I learned something new about ksh. Apparently when you use the function keyword to define a function like: function foo { trap 'echo trapped' EXIT } echo before foo echo after then the trap runs when the function exits! If you declare the same function as: foo() { trap 'echo trapped' EXIT } it behaves differently. POSIX shell does not have the function keyword, of course, and we are not using it here. Bash _does_ have the function keyword, but seems to behave POSIX-y even when it is present. I.e., running the first script: $ ksh foo.sh before trapped after $ bash foo.sh before after trapped $ dash foo.sh foo.sh: 3: foo.sh: function: not found foo.sh: 5: foo.sh: Syntax error: } unexpected Switching to the second form, all three produce: before after trapped I don't know if that is all helpful to your bug-tracking or analysis, but for whatever reason it looks like your ksh is using localized traps for both forms of function. But as far as I know, bash has never behaved that way (I just grepped its CHANGES file for mentions of trap and found nothing likely). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t5570 trap use in start/stop_git_daemon
Jeff King peff at peff.net writes: On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote: On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote: snip Hmm, today I learned something new about ksh. Apparently when you use the function keyword to define a function like: function foo { trap 'echo trapped' EXIT } echo before foo echo after then the trap runs when the function exits! If you declare the same function as: foo() { trap 'echo trapped' EXIT } it behaves differently. POSIX shell does not have the function keyword, of course, and we are not using it here. Bash _does_ have the function keyword, but seems to behave POSIX-y even when it is present. I.e., running the first script: $ ksh foo.sh before trapped after $ bash foo.sh before after trapped $ dash foo.sh foo.sh: 3: foo.sh: function: not found foo.sh: 5: foo.sh: Syntax error: } unexpected Switching to the second form, all three produce: before after trapped I don't know if that is all helpful to your bug-tracking or analysis, but for whatever reason it looks like your ksh is using localized traps for both forms of function. But as far as I know, bash has never behaved that way (I just grepped its CHANGES file for mentions of trap and found nothing likely). -Peff Both versions produce your first output on our platform $ ksh foo1.sh before trapped after $ bash foo1.sh before after trapped $ ksh foo2.sh before trapped after $ bash foo2.sh before after trapped $ This might have been one (or even _the_) reason why we picked bash as our SHELL_PATH in config.mak.uname (I don't remember, it's more than 2 years ago), not sure which shell Randall's test used? bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: t5570 trap use in start/stop_git_daemon
On 2015/02/13 3:58AM Joachim Schmitz wrote: Jeff King peff at peff.net writes: On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote: On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote: snip Hmm, today I learned something new about ksh. Apparently when you use the function keyword to define a function like: function foo { trap 'echo trapped' EXIT } echo before foo echo after then the trap runs when the function exits! If you declare the same function as: foo() { trap 'echo trapped' EXIT } it behaves differently. POSIX shell does not have the function keyword, of course, and we are not using it here. Bash _does_ have the function keyword, but seems to behave POSIX-y even when it is present. I.e., running the first script: $ ksh foo.sh before trapped after $ bash foo.sh before after trapped snip Both versions produce your first output on our platform $ ksh foo1.sh before trapped after $ bash foo1.sh before after trapped $ ksh foo2.sh before trapped after $ bash foo2.sh before after trapped $ This might have been one (or even _the_) reason why we picked bash as our SHELL_PATH in config.mak.uname (I don't remember, it's more than 2 years ago), not sure which shell Randall's test used? I tested both for trying to get t5570 to work. No matter which, without resetting the trap, function return would kill the git-daemon and the test would fail. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t5570 trap use in start/stop_git_daemon
On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote: On the NonStop port, we found that trap was causing an issue with test success for t5570. When start_git_daemon completes, the shell (ksh,bash) on this platform is sending a signal 0 that is being caught and acted on by the trap command within the start_git_daemon and stop_git_daemon functions. I am taking this up with the operating system group, Yeah, that seems wrong. If it were a subshell, even, I could see some argument for it, but it seems odd to trap 0 when a function returns (bash does have a RETURN trap, which AFAIK is bash-specific, but it should not trigger a 0-trap). but in any case, it may be appropriate to include a trap reset at the end of both functions, as below. I verified this change on SUSE Linux. diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index bc4b341..543e98a 100644 --- a/t/lib-git-daemon.sh +++ b/t/lib-git-daemon.sh @@ -62,6 +62,7 @@ start_git_daemon() { test_skip_or_die $GIT_TEST_GIT_DAEMON \ git daemon failed to start fi + trap '' EXIT } I don't think this is the right thing to do. That trap is meant to live beyond the function's return. Without it, there is nothing to clean up the running git-daemon if we exit the test script prematurely (e.g., by a test failing in immediate-mode). We pollute the environment with a running process which would cause subsequent test runs to fail. stop_git_daemon() { @@ -84,4 +85,6 @@ stop_git_daemon() { fi GIT_DAEMON_PID= rm -f git_daemon_output + + trap '' EXIT } This one is slightly less bad, in that we are dropping our daemon-specific cleanup here anyway. But the appropriate trap is still: trap 'die' EXIT which we set earlier in the function. Without it, the test harness's ability to detect a premature failure is lost. So I do not know quite what is going on with your shell, but turning off the traps in these functions is definitely not an acceptable (general) workaround; it makes things much worse on working platforms. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html