[Possible bug] diff-tree --stat info does not count copies

While testing qgit with the new rename/copy support I found this (possible) bug playing on git tree.

$ git-diff-tree -r --stat 6973dca
6973dcaee76ef7b7bfcabd2f26e76205aae07858
  Makefile                      |    2
 diff-files.c                  |  212 +----
 diff-lib.c                    | 1862 ++---------------------------------------
 diff.c                        | 1795 ++++++++++++++++++++++++++++++++++++++++
 diff.h                        |    7
 t/t1001-read-tree-m-2way.sh   |    2
 t/t1002-read-tree-m-u-2way.sh |    2
 7 files changed, 1929 insertions(+), 1953 deletions(-)

$ git-diff-tree -r --stat -C 6973dca
6973dcaee76ef7b7bfcabd2f26e76205aae07858
  Makefile                      |    2
 diff-files.c                  |  212 +----
 diff-lib.c                    | 1862 ++---------------------------------------
 diff-lib.c => diff.c          |    0
 diff.h                        |    7
 t/t1001-read-tree-m-2way.sh   |    2
 t/t1002-read-tree-m-u-2way.sh |    2
 7 files changed, 134 insertions(+), 1953 deletions(-)

IMHO the bug is

   "diff-lib.c => diff.c          |    0"

instead of

   "diff-lib.c => diff.c          |    1795"

because, after the patch applied, in the repository we have 1953-1929=24 lines of code more, not 1953-134= 1819 less.

Thanks Marco

"Marco Costalba" <mcostalba@gmail.com> writes:

Interesting.  That's really a matter of taste and interpretation.

If it were a straight rename without changing a single line, then would you say 1795 lines were removed (from the LHS file) and 1795 lines were added (to the RHS file)?

I personally find that output would be useless and would prefer it to say "I renamed file A to file B. Content-wise, there were N lines added and M lines removed, compared to the straight rename case, by the way".

And that is what the current output does. I do not see why it should be different in the case of a copy instead of a rename.

On 8/17/06, Junio C Hamano <junkio@cox.net> wrote:

Because after a copy you have 2 files, not still one. And the 'after copied' file cannot be the original, but a new file, because the original is still alive.

Perhaps I can explain better myself taking in account the _couple_ of files involved in both cases:

Without -C we have

diff-lib.c | 1862 ++--------------------------------------- diff.c | 1795 ++++++++++++++++++++++++++++++++++++++++

With -C option is

 diff-lib.c                    | 1862 ++---------------------------------------
 diff-lib.c => diff.c          |    0

If it was a rename we had something like:

diff-lib.c | 1795 ----------------------------------------- diff.c | 1795 ++++++++++++++++++++++++++++++++++++++++

and, with -C

diff-lib.c => diff.c          |    0

and _this_ is correct. But with copy diff-lib.c => diff.c should not stay at zero lines changed because diff.c is not the same of diff-lib.c, but it's a _new_ file created with the same content of diff-lib.c and _then_ the original and only diff-lib.c file is further modified on his own (in our case changing 1862 lines).

Please tell me where I get wrong.

Thanks Marco

"Marco Costalba" <mcostalba@gmail.com> writes:

That's why I said this is a matter of taste and interpretation. Our differences lie in what we expect from these numbers. I do not think your interpretation is wrong. It is just different from mine, and I happen to think my interpretation is more useful for my purposes.

The numbers currently shown give "how big a patch do I have to go through if I were to review this change in a patch form, assuming I am reasonably familiar with the current, pre-image, code?" So straight copy does not count -- if the patch tells me that new file diff.c has the same contents as old diff-lib.c, then I know what the resulting diff.c would contain without looking at the patch text to judge what its implications are (including things like 'if file A.c is moved to sub/B.c, I know A.c includes "C.h" and it needs to be adjusted to include "../C.h"). In "copy plus edit" case, the change shown in the diff text plus the fact that the file was copied from something I know makes the patch part the only thing that needs to be reviewed. The same logic applies to rename with or without editing.

What you expect seems to be different. If I understand you correctly, you are asking for the number of lines that need to be touched (inserted and removed) in order to make the original into the patched, when the source tree is taken as a whole. I do not think it is wrong to want to know that number, which is pretty much the same number the command gives without -M/-C [*1*].

To make this topic even more interesting, we can compare these three commands:

    $ paths='diff-lib.c diff.c'
    $ git diff-tree -r --stat --summary 6973dca -- $paths
    $ git diff-tree -r --stat --summary -C 6973dca -- $paths
    $ git diff-tree -r --stat --summary -B -C 6973dca -- $paths

The last one says:

6973dcaee76ef7b7bfcabd2f26e76205aae07858
 diff-lib.c           | 1928 +++-----------------------------------------------
 diff-lib.c => diff.c |    0 
 2 files changed, 133 insertions(+), 1795 deletions(-)
 rewrite diff-lib.c (99%)
 rename diff-lib.c => diff.c (100%)

which, personally, I think tells the story closest to what really happened for this change. What was known to be diff-lib.c was renamed to diff.c without any change, while diff-lib.c was replaced by completely new contents, losing all 1795 lines that were there and acquiring 133 brand new lines.

In this case, I do not _need_ to look at the 1795 lines that were lost from diff-lib.c to review the new implementation of diff-lib.c, assuming that I am familiar with the pre-image code (so what I said earlier about the number of lines I have to review is not quite true, but is close enough).

[Footnote]

*1* But if that is the number you are after, then you should count rename and copy the same way by saying rename removes N lines and adds N lines (that happen to be the same) elsewhere, so you would not say 0 is the right answer.