{"thread":{"id":"23191","subject":"[PATCH] t0005: add test for trap handling from deeply nested function calls","startedAt":"2010-03-26T16:34:05Z","lastAt":"2010-03-27T00:24:20Z","messageCount":4,"participants":["Brandon Casey","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"137875","messageId":"vm2zHZjrZOha7LnIM_9fAEAvwVZrJtZlAI9f2XI_VUsdqr51ihAMW68a5BT9tvMdRDSqk6GYlSE@cipher.nrlssc.navy.mil","threadId":"23191","inReplyTo":null,"subject":"[PATCH] t0005: add test for trap handling from deeply nested function calls","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-26T16:34:05Z","receivedAt":"2010-03-26T16:34:05Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nThe /usr/xpg4/bin/sh shell on Solaris only executes a trap handler set\nwithin a function from the first-level function call made from within the\nfunction that set the trap.  In other words, if func1 sets a trap, then\ncalls func2 which triggers the trap, then the trap handler will be executed\ncorrectly.  If instead of exiting, func2 calls func3, and func3 satisfies\nthe conditions for triggering the trap, the trap handler will NOT be\nexecuted.\n\nThis trap sequence exists in git-bisect.sh and is exercised by tests\nt6030.12 and t6030.13 which fail when the /usr/xpg4/bin/sh shell is used.\n\nAdd a test that will fail sooner and more precisely than the t6030 tests.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nImprovements to the commit message are welcome.\n\n-brandon\n\n\n t/t0005-signals.sh |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t0005-signals.sh b/t/t0005-signals.sh\nindex 09f855a..cb68d1f 100755\n--- a/t/t0005-signals.sh\n+++ b/t/t0005-signals.sh\n@@ -19,4 +19,12 @@ test_expect_success 'sigchain works' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'trap triggered in deeply nested function works correctly' '\n+\t(atrap () { exit 0; }\n+\t func3 () { exit 1; }\n+\t func2 () { func3; }\n+\t func1 () { trap atrap EXIT; func2; }\n+\t func1)\n+'\n+\n test_done\n-- \n1.6.6.2\n"},{"id":"137893","messageId":"7v7hoykfo7.fsf@alter.siamese.dyndns.org","threadId":"23191","inReplyTo":"vm2zHZjrZOha7LnIM_9fAEAvwVZrJtZlAI9f2XI_VUsdqr51ihAMW68a5BT9tvMdRDSqk6GYlSE@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t0005: add test for trap handling from deeply nested function calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-26T20:10:16Z","receivedAt":"2010-03-26T20:10:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> From: Brandon Casey <drafnel@gmail.com>\n>\n> The /usr/xpg4/bin/sh shell on Solaris only executes a trap handler set\n> within a function from the first-level function call made from within the\n> function that set the trap.  In other words, if func1 sets a trap, then\n> calls func2 which triggers the trap, then the trap handler will be executed\n> correctly.  If instead of exiting, func2 calls func3, and func3 satisfies\n> the conditions for triggering the trap, the trap handler will NOT be\n> executed.\n>\n> This trap sequence exists in git-bisect.sh and is exercised by tests\n> t6030.12 and t6030.13 which fail when the /usr/xpg4/bin/sh shell is used.\n>\n> Add a test that will fail sooner and more precisely than the t6030 tests.\n\nThat would be useful if you run all the tests in sequence.\n\nIt smells like the real solution is to fix git-bisect.sh not to do that\ntrap trick, if we can.\n\nOr perhaps have this not as an individual test but as a code to set a test\nprerequiste token (e.g. NESTED_TRAP) to run tests of bisect conditionally?\n\n>\n> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n> ---\n>\n>\n> Improvements to the commit message are welcome.\n>\n> -brandon\n>\n>\n>  t/t0005-signals.sh |    8 ++++++++\n>  1 files changed, 8 insertions(+), 0 deletions(-)\n>\n> diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh\n> index 09f855a..cb68d1f 100755\n> --- a/t/t0005-signals.sh\n> +++ b/t/t0005-signals.sh\n> @@ -19,4 +19,12 @@ test_expect_success 'sigchain works' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'trap triggered in deeply nested function works correctly' '\n> +\t(atrap () { exit 0; }\n> +\t func3 () { exit 1; }\n> +\t func2 () { func3; }\n> +\t func1 () { trap atrap EXIT; func2; }\n> +\t func1)\n> +'\n> +\n>  test_done\n> -- \n> 1.6.6.2\n"},{"id":"137896","messageId":"201003262119.45110.j6t@kdbg.org","threadId":"23191","inReplyTo":"vm2zHZjrZOha7LnIM_9fAEAvwVZrJtZlAI9f2XI_VUsdqr51ihAMW68a5BT9tvMdRDSqk6GYlSE@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t0005: add test for trap handling from deeply nested function calls","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-03-26T20:19:45Z","receivedAt":"2010-03-26T20:19:45Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Freitag, 26. März 2010, Brandon Casey wrote:\n> +test_expect_success 'trap triggered in deeply nested function works\n> correctly' ' +\t(atrap () { exit 0; }\n> +\t func3 () { exit 1; }\n> +\t func2 () { func3; }\n> +\t func1 () { trap atrap EXIT; func2; }\n> +\t func1)\n> +'\n\nWhat do you want to achieve with this test? That /usr/xpg4/bin/sh is not used \non Solaris?\n\nIf git-bisect triggers this bug in /usr/xpg4/bin/sh and if this is our shell \nof choice on Solaris, wouldn't it be better to work around it in git-bisect?\n\n-- Hannes\n"},{"id":"137922","messageId":"AgB5UDyfK4zUe0xzr08eZFJO1S5IvcsZv3v3AjCYT2qtRDoguqnJ3w@cipher.nrlssc.navy.mil","threadId":"23191","inReplyTo":"201003262119.45110.j6t@kdbg.org","subject":"Re: [PATCH] t0005: add test for trap handling from deeply nested function calls","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-27T00:24:20Z","receivedAt":"2010-03-27T00:24:20Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 03/26/2010 03:19 PM, Johannes Sixt wrote:\n> On Freitag, 26. März 2010, Brandon Casey wrote:\n>> +test_expect_success 'trap triggered in deeply nested function works\n>> correctly' ' +\t(atrap () { exit 0; }\n>> +\t func3 () { exit 1; }\n>> +\t func2 () { func3; }\n>> +\t func1 () { trap atrap EXIT; func2; }\n>> +\t func1)\n>> +'\n> \n> What do you want to achieve with this test? That /usr/xpg4/bin/sh is not used \n> on Solaris?\n\nYeah, pretty much.  At least until bisect is rewritten in c.\n\n> If git-bisect triggers this bug in /usr/xpg4/bin/sh and if this is our shell \n> of choice on Solaris,\n\nIt's not currently git's shell of choice on Solaris.  The Makefile still sets\nSHELL_PATH = /bin/bash in the SunOS section.\n\n> wouldn't it be better to work around it in git-bisect?\n\nYes.  You and Junio are giving the same advice, but I don't know how to work\naround it.  I don't think there is anything wrong or tricky about what\nbisect does.  I just think the shell is broken with respect to this\nbehavior.\n\nThis shell ignores traps that are set in the second function level away from\nwhere the trap was set (I found this difficult to describe in my commit\nmessage).  So even if the trap in my test above was set at the top level of\nthe script, it would not be executed after a call to exit from func3 or from\nfunc2.\n\nTake a look at this script:\n\n--- >8 ---\n#!/bin/ksh\n\n(atrap () { exit 0; }\n func3 () { exit 1; }\n func2 () { func3; }\n func1 () { trap atrap EXIT; func2; }\n func1) &&\n echo 'SUCCESS: Trap set in function catches exit two levels deeper' ||\n echo 'FAILURE: Trap set in function fails to catch exit two levels deeper'\n\n(atrap () { exit 0; }\n func2 () { exit 1; }\n func1 () { trap atrap EXIT; func2; }\n func1) &&\n echo 'SUCCESS: Trap set in function catches exit one level deeper' ||\n echo 'FAILURE: Trap set in function fails to catch exit one level deeper'\n\n(atrap () { exit 0; }\n func3 () { exit 1; }\n func2 () { func3; }\n func1 () { func2; }\n trap atrap EXIT\n func1) &&\n echo 'SUCCESS: Trap set at top-level catches exit three levels deeper' ||\n echo 'FAILURE: Trap set at top-level fails to catch exit three levels deeper'\n\n(atrap () { exit 0; }\n func2 () { exit 1; }\n func1 () { func2; }\n trap atrap EXIT\n func1) &&\n echo 'SUCCESS: Trap set at top-level catches exit two levels deeper' ||\n echo 'FAILURE: Trap set at top-level fails to catch exit two levels deeper'\n\n(atrap () { exit 0; }\n func1 () { exit 1; }\n trap atrap EXIT\n func1) &&\n echo 'SUCCESS: Trap set at top-level catches exit one level deeper' ||\n echo 'FAILURE: Trap set at top-level fails to catch exit one level deeper'\n--- >8 ---\n\nOn Linux or IRIX with bash or ksh, it prints\nSUCCESS: Trap set in function catches exit two levels deeper\nSUCCESS: Trap set in function catches exit one level deeper\nSUCCESS: Trap set at top-level catches exit three levels deeper\nSUCCESS: Trap set at top-level catches exit two levels deeper\nSUCCESS: Trap set at top-level catches exit one level deeper\n\nUsing Solaris's /usr/xpg4/bin/sh it prints:\nFAILURE: Trap set in function fails to catch exit two levels deeper\nSUCCESS: Trap set in function catches exit one level deeper\nFAILURE: Trap set at top-level fails to catch exit three levels deeper\nFAILURE: Trap set at top-level fails to catch exit two levels deeper\nSUCCESS: Trap set at top-level catches exit one level deeper\n\nFunny enough, Solaris's /bin/sh works correctly.\n\nSo, I don't see any way to catch a termination by signal in the scripts\nthat are currently using trap and which may make a function call from\nwithin another function.\n\nThe explicit exit's within the script could be handled without a trap\nthough.  Maybe that is enough.  We'll see how ugly it is...\n\n-brandon\n"}]}