{"thread":{"id":"7240","subject":"[PATCH] Allow git-diff exit with codes similar to diff(1)","startedAt":"2007-03-14T00:17:04Z","lastAt":"2007-03-15T13:56:16Z","messageCount":32,"participants":["Alex Riesen","Johannes Schindelin","Linus Torvalds","Junio C Hamano","Simon 'corecode' Schubert"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"37037","messageId":"81b0412b0703131717k7106ee1cg964628f0bda2c83e@mail.gmail.com","threadId":"7240","inReplyTo":null,"subject":"[PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T00:17:04Z","receivedAt":"2007-03-14T00:17:04Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"This introduces a new command-line option: --exit-code. The diff\nprograms will return 1 for differences, return 0 for equality, and\nsomething else for errors.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nStill working on my e-mail problems. Sorry for duplications.\n\nAs promised on irc. I'm somewhat confused about diff_tree: it used to\nunconditionally return 0, yet every caller of it saves and passes the\nvalue!\n\n Documentation/diff-options.txt |    5 ++++\n builtin-diff-tree.c            |   12 +++++----\n builtin-diff.c                 |    5 ++-\n diff-lib.c                     |   12 +++++++--\n diff.c                         |    2 +\n diff.h                         |    3 +-\n t/t4017-diff-retval.sh         |   51 ++++++++++++++++++++++++++++++++++++++++\n tree-diff.c                    |   11 +++++++-\n 8 files changed, 88 insertions(+), 13 deletions(-)\n create mode 100755 t/t4017-diff-retval.sh\n\n\nFrom 2309f82f88274a0ddac0d5b06c5df580f5cb7d81 Mon Sep 17 00:00:00 2001\nFrom: Alex Riesen <raa.lkml@gmail.com>\nDate: Tue, 13 Mar 2007 18:06:08 +0100\nSubject: [PATCH] Allow git-diff exit with codes similar to diff(1)\n\nThis introduces a new command-line option: --exit-code. The diff\nprograms will return 1 for differences, return 0 for equality, and\nsomething else for errors.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n Documentation/diff-options.txt |    5 ++++\n builtin-diff-tree.c            |   12 +++++----\n builtin-diff.c                 |    5 ++-\n diff-lib.c                     |   12 +++++++--\n diff.c                         |    2 +\n diff.h                         |    3 +-\n t/t4017-diff-retval.sh         |   51 ++++++++++++++++++++++++++++++++++++++++\n tree-diff.c                    |   11 +++++++-\n 8 files changed, 88 insertions(+), 13 deletions(-)\n create mode 100755 t/t4017-diff-retval.sh\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex d8696b7..77a3f78 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -159,5 +159,10 @@\n -w::\n \tShorthand for \"--ignore-all-space\".\n \n+--exit-code::\n+\tMake the program exit with codes similar to diff(1).\n+\tThat is, it exits with 1 if there were differences and\n+\t0 means no differences.\n+\n For more detailed explanation on these common options, see also\n link:diffcore.html[diffcore documentation].\ndiff --git a/builtin-diff-tree.c b/builtin-diff-tree.c\nindex 24cb2d7..d293a5e 100644\n--- a/builtin-diff-tree.c\n+++ b/builtin-diff-tree.c\n@@ -61,6 +61,7 @@ COMMON_DIFF_OPTIONS_HELP;\n \n int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n {\n+\tint result = 0;\n \tint nr_sha1;\n \tchar line[1000];\n \tstruct object *tree1, *tree2;\n@@ -110,7 +111,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \t\t\ttree2 = tree1;\n \t\t\ttree1 = tmp;\n \t\t}\n-\t\tdiff_tree_sha1(tree1->sha1,\n+\t\tresult = diff_tree_sha1(tree1->sha1,\n \t\t\t       tree2->sha1,\n \t\t\t       \"\", &opt->diffopt);\n \t\tlog_tree_diff_flush(opt);\n@@ -118,7 +119,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (!read_stdin)\n-\t\treturn 0;\n+\t\treturn opt->diffopt.diff_exit_code ? result: 0;\n \n \tif (opt->diffopt.detect_rename)\n \t\topt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |\n@@ -130,8 +131,9 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \t\t\tfputs(line, stdout);\n \t\t\tfflush(stdout);\n \t\t}\n-\t\telse\n-\t\t\tdiff_tree_stdin(line);\n+\t\telse if (diff_tree_stdin(line)) {\n+\t\t\tresult = 1;\n+\t\t}\n \t}\n-\treturn 0;\n+\treturn opt->diffopt.diff_exit_code ? result: 0;\n }\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 4efbb82..5e6265f 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -130,6 +130,7 @@ static int builtin_diff_tree(struct rev_info *revs,\n {\n \tconst unsigned char *(sha1[2]);\n \tint swap = 0;\n+\tint result = 0;\n \n \tif (argc > 1)\n \t\tusage(builtin_diff_usage);\n@@ -141,9 +142,9 @@ static int builtin_diff_tree(struct rev_info *revs,\n \t\tswap = 1;\n \tsha1[swap] = ent[0].item->sha1;\n \tsha1[1-swap] = ent[1].item->sha1;\n-\tdiff_tree_sha1(sha1[0], sha1[1], \"\", &revs->diffopt);\n+\tresult = diff_tree_sha1(sha1[0], sha1[1], \"\", &revs->diffopt);\n \tlog_tree_diff_flush(revs);\n-\treturn 0;\n+\treturn result;\n }\n \n static int builtin_diff_combined(struct rev_info *revs,\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 6abb981..f943b6f 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -170,8 +170,10 @@ static int handle_diff_files_args(struct rev_info *revs,\n \t\telse if (!strcmp(argv[1], \"--theirs\"))\n \t\t\trevs->max_count = 3;\n \t\telse if (!strcmp(argv[1], \"-n\") ||\n-\t\t\t\t!strcmp(argv[1], \"--no-index\"))\n+\t\t\t\t!strcmp(argv[1], \"--no-index\")) {\n \t\t\trevs->max_count = -2;\n+\t\t\trevs->diffopt.diff_exit_code = 1;\n+\t\t}\n \t\telse if (!strcmp(argv[1], \"-q\"))\n \t\t\t*silent = 1;\n \t\telse\n@@ -237,6 +239,7 @@ int setup_diff_no_index(struct rev_info *revs,\n \t\t\tbreak;\n \t\t} else if (i < argc - 3 && !strcmp(argv[i], \"--no-index\")) {\n \t\t\ti = argc - 3;\n+\t\t\trevs->diffopt.diff_exit_code = 1;\n \t\t\tbreak;\n \t\t}\n \tif (argc != i + 2 || (!is_outside_repo(argv[i + 1], nongit, prefix) &&\n@@ -309,6 +312,7 @@ int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)\n \n int run_diff_files(struct rev_info *revs, int silent_on_removed)\n {\n+\tint result = 0;\n \tint entries, i;\n \tint diff_unmerged_stage = revs->max_count;\n \n@@ -433,8 +437,9 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)\n \n \t}\n \tdiffcore_std(&revs->diffopt);\n+\tresult = revs->diffopt.diff_exit_code && diff_queued_diff.nr ? 1: 0;\n \tdiff_flush(&revs->diffopt);\n-\treturn 0;\n+\treturn result;\n }\n \n /*\n@@ -664,9 +669,10 @@ int run_diff_index(struct rev_info *revs, int cached)\n \t\treturn error(\"bad tree object %s\", tree_name);\n \tif (read_tree(tree, 1, revs->prune_data))\n \t\treturn error(\"unable to read tree object %s\", tree_name);\n-\tret = diff_cache(revs, active_cache, active_nr, revs->prune_data,\n+\tdiff_cache(revs, active_cache, active_nr, revs->prune_data,\n \t\t\t cached, match_missing);\n \tdiffcore_std(&revs->diffopt);\n+\tret = revs->diffopt.diff_exit_code && diff_queued_diff.nr ? 1: 0;\n \tdiff_flush(&revs->diffopt);\n \treturn ret;\n }\ndiff --git a/diff.c b/diff.c\nindex 954ca83..e2f91ed 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2134,6 +2134,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->color_diff = options->color_diff_words = 1;\n \telse if (!strcmp(arg, \"--no-renames\"))\n \t\toptions->detect_rename = 0;\n+\telse if (!strcmp(arg, \"--exit-code\"))\n+\t\toptions->diff_exit_code = 1;\n \telse\n \t\treturn 0;\n \treturn 1;\ndiff --git a/diff.h b/diff.h\nindex 4b435e8..cea61b5 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -56,7 +56,8 @@ struct diff_options {\n \t\t silent_on_remove:1,\n \t\t find_copies_harder:1,\n \t\t color_diff:1,\n-\t\t color_diff_words:1;\n+\t\t color_diff_words:1,\n+\t\t diff_exit_code:1;\n \tint context;\n \tint break_opt;\n \tint detect_rename;\ndiff --git a/t/t4017-diff-retval.sh b/t/t4017-diff-retval.sh\nnew file mode 100755\nindex 0000000..81a9393\n--- /dev/null\n+++ b/t/t4017-diff-retval.sh\n@@ -0,0 +1,51 @@\n+#!/bin/sh\n+\n+test_description='Return value of diffs'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo 1 >a &&\n+\tgit add . &&\n+\tgit commit -m first &&\n+\techo 2 >b &&\n+\tgit add . &&\n+\tgit commit -a -m second\n+'\n+\n+test_expect_failure 'git diff-tree HEAD^ HEAD' '\n+\tgit diff-tree --exit-code HEAD^ HEAD\n+'\n+test_expect_failure 'echo HEAD | git diff-tree --stdin' '\n+\techo $(git rev-parse HEAD) | git diff-tree --exit-code --stdin\n+'\n+test_expect_success 'git diff-tree HEAD HEAD' '\n+\tgit diff-tree --exit-code HEAD HEAD\n+'\n+\n+test_expect_success 'git diff-files' '\n+\tgit diff-files --exit-code\n+'\n+\n+test_expect_success 'git diff-index --cached HEAD' '\n+\tgit diff-index --exit-code --cached HEAD\n+'\n+test_expect_failure 'git diff-index --cached HEAD^' '\n+\tgit diff-index --exit-code --cached HEAD^\n+'\n+test_expect_failure 'git diff-index --cached HEAD^' '\n+\techo 3 >c &&\n+\tgit add . &&\n+\tgit diff-index --exit-code --cached HEAD^\n+'\n+git commit -m 'third' >/dev/null\n+test_expect_failure 'git diff-files' '\n+\techo 3 >>c &&\n+\tgit diff-files --exit-code\n+'\n+test_expect_failure 'git diff-index --cached HEAD' '\n+\tgit update-index c &&\n+\tgit diff-index --exit-code --cached HEAD\n+'\n+\n+test_done\ndiff --git a/tree-diff.c b/tree-diff.c\nindex c827582..47c516f 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -160,6 +160,8 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree\n \n int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt)\n {\n+\tint result = 0;\n+\n \twhile (t1->size | t2->size) {\n \t\tif (opt->nr_paths && t1->size && !interesting(t1, base, opt)) {\n \t\t\tupdate_tree_entry(t1);\n@@ -170,29 +172,34 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru\n \t\t\tcontinue;\n \t\t}\n \t\tif (!t1->size) {\n+\t\t\tresult = 1;\n \t\t\tshow_entry(opt, \"+\", t2, base);\n \t\t\tupdate_tree_entry(t2);\n \t\t\tcontinue;\n \t\t}\n \t\tif (!t2->size) {\n+\t\t\tresult = 1;\n \t\t\tshow_entry(opt, \"-\", t1, base);\n \t\t\tupdate_tree_entry(t1);\n \t\t\tcontinue;\n \t\t}\n \t\tswitch (compare_tree_entry(t1, t2, base, opt)) {\n \t\tcase -1:\n+\t\t\tresult = 1;\n \t\t\tupdate_tree_entry(t1);\n \t\t\tcontinue;\n \t\tcase 0:\n \t\t\tupdate_tree_entry(t1);\n-\t\t\t/* Fallthrough */\n+\t\t\tupdate_tree_entry(t2);\n+\t\t\tcontinue;\n \t\tcase 1:\n+\t\t\tresult = 1;\n \t\t\tupdate_tree_entry(t2);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"git-diff-tree: internal error\");\n \t}\n-\treturn 0;\n+\treturn opt->diff_exit_code ? result: 0;\n }\n \n int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const char *base, struct diff_options *opt)\n-- \n1.5.0.3.545.g5d3ba\n\n"},{"id":"37038","messageId":"Pine.LNX.4.63.0703140201240.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7240","inReplyTo":"81b0412b0703131717k7106ee1cg964628f0bda2c83e@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-14T01:03:16Z","receivedAt":"2007-03-14T01:03:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 Mar 2007, Alex Riesen wrote:\n\n> This introduces a new command-line option: --exit-code. The diff \n> programs will return 1 for differences, return 0 for equality, and \n> something else for errors.\n\nI am not convinced that this exit code (even if expected by experience) \nmakes sense. A diff should _only_ fail if anything goes _wrong_, not if \nthe files are _different. That is what cmp is for, no?\n\nCiao,\nDscho\n"},{"id":"37041","messageId":"Pine.LNX.4.64.0703131757080.9690@woody.linux-foundation.org","threadId":"7240","inReplyTo":"81b0412b0703131717k7106ee1cg964628f0bda2c83e@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-14T01:13:12Z","receivedAt":"2007-03-14T01:13:12Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 14 Mar 2007, Alex Riesen wrote:\n>\n> This introduces a new command-line option: --exit-code. The diff\n> programs will return 1 for differences, return 0 for equality, and\n> something else for errors.\n\nI don't think you should need a new command-line option.\n\nIs there any reason to not just do this unconditionally?\n\n> As promised on irc. I'm somewhat confused about diff_tree: it used to\n> unconditionally return 0, yet every caller of it saves and passes the\n> value!\n\nI think we just never implemented the error codes, but they were always \nmeant to be there.\n\nI also thought I did some early-out logic (for the revision list pruning \nthing), where the \"show_entry()\" routine could return a negative value to \nsay \"Ok, no need to do anything more\" but apparently I never added that..\n\n[ I have this very distinct memory of working on it, but either I was \n  dreaming or I never got it working.. ]\n\n\t\t\tLinus\n"},{"id":"37042","messageId":"Pine.LNX.4.63.0703140216480.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7240","inReplyTo":"Pine.LNX.4.64.0703131757080.9690@woody.linux-foundation.org","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-14T01:18:12Z","receivedAt":"2007-03-14T01:18:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 13 Mar 2007, Linus Torvalds wrote:\n\n> On Wed, 14 Mar 2007, Alex Riesen wrote:\n> >\n> > This introduces a new command-line option: --exit-code. The diff \n> > programs will return 1 for differences, return 0 for equality, and \n> > something else for errors.\n> \n> I don't think you should need a new command-line option.\n> \n> Is there any reason to not just do this unconditionally?\n\nSo, big master to hom everybody bows, how to return the correct value when \nexecuting a pager? Because this _has_ to be done if we go that way.\n\nCiao,\nDscho\n"},{"id":"37043","messageId":"Pine.LNX.4.64.0703131816360.9690@woody.linux-foundation.org","threadId":"7240","inReplyTo":"Pine.LNX.4.63.0703140201240.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-14T01:20:49Z","receivedAt":"2007-03-14T01:20:49Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 14 Mar 2007, Johannes Schindelin wrote:\n> \n> I am not convinced that this exit code (even if expected by experience) \n> makes sense. A diff should _only_ fail if anything goes _wrong_, not if \n> the files are _different. That is what cmp is for, no?\n\nWell, \"cmp\" doesn't do a sane job for recursive diffs.\n\nAnd Alex is certainly correct in stating that people historically expect \nand use the exit status of a regular \"diff\". I don't think there is any \nreal down-side to making \"git diff\" have the same semantics, especially \nsince the current exit-status isn't exactly useful.\n\n\t\t\tLinus\n"},{"id":"37044","messageId":"7v1wjsipnd.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"Pine.LNX.4.63.0703140201240.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T01:23:50Z","receivedAt":"2007-03-14T01:23:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Wed, 14 Mar 2007, Alex Riesen wrote:\n>\n>> This introduces a new command-line option: --exit-code. The diff \n>> programs will return 1 for differences, return 0 for equality, and \n>> something else for errors.\n>\n> I am not convinced that this exit code (even if expected by experience) \n> makes sense. A diff should _only_ fail if anything goes _wrong_, not if \n> the files are _different. That is what cmp is for, no?\n\nNot really, if you are used to use diff as a replacement for\ncmp.\n"},{"id":"37045","messageId":"7vveh4hapk.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"Pine.LNX.4.64.0703131757080.9690@woody.linux-foundation.org","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T01:31:51Z","receivedAt":"2007-03-14T01:31:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Wed, 14 Mar 2007, Alex Riesen wrote:\n>>\n>> This introduces a new command-line option: --exit-code. The diff\n>> programs will return 1 for differences, return 0 for equality, and\n>> something else for errors.\n>\n> I don't think you should need a new command-line option.\n>\n> Is there any reason to not just do this unconditionally?\n\nExiting with 0 for no-change, 1 for has-change and other value\nfor error is something that falls into the\n\n    \"I wish if we did it from day one, but now many people's\n    scripts depend on the behaviour, and heck we ourselves say\n    that the right way to see if there is difference is to check\n    if the output is an empty string (look at a few scripts of\n    our own), so it would be a huge backward compatibility\n    hassle\"\n\ncategory.\n\ne.g.\n\ngit-am.sh:\tfiles=$(git-diff-index --cached --name-only HEAD) || exit\ngit-am.sh:      changed=\"$(git-diff-index --cached --name-only HEAD)\"\nt/t0020-crlf.sh:\tdiffers=`git diff-index --cached HEAD` &&\nt/t0020-crlf.sh:\tdiffers=`git diff-index --cached HEAD` &&\nt/t0020-crlf.sh:\tdiffers=`git diff-index --cached HEAD` &&\nt/t0020-crlf.sh:\tdiffers=`git diff-index --cached HEAD` &&\n\nAnd I expect people's scripts are modelled after how git's\nscripts do things --- if they were half competent, they'd know\nthat would be the way to make sure their scripts would be\ncompatible with future plumbing changes.\n"},{"id":"37047","messageId":"Pine.LNX.4.64.0703131831390.9690@woody.linux-foundation.org","threadId":"7240","inReplyTo":"Pine.LNX.4.63.0703140216480.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-14T01:34:02Z","receivedAt":"2007-03-14T01:34:02Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 14 Mar 2007, Johannes Schindelin wrote:\n> \n> So, big master to hom everybody bows, how to return the correct value when \n> executing a pager? Because this _has_ to be done if we go that way.\n\nWhy? If you execute the pager, nobody cares about the error value anyway. \n\nI don't see why you would mix in a pager here. If you do\n\n\tdiff -u file1 file2 | less -S\n\nthe return value of the pipe will not only generally be totally \nuninteresting and never used, but it will be the return value of \"less\" \nanyway. Which is what we'd get quite naturally.\n\nThe return value of \"git diff\" really only is sensibly and meaningful when \nyou use it in a script *without* a pager (whether built-in or not).\n\n\t\tLinus\n"},{"id":"37049","messageId":"Pine.LNX.4.63.0703140237270.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7240","inReplyTo":"Pine.LNX.4.64.0703131831390.9690@woody.linux-foundation.org","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-14T01:38:40Z","receivedAt":"2007-03-14T01:38:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 13 Mar 2007, Linus Torvalds wrote:\n\n> On Wed, 14 Mar 2007, Johannes Schindelin wrote:\n> > \n> > So, big master to hom everybody bows, how to return the correct value \n> > when executing a pager? Because this _has_ to be done if we go that \n> > way.\n> \n> Why? If you execute the pager, nobody cares about the error value \n> anyway.\n> \n> I don't see why you would mix in a pager here. If you do\n> \n> \tdiff -u file1 file2 | less -S\n> \n> the return value of the pipe will not only generally be totally \n> uninteresting and never used, but it will be the return value of \"less\" \n> anyway. Which is what we'd get quite naturally.\n\nThe thing is, most people do not realize that\n\n\tgit diff file1 file2\n\n_will_ execute a pager. As foreground process. And the return value is \nthat of the pager.\n\nCiao,\nDscho\n"},{"id":"37051","messageId":"7v8xe0h19a.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"81b0412b0703131717k7106ee1cg964628f0bda2c83e@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T04:56:01Z","receivedAt":"2007-03-14T04:56:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" <raa.lkml@gmail.com> writes:\n\n> diff --git a/builtin-diff.c b/builtin-diff.c\n> index 4efbb82..5e6265f 100644\n> --- a/builtin-diff.c\n> +++ b/builtin-diff.c\n> @@ -130,6 +130,7 @@ static int builtin_diff_tree(struct rev_info *revs,\n>  {\n>  \tconst unsigned char *(sha1[2]);\n>  \tint swap = 0;\n> +\tint result = 0;\n>  \n>  \tif (argc > 1)\n>  \t\tusage(builtin_diff_usage);\n> @@ -141,9 +142,9 @@ static int builtin_diff_tree(struct rev_info *revs,\n>  \t\tswap = 1;\n>  \tsha1[swap] = ent[0].item->sha1;\n>  \tsha1[1-swap] = ent[1].item->sha1;\n> -\tdiff_tree_sha1(sha1[0], sha1[1], \"\", &revs->diffopt);\n> +\tresult = diff_tree_sha1(sha1[0], sha1[1], \"\", &revs->diffopt);\n>  \tlog_tree_diff_flush(revs);\n> -\treturn 0;\n> +\treturn result;\n>  }\n>  \n>  static int builtin_diff_combined(struct rev_info *revs,\n\nThe change to diff-tree side is completely borked.  (1) You did\nnot notice compare_tree_entry() in tree-diff.c returns 0 only to\nsignal that it has dealt with an entry from both sides (so the\ncaller can do update_tree_entry() on both), and the return value\ndoes not mean they are the same.  (2) You are checking if there\nare differences at wrong level, before letting diffcore_std() to\nprocess the queue.  Because of the bug (1) I cannot test that\nbut after you fix (1) you would notice that it would not work if\nyou say \"-Spickaxe\"; your changes to diff-files and diff-index\nare correct on this regard.\n\nA slight tangent, but what Linus recalled he thought he did but\nhe didn't is related to the parts you touched in diff-tree\nabove.  Because of the interaction with diffcore, these changes\nshould not be used for the purpose of -exit-code, but catching\nthe tree level change in the above places and leaving early\nwould be the right thing to do for comparing the whole tree for\nthe purpose of simplifying the parents.  Tomorrow will be my git\nday so I might whip up a patch to speed that up.\n\n> diff --git a/diff-lib.c b/diff-lib.c\n> index 6abb981..f943b6f 100644\n> --- a/diff-lib.c\n> +++ b/diff-lib.c\n> @@ -433,8 +437,9 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)\n>  \n>  \t}\n>  \tdiffcore_std(&revs->diffopt);\n> +\tresult = revs->diffopt.diff_exit_code && diff_queued_diff.nr ? 1: 0;\n>  \tdiff_flush(&revs->diffopt);\n> -\treturn 0;\n> +\treturn result;\n>  }\n>  \n>  /*\n> @@ -664,9 +669,10 @@ int run_diff_index(struct rev_info *revs, int cached)\n>  \t\treturn error(\"bad tree object %s\", tree_name);\n>  \tif (read_tree(tree, 1, revs->prune_data))\n>  \t\treturn error(\"unable to read tree object %s\", tree_name);\n> -\tret = diff_cache(revs, active_cache, active_nr, revs->prune_data,\n> +\tdiff_cache(revs, active_cache, active_nr, revs->prune_data,\n>  \t\t\t cached, match_missing);\n>  \tdiffcore_std(&revs->diffopt);\n> +\tret = revs->diffopt.diff_exit_code && diff_queued_diff.nr ? 1: 0;\n>  \tdiff_flush(&revs->diffopt);\n>  \treturn ret;\n>  }\n\nThis side looks correct, as you are counting queued_diff.nr after\nletting diffcore_std() to filter the results.\n\n> +test_expect_failure 'git diff-tree HEAD^ HEAD' '\n> +\tgit diff-tree --exit-code HEAD^ HEAD\n> +'\n> ...\n> +test_expect_failure 'git diff-index --cached HEAD' '\n> +\tgit update-index c &&\n> +\tgit diff-index --exit-code --cached HEAD\n> +'\n\nIn general, expect_failure should not be used for complex cases\nlike this.  The first one I quoted is fine, but the latter one\nis not.  update-index may fail (perhaps somebody screwed up\nwhile updating read-cache.c or sha1_file.c) and the whole test\nwould say \"happy that the command chain as a whole failed\".\n"},{"id":"37062","messageId":"81b0412b0703140119s4691211bw8a5ef3ba25c5cbdf@mail.gmail.com","threadId":"7240","inReplyTo":"7vveh4hapk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T08:19:52Z","receivedAt":"2007-03-14T08:19:52Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> Linus Torvalds <torvalds@linux-foundation.org> writes:\n>\n> > On Wed, 14 Mar 2007, Alex Riesen wrote:\n> >>\n> >> This introduces a new command-line option: --exit-code. The diff\n> >> programs will return 1 for differences, return 0 for equality, and\n> >> something else for errors.\n> >\n> > I don't think you should need a new command-line option.\n> >\n> > Is there any reason to not just do this unconditionally?\n>\n> Exiting with 0 for no-change, 1 for has-change and other value\n> for error is something that falls into the\n>\n>     \"I wish if we did it from day one, but now many people's\n>     scripts depend on the behaviour, and heck we ourselves say\n>     that the right way to see if there is difference is to check\n>     if the output is an empty string (look at a few scripts of\n>     our own), so it would be a huge backward compatibility\n>     hassle\"\n>\n> category.\n>\n> e.g.\n>\n> git-am.sh:      files=$(git-diff-index --cached --name-only HEAD) || exit\n> git-am.sh:      changed=\"$(git-diff-index --cached --name-only HEAD)\"\n\nIsn't this crazy? Get the information and never really use it?\nThe second one does not check for errors, BTW. Will\nbreak if someone sends enough files in one commit to go past\nlocal limit on shell variable size (or the local limit on process size,\nwhich is much worse situation: everything gets swapped out).\n\n> And I expect people's scripts are modelled after how git's\n> scripts do things --- if they were half competent, they'd know\n> that would be the way to make sure their scripts would be\n> compatible with future plumbing changes.\n\nThat is expert level. The half competents are used to diff(1).\n"},{"id":"37063","messageId":"81b0412b0703140128y46ff6bb6m503eeae00c043ddf@mail.gmail.com","threadId":"7240","inReplyTo":"7v8xe0h19a.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T08:28:55Z","receivedAt":"2007-03-14T08:28:55Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> \"Alex Riesen\" <raa.lkml@gmail.com> writes:\n>\n> > diff --git a/builtin-diff.c b/builtin-diff.c\n> > index 4efbb82..5e6265f 100644\n> > --- a/builtin-diff.c\n> > +++ b/builtin-diff.c\n> > @@ -130,6 +130,7 @@ static int builtin_diff_tree(struct rev_info *revs,\n> >  {\n> >       const unsigned char *(sha1[2]);\n> >       int swap = 0;\n> > +     int result = 0;\n> >\n> >       if (argc > 1)\n> >               usage(builtin_diff_usage);\n> > @@ -141,9 +142,9 @@ static int builtin_diff_tree(struct rev_info *revs,\n> >               swap = 1;\n> >       sha1[swap] = ent[0].item->sha1;\n> >       sha1[1-swap] = ent[1].item->sha1;\n> > -     diff_tree_sha1(sha1[0], sha1[1], \"\", &revs->diffopt);\n> > +     result = diff_tree_sha1(sha1[0], sha1[1], \"\", &revs->diffopt);\n> >       log_tree_diff_flush(revs);\n> > -     return 0;\n> > +     return result;\n> >  }\n> >\n> >  static int builtin_diff_combined(struct rev_info *revs,\n>\n> The change to diff-tree side is completely borked.  (1) You did\n> not notice compare_tree_entry() in tree-diff.c returns 0 only to\n> signal that it has dealt with an entry from both sides (so the\n> caller can do update_tree_entry() on both), and the return value\n> does not mean they are the same.  (2) You are checking if there\n> are differences at wrong level, before letting diffcore_std() to\n> process the queue.  Because of the bug (1) I cannot test that\n> but after you fix (1) you would notice that it would not work if\n> you say \"-Spickaxe\"; your changes to diff-files and diff-index\n> are correct on this regard.\n\nChallenging... Now, if someone just told me where to look for\ndifferences in diff-tree case...\n\n> A slight tangent, but what Linus recalled he thought he did but\n> he didn't is related to the parts you touched in diff-tree\n> above.  Because of the interaction with diffcore, these changes\n> should not be used for the purpose of -exit-code, but catching\n> the tree level change in the above places and leaving early\n> would be the right thing to do for comparing the whole tree for\n> the purpose of simplifying the parents.  Tomorrow will be my git\n> day so I might whip up a patch to speed that up.\n\nCan it eventually be wired to \"-s\" (DIFF_FORMAT_NO_OUTPUT)?\n\n> >       diffcore_std(&revs->diffopt);\n> > +     ret = revs->diffopt.diff_exit_code && diff_queued_diff.nr ? 1: 0;\n> >       diff_flush(&revs->diffopt);\n> >       return ret;\n> >  }\n>\n> This side looks correct, as you are counting queued_diff.nr after\n> letting diffcore_std() to filter the results.\n>\n\nAnd it will continue to work if the diffing is left early because of\nno output needed. Err, will it?\n\n> > +test_expect_failure 'git diff-index --cached HEAD' '\n> > +     git update-index c &&\n> > +     git diff-index --exit-code --cached HEAD\n> > +'\n>\n> In general, expect_failure should not be used for complex cases\n> like this.  The first one I quoted is fine, but the latter one\n> is not.  update-index may fail (perhaps somebody screwed up\n> while updating read-cache.c or sha1_file.c) and the whole test\n> would say \"happy that the command chain as a whole failed\".\n\nRight. Fixed.\n"},{"id":"37064","messageId":"81b0412b0703140137i6d0df44aua9205e81249570ea@mail.gmail.com","threadId":"7240","inReplyTo":"Pine.LNX.4.63.0703140237270.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T08:37:54Z","receivedAt":"2007-03-14T08:37:54Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/14/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> > > So, big master to hom everybody bows, how to return the correct value\n> > > when executing a pager? Because this _has_ to be done if we go that\n> > > way.\n> >\n> > Why? If you execute the pager, nobody cares about the error value\n> > anyway.\n> >\n> > I don't see why you would mix in a pager here. If you do\n> >\n> >       diff -u file1 file2 | less -S\n> >\n> > the return value of the pipe will not only generally be totally\n> > uninteresting and never used, but it will be the return value of \"less\"\n> > anyway. Which is what we'd get quite naturally.\n>\n> The thing is, most people do not realize that\n>\n>         git diff file1 file2\n>\n> _will_ execute a pager. As foreground process. And the return value is\n> that of the pager.\n\nIn this example this is obviously (sometimes it is obscurely) interactive.\nThe return code is seldom expected.\n\nMore sneaky case could be this:\n\n  git diff file1 file2 > tmp && do_something\n  rm -f tmp\n\nBut we have isatty in setup_pager, so this works properly.\n"},{"id":"37065","messageId":"7vfy88dwvw.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"81b0412b0703140119s4691211bw8a5ef3ba25c5cbdf@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T08:58:43Z","receivedAt":"2007-03-14T08:58:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" <raa.lkml@gmail.com> writes:\n\n> On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> ...\n>> Exiting with 0 for no-change, 1 for has-change and other value\n>> for error is something that falls into the\n>>\n>>     \"I wish if we did it from day one, but now many people's\n>>     scripts depend on the behaviour, and heck we ourselves say\n>>     that the right way to see if there is difference is to check\n>>     if the output is an empty string (look at a few scripts of\n>>     our own), so it would be a huge backward compatibility\n>>     hassle\"\n>>\n>> category.\n>>\n>> e.g.\n>>\n>> git-am.sh:      files=$(git-diff-index --cached --name-only HEAD) || exit\n>> git-am.sh:      changed=\"$(git-diff-index --cached --name-only HEAD)\"\n>\n> Isn't this crazy? Get the information and never really use it?\n\nThe information is used.\n\nThe first one says \"see if we have already difference stashed in\nthe index for our own committing --- by the way, if there is an\nerror, do not do any further harm but error out\".  If we do not\nget an error, that line is followed by this:\n\n\tfiles=$(git-diff-index --cached --name-only HEAD) || exit\n        if [ \"$files\" ]\n        then\n        \techo \"Dirty index: ...\" >&2\n                exit 1\n\tfi\n\nI've been telling you since the #git session that I know that is\n*different* from how \"diff\" works, and I think everybody agrees\nif we were doing git from scratch today we would have done exit\nstatus with 0/1/other to signal no-change, have-diff and error.\n\nBut the established way for scripts that use plumbing is\n\n - to check error with $? (or ... || )\n - to check modified-or-not with output\n\nand people who have been learning from the scripts (we used to\nhave lot more scripts) would have picked up that pattern.\nThat's why I already told you that --exit-status is the right\nthing to do if we were doing it from scratch, but is a wrong\nthing to do at this point.  Maybe in a release as big as 1.5.0\nthat we pre-announce a lot of interface changes.\n\nIn short, Linus is right in that the current exit code is not\nuseful to see what the end users are interested in (and they are\nnot in the business of debugging git, and diff would error out\nonly when the repository has problems, perhaps a corrupt object\nor something like that).  But being not useful and being\ncurrently not relied upon are two different things.\n\nAnd I am being conservative, especially after a big release.\n"},{"id":"37066","messageId":"7v6494dwms.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"81b0412b0703140128y46ff6bb6m503eeae00c043ddf@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T09:04:11Z","receivedAt":"2007-03-14T09:04:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" <raa.lkml@gmail.com> writes:\n\n> Challenging... Now, if someone just told me where to look for\n> differences in diff-tree case...\n\nAs the resident git-diff expert I might hack on this myself as\nthe --quiet options are useful, and --exit-code comes almost\nfree when you properly do --quiet that implies --quick.\n\n>> A slight tangent, but what Linus recalled he thought he did but\n>> he didn't is related to the parts you touched in diff-tree\n>> above.  Because of the interaction with diffcore, these changes\n>> should not be used for the purpose of -exit-code, but catching\n>> the tree level change in the above places and leaving early\n>> would be the right thing to do for comparing the whole tree for\n>> the purpose of simplifying the parents.  Tomorrow will be my git\n>> day so I might whip up a patch to speed that up.\n>\n> Can it eventually be wired to \"-s\" (DIFF_FORMAT_NO_OUTPUT)?\n\nI do not think so.  When we run diff internally to pick the set\nof paths (i.e. run diffcore and check contents of diff_queue,\njust like you did for diff-index/diff-files), we internally use\nNO_OUTPUT.  See merge-recursive.c for an example.\n\n>> >       diffcore_std(&revs->diffopt);\n>> > +     ret = revs->diffopt.diff_exit_code && diff_queued_diff.nr ? 1: 0;\n>> >       diff_flush(&revs->diffopt);\n>> >       return ret;\n>> >  }\n>>\n>> This side looks correct, as you are counting queued_diff.nr after\n>> letting diffcore_std() to filter the results.\n>\n> And it will continue to work if the diffing is left early because of\n> no output needed. Err, will it?\n\nTo implement --quick correctly, you need to know when it is safe\nto leave early.  Presence of -S (pickaxe) would most likely mean\nyou shouldn't leave early.\n"},{"id":"37067","messageId":"7v1wjsdwj2.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"7vfy88dwvw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T09:06:25Z","receivedAt":"2007-03-14T09:06:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> I've been telling you since the #git session that I know that is\n> *different* from how \"diff\" works, and I think everybody agrees\n> if we were doing git from scratch today we would have done exit\n> status with 0/1/other to signal no-change, have-diff and error.\n>\n> But the established way for scripts that use plumbing is\n>\n>  - to check error with $? (or ... || )\n>  - to check modified-or-not with output\n>\n> and people who have been learning from the scripts (we used to\n> have lot more scripts) would have picked up that pattern.\n> That's why I already told you that --exit-status is the right\n> thing to do if we were doing it from scratch, but is a wrong\n> thing to do at this point.\n\nCorrection.\n\ns/--exit-status is/doing --exit-status without such an explicit option is/.\n\n> Maybe in a release as big as 1.5.0\n> that we pre-announce a lot of interface changes.\n>\n> In short, Linus is right in that the current exit code is not\n> useful to see what the end users are interested in (and they are\n> not in the business of debugging git, and diff would error out\n> only when the repository has problems, perhaps a corrupt object\n> or something like that).  But being not useful and being\n> currently not relied upon are two different things.\n>\n> And I am being conservative, especially after a big release.\n"},{"id":"37068","messageId":"81b0412b0703140207kf887738od482022497d2a63d@mail.gmail.com","threadId":"7240","inReplyTo":"7vfy88dwvw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T09:07:21Z","receivedAt":"2007-03-14T09:07:21Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> and people who have been learning from the scripts (we used to\n> have lot more scripts) would have picked up that pattern.\n> That's why I already told you that --exit-status is the right\n> thing to do if we were doing it from scratch, but is a wrong\n> thing to do at this point.  Maybe in a release as big as 1.5.0\n> that we pre-announce a lot of interface changes.\n\nThis is something I don't understand: why is a new option\n--exit-code wrong? Noone uses it yet and the default exit code\nis at it was before (modulo the bug with diff-tree). I am *not*\nproposing to change default exit codes (well, I suggest that).\nSo what's the problem?\n"},{"id":"37071","messageId":"7vk5xkcgkb.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"81b0412b0703140207kf887738od482022497d2a63d@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T09:36:36Z","receivedAt":"2007-03-14T09:36:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" <raa.lkml@gmail.com> writes:\n\n> On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n>> and people who have been learning from the scripts (we used to\n>> have lot more scripts) would have picked up that pattern.\n>> That's why I already told you that --exit-status is the right\n>> thing to do if we were doing it from scratch, but is a wrong\n>> thing to do at this point.  Maybe in a release as big as 1.5.0\n>> that we pre-announce a lot of interface changes.\n>\n> This is something I don't understand: why is a new option\n> --exit-code wrong?\n\nSorry, what I said was very confused (I sent out a\ncorrection/clarification didn't I?).\n\nIn the ideal world we would sensibly make our diff to report\nwith its exit status.  Unfortunately we cannot for the above\nbackward compatibility worries, so a new option is an ugly\nbut necessary workaround.\n"},{"id":"37073","messageId":"81b0412b0703140246h13f6f18fo88395ad603885dc3@mail.gmail.com","threadId":"7240","inReplyTo":"7vk5xkcgkb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T09:46:11Z","receivedAt":"2007-03-14T09:46:11Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> In the ideal world we would sensibly make our diff to report\n> with its exit status.  Unfortunately we cannot for the above\n> backward compatibility worries, so a new option is an ugly\n> but necessary workaround.\n\nright\n"},{"id":"37081","messageId":"Pine.LNX.4.63.0703141301210.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7240","inReplyTo":"81b0412b0703140137i6d0df44aua9205e81249570ea@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-14T12:05:24Z","receivedAt":"2007-03-14T12:05:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 Mar 2007, Alex Riesen wrote:\n\n> On 3/14/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> > > > So, big master to hom everybody bows, how to return the correct value\n> > > > when executing a pager? Because this _has_ to be done if we go that\n> > > > way.\n> > >\n> > > Why? If you execute the pager, nobody cares about the error value\n> > > anyway.\n> > >\n> > > I don't see why you would mix in a pager here. If you do\n> > >\n> > >       diff -u file1 file2 | less -S\n> > >\n> > > the return value of the pipe will not only generally be totally\n> > > uninteresting and never used, but it will be the return value of \"less\"\n> > > anyway. Which is what we'd get quite naturally.\n> > \n> > The thing is, most people do not realize that\n> > \n> >         git diff file1 file2\n> > \n> > _will_ execute a pager. As foreground process. And the return value is\n> > that of the pager.\n> \n> In this example this is obviously (sometimes it is obscurely) interactive.\n> The return code is seldom expected.\n> \n> More sneaky case could be this:\n> \n>  git diff file1 file2 > tmp && do_something\n>  rm -f tmp\n> \n> But we have isatty in setup_pager, so this works properly.\n\nThe problem is test cases. I think that we pipe the output of the test \ncase _anyway_, so the isatty() call is helping us there.\n\nIf we did not (this applies to test cases _outside_ of Git, too), then a \nsimple\n\n\tgit diff bla || exit\n\nwould not work as expected. Even worse, as long as it is piped somewhere, \neven cat, it works. But once you no longer pipe it (to get the nice pager, \nfor example), it stops working.\n\nCiao,\nDscho\n"},{"id":"37083","messageId":"81b0412b0703140526p454ee85fk74dcd76fb684217c@mail.gmail.com","threadId":"7240","inReplyTo":"Pine.LNX.4.63.0703141301210.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T12:26:33Z","receivedAt":"2007-03-14T12:26:33Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/14/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> > But we have isatty in setup_pager, so this works properly.\n>\n> The problem is test cases. I think that we pipe the output of the test\n> case _anyway_, so the isatty() call is helping us there.\n>\n> If we did not (this applies to test cases _outside_ of Git, too), then a\n> simple\n>\n>         git diff bla || exit\n>\n> would not work as expected. Even worse, as long as it is piped somewhere,\n> even cat, it works. But once you no longer pipe it (to get the nice pager,\n> for example), it stops working.\n\nWe have \"PAGER=cat\" in test-lib.sh which just disables pager,\nso gits tests do not have the problem (maybe it was disabled just\nbecause of this?). And if someone has own tests _with_ pager active\nthe one better be aware of what the one doing.\n"},{"id":"37084","messageId":"Pine.LNX.4.63.0703141329460.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7240","inReplyTo":"81b0412b0703140526p454ee85fk74dcd76fb684217c@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-14T12:31:48Z","receivedAt":"2007-03-14T12:31:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 Mar 2007, Alex Riesen wrote:\n\n> We have \"PAGER=cat\" in test-lib.sh which just disables pager, so gits \n> tests do not have the problem (maybe it was disabled just because of \n> this?).\n\nI don't know if it was disabled just because of this, but I guess that it \nwas to prevent interactive tests from stopping with the output of _every_ \n_single_ test until the user quits the pager.\n\n> And if someone has own tests _with_ pager active the one better be aware \n> of what the one doing.\n\nNo. This is wrong. The whole purpose of the patch you sent is to meet \nexpectations. What you just wrote flies in the face of that.\n\nCiao,\nDscho\n"},{"id":"37086","messageId":"81b0412b0703140701h60982fddw3ed8fa71288cb220@mail.gmail.com","threadId":"7240","inReplyTo":"7v6494dwms.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T14:01:18Z","receivedAt":"2007-03-14T14:01:18Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"This introduces a new command-line option: --exit-code. The diff\nprograms will return 1 for differences, return 0 for equality, and\nsomething else for errors.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\nUpdated the patch:\n\n- added more tests: path limiter and -S\n- moved exit code into diff_options and used diffcore_std* for its evaluation\n\nOn 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> > Challenging... Now, if someone just told me where to look for\n> > differences in diff-tree case...\n>\n> As the resident git-diff expert I might hack on this myself as\n> the --quiet options are useful, and --exit-code comes almost\n> free when you properly do --quiet that implies --quick.\n\nThanks! That would _most_ welcome :)\n\n> > Can it eventually be wired to \"-s\" (DIFF_FORMAT_NO_OUTPUT)?\n>\n> I do not think so.  When we run diff internally to pick the set\n> of paths (i.e. run diffcore and check contents of diff_queue,\n> just like you did for diff-index/diff-files), we internally use\n> NO_OUTPUT.  See merge-recursive.c for an example.\n\nRight, is not that simple as it looks...\n\n> >> >       diffcore_std(&revs->diffopt);\n> >> > +     ret = revs->diffopt.diff_exit_code && diff_queued_diff.nr ? 1: 0;\n> >> >       diff_flush(&revs->diffopt);\n> >> >       return ret;\n> >> >  }\n> >>\n> >> This side looks correct, as you are counting queued_diff.nr after\n> >> letting diffcore_std() to filter the results.\n> >\n> > And it will continue to work if the diffing is left early because of\n> > no output needed. Err, will it?\n>\n> To implement --quick correctly, you need to know when it is safe\n> to leave early.  Presence of -S (pickaxe) would most likely mean\n> you shouldn't leave early.\n\nThanks, that got me thinking. Moved all exit code evaluation\ninto diffcore_std, added a field for the code to diff_options,\nand use it if called with --exit-code.\n\n Documentation/diff-options.txt |    5 +++\n builtin-diff-files.c           |    4 ++-\n builtin-diff-index.c           |    4 ++-\n builtin-diff-tree.c            |    4 +-\n builtin-diff.c                 |   19 +++++++-----\n diff-lib.c                     |    5 ++-\n diff.c                         |    6 ++++\n diff.h                         |    5 ++-\n t/t4017-diff-retval.sh         |   64 ++++++++++++++++++++++++++++++++++++++++\n 9 files changed, 102 insertions(+), 14 deletions(-)\n create mode 100755 t/t4017-diff-retval.sh\n"},{"id":"37090","messageId":"7v1wjrby4w.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"81b0412b0703140701h60982fddw3ed8fa71288cb220@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T16:14:39Z","receivedAt":"2007-03-14T16:14:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" <raa.lkml@gmail.com> writes:\n\n>> To implement --quick correctly, you need to know when it is safe\n>> to leave early.  Presence of -S (pickaxe) would most likely mean\n>> you shouldn't leave early.\n>\n> Thanks, that got me thinking. Moved all exit code evaluation\n> into diffcore_std, added a field for the code to diff_options,\n> and use it if called with --exit-code.\n\nCertainly --quick would be \"interesting\" and useful to add on\ntop of your patch.\n\n> diff --git a/builtin-diff-files.c b/builtin-diff-files.c\n> index aec8338..2525ae6 100644\n> --- a/builtin-diff-files.c\n> +++ b/builtin-diff-files.c\n> @@ -17,6 +17,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n>  {\n>  \tstruct rev_info rev;\n>  \tint nongit = 0;\n> +\tint result;\n>  \n>  \tprefix = setup_git_directory_gently(&nongit);\n>  \tinit_revisions(&rev, prefix);\n> @@ -29,5 +30,6 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n>  \t\targc = setup_revisions(argc, argv, &rev, NULL);\n>  \tif (!rev.diffopt.output_format)\n>  \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n> -\treturn run_diff_files_cmd(&rev, argc, argv);\n> +\tresult = run_diff_files_cmd(&rev, argc, argv);\n> +\treturn rev.diffopt.diff_exit_code ? rev.diffopt.exit_code: result;\n>  }\n\nYuck.  Let's call the former \"exit_with_status\" (meaning, the\ncaller instructed us to do that) and the latter \"has_changes\".\n\n> +test_expect_failure 'git diff-index --cached HEAD^' '\n> +\techo text >>b &&\n> +\techo 3 >c &&\n> +\tgit add . &&\n> +\tgit diff-index --exit-code --cached HEAD^\n> +'\n\nPlease:\n\n        test_expect_success '...\n                echo ... &&\n                git add . &&\n                ! git diff-index ...\n        '\n\nI recall somebody on the list had a buggy shell that cannot\nhandle \"a && ! b\" and tweaked a few tests to work around it.\nIn that case...\n\n\techo ... &&\n        git add . &&\n\t{\n        \tgit diff-index ...; test $? != 0\n\t}\n\n> +test_expect_failure 'git diff-files' '\n> +\techo 3 >>c &&\n> +\tgit diff-files --exit-code\n> +'\n\nLikewise\n\n> +git update-index c || error \"update-index failed\"\n\nPlease do not have command outside test_expect without good\nreason.\n"},{"id":"37091","messageId":"81b0412b0703140933v5c684aa9g1f61d61e502c6783@mail.gmail.com","threadId":"7240","inReplyTo":"7v1wjrby4w.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T16:33:21Z","receivedAt":"2007-03-14T16:33:21Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> > -     return run_diff_files_cmd(&rev, argc, argv);\n> > +     result = run_diff_files_cmd(&rev, argc, argv);\n> > +     return rev.diffopt.diff_exit_code ? rev.diffopt.exit_code: result;\n> >  }\n>\n> Yuck.  Let's call the former \"exit_with_status\" (meaning, the\n> caller instructed us to do that) and the latter \"has_changes\".\n\nI like \"exit_with_status\". But has_changes looks confusing\ngood near return value of run_diff_files_cmd, which \"has\"\nnothing. Or do you mean to highlight this \"difference\"?\n\n> > +test_expect_failure 'git diff-index --cached HEAD^' '\n> > +     echo text >>b &&\n> > +     echo 3 >c &&\n> > +     git add . &&\n> > +     git diff-index --exit-code --cached HEAD^\n> > +'\n>\n> Please:\n>\n>         test_expect_success '...\n>                 echo ... &&\n>                 git add . &&\n>                 ! git diff-index ...\n>         '\n>\n> I recall somebody on the list had a buggy shell that cannot\n> handle \"a && ! b\" and tweaked a few tests to work around it.\n> In that case...\n>\n>         echo ... &&\n>         git add . &&\n>         {\n>                 git diff-index ...; test $? != 0\n>         }\n\nConfused. What's wrong with test_expect_failure which\ndoes not need any of the both ugly constructions?\nHmm... Do you mean: \"we expect successful\noperation with exit code 1\"? I like this, easier to follow.\nWill do.\n\n> > +test_expect_failure 'git diff-files' '\n> > +     echo 3 >>c &&\n> > +     git diff-files --exit-code\n> > +'\n>\n> Likewise\n\nYep, I'll change this too.\n\n> > +git update-index c || error \"update-index failed\"\n>\n> Please do not have command outside test_expect without good\n> reason.\n\nWell, it does not actually belongs to the test logic\n(as well as the echo's, one may notice).\n"},{"id":"37092","messageId":"7vwt1jaiiu.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"81b0412b0703140933v5c684aa9g1f61d61e502c6783@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T16:37:13Z","receivedAt":"2007-03-14T16:37:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" <raa.lkml@gmail.com> writes:\n\n>>         echo ... &&\n>>         git add . &&\n>>         {\n>>                 git diff-index ...; test $? != 0\n>>         }\n>\n> Confused. What's wrong with test_expect_failure which\n> does not need any of the both ugly constructions?\n> Hmm... Do you mean: \"we expect successful\n> operation with exit code 1\"?\n\nNo.  Think what happens if you broke \"git add\".\n"},{"id":"37093","messageId":"7vodmvah5e.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"81b0412b0703140933v5c684aa9g1f61d61e502c6783@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T17:06:53Z","receivedAt":"2007-03-14T17:06:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" <raa.lkml@gmail.com> writes:\n\n> On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n>> > -     return run_diff_files_cmd(&rev, argc, argv);\n>> > +     result = run_diff_files_cmd(&rev, argc, argv);\n>> > +     return rev.diffopt.diff_exit_code ? rev.diffopt.exit_code: result;\n>> >  }\n>>\n>> Yuck.  Let's call the former \"exit_with_status\" (meaning, the\n>> caller instructed us to do that) and the latter \"has_changes\".\n>\n> I like \"exit_with_status\". But has_changes looks confusing\n> good near return value of run_diff_files_cmd, which \"has\"\n> nothing. Or do you mean to highlight this \"difference\"?\n\nMaybe 'found_changes' would be a better name, then?\n\n\tif (exit_with_status)\n        \treturn !!found_changes;\n\telse\n        \treturn result;\n"},{"id":"37094","messageId":"81b0412b0703141012n6fc252ecsd5ed3bc7283cf10f@mail.gmail.com","threadId":"7240","inReplyTo":"7vwt1jaiiu.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T17:12:49Z","receivedAt":"2007-03-14T17:12:49Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"This introduces a new command-line option: --exit-code. The diff\nprograms will return 1 for differences, return 0 for equality, and\nsomething else for errors.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nUpdated patch:\n- implemented exit_with_status+has_changes\n- fixed test WRT intermediate failures\n\nOn 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> \"Alex Riesen\" <raa.lkml@gmail.com> writes:\n>\n> >>         echo ... &&\n> >>         git add . &&\n> >>         {\n> >>                 git diff-index ...; test $? != 0\n> >>         }\n> >\n> > Confused. What's wrong with test_expect_failure which\n> > does not need any of the both ugly constructions?\n> > Hmm... Do you mean: \"we expect successful\n> > operation with exit code 1\"?\n>\n> No.  Think what happens if you broke \"git add\".\n>\n\nRight. Figured that out myself while changing the test.\nRemoved all test_expect_failure anyway: not that I\nactually expect a failure, just exit code. Made the test\nstricter, too: it must be either 0 or 1. Anything else\nconsidered test failure.\n\n Documentation/diff-options.txt |    5 +++\n builtin-diff-files.c           |    4 ++-\n builtin-diff-index.c           |    4 ++-\n builtin-diff-tree.c            |    5 ++-\n builtin-diff.c                 |   19 ++++++----\n diff-lib.c                     |    5 ++-\n diff.c                         |    6 +++\n diff.h                         |    5 ++-\n t/t4017-diff-retval.sh         |   79 ++++++++++++++++++++++++++++++++++++++++\n 9 files changed, 118 insertions(+), 14 deletions(-)\n create mode 100755 t/t4017-diff-retval.sh\n"},{"id":"37095","messageId":"81b0412b0703141015m1f7cb68dkf89fe13a968a30d0@mail.gmail.com","threadId":"7240","inReplyTo":"7vodmvah5e.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-14T17:15:32Z","receivedAt":"2007-03-14T17:15:32Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:\n> >> > -     return run_diff_files_cmd(&rev, argc, argv);\n> >> > +     result = run_diff_files_cmd(&rev, argc, argv);\n> >> > +     return rev.diffopt.diff_exit_code ? rev.diffopt.exit_code: result;\n> >> >  }\n> >>\n> >> Yuck.  Let's call the former \"exit_with_status\" (meaning, the\n> >> caller instructed us to do that) and the latter \"has_changes\".\n> >\n> > I like \"exit_with_status\". But has_changes looks confusing\n> > good near return value of run_diff_files_cmd, which \"has\"\n> > nothing. Or do you mean to highlight this \"difference\"?\n>\n> Maybe 'found_changes' would be a better name, then?\n>\n>         if (exit_with_status)\n>                 return !!found_changes;\n>         else\n>                 return result;\n\nNah... has_changes has almost the same meaning,\nand I'm weary regarding making it a changing counter\n(which one can assume it to be when seeing !!found_changes).\nAnd I already sent the patch off.\n"},{"id":"37096","messageId":"7vejnragie.fsf@assigned-by-dhcp.cox.net","threadId":"7240","inReplyTo":"81b0412b0703141012n6fc252ecsd5ed3bc7283cf10f@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-14T17:20:41Z","receivedAt":"2007-03-14T17:20:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" <raa.lkml@gmail.com> writes:\n\n> Updated patch:\n> - implemented exit_with_status+has_changes\n> - fixed test WRT intermediate failures\n\nThanks.\n"},{"id":"37157","messageId":"45F940E0.60302@fs.ei.tum.de","threadId":"7240","inReplyTo":"81b0412b0703140526p454ee85fk74dcd76fb684217c@mail.gmail.com","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Simon 'corecode' Schubert","fromEmail":"corecode@fs.ei.tum.de","sentAt":"2007-03-15T12:49:36Z","receivedAt":"2007-03-15T12:49:36Z","isPatch":true,"sender":{"key":"corecode@fs.ei.tum.de","avatar":"https://gravatar.com/avatar/eff9dbf0cdac0d1e6a6cd7ed0e50763edcb376b493b5253a35ff167918ad79e1?d=mp&s=160"},"body":"Alex Riesen wrote:\n> We have \"PAGER=cat\" in test-lib.sh which just disables pager,\n> so gits tests do not have the problem (maybe it was disabled just\n> because of this?). And if someone has own tests _with_ pager active\n> the one better be aware of what the one doing.\n\nDoes PAGER=cat really disable the pager?  seems to me that it simply passes the data through cat.  That's not disabling it, but it is instead rendering the pager transparent.  Not for the exit code though.\n\ncheers\n  simon\n\n-- \nServe - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /\"\\\nWork - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \\ /\nParty Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \\\nDude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \\\n\n"},{"id":"37159","messageId":"81b0412b0703150656v6fb91c9bge73f64d4dc05a115@mail.gmail.com","threadId":"7240","inReplyTo":"45F940E0.60302@fs.ei.tum.de","subject":"Re: [PATCH] Allow git-diff exit with codes similar to diff(1)","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-15T13:56:16Z","receivedAt":"2007-03-15T13:56:16Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 3/15/07, Simon 'corecode' Schubert <corecode@fs.ei.tum.de> wrote:\n> Alex Riesen wrote:\n> > We have \"PAGER=cat\" in test-lib.sh which just disables pager,\n> > so gits tests do not have the problem (maybe it was disabled just\n> > because of this?). And if someone has own tests _with_ pager active\n> > the one better be aware of what the one doing.\n>\n> Does PAGER=cat really disable the pager?  seems to me that it simply\n> passes the data through cat.\n\nYes:\npager.c:\n\n\telse if (!*pager || !strcmp(pager, \"cat\"))\n\t\treturn;\n"}]}