# [PATCH 2/7] gitweb: Support comparing blobs with different names 7 messages from 2007-04-16 to 2007-05-19. Participants: Martin Koegler, Jakub Narebski, Petr Baudis. Thread: https://gitlist.dev/t/7698 ## Martin Koegler, 2007-04-16 20:18 Subject: [PATCH 2/7] gitweb: Support comparing blobs with different names Message-ID: <20070416201813.GA2592@auto.tuwien.ac.at> URL: https://gitlist.dev/e/20070416201813.GA2592%40auto.tuwien.ac.at In-Reply-To: ``` Currently, blobdiff can only compare blobs with different file names, if no hb/hpb parameters are present. This patch adds support for comparing two blobs specified by any combination of hb/f/h and hpb/fp/hp. Signed-off-by: Martin Koegler --- New version, as I found a bug in the expiration handling code. I unified all blobdiff variants and added support for comparing blobs with different names. If h/hp parameter are missing, I need to generate them with git_get_hash_by_path, as the are needed for the html header, which is generated before parsing the git-diff output. I currently ignore all mode changed, as they are part of the tree. I don't think that displaying a mode change message justifes two call to git-ls-tree for each blob diff (Currently it only calls git-ls-tree for each missing h/hp parameter). gitweb/gitweb.perl | 136 ++++++++++++++++------------------------------------ 1 files changed, 41 insertions(+), 95 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index cbd8d03..790041c 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -3902,109 +3902,54 @@ sub git_blobdiff { my $fd; my @difftree; my %diffinfo; - my $expires; - - # preparing $fd and %diffinfo for git_patchset_body - # new style URI - if (defined $hash_base && defined $hash_parent_base) { - if (defined $file_name) { - # read raw output - open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, - $hash_parent_base, $hash_base, - "--", (defined $file_parent ? $file_parent : ()), $file_name - or die_error(undef, "Open git-diff-tree failed"); - @difftree = map { chomp; $_ } <$fd>; - close $fd - or die_error(undef, "Reading git-diff-tree failed"); - @difftree - or die_error('404 Not Found', "Blob diff not found"); - - } elsif (defined $hash && - $hash =~ /[0-9a-fA-F]{40}/) { - # try to find filename from $hash - - # read filtered raw output - open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, - $hash_parent_base, $hash_base, "--" - or die_error(undef, "Open git-diff-tree failed"); - @difftree = - # ':100644 100644 03b21826... 3b93d5e7... M ls-files.c' - # $hash == to_id - grep { /^:[0-7]{6} [0-7]{6} [0-9a-fA-F]{40} $hash/ } - map { chomp; $_ } <$fd>; - close $fd - or die_error(undef, "Reading git-diff-tree failed"); - @difftree - or die_error('404 Not Found', "Blob diff not found"); - - } else { - die_error('404 Not Found', "Missing one of the blob diff parameters"); - } + my $expires = '+1d'; - if (@difftree > 1) { - die_error('404 Not Found', "Ambiguous blob diff specification"); - } - - %diffinfo = parse_difftree_raw_line($difftree[0]); - $file_parent ||= $diffinfo{'from_file'} || $file_name || $diffinfo{'file'}; - $file_name ||= $diffinfo{'to_file'} || $diffinfo{'file'}; - - $hash_parent ||= $diffinfo{'from_id'}; - $hash ||= $diffinfo{'to_id'}; + $file_parent ||= $file_name; - # non-textual hash id's can be cached - if ($hash_base =~ m/^[0-9a-fA-F]{40}$/ && - $hash_parent_base =~ m/^[0-9a-fA-F]{40}$/) { - $expires = '+1d'; - } + # non-textual hash id's can be cached + if (defined $hash && $hash !~ m/^[0-9a-fA-F]{40}$/) { + $expires = undef; + } elsif (defined $hash_parent && $hash_parent !~ m/^[0-9a-fA-F]{40}$/) { + $expires = undef; + } elsif (defined $hash_base && $hash_base !~ m/^[0-9a-fA-F]{40}$/) { + $expires = undef; + } elsif (defined $hash_parent_base && $hash_parent_base !~ m/^[0-9a-fA-F]{40}$/) { + $expires = undef; + } - # open patch output - open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, - '-p', ($format eq 'html' ? "--full-index" : ()), - $hash_parent_base, $hash_base, - "--", (defined $file_parent ? $file_parent : ()), $file_name - or die_error(undef, "Open git-diff-tree failed"); + # if hash parameter is missing, read it from the commit. + if (defined $hash_base && defined $file_name && !defined $hash) { + $hash = git_get_hash_by_path($hash_base, $file_name); } - # old/legacy style URI - if (!%diffinfo && # if new style URI failed - defined $hash && defined $hash_parent) { - # fake git-diff-tree raw output - $diffinfo{'from_mode'} = $diffinfo{'to_mode'} = "blob"; - $diffinfo{'from_id'} = $hash_parent; - $diffinfo{'to_id'} = $hash; - if (defined $file_name) { - if (defined $file_parent) { - $diffinfo{'status'} = '2'; - $diffinfo{'from_file'} = $file_parent; - $diffinfo{'to_file'} = $file_name; - } else { # assume not renamed - $diffinfo{'status'} = '1'; - $diffinfo{'from_file'} = $file_name; - $diffinfo{'to_file'} = $file_name; - } - } else { # no filename given - $diffinfo{'status'} = '2'; - $diffinfo{'from_file'} = $hash_parent; - $diffinfo{'to_file'} = $hash; - } + if (defined $hash_parent_base && defined $file_parent && !defined $hash_parent) { + $hash_parent = git_get_hash_by_path($hash_parent_base, $file_parent); + } + + if (!defined $hash || ! defined $hash_parent) { + die_error('404 Not Found', "Missing one of the blob diff parameters"); + } - # non-textual hash id's can be cached - if ($hash =~ m/^[0-9a-fA-F]{40}$/ && - $hash_parent =~ m/^[0-9a-fA-F]{40}$/) { - $expires = '+1d'; - } - # open patch output - open $fd, "-|", git_cmd(), "diff", @diff_opts, - '-p', ($format eq 'html' ? "--full-index" : ()), - $hash_parent, $hash, "--" - or die_error(undef, "Open git-diff failed"); - } else { - die_error('404 Not Found', "Missing one of the blob diff parameters") - unless %diffinfo; + # fake git-diff-tree raw output + $diffinfo{'from_mode'} = $diffinfo{'to_mode'} = "blob"; + $diffinfo{'from_id'} = $hash_parent; + $diffinfo{'to_id'} = $hash; + if (defined $file_name) { + $diffinfo{'status'} = '2'; + $diffinfo{'from_file'} = $file_parent; + $diffinfo{'to_file'} = $file_name; + } else { # no filename given + $diffinfo{'status'} = '2'; + $diffinfo{'from_file'} = $hash_parent; + $diffinfo{'to_file'} = $hash; } + # open patch output + open $fd, "-|", git_cmd(), "diff", @diff_opts, + $hash_parent, $hash, "--" + or die_error(undef, "Open git-diff failed"); + # header if ($format eq 'html') { my $formats_nav = @@ -4028,11 +3973,12 @@ sub git_blobdiff { } } elsif ($format eq 'plain') { + my $patch_file_name = $file_name || $hash; print $cgi->header( -type => 'text/plain', -charset => 'utf-8', -expires => $expires, - -content_disposition => 'inline; filename="' . "$file_name" . '.patch"'); + -content_disposition => 'inline; filename="' . "$patch_file_name" . '.patch"'); print "X-Git-Url: " . $cgi->self_url() . "\n\n"; -- 1.5.1.1.85.gf1888 ``` ## Martin Koegler, 2007-04-16 20:20 Subject: [PATCH 4/7] gitweb: Add treediff view Message-ID: <20070416202010.GB2592@auto.tuwien.ac.at> URL: https://gitlist.dev/e/20070416202010.GB2592%40auto.tuwien.ac.at In-Reply-To: <85de402216e82cc0f220df9d27370a33538a232a.1176659094.git.mkoegler@auto.tuwien.ac.at> ``` git_treediff supports comparing different trees. A tree can be specified either as hash or as base hash and filename. --- New version, as I found a bug in the expiration handling code. I removed the parent links for now, as they require more discussion. This patch uses empty fp/f for the root directory and does not propagte f to fp. Patch 6 adds this. gitweb/gitweb.perl | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 116 insertions(+), 0 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 1551d95..c2dc82d 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -450,6 +450,8 @@ my %actions = ( "tag" => \&git_tag, "tags" => \&git_tags, "tree" => \&git_tree, + "treediff" => \&git_treediff, + "treediff_plain" => \&git_treediff_plain, "snapshot" => \&git_snapshot, "object" => \&git_object, # those below don't need $project @@ -4174,6 +4176,120 @@ sub git_commitdiff_plain { git_commitdiff('plain'); } +sub git_treediff { + my $format = shift || 'html'; + my $expires = '+1d'; + + # non-textual hash id's can be cached + if (defined $hash && $hash !~ m/^[0-9a-fA-F]{40}$/) { + $expires = undef; + } elsif (defined $hash_parent && $hash_parent !~ m/^[0-9a-fA-F]{40}$/) { + $expires = undef; + } elsif (defined $hash_base && $hash_base !~ m/^[0-9a-fA-F]{40}$/) { + $expires = undef; + } elsif (defined $hash_parent_base && $hash_parent_base !~ m/^[0-9a-fA-F]{40}$/) { + $expires = undef; + } + + # we need to prepare $formats_nav before any parameter munging + my $formats_nav; + if ($format eq 'html') { + $formats_nav = + $cgi->a({-href => href(action=>"treediff_plain", + hash=>$hash, hash_parent=>$hash_parent, + hash_base=>$hash_base, hash_parent_base=>$hash_parent_base, + file_name=>$file_name, file_parent=>$file_parent)}, + "raw"); + } + + if (!defined $hash) { + if (!defined $hash_base) { + die_error(undef,'tree parameter missing'); + } + $hash = $hash_base; + $hash .= ":".$file_name if (defined $file_name); + } + + if (!defined $hash_parent) { + if (!defined $hash_parent_base) { + die_error(undef,'tree parameter missing'); + } + $hash_parent = $hash_parent_base; + $hash_parent .= ":".$file_parent if (defined $file_parent); + } + + # read treediff + my $fd; + my @difftree; + if ($format eq 'html') { + open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, + "--no-commit-id", "--patch-with-raw", "--full-index", + $hash_parent, $hash, "--" + or die_error(undef, "Open git-diff-tree failed"); + + while (my $line = <$fd>) { + chomp $line; + # empty line ends raw part of diff-tree output + last unless $line; + push @difftree, $line; + } + + } elsif ($format eq 'plain') { + open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, + '-p', $hash_parent, $hash, "--" + or die_error(undef, "Open git-diff-tree failed"); + + } else { + die_error(undef, "Unknown treediff format"); + } + + # write header + if ($format eq 'html') { + git_header_html(undef, $expires); + if (defined $hash_base && (my %co = parse_commit($hash_base))) { + git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav); + git_print_header_div('commit', esc_html($co{'title'}), $hash_base); + } else { + print "

