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

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

[Jakub Narebski]

Fix the bug that caused "blobdiff" view called with new style URI
for a rename with change diff to be show as new (added) file diff.

New style URI for "blobdiff" for rename means with $hash_base ('hb') and
$hash_parent_base ('hpb') paramaters denoting tree-ish (usually commit)
of a blobs being compared, together with both $file_name ('f') and
$file_parent ('fp') parameters.

It is done by adding $file_parent ('fp') to the path limiter, meaning
that diff command becomes:

	git diff-tree [options] hpb hb -- fp f

Other option would be finding hash of a blob using git_get_hash_by_path
subroutine and comparing blobs using git-diff, or using extended SHA-1
syntax and compare blobs using git-diff:

	git diff [options] hpb:fp hp:f

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio, could you apply this? It looks like it was lost in the noise.

This patch is least invasive, and it solves bug with "blobdiff" for
rename with change. It is not a complete solution for comparing two
arbitrary blobs (which takes longer than expected, and it looks like
it would be a while till it is ready), but it covers AFAICT all the
URL (all the situations) for "blobdiff" currently _generated_ by gitweb.

>From the lack of bugreports concerning this bug in "blobdiff" it looks
like "blobdiff" view is not often used. By the way: "commitdiff" view
works fine in the presence of renames, and renames with modification.

This version has slightly changed commit message.

Bug was noticed by Martin Koegler <mkoegler@auto.tuwien.ac.at>


 gitweb/gitweb.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5214050..c79bfeb 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3885,7 +3885,7 @@ sub git_blobdiff {
 			# read raw output
 			open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
 				$hash_parent_base, $hash_base,
-				"--", $file_name
+				"--", (defined $file_parent ? $file_parent : ()), $file_name
 				or die_error(undef, "Open git-diff-tree failed");
 			@difftree = map { chomp; $_ } <$fd>;
 			close $fd
@@ -3935,7 +3935,7 @@ sub git_blobdiff {
 		# open patch output
 		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
 			'-p', $hash_parent_base, $hash_base,
-			"--", $file_name
+			"--", (defined $file_parent ? $file_parent : ()), $file_name
 			or die_error(undef, "Open git-diff-tree failed");
 	}
 
-- 
1.5.0.5

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

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Fix the bug that caused "blobdiff" view called with new style URI
> for a rename with change diff to be show as new (added) file diff.
>
> New style URI for "blobdiff" for rename means with $hash_base ('hb') and
> $hash_parent_base ('hpb') paramaters denoting tree-ish (usually commit)
> of a blobs being compared, together with both $file_name ('f') and
> $file_parent ('fp') parameters.
>
> It is done by adding $file_parent ('fp') to the path limiter, meaning
> that diff command becomes:
>
> 	git diff-tree [options] hpb hb -- fp f
>
> Other option would be finding hash of a blob using git_get_hash_by_path
> subroutine and comparing blobs using git-diff, or using extended SHA-1
> syntax and compare blobs using git-diff:
>
> 	git diff [options] hpb:fp hp:f
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> Junio, could you apply this? It looks like it was lost in the noise.

I was waiting for dust to settle.  Ack's from people involved in
the discussion would be nice.

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

[Jakub Narebski]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Fix the bug that caused "blobdiff" view called with new style URI
>> for a rename with change diff to be show as new (added) file diff.
[...]
>> ---
>> Junio, could you apply this? It looks like it was lost in the noise.
> 
> I was waiting for dust to settle.  Ack's from people involved in
> the discussion would be nice.

I can understand that.

This patch is _a_ solution: it fixes "blobdiff" view for all blobdiff 
URIs gitweb generates now. But it is not _the_ solution, as it doesn't 
work with Martin work on adding "(base | diff)" links to allow to view 
diff between arbitrary commits, trees or blobs; which includes 
"blobdiff" view of arbitrary blobs.

So if/when Martin finishes his series of patches, this part would be 
rewritten. But it is some time in the future...
-- 
Jakub Narebski
Poland