Re: [PATCH 2/2] annotate: Fix bug when parsing merges with differing real and logical parents.

[Ryan Anderson <ryan@michonline.com>]

On Mon, Aug 07, 2006 at 12:16:27PM -0700, Junio C Hamano wrote:
> This does stop it from dying on Jeff King's bug report
> (<20060803203848.GA15121@coredump.intra.peff.net>) to annotate
> builtin-read-tree.c.  However,...
> 
> Comparing output between "annotate" and "blame -c" on the same
> file shows a rather interesting differences.

Yes, annotate is still mis-attributing things to merges that it should
not.  I was kind of hoping this fix would have a side-effect of fixing
it, but I had no logical reason to think it would.   I'm going to try
again at fixing it later this week, maybe the weekend, but I'm starting
to think that maybe blame has won. :)

> 
> (1) "annotate" blames the first line "/*" to d147e501 made on
>     2006-05-23, while "blame" annotates it with 8bc9a0c7 made on
>     2005-04-07.  As far as I remember this line has never been
>     touched by anybody during the life of the file, so the
>     latter sounds more plausible.
> 
> (2) The 13th line to #include "cache-tree.h" is blamed on
>     3f69d405 by "annotate", while annotation by "blame" reads
>     that it came from bad68ec9.
> 
>     3f69d405 is a merge to incorporate the line into "master"
>     branch.  "git show -M 3f69d405 -- builtin-read-tree.c read-tree.c"
>     starts with:
> 
>         diff --cc builtin-read-tree.c
>         index ec40d01,99e7c75..716f792
>         --- a/builtin-read-tree.c
>         +++ b/builtin-read-tree.c
>         @@@ -9,9 -9,9 +9,10 @@@
> 
>           #include "object.h"
>           #include "tree.h"
>         + #include "cache-tree.h"
> 	  #include <sys/time.h>
> 
>     Which means the line is new from the point of view of the
>     "master" branch, but existed in the branch that this commit
>     merges from (namely, jc/cache-tree) already.
> 
>     "git show -M bad68ec9 -- read-tree.c" shows that it is the
>     one that added the #include line to the history.
> 
> (3) The 14th line to #include <sys/time.h> is blamed on d147e501
>     by "annotate" but "blame" says it came from 744633cb.  It is
>     obvious the latter is correct, if you look at "git show" on
>     these commits.  The eye-candy was what made <sys/time.h>
>     necessary (it needed to do the progress-signal stuff).
> 
> In fact, "annotate" attributes too many lines to d147e501 (839
> lines).
> 
> Given that "git show -M d147e501 -- read-tree.c builtin-read-tree.c"
> shows only 2 insertions and 1 deletion, it does not sound
> right.  The commit adds #include "builtin.h" and changes the
> function name "main" to "cmd_read_tree".
> 
> On the other hand, "blame" attributes only 1 line to this
> commit; the line to #include "builtin.h".  The other change this
> commit introduces (main -> cmd_read_tree) is not counted because
> the third parameter to the function changes after this commit by
> a91af794 (which, by the way, both "annotate" and "blame" get
> right), and the line this commit introduces did not survive.
> 
> 
> 

-- 

Ryan Anderson
  sometimes Pug Majere