Re: [PATCH] gitweb: Deal with HEAD pointing to unborn branch in "heads" view
On Mon, 6 Feb 2012, rajesh boyapati wrote:
>
> Thanks for your work.
I'm sorry I was able to find a fix only for the part of issue.
Show 31 quoted lines
>>>>>>>>> [2012-01-25 18:50:35,658] ERROR
>>>>>>>>> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: [Wed Jan 25
>>>>>>>>> 18:50:35 2012] gitweb.cgi: Use of uninitialized value $commit_id
>>>>>>>>> in open at /usr/lib/cgi-bin/gitweb.cgi line 2817.
>>>>>>>
>>>>>>> sub parse_commits {
>>>>>>> my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
>>>>>>> my @cos;
>>>>>>>
>>>>>>> $maxcount ||= 1;
>>>>>>> $skip ||= 0;
>>>>>>>
>>>>>>> local $/ = "\0";
>>>>>>>
>>>>>>> open my $fd, "-|", git_cmd(), "rev-list",
>>>>>>> "--header",
>>>>>>> @args,
>>>>>>> ("--max-count=" . $maxcount),
>>>>>>> ("--skip=" . $skip),
>>>>>>> @extra_options,
>>>>>>> $commit_id,
>>>>>>> "--",
>>>>>>> ($filename ? ($filename) : ())
>>>>>>> or die_error(500, "Open git-rev-list failed");
>>
>> But I was not able to fix this, at least not currently. I wrote a failing
>> test case for "commit" and similar views on unborn HEAD... but they fail
>> _without_ error message like the one quoted.
>>
>> I'd have to go slower route of examining gitweb code in how it deals with
>> "invalid" HEAD (i.e. HEAD not pointing to commit, being on unborn branch).Show 5 quoted lines
>> And here is the patch:
>> -->8 ------------>8 ---
>> From: Jakub Narebski <jnareb@gmail.com>
>> Subject: [PATCH] gitweb: Deal with HEAD pointing to unborn branch in
>> "heads" view
Show 15 quoted lines
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 9cf7e71..1f0ec12 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -5570,7 +5570,7 @@ sub git_tags_body {
>>
>> sub git_heads_body {
>> # uses global variable $project
>> - my ($headlist, $head, $from, $to, $extra) = @_;
>> + my ($headlist, $head_at, $from, $to, $extra) = @_;
>> $from = 0 unless defined $from;
>> $to = $#{$headlist} if (!defined $to || $#{$headlist} < $to);
>>
>
> I didn't see a file called "gitweb.perl" in /usr/share/gitweb/The file "gitweb.perl", or rather "gitweb/gitweb.perl" is the name of the script in git.git repository. From it "make gitweb" would generate "gitweb.cgi" file...
> I applied this patch to file "index.cgi" in /usr/share/gitweb/index.cgi at
> line 4711.
>
> I applied this patch to file "index.cgi" in /usr/share/gitweb/index.cgi at
> line 4720.
...and I guess Gerrit build process generates "index.cgi" from that.
> Had I applied the patch to the correct file "index.cgi", which is a link to
> file "gitweb.cgi" in /usr/lib/cgi-bin/gitweb.cgi ?
> Then, I restarted gerrit server to take changes.
> Now the error log of gerrit shows:
Show 9 quoted lines
> [2012-02-06 11:21:46,726] ERROR
> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision
> 'HEAD'
> [2012-02-06 11:21:49,167] ERROR
> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: [Mon Feb 6 11:21:49
> 2012] gitweb.cgi: Use of uninitialized value $commit_id in open at
> /usr/lib/cgi-bin/gitweb.cgi line 2817.
> [2012-02-06 11:21:49,169] ERROR
> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision ''
[the same errors repeated few times]
> <<<<<<<<<<<<<<<<
> Previously, there is a error showing at line 4720. Now, with this patch,
> that error has gone.
As I said I was able to find a fix only for part of the issue. Unfortunately I was not able to reproduce this error in this form. Note that the error location doesn't help much, because it is more interesting for find which callers of parse_commits() pass undefined $commit_id.
I can try to harden parse_commits() against bogus parameters; maybe
this would help.
> I tried to upgrade gitweb with the command "sudo apt-get install gitweb",
> but, it didn't find any upgrade.
> Am I doing in a right way?
There is no new version of gitweb yet; it haven't even been accepted by Junio Hamano, maintainer of git of which gitweb is part, into git repository (I might have to resend this patch for better visibility).
> Is there any place like "Github" (where we can place git projects) for
> gitweb ?
Gitweb is for quite some time developed within git repository. From it the 'gitweb' package is created.
Clones of canonical, official git repository can be found in a few places:
git://git.kernel.org/pub/scm/git/git.git
git://repo.or.cz/alt-git.git
https://code.google.com/p/git-core/
https://github.com/git/gitMy own clone of git, with my work, can be found at:
git://repo.or.cz/git/jnareb-git.git
https://github.com/jnareb/gitShow 25 quoted lines
>> diff --git a/t/t9500-gitweb-standalone-no-errors.sh
>> b/t/t9500-gitweb-standalone-no-errors.sh
>> index 0f771c6..81246a6 100755
>> --- a/t/t9500-gitweb-standalone-no-errors.sh
>> +++ b/t/t9500-gitweb-standalone-no-errors.sh
>> @@ -739,4 +739,13 @@ test_expect_success \
>> 'echo "\$projects_list_group_categories = 1;">>gitweb_config.perl
>> &&
>> gitweb_run'
>>
>> +# ----------------------------------------------------------------------
>> +# unborn branches
>> +
>> +test_expect_success \
>> + 'unborn HEAD: "summary" page (with "heads" subview)' \
>> + 'git checkout orphan_branch || git checkout --orphan orphan_branch
>> &&
>> + test_when_finished "git checkout master" &&
>> + gitweb_run "p=.git;a=summary"'
>> +
>> test_done
>>
>
> I didn't find a file where to apply this patch.
> Is this file to test your patch for you?
Yes, this is to test that my patch fixes the issue correctly, and to ensure that further changes don't re-break it. It is not usually installed with git or gitweb, so don't worry about it.
--
Jakub Narebski
Poland
