Re: Inconsistent results of git blame --porcelain when detecting copies from other files

[Jeff King <peff@peff.net>]

On Mon, Feb 20, 2017 at 01:30:29PM -0800, Junio C Hamano wrote:

> "Sokolov, Konstantin" <konstantin.sokolov.ext@siemens.com> writes:
> 
> > However, when using --porcelain DirectoryReader.java is reported as the origin of lines 502-504:
> > ...
> > This is not only inconsistent with the other outputs but the output is also inconsistent in itself because lines 496 -498 do not even exist in a previous version of DirectoryReader.java.
> 
> Hmph, this sounds vaguely familiar with
> 
>     http://public-inbox.org/git/20170106042051.nwjiuyyp7ljhs4sr@sigill.intra.peff.net
> 
> which is part of Git 2.12-rc0

Yeah, I had the same thought while reading Konstantin's report.

I'm not sure Git is wrong, though. I think it's just the way the
porcelain output works.

Here's a minimal reproduction; the interesting thing is when a commit is
mentioned twice (as happens on lines 1 and 5 here):

  git init repo
  cd repo
  
  # use long lines to make sure we trigger content-movement detection
  for i in $(seq 1 5); do
  	echo this is really long line number $i
  done >file
  git add file
  git commit -m initial
  
  sed 's/1/one/; s/5/five/' <file >renamed
  git rm file
  git add renamed
  git commit -m 'rename and use english'
  
  git blame renamed
  git blame --line-porcelain renamed
  git blame --porcelain renamed

The first blame output looks something like this:

  bab03701 renamed ... line number 1
  ^dda1349 file    ... line number 2
  ^dda1349 file    ... line number 3
  ^dda1349 file    ... line number 4
  bab03701 renamed ... line number 5

so we can see it's the same case. The --line-porcelain similarly matches
the commits and filenames.

But the --porcelain output is:

  bab037010dcabaf0509db27bf232d25659b180fa 1 1 1
  ...
  filename renamed
          this is really long line number one
  dda1349d41da859f4c37e018dbed714ba6c1aa18 2 2 3
  ...
  filename file
          this is really long line number 2
  dda1349d41da859f4c37e018dbed714ba6c1aa18 3 3
          this is really long line number 3
  dda1349d41da859f4c37e018dbed714ba6c1aa18 4 4
          this is really long line number 4
  bab037010dcabaf0509db27bf232d25659b180fa 5 5 1
          this is really long line number five

You might be tempted to say that the fifth line comes from "filename
file", because that was the last "filename" entry we saw. But that's
_not_ how the porcelain output works. That "filename" entry was
associated with dda1349, but the line comes from bab0370 here.

The simplest way (IMHO) to parse --porcelain output is:

  - maintain a mapping of commit sha1s to the commit's details

  - whenever you see a "<sha1> <line_nr> <orig_nr> [<size-of-hunk>]"
    line, any key-value fields which follow impact _only_ that sha1, and
    you should update the details for that map entry

  - when you see the actual tab-indented line content, you have gotten
    all of the key-value updates for that sha1. You can now safely do
    what you like with the line entry.

Another way, if you don't want to update your mapping, is to actually
pay attention to the size-of-hunk headers. In this case the middle three
lines come in their own hunk (which you can see from the "2 2 3" header
on the second line). The "filename" field we get applies to that hunk,
but once we switch to a different one, the filename field needs to be
looked up in the commit mapping.

But it's definitely not correct to blindly apply one "filename" field to
subsequent lines in other hunks.

And yes, I do think this is probably more complex than it needs to be.
I didn't write it. And I don't think it is worth the backwards
compatibility headache of trying to change it now. It's possible this
could be better documented (I didn't look at the documentation to write
that explanation; I happened to puzzle it out for somebody else
recently who had a similar case. That's what led to the bug-fix in the
message you linked).

-Peff