{"thread":{"id":"5183","subject":"Strange output of git-diff-tree","startedAt":"2006-08-09T13:24:54Z","lastAt":"2006-08-09T20:17:43Z","messageCount":6,"participants":["Jakub Narebski","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"25029","messageId":"ebcnml$btf$1@sea.gmane.org","threadId":"5183","inReplyTo":null,"subject":"Strange output of git-diff-tree","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-08-09T13:24:54Z","receivedAt":"2006-08-09T13:24:54Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"First (noticed by matled) is that for git-diff-tree with single tree\nas an argument it outputs fist commit-id of commit given at input. \nIt is not mentioned in documentation and I think totally unnecessary:\n\n1038:jnareb@roke:~/git> git diff-tree --abbrev origin\nd5dc6a76d49367cddc015e01d2e9aa22e64d7e28\n:040000 040000 44fb36d... 1c26294... M Documentation\n\n\nSecond, for some combination of options for it returns \"...\" instead of\n0{40} for file creation. It seems that the culprit is \"--find-copies-harder\"\noption:\n\n1043:jnareb@roke:~/git> git rev-list --full-history next | \\\n git diff-tree --find-copies-harder -B -C -M -r --full-history --stdin \\\n -- gitweb/gitweb.perl gitweb/gitweb.cgi gitweb.cgi | less | grep \" A\"\n:000000 100755 ... 017664b8f440f5ec151cf5653245ee02aefd3db2 A gitweb.cgi\n\n1047:jnareb@roke:~/git> git rev-list --full-history next | \\\n git diff-tree -B -C -M -r --stdin \\\n -- gitweb/gitweb.perl gitweb/gitweb.cgi gitweb.cgi | less | grep \" A\"\n:000000 100755 0000000000000000000000000000000000000000 017664b8f440f5ec151cf5653245ee02aefd3db2 A gitweb.cgi\n\n\nThird, while it detects that gitweb/gitweb.perl was renamed from \ngitweb/gitweb.cgi:\n [...] R100 gitweb/gitweb.cgi gitweb/gitweb.perl\nit does not notice that gitweb/gitweb.cgi was gitweb.cgi in \n1130ef362fc8d9c3422c23f5d5a833e93d3f5c13.\n\n\nAll those for git version 1.4.1.1\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"25038","messageId":"7vfyg54vjb.fsf@assigned-by-dhcp.cox.net","threadId":"5183","inReplyTo":"ebcnml$btf$1@sea.gmane.org","subject":"Re: Strange output of git-diff-tree","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-09T18:24:24Z","receivedAt":"2006-08-09T18:24:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> First (noticed by matled) is that for git-diff-tree with single tree\n> as an argument it outputs fist commit-id of commit given at input. \n> It is not mentioned in documentation and I think totally unnecessary:\n>\n> 1038:jnareb@roke:~/git> git diff-tree --abbrev origin\n> d5dc6a76d49367cddc015e01d2e9aa22e64d7e28\n> :040000 040000 44fb36d... 1c26294... M Documentation\n\nWorking as specified as in the original version. See\ncore-tutorial and look for \"git-diff-tree -p HEAD\".\n\n> Second, for some combination of options for it returns \"...\" instead of\n> 0{40} for file creation.\n\nWell spotted. The minimum reproduction recipe is:\n\n $ git diff-tree --find-copies-harder 2ad9331\n\nWill look into it.\n\n> Third, while it detects that gitweb/gitweb.perl was renamed from \n> gitweb/gitweb.cgi:\n> [...] R100 gitweb/gitweb.cgi gitweb/gitweb.perl\n> it does not notice that gitweb/gitweb.cgi was gitweb.cgi in \n> 1130ef362fc8d9c3422c23f5d5a833e93d3f5c13.\n\nDepends on how you ask.\n\ngit diff-tree --abbrev --pretty -m -M -r --diff-filter=R 0a8f4f00\n\nor\n\ngit diff-tree --abbrev --pretty -c -M -r 0a8f4f00\n"},{"id":"25039","messageId":"7vbqqt4vct.fsf@assigned-by-dhcp.cox.net","threadId":"5183","inReplyTo":"7vfyg54vjb.fsf@assigned-by-dhcp.cox.net","subject":"Re: Strange output of git-diff-tree","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-09T18:28:18Z","receivedAt":"2006-08-09T18:28:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jakub Narebski writes:\n>\n>> First (noticed by matled) is that for git-diff-tree with single tree\n>> as an argument it outputs fist commit-id of commit given at input. \n>> It is not mentioned in documentation and I think totally unnecessary:\n>>\n>> 1038:jnareb@roke:~/git> git diff-tree --abbrev origin\n>> d5dc6a76d49367cddc015e01d2e9aa22e64d7e28\n>> :040000 040000 44fb36d... 1c26294... M Documentation\n>\n> Working as specified as in the original version. See\n> core-tutorial and look for \"git-diff-tree -p HEAD\".\n\nMy mistake -- if you are talking about the first line that shows\nthe commit object name, I am not sure if this was from the\nbeginning or a recent slip-up. Will need to look into it.\n\nI thought you were talking about 1-argument diff-tree, sorry.\n"},{"id":"25041","messageId":"7vwt9h3e35.fsf@assigned-by-dhcp.cox.net","threadId":"5183","inReplyTo":"7vbqqt4vct.fsf@assigned-by-dhcp.cox.net","subject":"Re: Strange output of git-diff-tree","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-09T19:26:38Z","receivedAt":"2006-08-09T19:26:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> Jakub Narebski writes:\n>>\n>>> First (noticed by matled) is that for git-diff-tree with single tree\n>>> as an argument it outputs fist commit-id of commit given at input. \n>>> It is not mentioned in documentation and I think totally unnecessary:\n>>>\n>>> 1038:jnareb@roke:~/git> git diff-tree --abbrev origin\n>>> d5dc6a76d49367cddc015e01d2e9aa22e64d7e28\n>>> :040000 040000 44fb36d... 1c26294... M Documentation\n>>\n>> Working as specified as in the original version. See\n>> core-tutorial and look for \"git-diff-tree -p HEAD\".\n>\n> My mistake -- if you are talking about the first line that shows\n> the commit object name, I am not sure if this was from the\n> beginning or a recent slip-up. Will need to look into it.\n>\n> I thought you were talking about 1-argument diff-tree, sorry.\n\nTurns out that git-diff-tree from 1.0.0 behaves this way. Since\n7384889 (May 18, 2005), one-tree form of diff-tree showed the\n\"header\" line that has the commit object name at the top, and\nsince 1809266 (Jun 23, 2005) we have exactly one commit object\nname there; before that we used to say \"commitA (from commitB)\".\n\nI agree that showing the single argument that is given on the\ncommand line as the first output line might seem redundant, but\nit has been done this way for a long time (eternity in git\ntimescale), so let's not risk breaking people's scripts by\nchanging it.\n"},{"id":"25043","messageId":"7vodut3dwn.fsf_-_@assigned-by-dhcp.cox.net","threadId":"5183","inReplyTo":"7vwt9h3e35.fsf@assigned-by-dhcp.cox.net","subject":"(Unsolicited hint) straightening out stray \"git bisect\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-09T19:30:32Z","receivedAt":"2006-08-09T19:30:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> Jakub Narebski writes:\n>>>\n>>>> First (noticed by matled) is that for git-diff-tree with single tree\n>>>> as an argument it outputs fist commit-id of commit given at input. \n>>>> It is not mentioned in documentation and I think totally unnecessary:\n>>>>\n>>>> 1038:jnareb@roke:~/git> git diff-tree --abbrev origin\n>>>> d5dc6a76d49367cddc015e01d2e9aa22e64d7e28\n>>>> :040000 040000 44fb36d... 1c26294... M Documentation\n>>>\n> Turns out that git-diff-tree from 1.0.0 behaves this way. Since\n> 7384889 (May 18, 2005), one-tree form of diff-tree showed the\n> \"header\" line that has the commit object name at the top, and\n> since 1809266 (Jun 23, 2005) we have exactly one commit object\n> name there; before that we used to say \"commitA (from commitB)\".\n\nBTW, I got curious and run \"git bisect\" to see when the \" (from\ncommitB)\" was dropped. This turned out to be quite an\ninteresting one. You would start from these:\n\n $ git bisect start\n $ git bisect good 7384889 ;# this said \"( from commitB)\"\n $ git bisect bad v1.0.0 ;# we know this did not\n\nand keep building and testing \"git-diff-tree\", to mark the ones\nthat do not say \"( from commitB)\" as bad and the ones that do as\ngood.\n\nAt some point, it crosses the \"coolest merge ever\" and suggests\nc2f6a02 to be checked, which is on the original \"gitk\" branch.\nThere is no way to build git-diff-tree in that revision ;-).\n\nAt this point, \"visualize\" helps.\n\n\t$ git bisect visualize\n\nshows (pardon the ASCII art):\n\n *--bisect/bad git-rev-list: add option to list all objects\n * git-rev-parse: re-organize and be more careful\n .\n * Do a cross-project merge of Paul Mackerras' gitk visualizer \n |* Try to assign colors so crossing lines have different colors\n |* Account for indentation of the checkin commments by git-rev-list\n |.\n |*-bisect Show heads as well as tags\n |.\n |* Add initial version of gitk to the CVS repository\n o--bisect/good-fae22ac [PATCH] git-apply: tests for --stat and --summary.\n\nNow, we know that gitk did not have diff-tree, so up to the\ncommit \"Try to assign colors...\" are irrelevant to the \"bug\" we\nare hunting. We say:\n\n\t$ git bisect good 6c20ff3\n\n(where 6c20ff3 is the commit \"Try to assign colors...\") to mark\neverything leading to that commit (ones on the line on the right\nhand side in the above ASCII graph) are good. After that bisect\ncontinues normally.\n"},{"id":"25048","messageId":"7v7j1h3bq0.fsf@assigned-by-dhcp.cox.net","threadId":"5183","inReplyTo":"7vfyg54vjb.fsf@assigned-by-dhcp.cox.net","subject":"Re: Strange output of git-diff-tree","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-09T20:17:43Z","receivedAt":"2006-08-09T20:17:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Well spotted. The minimum reproduction recipe is:\n>\n> $ git diff-tree --find-copies-harder 2ad9331\n>\n> Will look into it.\n\nThis is because the original command line has -C and -M in this\norder, and the minimum reproduction does not have either.\n\ndiff_setup_done() \"silently\" complains about it but the caller\nwas not taking notice.\n\nWhat happens is that the check at the top of diff_setup_done()\nfails, and does not set up options->abbrev to 40 upon seeing\nabbrev is set to zero. This later causes find_unique_abbrev()\nto be called with len=0, which returns an empty string, because\nit is not expecing to get something like that.\n\nIncidentally, there is another funny breakage related to this.\n\n\t$ git diff-tree --find-copies-harder v1.4.1\n\ncomplains quite a lot about \"feeding unmodified\" file pairs.\nThis is because \"harder\" is given without -C.\n\nSo there are three things to fix:\n\n (1) callers of diff_setup_done() that do not check and die\n should do so like others.\n\n (2) find_unique_abbrev() should prepare being called with\n len=0.\n\n (3) make --find-copies-harder imply -C.\n\nStrictly speaking, the third is an incompatible change, but it\nmakes something that used to be an invalid/undefined operation\nto a valid one, so it would not be so bad.\n\n*1*\n\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 1075855..dd9886c 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -253,7 +253,8 @@ int cmd_diff(int argc, const char **argv\n \targc = setup_revisions(argc, argv, &rev, NULL);\n \tif (!rev.diffopt.output_format) {\n \t\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n-\t\tdiff_setup_done(&rev.diffopt);\n+\t\tif (diff_setup_done(&rev.diffopt) < 0)\n+\t\t\tdie(\"diff_setup_done failed\");\n \t}\n \n \t/* Do we have --cached and not have a pending object, then\ndiff --git a/revision.c b/revision.c\nindex a58257a..5a91d06 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -936,7 +936,8 @@ int setup_revisions(int argc, const char\n \t\t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n \t}\n \trevs->diffopt.abbrev = revs->abbrev;\n-\tdiff_setup_done(&revs->diffopt);\n+\tif (diff_setup_done(&revs->diffopt) < 0)\n+\t\tdie(\"diff_setup_done failed\");\n \n \treturn left;\n }\n\n*2*\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 5fe8e5d..c5a05fa 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -193,7 +193,7 @@ const char *find_unique_abbrev(const uns\n \n \tis_null = !memcmp(sha1, null_sha1, 20);\n \tmemcpy(hex, sha1_to_hex(sha1), 40);\n-\tif (len == 40)\n+\tif (len == 40 || !len)\n \t\treturn hex;\n \twhile (len < 40) {\n \t\tunsigned char sha1_ret[20];\n\n*3*\n\ndiff --git a/diff.c b/diff.c\nindex 895c137..02a409d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1515,9 +1515,10 @@ void diff_setup(struct diff_options *opt\n \n int diff_setup_done(struct diff_options *options)\n {\n-\tif ((options->find_copies_harder &&\n-\t options->detect_rename != DIFF_DETECT_COPY) ||\n-\t (0 <= options->rename_limit && !options->detect_rename))\n+\tif (options->find_copies_harder)\n+\t\toptions->detect_rename = DIFF_DETECT_COPY;\n+\n+\tif ((0 <= options->rename_limit && !options->detect_rename)\n \t\treturn -1;\n \n \tif (options->output_format & (DIFF_FORMAT_NAME |\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 5fe8e5d..c5a05fa 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -193,7 +193,7 @@ const char *find_unique_abbrev(const uns\n \n \tis_null = !memcmp(sha1, null_sha1, 20);\n \tmemcpy(hex, sha1_to_hex(sha1), 40);\n-\tif (len == 40)\n+\tif (len == 40 || !len)\n \t\treturn hex;\n \twhile (len < 40) {\n \t\tunsigned char sha1_ret[20];\n"}]}