# [PATCH 4/4] gitweb: Fix broken blob action parameters on blob/commitdiff pages 3 messages from 2013-04-08 to 2013-04-09. Participants: Jürgen Kreileder, Jakub Narębski. Thread: https://gitlist.dev/t/33443 ## Jürgen Kreileder, 2013-04-08 20:10 Subject: [PATCH 4/4] gitweb: Fix broken blob action parameters on blob/commitdiff pages Message-ID: URL: https://gitlist.dev/e/m2vc7wbx76.fsf%40blackdown.de ``` Fix broken blob action parameters on blobdiff and commitdiff pages by explicitly passing variables instead of relying on global ones. (The broken parameters on blob links lead to blob pages which show the blob but with a hash instead of a commit message and have broken blob_plain (404 - Cannot find file) and tree links (404 - Reading tree failed)) Signed-off-by: Jürgen Kreileder --- gitweb/gitweb.perl | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 1309196..995e8fb 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -2258,7 +2258,7 @@ sub format_extended_diff_header_line { # format from-file/to-file diff header sub format_diff_from_to_header { - my ($from_line, $to_line, $diffinfo, $from, $to, @parents) = @_; + my ($from_line, $to_line, $diffinfo, $from, $to, $hash, @parents) = @_; my $line; my $result = ''; @@ -2324,7 +2324,7 @@ sub format_diff_from_to_header { # create note for patch simplified by combined diff sub format_diff_cc_simplified { - my ($diffinfo, @parents) = @_; + my ($diffinfo, $hash, @parents) = @_; my $result = ''; $result .= "
" . @@ -3592,7 +3592,8 @@ sub parse_ls_tree_line { # generates _two_ hashes, references to which are passed as 2 and 3 argument sub parse_from_to_diffinfo { - my ($diffinfo, $from, $to, @parents) = @_; + my ($diffinfo, $from, $to, $hash, @parents) = @_; + my ($hash_parent) = $parents[0]; if ($diffinfo->{'nparents'}) { # combined diff @@ -5312,7 +5313,7 @@ sub git_patchset_body { if ($is_combined) { while ($to_name ne $diffinfo->{'to_file'}) { print "
\n" . - format_diff_cc_simplified($diffinfo, @hash_parents) . + format_diff_cc_simplified($diffinfo, $hash, @hash_parents) . "
\n"; # class="patch" $patch_idx++; @@ -5324,7 +5325,7 @@ sub git_patchset_body { } # modifies %from, %to hashes - parse_from_to_diffinfo($diffinfo, \%from, \%to, @hash_parents); + parse_from_to_diffinfo($diffinfo, \%from, \%to, $hash, @hash_parents); # this is first patch for raw difftree line with $patch_idx index # we index @$difftree array from 0, but number patches from 1 @@ -5367,7 +5368,7 @@ sub git_patchset_body { print format_diff_from_to_header($last_patch_line, $patch_line, $diffinfo, \%from, \%to, - @hash_parents); + $hash, @hash_parents); # the patch itself LINE: @@ -5404,7 +5405,7 @@ sub git_patchset_body { # generate anchor for "patch" links in difftree / whatchanged part print "
\n" . - format_diff_cc_simplified($diffinfo, @hash_parents) . + format_diff_cc_simplified($diffinfo, $hash, @hash_parents) . "
\n"; # class="patch" $patch_number++; -- 1.7.10.4 ``` ## Jakub Narębski, 2013-04-09 15:25 Subject: Re: [PATCH 4/4] gitweb: Fix broken blob action parameters on blob/commitdiff pages Message-ID: <51643307.9090603@gmail.com> URL: https://gitlist.dev/e/51643307.9090603%40gmail.com In-Reply-To: ``` W dniu 08.04.2013 22:10, Jürgen Kreileder pisze: > Fix broken blob action parameters on blobdiff and commitdiff pages by > explicitly passing variables instead of relying on global ones. Do I understand it correctly that those variables (e.g. $hash variable in git_patchset_body in second chunk below, after this change passed as parameter to format_diff_cc_simplified()) can be filled in then, or at least adjusted correctly? > (The broken parameters on blob links lead to blob pages which show the > blob but with a hash instead of a commit message and have broken > blob_plain (404 - Cannot find file) and tree links (404 - Reading tree > failed)) > > Signed-off-by: Jürgen Kreileder I wonder how we missed this. Does this happen always, or in some specific conditions? Anyway, this change is a good change. Internal subroutines should not use global variables. I hope that in the future we would get rid of most global variables... Acked-by: Jakub Narebski [not tested] > # create note for patch simplified by combined diff > sub format_diff_cc_simplified { > - my ($diffinfo, @parents) = @_; > + my ($diffinfo, $hash, @parents) = @_; > my $result = ''; > > $result .= "
" . [...] > @@ -5404,7 +5405,7 @@ sub git_patchset_body { > > # generate anchor for "patch" links in difftree / whatchanged part > print "
\n" . > - format_diff_cc_simplified($diffinfo, @hash_parents) . > + format_diff_cc_simplified($diffinfo, $hash, @hash_parents) . > "
\n"; # class="patch" > > $patch_number++; > -- Jakub Narębski ``` ## Jürgen Kreileder, 2013-04-09 20:17 Subject: Re: [PATCH 4/4] gitweb: Fix broken blob action parameters on blob/commitdiff pages Message-ID: URL: https://gitlist.dev/e/m2d2u3a27u.fsf%40zahir.fritz.box In-Reply-To: <51643307.9090603@gmail.com> ``` Jakub Narębski writes: > W dniu 08.04.2013 22:10, Jürgen Kreileder pisze: > >> Fix broken blob action parameters on blobdiff and commitdiff pages by >> explicitly passing variables instead of relying on global ones. > > Do I understand it correctly that those variables (e.g. $hash variable > in git_patchset_body in second chunk below, after this change passed > as parameter to format_diff_cc_simplified()) can be filled in then, > or at least adjusted correctly? Yep. >> (The broken parameters on blob links lead to blob pages which show the >> blob but with a hash instead of a commit message and have broken >> blob_plain (404 - Cannot find file) and tree links (404 - Reading tree >> failed)) > > I wonder how we missed this. Does this happen always, or in some > specific conditions? Just having a hash in the diff header and missing navigation links for lhs links in diffs happen pretty much all the time: E.g. follow any a/... link on http://repo.or.cz/w/alt-git.git/commitdiff/5bda18c186e455f8e65f976d3bf333ab1f4b5b53 (Btw, on http://repo.or.cz/w/alt-git.git/blobdiff/6466fbbeef67ec398f6f5d5b5da5d6f107a647b2..5bda18c186e455f8e65f976d3bf333ab1f4b5b53:/GIT-VERSION-GEN both the a/ und b/ link are broken. No idea if my patch fixes that, I don't use pathinfo because I've had similar problems with it when I tested it a year ago). Broken a/ and b/ + broken tree + broken blob_plain links are more rare. I see them on https://git.blackdown.de/old.cgi?p=contactalbum.git;a=blobdiff;f=Classes/ContactAlbum.h;h=8f2b6a772ba0a4530e0c441db53ae5bbc0f2e277;hp=03c43ea01a3a88bbce3c73df9bfde64ee7a13717;hb=cc4eaa64c2b399dd9bdbf1f67f6d621aa24df5f8;hpb=43ff97d2c247517f43d83314aaa215e9f61d5d0c;js=1 for example: When following either a/ or b/, all links in "[contactalbum.git] / Classes / ContactAlbum.h" are broken. With my patch I don't see these problems: https://git.blackdown.de/?p=contactalbum.git;a=blobdiff;f=Classes/ContactAlbum.h;h=8f2b6a772ba0a4530e0c441db53ae5bbc0f2e277;hp=03c43ea01a3a88bbce3c73df9bfde64ee7a13717;hb=cc4eaa64c2b399dd9bdbf1f67f6d621aa24df5f8;hpb=43ff97d2c247517f43d83314aaa215e9f61d5d0c;js=1 Juergen ```