Re: [PATCH] gitweb: Support comparing blobs (files) with different names

[Jakub Narebski <jnareb@gmail.com>]

On Thu, Mar 27, 2007, Martin Koegler wrote:
> On Tue, Mar 27, 2007 at 01:56:24AM +0100, Jakub Narebski wrote:
>> Martin Koegler wrote:
>>> My idea is, that if I got hb:f and hpb:fp, the user exactly specified
>>> the blobs to be compared. Then I don't want any guessing logic.
>> 
>> I'd rather you reuse the "no hash_parent" code, which also hand-crafts
>> diffinfo. Perhaps removing "git-diff-tree hpb hb -- f" code entirely.
>> Besides, code dealing with "blobdiff" coming from "commit", "commitdiff"
>> and "history" views are tested to work as expected, not so with
>> arbitrary diffs.
> 
> I don't like the whole rename detection code, so I offer to simplify
> git_blobdiff. For all calls to git_blobdiff (except those from git_history),
> I'm sure, that I can assume $file_parent ||= $file_name.

That was the idea. Perhaps I haven't said it clearly, but I wanted to
suggest to remove the whole git-diff-tree code, and use git-diff to
generate diff between blobs.

> If you think, its safe, I can simplify git_blobdiff. I propose
> doing the following way (pseudo-code):

> $file_parent ||= $file_name;
[...]
> $hash=git_get_hash_by_path($hash_base,$file_name);
[...]
> $hash_parent=git_get_hash_by_path($hash_parent_base,$file_parent);
[...]
>  open $fd, "-|", git_cmd(), "diff", '-p', @diff_opts,
>     $hash_parent, $hash
>     or die_error(undef, "Open git-diff failed");
[...]
> Else I will keep a reworked version of my patch.

The trouble with this is that we may lose mode change (symlink to
ordinary file etc.) because we hand-generate %diffinfo.

>> By the way, if you call git_get_hash_by_path (which is expensive, as it
>> calls git command), you can use resulting hash in place of
>> hash_base:filename as an argument to git-diff.
>  
> I must check, if we need to resolve $hash ($hash_parent) by
> git_get_hash_by_path, if we construct it out of $hash_base and
> $file_name. Maybe we can avoid this call.

We can use "$hash_base:$file_name" as second parameter to git-diff etc.,
but I don't think we want to create links with "$hash_base:$file_name"
instead of sha-1 id of a blob as 'h' parameter.

It can be first implementation, thought, and later we can try to use
"index <hash>..<hash> <mode>" lines from extended header to get $hash
and $hash_parent (with exception of pure rename, but then we need only
one invocation of git_get_hash_by_path subroutine).

But I think it is better left for later patch.
-- 
Jakub Narebski
Poland