Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame

[Jakub Narebski <jnareb@gmail.com>]

On Wed, 4 Jun 2008, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> On Tue, 3 Jan 2008, Luben Tuikov wrote:
>>>
>>> The intention was that it shouldn't necessarily be the (strict) parent
>>> of the change (changed segment), since it may or may not have changed
>>> in the strict parent commit.  The intention was that it
>>> "starts"/"opens" the parent commit so that "git" would start from
>>> there and find the actual change/commit where that line/segment has
>>> changed.  And it has worked pretty fine for me when data-mining
>>> (something I do quite often) code evolution.
> 
> Yes, but the current scheme breaks down in another way.  When $full_rev
> added many lines to the file, and you are adding the link to for a line
> near the end of the file and such a line may not exist.  This cannot be
> cheaply done even inside blame itself.

I think the scheme could be fixed by proposed belo git-blame porcelain
format output extension.  Can it be done cheaply?  I don't know,
generating extended info as described below should be cheap if we
have equivalent of textual (patch) diff between commit blamed for
given line, and its parent; actually what we need is more of 'context'
diff than of default 'unified' diff.

> Another breakage is even though $full_rev^ _may_ exist (iow, $full_rev
> might not be the root commit), the file being blamed may not exist there
> (iow $full_rev might have introduced the file).  Instead of running
> "rev-parse $full_rev^", you would at least need to ask "rev-list -1
> $full_rev^ -- $path" or something from the Porcelain layer, but
> unfortunately this is rather expensive.

Doesn't blame know revision graph for history of a given file already?

But even without it (i.e. ony 'parents' header showing true, not
rewritten parents) what we need is some info about pre-image for blamed
line.  We would need line number of the line in pre-image (or NUL
if the page was added in blamed commit), and pre-image filename.

I don't know if it could be done cheaply, and if it could be done
simply; currently git-diff doesn't have "context diff" format output,
and what I though about by pre-image line number requires finding if
a line was added in a commit, or was modified in a commit.  (If it
was removed, it wouldn't be in final image and hence wouldn't be
blamed; if it was moved, it wouldn't be blamed, as blame follows
code movement).

> Because blame already almost knows if the commit the final blame lies on
> has a parent, it would be reasonably cheap to add that "parent or nothing"
> information to its --porcelain (and its --incremental) format if we wanted
> to.

It would be easy to add 'parents' header, perhaps empty if we blame
root commit, or a boundary commit (do we say 'boundary' then?) when
doing revision limited blaming.

>From what you write it wouldn't be easy to add "history of a given
line begins here", or even "history of a given file begins there"...

-- 
Jakub Narebski
Poland