{"thread":{"id":"9497","subject":"--exit-code (and --quiet) broken in git-diff?","startedAt":"2007-08-11T23:12:40Z","lastAt":"2007-08-13T23:42:33Z","messageCount":12,"participants":["Wincent Colaiuta","René Scharfe","David Kastrup","Steffen Prohaska","Steven Grimm","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"50517","messageId":"17875.88.10.191.55.1186873960.squirrel@secure.wincent.com","threadId":"9497","inReplyTo":null,"subject":"--exit-code (and --quiet) broken in git-diff?","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-08-11T23:12:40Z","receivedAt":"2007-08-11T23:12:40Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"The git-diff man page documents an \"--exit-code\" option, as well as a\n\"--quiet\" option which automatically implies the former.\n\nIn my tests on Mac OS X and Bash 3, however, \"git diff\" always return an\nexit code of 0, never of 1, regardless of how I use the \"--quiet\" and\n\"--exit-code\" options. I see that there are tests in t/t4017-quiet.sh for\nthe lower-level git-diff-files, git-diff-index, git-diff-tree commands,\nbut none for the porcelain git-diff.\n\nIs this a bug with a missing test case? Or am I using this incorrectly? In\nthe example below I'm looking for differences between the working tree and\nthe last commit, so I'm using \"git diff HEAD\", but as you can see, the\nexit code is always 0 for \"git diff\" and \"git diff --cached\" as well:\n\n$ git --version\ngit version 1.5.2.4\n$ mkdir example\n$ cd example\n$ git init\nInitialized empty Git repository in .git/\n$ echo \"start\" > foo\n$ git add foo\n$ git commit -m \"Add foo\"\nCreated initial commit 85954f6: Add foo\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 foo\n$ git diff --quiet HEAD; echo $?\n0\n$ echo \"more\" >> foo\n$ git diff --quiet HEAD; echo $?\n0\n$ git add foo\n$ git diff --quiet HEAD; echo $?\n0\n$ git diff --quiet; echo $?\n0\n$ git diff --exit-code; echo $?\n0\n$ git diff --cached --quiet; echo $?\n0\n\nCheers,\nWincent\n"},{"id":"50540","messageId":"46BED5AA.7050900@lsrfire.ath.cx","threadId":"9497","inReplyTo":"17875.88.10.191.55.1186873960.squirrel@secure.wincent.com","subject":"Re: --exit-code (and --quiet) broken in git-diff?","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-08-12T09:40:58Z","receivedAt":"2007-08-12T09:40:58Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Wincent Colaiuta schrieb:\n> The git-diff man page documents an \"--exit-code\" option, as well as a\n> \"--quiet\" option which automatically implies the former.\n> \n> In my tests on Mac OS X and Bash 3, however, \"git diff\" always return an\n> exit code of 0, never of 1, regardless of how I use the \"--quiet\" and\n> \"--exit-code\" options. I see that there are tests in t/t4017-quiet.sh for\n> the lower-level git-diff-files, git-diff-index, git-diff-tree commands,\n> but none for the porcelain git-diff.\n> \n> Is this a bug with a missing test case? Or am I using this incorrectly? In\n> the example below I'm looking for differences between the working tree and\n> the last commit, so I'm using \"git diff HEAD\", but as you can see, the\n> exit code is always 0 for \"git diff\" and \"git diff --cached\" as well:\n> \n> $ git --version\n> git version 1.5.2.4\n> $ mkdir example\n> $ cd example\n> $ git init\n> Initialized empty Git repository in .git/\n> $ echo \"start\" > foo\n> $ git add foo\n> $ git commit -m \"Add foo\"\n> Created initial commit 85954f6: Add foo\n>  1 files changed, 1 insertions(+), 0 deletions(-)\n>  create mode 100644 foo\n> $ git diff --quiet HEAD; echo $?\n> 0\n> $ echo \"more\" >> foo\n> $ git diff --quiet HEAD; echo $?\n> 0\n> $ git add foo\n> $ git diff --quiet HEAD; echo $?\n> 0\n> $ git diff --quiet; echo $?\n> 0\n> $ git diff --exit-code; echo $?\n> 0\n> $ git diff --cached --quiet; echo $?\n> 0\n\ngit diff passes the output through your pager by default, so you see the\nexit code of that instead of diff's.  Set PAGER=cat or redirect the\noutput to /dev/null to get rid of it.\n\nA test case for diff would be nice regardless, though. :)\n\nRené\n"},{"id":"50546","messageId":"24332.88.10.191.55.1186917895.squirrel@secure.wincent.com","threadId":"9497","inReplyTo":"46BED5AA.7050900@lsrfire.ath.cx","subject":"Re: --exit-code (and --quiet) broken in git-diff?","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-08-12T11:24:55Z","receivedAt":"2007-08-12T11:24:55Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"René Scharfe wrote:\n>\n> git diff passes the output through your pager by default, so you see the\n> exit code of that instead of diff's.  Set PAGER=cat or redirect the\n> output to /dev/null to get rid of it.\n\nAh, thanks very much René!\n\nBest wishes,\nWincent\n"},{"id":"50547","messageId":"85643lq99h.fsf@lola.goethe.zz","threadId":"9497","inReplyTo":"24332.88.10.191.55.1186917895.squirrel@secure.wincent.com","subject":"Re: --exit-code (and --quiet) broken in git-diff?","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-12T11:31:38Z","receivedAt":"2007-08-12T11:31:38Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\"Wincent Colaiuta\" <win@wincent.com> writes:\n\n> René Scharfe wrote:\n>>\n>> git diff passes the output through your pager by default, so you see the\n>> exit code of that instead of diff's.  Set PAGER=cat or redirect the\n>> output to /dev/null to get rid of it.\n>\n> Ah, thanks very much René!\n\nI think I would call that a mistake.  However, I don't see that fixing\nit would actually be useful: if a pager gets called, this means that\ngit-diff might die with SIGPIPE (when the user quits the pager), and\nthat in turn has pretty much no meaning.  So one really needs to\nredirect the output, anyway.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"50548","messageId":"7C233CA0-D6A4-42F2-B072-B04B1874D991@zib.de","threadId":"9497","inReplyTo":"46BED5AA.7050900@lsrfire.ath.cx","subject":"Re: --exit-code (and --quiet) broken in git-diff?","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-08-12T11:33:28Z","receivedAt":"2007-08-12T11:33:28Z","isPatch":false,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 12, 2007, at 11:40 AM, René Scharfe wrote:\n\n>\n> git diff passes the output through your pager by default, so you  \n> see the\n> exit code of that instead of diff's.  Set PAGER=cat or redirect the\n> output to /dev/null to get rid of it.\n>\n> A test case for diff would be nice regardless, though. :)\n\nWhich is not that easy, because of the redirections done\nby test-lib.sh. I think git-diff needs a tty to exhibit\nthe wrong behaviour, which it doesn't get from test-lib\nif run in standard mode (non verbose).\n\nI tried to write a test case once, but after it got more\nand more complex, I was too lazy to complete it.\n\n\tSteffen"},{"id":"50552","messageId":"46BF04F0.5020304@midwinter.com","threadId":"9497","inReplyTo":"85643lq99h.fsf@lola.goethe.zz","subject":"Re: --exit-code (and --quiet) broken in git-diff?","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-08-12T13:02:40Z","receivedAt":"2007-08-12T13:02:40Z","isPatch":false,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"David Kastrup wrote:\n> I think I would call that a mistake.  However, I don't see that fixing\n> it would actually be useful: if a pager gets called, this means that\n> git-diff might die with SIGPIPE (when the user quits the pager), and\n> that in turn has pretty much no meaning.  So one really needs to\n> redirect the output, anyway.\n>   \n\nIt does sort of make one wonder, though, if there's much point ever \nlaunching a pager when git-diff is run with --quiet -- it will never \nproduce any output to page, so running a pager is guaranteed to always \nbe a waste of cycles.\n\nUnfortunately the pager is launched before the option processing code \nknows whether --quiet is being used or not; I'm not sure it's worth \nrefactoring the pager launch code just to handle this one special case. \n(Or are there other cases where programs would want to be able to \ncontrol the use of the pager?)\n\n-Steve\n"},{"id":"50554","messageId":"85bqdcq3sv.fsf@lola.goethe.zz","threadId":"9497","inReplyTo":"46BF04F0.5020304@midwinter.com","subject":"Re: --exit-code (and --quiet) broken in git-diff?","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-12T13:29:36Z","receivedAt":"2007-08-12T13:29:36Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Steven Grimm <koreth@midwinter.com> writes:\n\n> David Kastrup wrote:\n>> I think I would call that a mistake.  However, I don't see that fixing\n>> it would actually be useful: if a pager gets called, this means that\n>> git-diff might die with SIGPIPE (when the user quits the pager), and\n>> that in turn has pretty much no meaning.  So one really needs to\n>> redirect the output, anyway.\n>>   \n>\n> It does sort of make one wonder, though, if there's much point ever\n> launching a pager when git-diff is run with --quiet -- it will never\n> produce any output to page, so running a pager is guaranteed to always\n> be a waste of cycles.\n>\n> Unfortunately the pager is launched before the option processing code\n> knows whether --quiet is being used or not; I'm not sure it's worth\n> refactoring the pager launch code just to handle this one special\n> case. (Or are there other cases where programs would want to be able\n> to control the use of the pager?)\n\nI think it is reasonable not to start the pager at all when there is\nno bulk material, but just a fixed amount of output such as a summary\nlines.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"50561","messageId":"8C80CB6C-83A5-437D-9360-AC93B73EAA54@wincent.com","threadId":"9497","inReplyTo":"46BF04F0.5020304@midwinter.com","subject":"Re: --exit-code (and --quiet) broken in git-diff?","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-08-12T16:57:20Z","receivedAt":"2007-08-12T16:57:20Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 12/8/2007, a las 15:02, Steven Grimm escribió:\n\n> David Kastrup wrote:\n>> I think I would call that a mistake.  However, I don't see that  \n>> fixing\n>> it would actually be useful: if a pager gets called, this means that\n>> git-diff might die with SIGPIPE (when the user quits the pager), and\n>> that in turn has pretty much no meaning.  So one really needs to\n>> redirect the output, anyway.\n>>\n>\n> It does sort of make one wonder, though, if there's much point ever  \n> launching a pager when git-diff is run with --quiet -- it will  \n> never produce any output to page, so running a pager is guaranteed  \n> to always be a waste of cycles.\n\nYes, I thought the same thing when I read in the initial release  \nnotes (<http://www.kernel.org/pub/software/scm/git/docs/ \nRelNotes-1.5.1.txt>) that the \"--quiet\" option was explicitly \"meant  \nfor scripted use\", where naturally you don't want or need a pager...  \nturns out I was writing a script, you see, and I wanted to test the  \ncommands out manually to be sure they did what I thought they would...\n\nBut as you say, refactoring this to bypass the pager when \"--quiet\"  \nis passed may be unpleasantly complicated.\n\nCheers,\nWincent\n"},{"id":"50565","messageId":"46BF478F.7030603@lsrfire.ath.cx","threadId":"9497","inReplyTo":"17875.88.10.191.55.1186873960.squirrel@secure.wincent.com","subject":"[PATCH] diff: don't run pager if user asked for a diff style exit code","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-08-12T17:46:55Z","receivedAt":"2007-08-12T17:46:55Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"As Wincent Colaiuta found out, it's a bit unexpected for git diff to\nstart a pager even when the --quiet option is specified.  The problem\nis that the pager hides the return code -- which is the only output\nwe're interested in in this case.\n\nPush pager setup down into builtin-diff.c and don't start the pager\nif --exit-code or --quiet (which implies --exit-code) was specified.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n\n builtin-diff.c |    6 ++++++\n git.c          |    2 +-\n 2 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex b48121e..8dc17b0 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -235,6 +235,12 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \trev.diffopt.allow_external = 1;\n \trev.diffopt.recursive = 1;\n \n+\t/* If the user asked for our exit code then don't start a\n+\t * pager or we would end up reporting its exit code instead.\n+\t */\n+\tif (!rev.diffopt.exit_with_status)\n+\t\tsetup_pager();\n+\n \t/* Do we have --cached and not have a pending object, then\n \t * default to HEAD by hand.  Eek.\n \t */\ndiff --git a/git.c b/git.c\nindex e5daae0..cab0e72 100644\n--- a/git.c\n+++ b/git.c\n@@ -325,7 +325,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"config\", cmd_config },\n \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n \t\t{ \"describe\", cmd_describe, RUN_SETUP },\n-\t\t{ \"diff\", cmd_diff, USE_PAGER },\n+\t\t{ \"diff\", cmd_diff },\n \t\t{ \"diff-files\", cmd_diff_files },\n \t\t{ \"diff-index\", cmd_diff_index, RUN_SETUP },\n \t\t{ \"diff-tree\", cmd_diff_tree, RUN_SETUP },\n"},{"id":"50627","messageId":"13516449-DEDB-41A1-8880-42CB124A6C0A@wincent.com","threadId":"9497","inReplyTo":"46BF478F.7030603@lsrfire.ath.cx","subject":"Re: [PATCH] diff: don't run pager if user asked for a diff style exit code","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-08-13T09:57:32Z","receivedAt":"2007-08-13T09:57:32Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 12/8/2007, a las 19:46, René Scharfe escribió:\n\n> Push pager setup down into builtin-diff.c and don't start the pager\n> if --exit-code or --quiet (which implies --exit-code) was specified.\n\nGreat stuff, René. The change looks much simpler than I thought it  \nwould.\n\nCheers,\nWincent\n"},{"id":"50628","messageId":"86d4xrka23.fsf@lola.quinscape.zz","threadId":"9497","inReplyTo":"13516449-DEDB-41A1-8880-42CB124A6C0A@wincent.com","subject":"Re: [PATCH] diff: don't run pager if user asked for a diff style exit code","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-13T10:23:16Z","receivedAt":"2007-08-13T10:23:16Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Wincent Colaiuta <win@wincent.com> writes:\n\n> El 12/8/2007, a las 19:46, René Scharfe escribió:\n>\n>> Push pager setup down into builtin-diff.c and don't start the pager\n>> if --exit-code or --quiet (which implies --exit-code) was specified.\n>\n> Great stuff, René. The change looks much simpler than I thought it  \n> would.\n\nPersonally, I'd start the pager only when the first diff or stat gets\noutput, anyway.  For a summary message or even a silent exit, a pager\nis annoying, regardless of whether --exit-code was specified.\n\n-- \nDavid Kastrup\n"},{"id":"50666","messageId":"7v7inzf1cm.fsf@assigned-by-dhcp.cox.net","threadId":"9497","inReplyTo":"46BF478F.7030603@lsrfire.ath.cx","subject":"Re: [PATCH] diff: don't run pager if user asked for a diff style exit code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-13T23:42:33Z","receivedAt":"2007-08-13T23:42:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n> As Wincent Colaiuta found out, it's a bit unexpected for git diff to\n> start a pager even when the --quiet option is specified.  The problem\n> is that the pager hides the return code -- which is the only output\n> we're interested in in this case.\n>\n> Push pager setup down into builtin-diff.c and don't start the pager\n> if --exit-code or --quiet (which implies --exit-code) was specified.\n\nSurprisingly nice, although I would have said \"don't use\ngit-diff wrapper from scripts, use underlying diff-* plumbing\"\n;-).\n\nThanks.\n"}]}