{"thread":{"id":"5304","subject":"[Possible bug] diff-tree --stat info does not count copies","startedAt":"2006-08-17T09:19:06Z","lastAt":"2006-08-18T04:09:13Z","messageCount":4,"participants":["Marco Costalba","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"25461","messageId":"e5bfff550608170219q12fcb34ewf93a195eabe0b94a@mail.gmail.com","threadId":"5304","inReplyTo":null,"subject":"[Possible bug] diff-tree --stat info does not count copies","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-08-17T09:19:06Z","receivedAt":"2006-08-17T09:19:06Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"While testing qgit with the new rename/copy support I found this\n(possible) bug playing on git tree.\n\n$ git-diff-tree -r --stat 6973dca\n6973dcaee76ef7b7bfcabd2f26e76205aae07858\n Makefile | 2\n diff-files.c | 212 +----\n diff-lib.c | 1862 ++---------------------------------------\n diff.c | 1795 ++++++++++++++++++++++++++++++++++++++++\n diff.h | 7\n t/t1001-read-tree-m-2way.sh | 2\n t/t1002-read-tree-m-u-2way.sh | 2\n 7 files changed, 1929 insertions(+), 1953 deletions(-)\n\n$ git-diff-tree -r --stat -C 6973dca\n6973dcaee76ef7b7bfcabd2f26e76205aae07858\n Makefile | 2\n diff-files.c | 212 +----\n diff-lib.c | 1862 ++---------------------------------------\n diff-lib.c => diff.c | 0\n diff.h | 7\n t/t1001-read-tree-m-2way.sh | 2\n t/t1002-read-tree-m-u-2way.sh | 2\n 7 files changed, 134 insertions(+), 1953 deletions(-)\n\nIMHO the bug is\n\n \"diff-lib.c => diff.c | 0\"\n\ninstead of\n\n \"diff-lib.c => diff.c | 1795\"\n\nbecause, after the patch applied, in the repository we have\n1953-1929=24 lines of code more, not 1953-134= 1819 less.\n\nThanks\nMarco\n"},{"id":"25472","messageId":"7vveorsnt3.fsf@assigned-by-dhcp.cox.net","threadId":"5304","inReplyTo":"e5bfff550608170219q12fcb34ewf93a195eabe0b94a@mail.gmail.com","subject":"Re: [Possible bug] diff-tree --stat info does not count copies","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-17T09:40:08Z","receivedAt":"2006-08-17T09:40:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> While testing qgit with the new rename/copy support I found this\n> (possible) bug playing on git tree.\n>\n> $ git-diff-tree -r --stat 6973dca\n> 6973dcaee76ef7b7bfcabd2f26e76205aae07858\n> Makefile | 2\n> diff-files.c | 212 +----\n> diff-lib.c | 1862 ++---------------------------------------\n> diff.c | 1795 ++++++++++++++++++++++++++++++++++++++++\n> diff.h | 7\n> t/t1001-read-tree-m-2way.sh | 2\n> t/t1002-read-tree-m-u-2way.sh | 2\n> 7 files changed, 1929 insertions(+), 1953 deletions(-)\n>\n> $ git-diff-tree -r --stat -C 6973dca\n> 6973dcaee76ef7b7bfcabd2f26e76205aae07858\n> Makefile | 2\n> diff-files.c | 212 +----\n> diff-lib.c | 1862 ++---------------------------------------\n> diff-lib.c => diff.c | 0\n> diff.h | 7\n> t/t1001-read-tree-m-2way.sh | 2\n> t/t1002-read-tree-m-u-2way.sh | 2\n> 7 files changed, 134 insertions(+), 1953 deletions(-)\n>\n> IMHO the bug is\n>\n> \"diff-lib.c => diff.c | 0\"\n>\n> instead of\n>\n> \"diff-lib.c => diff.c | 1795\"\n>\n> because, after the patch applied, in the repository we have\n> 1953-1929=24 lines of code more, not 1953-134= 1819 less.\n\nInteresting. That's really a matter of taste and interpretation.\n\nIf it were a straight rename without changing a single line,\nthen would you say 1795 lines were removed (from the LHS file)\nand 1795 lines were added (to the RHS file)?\n\nI personally find that output would be useless and would prefer\nit to say \"I renamed file A to file B. Content-wise, there were\nN lines added and M lines removed, compared to the straight\nrename case, by the way\".\n\nAnd that is what the current output does. I do not see why it\nshould be different in the case of a copy instead of a rename.\n"},{"id":"25479","messageId":"e5bfff550608170354i45104458h3b7e2ec8c668fb9c@mail.gmail.com","threadId":"5304","inReplyTo":"7vveorsnt3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [Possible bug] diff-tree --stat info does not count copies","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-08-17T10:54:15Z","receivedAt":"2006-08-17T10:54:15Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 8/17/06, Junio C Hamano wrote:\n> \"Marco Costalba\" writes:\n>\n> > While testing qgit with the new rename/copy support I found this\n> > (possible) bug playing on git tree.\n> >\n> > $ git-diff-tree -r --stat 6973dca\n> > 6973dcaee76ef7b7bfcabd2f26e76205aae07858\n> > Makefile | 2\n> > diff-files.c | 212 +----\n> > diff-lib.c | 1862 ++---------------------------------------\n> > diff.c | 1795 ++++++++++++++++++++++++++++++++++++++++\n> > diff.h | 7\n> > t/t1001-read-tree-m-2way.sh | 2\n> > t/t1002-read-tree-m-u-2way.sh | 2\n> > 7 files changed, 1929 insertions(+), 1953 deletions(-)\n> >\n> > $ git-diff-tree -r --stat -C 6973dca\n> > 6973dcaee76ef7b7bfcabd2f26e76205aae07858\n> > Makefile | 2\n> > diff-files.c | 212 +----\n> > diff-lib.c | 1862 ++---------------------------------------\n> > diff-lib.c => diff.c | 0\n> > diff.h | 7\n> > t/t1001-read-tree-m-2way.sh | 2\n> > t/t1002-read-tree-m-u-2way.sh | 2\n> > 7 files changed, 134 insertions(+), 1953 deletions(-)\n> >\n> > IMHO the bug is\n> >\n> > \"diff-lib.c => diff.c | 0\"\n> >\n> > instead of\n> >\n> > \"diff-lib.c => diff.c | 1795\"\n> >\n> > because, after the patch applied, in the repository we have\n> > 1953-1929=24 lines of code more, not 1953-134= 1819 less.\n>\n> Interesting. That's really a matter of taste and interpretation.\n>\n> If it were a straight rename without changing a single line,\n> then would you say 1795 lines were removed (from the LHS file)\n> and 1795 lines were added (to the RHS file)?\n>\n> I personally find that output would be useless and would prefer\n> it to say \"I renamed file A to file B. Content-wise, there were\n> N lines added and M lines removed, compared to the straight\n> rename case, by the way\".\n>\n> And that is what the current output does. I do not see why it\n> should be different in the case of a copy instead of a rename.\n>\n\nBecause after a copy you have 2 files, not still one. And the 'after\ncopied' file cannot be the original, but a new file, because the\noriginal is still alive.\n\nPerhaps I can explain better myself taking in account the _couple_ of\nfiles involved in both cases:\n\nWithout -C we have\n\ndiff-lib.c | 1862 ++---------------------------------------\ndiff.c | 1795 ++++++++++++++++++++++++++++++++++++++++\n\nWith -C option is\n\n diff-lib.c | 1862 ++---------------------------------------\n diff-lib.c => diff.c | 0\n\nIf it was a rename we had something like:\n\ndiff-lib.c | 1795 -----------------------------------------\ndiff.c | 1795 ++++++++++++++++++++++++++++++++++++++++\n\nand, with -C\n\ndiff-lib.c => diff.c | 0\n\nand _this_ is correct. But with copy diff-lib.c => diff.c should not\nstay at zero lines changed because diff.c is not the same of\ndiff-lib.c, but it's a _new_ file created with the same content of\ndiff-lib.c and _then_ the original and only diff-lib.c file is further\nmodified on his own (in our case changing 1862 lines).\n\nPlease tell me where I get wrong.\n\nThanks\nMarco\n"},{"id":"25519","messageId":"7v8xlmelcm.fsf@assigned-by-dhcp.cox.net","threadId":"5304","inReplyTo":"e5bfff550608170354i45104458h3b7e2ec8c668fb9c@mail.gmail.com","subject":"Re: [Possible bug] diff-tree --stat info does not count copies","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-18T04:09:13Z","receivedAt":"2006-08-18T04:09:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On 8/17/06, Junio C Hamano wrote:\n>\n>> Interesting. That's really a matter of taste and interpretation.\n>>...\n>\n> If it was a rename we had something like:\n>\n> diff-lib.c | 1795 -----------------------------------------\n> diff.c | 1795 ++++++++++++++++++++++++++++++++++++++++\n>\n> and, with -C\n>\n> diff-lib.c => diff.c | 0\n>\n> and _this_ is correct. But with copy diff-lib.c => diff.c should not\n> stay at zero lines changed because diff.c is not the same of\n> diff-lib.c, but it's a _new_ file created with the same content of\n> diff-lib.c and _then_ the original and only diff-lib.c file is further\n> modified on his own (in our case changing 1862 lines).\n>\n> Please tell me where I get wrong.\n\nThat's why I said this is a matter of taste and interpretation.\nOur differences lie in what we expect from these numbers. I do\nnot think your interpretation is wrong. It is just different\nfrom mine, and I happen to think my interpretation is more\nuseful for my purposes.\n\nThe numbers currently shown give \"how big a patch do I have to\ngo through if I were to review this change in a patch form,\nassuming I am reasonably familiar with the current, pre-image,\ncode?\" So straight copy does not count -- if the patch tells me\nthat new file diff.c has the same contents as old diff-lib.c,\nthen I know what the resulting diff.c would contain without\nlooking at the patch text to judge what its implications are\n(including things like 'if file A.c is moved to sub/B.c, I know\nA.c includes \"C.h\" and it needs to be adjusted to include\n\"../C.h\"). In \"copy plus edit\" case, the change shown in the\ndiff text plus the fact that the file was copied from something\nI know makes the patch part the only thing that needs to be\nreviewed. The same logic applies to rename with or without\nediting.\n\nWhat you expect seems to be different. If I understand you\ncorrectly, you are asking for the number of lines that need to\nbe touched (inserted and removed) in order to make the original\ninto the patched, when the source tree is taken as a whole. I\ndo not think it is wrong to want to know that number, which is\npretty much the same number the command gives without -M/-C\n[*1*].\n\nTo make this topic even more interesting, we can compare these\nthree commands:\n\n $ paths='diff-lib.c diff.c'\n $ git diff-tree -r --stat --summary 6973dca -- $paths\n $ git diff-tree -r --stat --summary -C 6973dca -- $paths\n $ git diff-tree -r --stat --summary -B -C 6973dca -- $paths\n\nThe last one says:\n\n6973dcaee76ef7b7bfcabd2f26e76205aae07858\n diff-lib.c | 1928 +++-----------------------------------------------\n diff-lib.c => diff.c | 0 \n 2 files changed, 133 insertions(+), 1795 deletions(-)\n rewrite diff-lib.c (99%)\n rename diff-lib.c => diff.c (100%)\n\nwhich, personally, I think tells the story closest to what\nreally happened for this change. What was known to be\ndiff-lib.c was renamed to diff.c without any change, while\ndiff-lib.c was replaced by completely new contents, losing all\n1795 lines that were there and acquiring 133 brand new lines.\n\nIn this case, I do not _need_ to look at the 1795 lines that\nwere lost from diff-lib.c to review the new implementation of\ndiff-lib.c, assuming that I am familiar with the pre-image code\n(so what I said earlier about the number of lines I have to\nreview is not quite true, but is close enough).\n\n[Footnote]\n\n*1* But if that is the number you are after, then you should\ncount rename and copy the same way by saying rename removes N\nlines and adds N lines (that happen to be the same) elsewhere,\nso you would not say 0 is the right answer.\n"}]}