{"thread":{"id":"45406","subject":"[PATCH] tests: create an interactive gdb session with the 'debug' helper","startedAt":"2017-03-17T14:50:45Z","lastAt":"2017-03-21T11:07:31Z","messageCount":9,"participants":["SZEDER Gábor","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"314337","messageId":"20170317144646.9349-1-szeder.dev@gmail.com","threadId":"45406","inReplyTo":null,"subject":"[PATCH] tests: create an interactive gdb session with the 'debug' helper","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-03-17T14:46:46Z","receivedAt":"2017-03-17T14:50:45Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The 'debug' test helper is supposed to facilitate debugging by running\na command of the test suite under gdb.  Unfortunately, its usefulness\nis severely limited, because that gdb session is not interactive,\nsince the test's, and thus gdb's standard input is attached to\n/dev/null (for a good reason, see 781f76b15 (test-lib: redirect stdin\nof tests, 2011-12-15)).\n\nRe-attach the original standard input in the debug helper, thus\ncreating an interactive gdb session, which is much much more useful.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/test-lib-functions.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex bd357704c..9567dc986 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -154,7 +154,7 @@ test_pause () {\n #\n # Example: \"debug git checkout master\".\n debug () {\n-\t GIT_TEST_GDB=1 \"$@\"\n+\t GIT_TEST_GDB=1 \"$@\" <&6\n }\n \n # Call test_commit with the arguments\n-- \n2.12.0.375.g7a2ebd56b\n\n"},{"id":"314345","messageId":"20170317145504.utysxd4qqaqetm7t@sigill.intra.peff.net","threadId":"45406","inReplyTo":"20170317144646.9349-1-szeder.dev@gmail.com","subject":"Re: [PATCH] tests: create an interactive gdb session with the 'debug' helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-17T14:55:05Z","receivedAt":"2017-03-17T16:43:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 17, 2017 at 03:46:46PM +0100, SZEDER Gábor wrote:\n\n> The 'debug' test helper is supposed to facilitate debugging by running\n> a command of the test suite under gdb.  Unfortunately, its usefulness\n> is severely limited, because that gdb session is not interactive,\n> since the test's, and thus gdb's standard input is attached to\n> /dev/null (for a good reason, see 781f76b15 (test-lib: redirect stdin\n> of tests, 2011-12-15)).\n> \n> Re-attach the original standard input in the debug helper, thus\n> creating an interactive gdb session, which is much much more useful.\n\nYeah, I think I've had to do a similar hack manually. This is an obvious\nimprovement. Though...\n\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index bd357704c..9567dc986 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -154,7 +154,7 @@ test_pause () {\n>  #\n>  # Example: \"debug git checkout master\".\n>  debug () {\n> -\t GIT_TEST_GDB=1 \"$@\"\n> +\t GIT_TEST_GDB=1 \"$@\" <&6\n>  }\n\nYour stderr and stdout may be redirected, too. That's usually not a big\nproblem because you'll be using \"-v\", but perhaps this should add:\n\n  >&3 2>&4\n\nto be on the safe side. That covers running without \"-v\", as well as\ncases where the test script is redirecting output (I _actually_ wish gdb\nbehaved like perl's debugger and unconditionally used the terminal,\nretaining existing redirections, but I AFAIK there's no easy way to get\nit to do that).\n\n-Peff\n"},{"id":"314503","messageId":"CAM0VKj=k8kygEPpfX+-n0ODd70A8PEYAviKosvc0D34jT02N5w@mail.gmail.com","threadId":"45406","inReplyTo":"20170317145504.utysxd4qqaqetm7t@sigill.intra.peff.net","subject":"Re: [PATCH] tests: create an interactive gdb session with the 'debug' helper","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-03-18T16:10:18Z","receivedAt":"2017-03-18T16:10:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Mar 17, 2017 at 3:55 PM, Jeff King <peff@peff.net> wrote:\n> On Fri, Mar 17, 2017 at 03:46:46PM +0100, SZEDER Gábor wrote:\n>\n>> The 'debug' test helper is supposed to facilitate debugging by running\n>> a command of the test suite under gdb.  Unfortunately, its usefulness\n>> is severely limited, because that gdb session is not interactive,\n>> since the test's, and thus gdb's standard input is attached to\n>> /dev/null (for a good reason, see 781f76b15 (test-lib: redirect stdin\n>> of tests, 2011-12-15)).\n>>\n>> Re-attach the original standard input in the debug helper, thus\n>> creating an interactive gdb session, which is much much more useful.\n>\n> Yeah, I think I've had to do a similar hack manually. This is an obvious\n> improvement. Though...\n>\n>> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n>> index bd357704c..9567dc986 100644\n>> --- a/t/test-lib-functions.sh\n>> +++ b/t/test-lib-functions.sh\n>> @@ -154,7 +154,7 @@ test_pause () {\n>>  #\n>>  # Example: \"debug git checkout master\".\n>>  debug () {\n>> -      GIT_TEST_GDB=1 \"$@\"\n>> +      GIT_TEST_GDB=1 \"$@\" <&6\n>>  }\n>\n> Your stderr and stdout may be redirected, too. That's usually not a big\n> problem because you'll be using \"-v\",\n\nYeah, I didn't consider using this helper in non-verbose mode at all,\nand the resulting behaviour is not quite pleasant, indeed: gdb starts\nin interactive mode, but as its stdout goes straight to /dev/null the\nuser has no idea, and gdb does not quit on Ctrl-C.  A possible\nsolution would be to forbid using the 'debug' helper without '-v',\nlike in the 'test_pause' helper just above.\n\nHowever...\n\n> but perhaps this should add:\n>\n>   >&3 2>&4\n>\n> to be on the safe side. That covers running without \"-v\", as well as\n> cases where the test script is redirecting output\n\nThat wouldn't buy us much.  First, file descriptors 3 and 4 are the\ntest's stdout and stderr, i.e. any process started within the test\nwould be connected to those fds anyway without those explicit\nredirections.  Second, fds 3 and 4 are redirected to /dev/null in\nnon-verbose mode, so it would still not work without '-v'.\n\nPerhaps you meant '>&5 2>&7' here (and in the bash example in the\ncommit message of 781f76b15 (test-lib: redirect stdin of tests,\n2011-12-15), for that matter)?  Those redirections do in fact make\n'debug' work even without '-v'.  Furthermore, those redirections do\nmake 'test_pause' work without '-v', too.\n\nSo this is what v2 will do, then.\n\nGábor\n"},{"id":"314504","messageId":"20170318161400.19753-1-szeder.dev@gmail.com","threadId":"45406","inReplyTo":"CAM0VKj=k8kygEPpfX+-n0ODd70A8PEYAviKosvc0D34jT02N5w@mail.gmail.com","subject":"[PATCHv2 1/2] tests: create an interactive gdb session with the 'debug' helper","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-03-18T16:13:59Z","receivedAt":"2017-03-18T16:22:58Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The 'debug' test helper is supposed to facilitate debugging by running\na command of the test suite under gdb.  Unfortunately, its usefulness\nis severely limited, because that gdb session is not interactive,\nsince the test's, and thus gdb's standard input is redirected from\n/dev/null (for a good reason, see 781f76b15 (test-lib: redirect stdin\nof tests, 2011-12-15)).\n\nRedirect gdb's standard file descriptors from/to the test\nenvironment's stdin, stdout and stderr in the 'debug' helper, thus\ncreating an interactive gdb session (even in non-verbose mode), which\nis much, much more useful.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/test-lib-functions.sh | 2 +-\n t/test-lib.sh           | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex bd357704c..4a50a6104 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -154,7 +154,7 @@ test_pause () {\n #\n # Example: \"debug git checkout master\".\n debug () {\n-\t GIT_TEST_GDB=1 \"$@\"\n+\t GIT_TEST_GDB=1 \"$@\" <&6 >&5 2>&7\n }\n \n # Call test_commit with the arguments\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 86d77c16d..23c29bce6 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -342,6 +342,7 @@ fi\n \n exec 5>&1\n exec 6<&0\n+exec 7>&2\n if test \"$verbose_log\" = \"t\"\n then\n \texec 3>>\"$GIT_TEST_TEE_OUTPUT_FILE\" 4>&3\n-- \n2.12.0.377.g15f6ffe90\n\n"},{"id":"314505","messageId":"20170318161400.19753-2-szeder.dev@gmail.com","threadId":"45406","inReplyTo":"20170318161400.19753-1-szeder.dev@gmail.com","subject":"[PATCHv2 2/2] tests: make the 'test_pause' helper work in non-verbose mode","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-03-18T16:14:00Z","receivedAt":"2017-03-18T16:43:23Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"When the 'test_pause' helper function invokes the shell mid-test, it\nexplicitly redirects the shell's stdout and stderr to file descriptors\n3 and 4, which are the stdout and stderr of the tests (i.e. where they\nwould be connected anyway without those redirections).  These file\ndescriptors are only attached to the terminal in verbose mode, hence\nthe restriction of 'test_pause' to work only with '-v'.\n\nRedirect the shell's stdout and stderr to the test environment's\noriginal stdout and stderr, allowing it to work properly even in\nnon-verbose mode, and the restriction can be lifted.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/test-lib-functions.sh | 9 ++-------\n 1 file changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 4a50a6104..5ee124332 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -136,17 +136,12 @@ test_tick () {\n \texport GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n }\n \n-# Stop execution and start a shell. This is useful for debugging tests and\n-# only makes sense together with \"-v\".\n+# Stop execution and start a shell. This is useful for debugging tests.\n #\n # Be sure to remove all invocations of this command before submitting.\n \n test_pause () {\n-\tif test \"$verbose\" = t; then\n-\t\t\"$SHELL_PATH\" <&6 >&3 2>&4\n-\telse\n-\t\terror >&5 \"test_pause requires --verbose\"\n-\tfi\n+\t\"$SHELL_PATH\" <&6 >&5 2>&7\n }\n \n # Wrap git in gdb. Adding this to a command can make it easier to\n-- \n2.12.0.377.g15f6ffe90\n\n"},{"id":"314623","messageId":"20170320042908.2vwrqnqu2qvlywaf@sigill.intra.peff.net","threadId":"45406","inReplyTo":"CAM0VKj=k8kygEPpfX+-n0ODd70A8PEYAviKosvc0D34jT02N5w@mail.gmail.com","subject":"Re: [PATCH] tests: create an interactive gdb session with the 'debug' helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-20T04:29:08Z","receivedAt":"2017-03-20T04:30:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 18, 2017 at 05:10:18PM +0100, SZEDER Gábor wrote:\n\n> > but perhaps this should add:\n> >\n> >   >&3 2>&4\n> >\n> > to be on the safe side. That covers running without \"-v\", as well as\n> > cases where the test script is redirecting output\n> \n> That wouldn't buy us much.  First, file descriptors 3 and 4 are the\n> test's stdout and stderr, i.e. any process started within the test\n> would be connected to those fds anyway without those explicit\n> redirections.  Second, fds 3 and 4 are redirected to /dev/null in\n> non-verbose mode, so it would still not work without '-v'.\n> \n> Perhaps you meant '>&5 2>&7' here (and in the bash example in the\n> commit message of 781f76b15 (test-lib: redirect stdin of tests,\n> 2011-12-15), for that matter)?  Those redirections do in fact make\n> 'debug' work even without '-v'.  Furthermore, those redirections do\n> make 'test_pause' work without '-v', too.\n\nYeah, sorry, I got my descriptors mixed up. \">&5 2>&1\" is the right\ninvocation (unless you add a new \"7\" to save the original stderr).\n\n-Peff\n"},{"id":"314624","messageId":"20170320043134.exaiz6adzcxp4zld@sigill.intra.peff.net","threadId":"45406","inReplyTo":"20170318161400.19753-1-szeder.dev@gmail.com","subject":"Re: [PATCHv2 1/2] tests: create an interactive gdb session with the 'debug' helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-20T04:31:34Z","receivedAt":"2017-03-20T04:31:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 18, 2017 at 05:13:59PM +0100, SZEDER Gábor wrote:\n\n> The 'debug' test helper is supposed to facilitate debugging by running\n> a command of the test suite under gdb.  Unfortunately, its usefulness\n> is severely limited, because that gdb session is not interactive,\n> since the test's, and thus gdb's standard input is redirected from\n> /dev/null (for a good reason, see 781f76b15 (test-lib: redirect stdin\n> of tests, 2011-12-15)).\n> \n> Redirect gdb's standard file descriptors from/to the test\n> environment's stdin, stdout and stderr in the 'debug' helper, thus\n> creating an interactive gdb session (even in non-verbose mode), which\n> is much, much more useful.\n>\n> [...]\n>\n>  debug () {\n> -\t GIT_TEST_GDB=1 \"$@\"\n> +\t GIT_TEST_GDB=1 \"$@\" <&6 >&5 2>&7\n>  }\n\nYep, this looks fine to me.\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 86d77c16d..23c29bce6 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -342,6 +342,7 @@ fi\n>  \n>  exec 5>&1\n>  exec 6<&0\n> +exec 7>&2\n\nAnd this obviously is required to keep stdout/stderr separate.\n\n-Peff\n"},{"id":"314625","messageId":"20170320043219.3y4ksmttvcchfpis@sigill.intra.peff.net","threadId":"45406","inReplyTo":"20170318161400.19753-2-szeder.dev@gmail.com","subject":"Re: [PATCHv2 2/2] tests: make the 'test_pause' helper work in non-verbose mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-20T04:32:20Z","receivedAt":"2017-03-20T04:32:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 18, 2017 at 05:14:00PM +0100, SZEDER Gábor wrote:\n\n> When the 'test_pause' helper function invokes the shell mid-test, it\n> explicitly redirects the shell's stdout and stderr to file descriptors\n> 3 and 4, which are the stdout and stderr of the tests (i.e. where they\n> would be connected anyway without those redirections).  These file\n> descriptors are only attached to the terminal in verbose mode, hence\n> the restriction of 'test_pause' to work only with '-v'.\n> \n> Redirect the shell's stdout and stderr to the test environment's\n> original stdout and stderr, allowing it to work properly even in\n> non-verbose mode, and the restriction can be lifted.\n> \n> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> ---\n>  t/test-lib-functions.sh | 9 ++-------\n>  1 file changed, 2 insertions(+), 7 deletions(-)\n\nThis makes sense, too. Thanks.\n\n-Peff\n"},{"id":"314788","messageId":"alpine.DEB.2.20.1703211206050.3767@virtualbox","threadId":"45406","inReplyTo":"20170318161400.19753-1-szeder.dev@gmail.com","subject":"Re: [PATCHv2 1/2] tests: create an interactive gdb session with the 'debug' helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-03-21T11:07:08Z","receivedAt":"2017-03-21T11:07:31Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Sat, 18 Mar 2017, SZEDER Gábor wrote:\n\n> The 'debug' test helper is supposed to facilitate debugging by running\n> a command of the test suite under gdb.  Unfortunately, its usefulness\n> is severely limited, because that gdb session is not interactive,\n> since the test's, and thus gdb's standard input is redirected from\n> /dev/null (for a good reason, see 781f76b15 (test-lib: redirect stdin\n> of tests, 2011-12-15)).\n> \n> Redirect gdb's standard file descriptors from/to the test\n> environment's stdin, stdout and stderr in the 'debug' helper, thus\n> creating an interactive gdb session (even in non-verbose mode), which\n> is much, much more useful.\n\nThank you for working on this!\n\nI meant to clean up my local patch that tried to avoid redirection\naltogether in the case of `debug`, but I never could make it elegant\nenough for my taste. Your approach is much better!\n\nACK,\nDscho"}]}