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

From: Jakub Narebski <jnareb@gmail.com>
Date: Mar 27, 2007, 23:58 UTC
Message-ID: <200703280158.54929.jnareb@gmail.com>
In-Reply-To: <20070327195627.GA23205@auto.tuwien.ac.at>

On Thu, Mar 27, 2007, Martin Koegler wrote:

Show 14 quoted lines

> 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.

Show 7 quoted lines

>> 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