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.

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

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

[...]

quoted

And here is the patch:
-->8 ------------>8 ---
From: Jakub Narebski <redacted>
Subject: [PATCH] gitweb: Deal with HEAD pointing to unborn branch in
  "heads"  view

[...]

quoted

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 ?

Ah, right.

Then, I restarted gerrit server to take changes.
Now the error log of gerrit shows:

[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/git

My own clone of git, with my work, can be found at:

       git://repo.or.cz/git/jnareb-git.git
       https://github.com/jnareb/git

quoted

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