$formats_nav
\n"; + print "
$hash vs $hash_parent
\n"; + } + print "
\n"; + + } elsif ($format eq 'plain') { + my $filename = basename($project) . "-$hash-$hash_parent.patch"; + + print $cgi->header( + -type => 'text/plain', + -charset => 'utf-8', + -expires => $expires, + -content_disposition => 'inline; filename="' . "$filename" . '"'); + + print "X-Git-Url: " . $cgi->self_url() . "\n\n"; + print "---\n\n"; + } + + # write patch + if ($format eq 'html') { + git_difftree_body(\@difftree, $hash_base, $hash_parent_base, $file_name, $file_parent); + print "
\n"; + + git_patchset_body($fd, \@difftree, $hash_base, $hash_parent_base, $file_name, $file_parent); + close $fd; + print "
\n"; # class="page_body" + git_footer_html(); + + } elsif ($format eq 'plain') { + local $/ = undef; + print <$fd>; + close $fd + or print "Reading git-diff-tree failed\n"; + } +} + +sub git_treediff_plain { + git_treediff('plain'); +} + sub git_history { if (!defined $hash_base) { $hash_base = git_get_head_hash($project); -- 1.5.1.1.85.gf1888 ``` ## Jakub Narebski, 2007-04-29 21:35 Subject: Re: [PATCH 2/7] gitweb: Support comparing blobs with different names Message-ID: <200704292335.50102.jnareb@gmail.com> URL: https://gitlist.dev/e/200704292335.50102.jnareb%40gmail.com In-Reply-To: <20070416201813.GA2592@auto.tuwien.ac.at> ``` On Monday, 16 April 2007, Martin Koegler wrote: > Currently, blobdiff can only compare blobs with different file > names, if no hb/hpb parameters are present. > Not true. Since commit 5ae917acdf40444945271e5e014cda37e202504e "gitweb: Support comparing blobs (files) with different names" git_blobdiff ('blobdiff' view) suports comparing arbitrary blobs (also with different file names) when no hp/hpb parameters are present (although there would be no mode change information), but supports comparing blobs with different file names in presence of hp/hpb parameters, provided that it is proper rename diff. Since this commit you can select rename diff which is part of 'commitdiff' view as a 'blobdiff' view, correctly. > This patch adds support for comparing two blobs specified by any > combination of hb/f/h and hpb/fp/hp. > But it used to lose mode change information. This info should be I think in commit message. Because your work on including mode changes for git-diff with : extended SHA-1 syntax (thanks a lot!) I guess that you would want to rewrite this patch. > Signed-off-by: Martin Koegler > --- > New version, as I found a bug in the expiration handling code. > > I unified all blobdiff variants and added support for comparing blobs > with different names. > > If h/hp parameter are missing, I need to generate them with > git_get_hash_by_path, as the are needed for the html header, which is > generated before parsing the git-diff output. git_get_hash_by_path uses git-ls-tree but it does not catch all the info; perhaps git_get_info_by_path would be called for here. > I currently ignore all mode changes, as they are part of the tree. I > don't think that displaying a mode change message justifes two call to > git-ls-tree for each blob diff (Currently it only calls git-ls-tree > for each missing h/hp parameter). That is not a problem now, thanks to your patches... although it would be nice to have subroutine which would get the difftree information from extended git diff header (if it is possible). > + my $expires = '+1d'; > + # non-textual hash id's can be cached > + if (defined $hash && $hash !~ m/^[0-9a-fA-F]{40}$/) { > + $expires = undef; > + } elsif (defined $hash_parent && $hash_parent !~ m/^[0-9a-fA-F]{40}$/) { > + $expires = undef; > + } elsif (defined $hash_base && $hash_base !~ m/^[0-9a-fA-F]{40}$/) { > + $expires = undef; > + } elsif (defined $hash_parent_base && $hash_parent_base !~ m/^[0-9a-fA-F]{40}$/) { > + $expires = undef; > + } Usually gitweb _changes_ $expires to '+1d' when appropriate, instead of defaulting to '+1d' and undefining it. But this might be the matter of style consistency, and personal preferences: one complicated boolean expression or set of if ... elsif ... conditionals. The idea is that both TO and FROM are to be immutable; this means either $hash_parent (or $hash_parent_base) being full SHA-1 and having filename defined, or if appropriate "parent" hash is not present (because "parent" hash has precedence over blob hash) it means that $hash (or $hash_parent) being full SHA-1. [...] > + if (defined $hash_parent_base && defined $file_parent && !defined $hash_parent) { > + $hash_parent = git_get_hash_by_path($hash_parent_base, $file_parent); > + } [...] > + # open patch output > + open $fd, "-|", git_cmd(), "diff", @diff_opts, > + $hash_parent, $hash, "--" > + or die_error(undef, "Open git-diff failed"); You would most probably use now "$hash_base:$file_name" instead of $hash if $hash_base is defined, i.e. defined $hash_base ? "$hash_base:$file_name" : $hash and similarly for $hash_parent parameter now that : form respects mode changes information. -- Jakub Narebski ShadeHawk on #git Poland ``` ## Martin Koegler, 2007-04-30 05:27 Subject: Re: [PATCH 2/7] gitweb: Support comparing blobs with different names Message-ID: <20070430052702.GA12744@auto.tuwien.ac.at> URL: https://gitlist.dev/e/20070430052702.GA12744%40auto.tuwien.ac.at In-Reply-To: <200704292335.50102.jnareb@gmail.com> ``` On Sun, Apr 29, 2007 at 11:35:49PM +0200, Jakub Narebski wrote: > On Monday, 16 April 2007, Martin Koegler wrote: > > > Currently, blobdiff can only compare blobs with different file > > names, if no hb/hpb parameters are present. > > I've already posted a new version of this patch: http://www.spinics.net/lists/git/msg28812.html Can you please look at it. > > Signed-off-by: Martin Koegler > > --- > > New version, as I found a bug in the expiration handling code. > > > > I unified all blobdiff variants and added support for comparing blobs > > with different names. > > > > If h/hp parameter are missing, I need to generate them with > > git_get_hash_by_path, as the are needed for the html header, which is > > generated before parsing the git-diff output. > > git_get_hash_by_path uses git-ls-tree but it does not catch all the info; > perhaps git_get_info_by_path would be called for here. I now only need the hash of the blob, if not passed as parameter, to generate a correct header. > [...] > > + if (defined $hash_parent_base && defined $file_parent && !defined $hash_parent) { > > + $hash_parent = git_get_hash_by_path($hash_parent_base, $file_parent); > > + } > [...] > > + # open patch output > > + open $fd, "-|", git_cmd(), "diff", @diff_opts, > > + $hash_parent, $hash, "--" > > + or die_error(undef, "Open git-diff failed"); > > You would most probably use now "$hash_base:$file_name" instead of $hash > if $hash_base is defined, i.e. > > defined $hash_base ? "$hash_base:$file_name" : $hash > > and similarly for $hash_parent parameter now that : form > respects mode changes information. Addressed in new patch. mfg Martin Kögler ``` ## Petr Baudis, 2007-05-18 08:49 Subject: Re: [PATCH 5/7] gitweb: Prototyp for selecting diffs in JavaScript Message-ID: <20070518084935.GV4489@pasky.or.cz> URL: https://gitlist.dev/e/20070518084935.GV4489%40pasky.or.cz In-Reply-To: <481946c2e3cff09ed4a623b1b20b9889666aedb0.1176659095.git.mkoegler@auto.tuwien.ac.at> ``` Hi, On Sun, Apr 15, 2007 at 10:46:08PM CEST, Martin Koegler wrote: > --- > This patch is only to test the other patches. I'm working on reimplementing it in perl. I think this is something that I'd quite like to testdrive at repo.or.cz, but I'm admittelly somewhat lost in all the patches, resends, new versions, something got applied, etc. - if it's not too big a hassle for you, would you please care to resend the latest series? Thanks, -- Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ Ever try. Ever fail. No matter. // Try again. Fail again. Fail better. -- Samuel Beckett ``` ## Martin Koegler, 2007-05-19 07:57 Subject: Re: [PATCH 5/7] gitweb: Prototyp for selecting diffs in JavaScript Message-ID: <20070519075701.GA30933@auto.tuwien.ac.at> URL: https://gitlist.dev/e/20070519075701.GA30933%40auto.tuwien.ac.at In-Reply-To: <20070518084935.GV4489@pasky.or.cz> ``` On Fri, May 18, 2007 at 10:49:35AM +0200, Petr Baudis wrote: > On Sun, Apr 15, 2007 at 10:46:08PM CEST, Martin Koegler wrote: > > --- > > This patch is only to test the other patches. I'm working on reimplementing it in perl. > > I think this is something that I'd quite like to testdrive at > repo.or.cz, but I'm admittelly somewhat lost in all the patches, > resends, new versions, something got applied, etc. - if it's not too big > a hassle for you, would you please care to resend the latest series? Againgst what version/repository do you want the patch? For the new version, I intend to move the JavaScript file into the head tag of the html page. I could integrate your incremental blame link rewrite as well. If I'm doing this, we could remove the blamelink class hack as well as the fixBlameLinks JavaScript function. mfg Martin Kögler ``` ## Petr Baudis, 2007-05-19 08:27 Subject: Re: [PATCH 5/7] gitweb: Prototyp for selecting diffs in JavaScript Message-ID: <20070519082744.GC4489@pasky.or.cz> URL: https://gitlist.dev/e/20070519082744.GC4489%40pasky.or.cz In-Reply-To: <20070519075701.GA30933@auto.tuwien.ac.at> ``` On Sat, May 19, 2007 at 09:57:01AM CEST, Martin Koegler wrote: > Againgst what version/repository do you want the patch? Whatever is comfortable for you :-) - next would be ideal for me, but can be something else as well. > For the new version, I intend to move the JavaScript file into the > head tag of the html page. I could integrate your incremental blame > link rewrite as well. If I'm doing this, we could remove the blamelink > class hack as well as the fixBlameLinks JavaScript function. It's not clear at all yet whether incremental blame is viable idea because I suspect Firefox might be just too horribly slow at working with DOM for it to make sense for any files of non-trivial size. So I wouldn't rely on this going in just yet. :-( -- Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ Ever try. Ever fail. No matter. // Try again. Fail again. Fail better. -- Samuel Beckett ```