{"thread":{"id":"38560","subject":"t5570 trap use in start/stop_git_daemon","startedAt":"2015-02-12T20:31:12Z","lastAt":"2015-02-13T12:27:01Z","messageCount":5,"participants":["Randall S. Becker","Jeff King","Joachim Schmitz"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"256018","messageId":"013601d04702$d7e721e0$87b565a0$@nexbridge.com","threadId":"38560","inReplyTo":null,"subject":"t5570 trap use in start/stop_git_daemon","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2015-02-12T20:31:12Z","receivedAt":"2015-02-12T20:31:12Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On the NonStop port, we found that “trap” was causing an issue with test\nsuccess for t5570. When start_git_daemon completes, the shell (ksh,bash) on\nthis platform is sending a signal 0 that is being caught and acted on by the\ntrap command within the start_git_daemon and stop_git_daemon functions. I am\ntaking this up with the operating system group, but in any case, it may be\nappropriate to include a trap reset at the end of both functions, as below.\nI verified this change on SUSE Linux.\n\ndiff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\nindex bc4b341..543e98a 100644\n--- a/t/lib-git-daemon.sh\n+++ b/t/lib-git-daemon.sh\n@@ -62,6 +62,7 @@ start_git_daemon() {\n                test_skip_or_die $GIT_TEST_GIT_DAEMON \\\n                        \"git daemon failed to start\"\n       fi\n+       trap '' EXIT\n}\n\nstop_git_daemon() {\n@@ -84,4 +85,6 @@ stop_git_daemon() {\n        fi\n        GIT_DAEMON_PID=\n        rm -f git_daemon_output\n+\n+       trap '' EXIT\n}\n\nCheers,\nRandall\n-- Brief whoami: NonStop&UNIX developer since approximately\nUNIX(421664400)/NonStop(211288444200000000)\n-- In real life, I talk too much.\n"},{"id":"256051","messageId":"20150213074403.GB26775@peff.net","threadId":"38560","inReplyTo":"013601d04702$d7e721e0$87b565a0$@nexbridge.com","subject":"Re: t5570 trap use in start/stop_git_daemon","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-13T07:44:03Z","receivedAt":"2015-02-13T07:44:03Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote:\n\n> On the NonStop port, we found that “trap” was causing an issue with test\n> success for t5570. When start_git_daemon completes, the shell (ksh,bash) on\n> this platform is sending a signal 0 that is being caught and acted on by the\n> trap command within the start_git_daemon and stop_git_daemon functions. I am\n> taking this up with the operating system group,\n\nYeah, that seems wrong. If it were a subshell, even, I could see some\nargument for it, but it seems odd to trap 0 when a function returns\n(bash does have a RETURN trap, which AFAIK is bash-specific, but it\nshould not trigger a 0-trap).\n\n> but in any case, it may be\n> appropriate to include a trap reset at the end of both functions, as below.\n> I verified this change on SUSE Linux.\n> \n> diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\n> index bc4b341..543e98a 100644\n> --- a/t/lib-git-daemon.sh\n> +++ b/t/lib-git-daemon.sh\n> @@ -62,6 +62,7 @@ start_git_daemon() {\n>                 test_skip_or_die $GIT_TEST_GIT_DAEMON \\\n>                         \"git daemon failed to start\"\n>        fi\n> +       trap '' EXIT\n> }\n\nI don't think this is the right thing to do. That trap is meant to live\nbeyond the function's return. Without it, there is nothing to clean up\nthe running git-daemon if we exit the test script prematurely (e.g., by\na test failing in immediate-mode). We pollute the environment with a\nrunning process which would cause subsequent test runs to fail.\n\n> stop_git_daemon() {\n> @@ -84,4 +85,6 @@ stop_git_daemon() {\n>         fi\n>         GIT_DAEMON_PID=\n>         rm -f git_daemon_output\n> +\n> +       trap '' EXIT\n> }\n\nThis one is slightly less bad, in that we are dropping our\ndaemon-specific cleanup here anyway. But the appropriate trap is still:\n\n  trap 'die' EXIT\n\nwhich we set earlier in the function. Without it, the test harness's\nability to detect a premature failure is lost.\n\nSo I do not know quite what is going on with your shell, but turning off\nthe traps in these functions is definitely not an acceptable (general)\nworkaround; it makes things much worse on working platforms.\n\n-Peff\n"},{"id":"256052","messageId":"20150213080359.GC26775@peff.net","threadId":"38560","inReplyTo":"20150213074403.GB26775@peff.net","subject":"Re: t5570 trap use in start/stop_git_daemon","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-13T08:03:59Z","receivedAt":"2015-02-13T08:03:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote:\n\n> On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote:\n> \n> > On the NonStop port, we found that “trap” was causing an issue with test\n> > success for t5570. When start_git_daemon completes, the shell (ksh,bash) on\n> > this platform is sending a signal 0 that is being caught and acted on by the\n> > trap command within the start_git_daemon and stop_git_daemon functions. I am\n> > taking this up with the operating system group,\n> \n> Yeah, that seems wrong. If it were a subshell, even, I could see some\n> argument for it, but it seems odd to trap 0 when a function returns\n> (bash does have a RETURN trap, which AFAIK is bash-specific, but it\n> should not trigger a 0-trap).\n\nHmm, today I learned something new about ksh. Apparently when you use\nthe \"function\" keyword to define a function like:\n\n  function foo {\n    trap 'echo trapped' EXIT\n  }\n  echo before\n  foo\n  echo after\n\nthen the trap runs when the function exits! If you declare the same\nfunction as:\n\n  foo() {\n    trap 'echo trapped' EXIT\n  }\n\nit behaves differently. POSIX shell does not have the function keyword,\nof course, and we are not using it here. Bash _does_ have the function\nkeyword, but seems to behave POSIX-y even when it is present. I.e.,\nrunning the first script:\n\n  $ ksh foo.sh\n  before\n  trapped\n  after\n\n  $ bash foo.sh\n  before\n  after\n  trapped\n\n  $ dash foo.sh\n  foo.sh: 3: foo.sh: function: not found\n  foo.sh: 5: foo.sh: Syntax error: \"}\" unexpected\n\nSwitching to the second form, all three produce:\n\n  before\n  after\n  trapped\n\nI don't know if that is all helpful to your bug-tracking or analysis,\nbut for whatever reason it looks like your ksh is using localized traps\nfor both forms of function. But as far as I know, bash has never behaved\nthat way (I just grepped its CHANGES file for mentions of trap and found\nnothing likely).\n\n-Peff\n"},{"id":"256053","messageId":"loom.20150213T094712-477@post.gmane.org","threadId":"38560","inReplyTo":"20150213080359.GC26775@peff.net","subject":"Re: t5570 trap use in start/stop_git_daemon","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-13T08:57:51Z","receivedAt":"2015-02-13T08:57:51Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Jeff King <peff <at> peff.net> writes:\n\n> \n> On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote:\n> \n> > On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote:\n> > \n<snip> \n> Hmm, today I learned something new about ksh. Apparently when you use\n> the \"function\" keyword to define a function like:\n> \n>   function foo {\n>     trap 'echo trapped' EXIT\n>   }\n>   echo before\n>   foo\n>   echo after\n> \n> then the trap runs when the function exits! If you declare the same\n> function as:\n> \n>   foo() {\n>     trap 'echo trapped' EXIT\n>   }\n> \n> it behaves differently. POSIX shell does not have the function keyword,\n> of course, and we are not using it here. Bash _does_ have the function\n> keyword, but seems to behave POSIX-y even when it is present. I.e.,\n> running the first script:\n> \n>   $ ksh foo.sh\n>   before\n>   trapped\n>   after\n> \n>   $ bash foo.sh\n>   before\n>   after\n>   trapped\n> \n>   $ dash foo.sh\n>   foo.sh: 3: foo.sh: function: not found\n>   foo.sh: 5: foo.sh: Syntax error: \"}\" unexpected\n> \n> Switching to the second form, all three produce:\n> \n>   before\n>   after\n>   trapped\n> \n> I don't know if that is all helpful to your bug-tracking or analysis,\n> but for whatever reason it looks like your ksh is using localized traps\n> for both forms of function. But as far as I know, bash has never behaved\n> that way (I just grepped its CHANGES file for mentions of trap and found\n> nothing likely).\n> \n> -Peff\n> \n\nBoth versions produce your first output on our platform\n\n$ ksh foo1.sh\nbefore\ntrapped\nafter\n$ bash foo1.sh\nbefore\nafter\ntrapped\n$ ksh foo2.sh\nbefore\ntrapped\nafter\n$ bash foo2.sh\nbefore\nafter\ntrapped\n$\n\nThis might have been one (or even _the_) reason why we picked bash as our \nSHELL_PATH in config.mak.uname (I don't remember, it's more than 2 years \nago), not sure which shell Randall's test used?\n\nbye, Jojo\n"},{"id":"256059","messageId":"019d01d04788$5e923970$1bb6ac50$@nexbridge.com","threadId":"38560","inReplyTo":"loom.20150213T094712-477@post.gmane.org","subject":"RE: t5570 trap use in start/stop_git_daemon","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2015-02-13T12:27:01Z","receivedAt":"2015-02-13T12:27:01Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On 2015/02/13 3:58AM Joachim Schmitz wrote:\n>Jeff King <peff <at> peff.net> writes:\n> > On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote:\n> > On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote:\n> > \n><snip> \n>> Hmm, today I learned something new about ksh. Apparently when you use\n>> the \"function\" keyword to define a function like:\n>> \n>>   function foo {\n>>     trap 'echo trapped' EXIT\n>>   }\n>>   echo before\n>>   foo\n>>   echo after\n>> \n>> then the trap runs when the function exits! If you declare the same\n>> function as:\n>> \n>>   foo() {\n>>     trap 'echo trapped' EXIT\n>>   }\n>> \n>> it behaves differently. POSIX shell does not have the function keyword,\n>> of course, and we are not using it here. Bash _does_ have the function\n>> keyword, but seems to behave POSIX-y even when it is present. I.e.,\n>> running the first script:\n>> \n>>   $ ksh foo.sh\n>>   before\n>>   trapped\n>>   after\n>> \n>>   $ bash foo.sh\n> >  before\n>>   after\n> >  trapped\n>> \n<snip>\n>Both versions produce your first output on our platform\n>$ ksh foo1.sh\n>before\n>trapped\n>after\n>$ bash foo1.sh\n>before\n>after\n>trapped\n>$ ksh foo2.sh\n>before\n>trapped\n>after\n>$ bash foo2.sh\n>before\n>after\n>trapped\n>$\n>This might have been one (or even _the_) reason why we picked bash as our \n>SHELL_PATH in config.mak.uname (I don't remember, it's more than 2 years \n>ago), not sure which shell Randall's test used?\n\nI tested both for trying to get t5570 to work. No matter which, without\nresetting the trap, function return would kill the git-daemon and the test\nwould fail.\n"}]}