{"thread":{"id":"41567","subject":"[PATCH] Documentation: talk about pager in api-trace.txt","startedAt":"2016-02-29T14:21:20Z","lastAt":"2016-03-03T16:51:21Z","messageCount":4,"participants":["Christian Couder","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"279781","messageId":"1456755680-4335-1-git-send-email-chriscool@tuxfamily.org","threadId":"41567","inReplyTo":null,"subject":"[PATCH] Documentation: talk about pager in api-trace.txt","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-02-29T14:21:20Z","receivedAt":"2016-02-29T14:21:20Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/technical/api-trace.txt | 43 +++++++++++++++++++++++++++++++++++\n 1 file changed, 43 insertions(+)\n\ndiff --git a/Documentation/technical/api-trace.txt b/Documentation/technical/api-trace.txt\nindex 097a651..a10b3a9 100644\n--- a/Documentation/technical/api-trace.txt\n+++ b/Documentation/technical/api-trace.txt\n@@ -95,3 +95,46 @@ for (;;) {\n }\n trace_performance(t, \"frotz\");\n ------------\n+\n+Bugs & Caveats\n+--------------\n+\n+Some git commands, like `git log`, are run by default using a\n+pager. In this case, stdout and stderr are redirected to the pager and\n+are closed when the pager exits.\n+\n+If a GIT_TRACE* environment variable has been set to \"1\" or \"2\" to\n+print traces on stderr, no trace output will be printed after the\n+pager has exited.\n+\n+This can be annoying, because GIT_TRACE_PERFORMANCE by default prints\n+the performance stats for the whole command at atexit() time which\n+happens after the pager has exited.\n+\n+So the following command will print no performance stat:\n+\n+------------\n+GIT_TRACE_PERFORMANCE=2 git log -1\n+------------\n+\n+To overcome this problem, you can use one of the following\n+work-arounds:\n+\n+  - redirect to another file descriptor which is redirected to stderr,\n+    like this:\n+\n+------------\n+GIT_TRACE_PERFORMANCE=3 3>&2 git log -1\n+------------\n+\n+  - redirect to a file specified by its absolute path, like this:\n+\n+------------\n+GIT_TRACE_PERFORMANCE=/path/to/log/file git log -1\n+------------\n+\n+  - use \"--no-pager\", like this:\n+\n+------------\n+GIT_TRACE_PERFORMANCE=2 git --no-pager log -1\n+------------\n-- \n2.7.1.289.gf4cc727\n"},{"id":"279849","messageId":"20160229213152.GC25342@sigill.intra.peff.net","threadId":"41567","inReplyTo":"1456755680-4335-1-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH] Documentation: talk about pager in api-trace.txt","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-29T21:31:52Z","receivedAt":"2016-02-29T21:31:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 29, 2016 at 03:21:20PM +0100, Christian Couder wrote:\n\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  Documentation/technical/api-trace.txt | 43 +++++++++++++++++++++++++++++++++++\n>  1 file changed, 43 insertions(+)\n\nI think this is fine. I'm not sure how many people would look at the\ntechnical/api documentation in such a case, but I don't think it hurts\nto document this stuff.\n\n-Peff\n"},{"id":"280107","messageId":"CAP8UFD38odsz=fwE9E+0fVJXFE-4vwL1vTQT0rshyCAUYO1fcw@mail.gmail.com","threadId":"41567","inReplyTo":"20160229213152.GC25342@sigill.intra.peff.net","subject":"Re: [PATCH] Documentation: talk about pager in api-trace.txt","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-03-03T09:17:49Z","receivedAt":"2016-03-03T09:17:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Feb 29, 2016 at 10:31 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Feb 29, 2016 at 03:21:20PM +0100, Christian Couder wrote:\n>\n>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>> ---\n>>  Documentation/technical/api-trace.txt | 43 +++++++++++++++++++++++++++++++++++\n>>  1 file changed, 43 insertions(+)\n>\n> I think this is fine. I'm not sure how many people would look at the\n> technical/api documentation in such a case, but I don't think it hurts\n> to document this stuff.\n\nYeah.\n\nJunio do you plan to merge this patch or would you prefer something like:\n\n+\n+Bugs & Caveats\n+--------------\n+\n+Some git commands, like `git log`, are run by default using a\n+pager. In this case, stdout and stderr are redirected to the pager and\n+are closed when the pager exits.\n+\n+If a GIT_TRACE* environment variable has been set to \"1\" or \"2\" to\n+print traces on stderr, no trace output will be printed after the\n+pager has exited.\n+\n+This can be annoying, because GIT_TRACE_PERFORMANCE by default prints\n+the performance stats for the whole command at atexit() time which\n+happens after the pager has exited.\n+\n+So the following command will print no performance stat:\n+\n+------------\n+GIT_TRACE_PERFORMANCE=2 git log -1\n+------------\n+\n+To overcome this problem, you can use for example:\n+\n+------------\n+GIT_TRACE_PERFORMANCE=2 git --no-pager log -1\n+------------\n\n?\n"},{"id":"280127","messageId":"xmqqfuw733p2.fsf@gitster.mtv.corp.google.com","threadId":"41567","inReplyTo":"CAP8UFD38odsz=fwE9E+0fVJXFE-4vwL1vTQT0rshyCAUYO1fcw@mail.gmail.com","subject":"Re: [PATCH] Documentation: talk about pager in api-trace.txt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-03T16:51:21Z","receivedAt":"2016-03-03T16:51:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> Junio do you plan to merge this patch or would you prefer something like:\n>\n> +\n> +Bugs & Caveats\n> +--------------\n> +\n> +Some git commands, like `git log`, are run by default using a\n> +pager. In this case, stdout and stderr are redirected to the pager and\n> +are closed when the pager exits.\n> +\n> +If a GIT_TRACE* environment variable has been set to \"1\" or \"2\" to\n> +print traces on stderr, no trace output will be printed after the\n> +pager has exited.\n> +\n> +This can be annoying, because GIT_TRACE_PERFORMANCE by default prints\n> +the performance stats for the whole command at atexit() time which\n> +happens after the pager has exited.\n> +\n> +So the following command will print no performance stat:\n> +\n> +------------\n> +GIT_TRACE_PERFORMANCE=2 git log -1\n> +------------\n> +\n> +To overcome this problem, you can use for example:\n> +\n> +------------\n> +GIT_TRACE_PERFORMANCE=2 git --no-pager log -1\n> +------------\n>\n> ?\n\nShould I take either one?  Which one do you prefer and why?\n\nI do not have strong preference between the two.  I understand that\nthe differences are only in the example workarounds.  And I do not\nthink the common parts of the two patches are that great.\n\nEven though I think the first two paragraphs do not tell a lie, I\nthink they are overly verbose, tell irrelevant details and does not\nhighlight the real issue.  You only want to say\n\n\tGIT_TRACE_* environment variables can be used to tell Git to\n\tshow trace output to its standard error stream.  Git can\n\toften spawn a pager internally to run its subcommand and\n\tsend its standard output and standard error to it.\n\nThe third paragraph is misleading.  \"by default prints ... at\natexit() time\" sounds as if you are hinting that the solution would\nbe to use some non-default way to tell it to print the stats at some\nother time before atexit() to ensure that the output is done before\nthe pager exits, but that is not actually what you are going to\nsuggest.\n\nThe real source of the annoyance is not that trace output will not\nbe seen after the pager exits, but that PERFORMANCE trace does not\nbegin until the pager exits, which by definition means you would see\nnothing.  \"This can be annoying\" is an understatement (because\nsending PERFORMANCE output to pager always gives an annoying\nresult), and blames a wrong thing at the same time (because the fact\nthat trace output are sent to pager together with the program output\nis not what makes it annoying; the real culprit is that PERFORMANCE\noutput comes only after pager exits).\n\nI'd replace it with something like this, if I were writing this patch.\n\n\tBecause GIT_TRACE_PERFORMANCE trace is generated only at the\n\tvery end of the program with atexit(), which happens after\n\tthe pager exits, it would not work well if you send its log\n\tto the standard error output and let Git spawn the pager at\n\tthe same time.\n\nThat would make \"So the following ... no performance stat:\"\nunnecessary, and the workarounds more obvious.  You can choose not\nto use the pager, or you can send the trace output to a file.\n"}]}