threads / patch / 29172

patchGitweb: Avoid warnings when a repo does not have a valid HEAD

Subject: [PATCH] Gitweb: Avoid warnings when a repo does not have a valid HEAD

## tl;dr

4 messages between Dec 15, 2011 and Dec 16, 2011. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Joe Ratterman· Dec 15, 2011, 20:58 UTC · lore

It is possible that the HEAD reference does not point to an existing branch. When viewing such a repository in gitweb, a message like this one was sent to the error log:

  gitweb.cgi: Use of uninitialized value in string eq at /usr/src/git/gitweb/gitweb.cgi line 5115.

Signed-off-by: Joe Ratterman <jratt0@gmail.com>
---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to gitweb/gitweb.perl +1 −1

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4f0c3bd..5af06d6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5440,7 +5440,7 @@ sub git_heads_body {
 	for (my $i = $from; $i <= $to; $i++) {
 		my $entry = $headlist->[$i];
 		my %ref = %$entry;
-		my $curr = $ref{'id'} eq $head;
+		my $curr = $head ? ($ref{'id'} eq $head) : 0;
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
 		} else {

-- 
1.7.7.1

Junio C Hamano· Dec 15, 2011, 22:02 UTC · re: Joe Ratterman · lore

Re: [PATCH] Gitweb: Avoid warnings when a repo does not have a valid HEAD

Joe Ratterman <jratt0@gmail.com> writes:

Show 21 quoted lines

> It is possible that the HEAD reference does not point to an existing
> branch.  When viewing such a repository in gitweb, a message like this
> one was sent to the error log:
>
>   gitweb.cgi: Use of uninitialized value in string eq at /usr/src/git/gitweb/gitweb.cgi line 5115.
>
> Signed-off-by: Joe Ratterman <jratt0@gmail.com>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4f0c3bd..5af06d6 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5440,7 +5440,7 @@ sub git_heads_body {
>  	for (my $i = $from; $i <= $to; $i++) {
>  		my $entry = $headlist->[$i];
>  		my %ref = %$entry;
> -		my $curr = $ref{'id'} eq $head;
> +		my $curr = $head ? ($ref{'id'} eq $head) : 0;

Makes one wonder if $head could be '0', but I presume this is about the case where (!defined $head) holds true. Also makes one wonder if a similar issue exists on the $ref{"id"} side. I suspect that won't be true unless you have a very screwed-up repository, but in that case a repository with a HEAD that points at an unborn branch _and_ have other refs that do point at existing commit is already screwed-up, so if we want to be extremely pedantic then perhaps ...

    my $curr = ((defined $head && exists $ref{"id"} && defined $ref{"id"})
		? ($ref{"id"} eq $head)
                : 0);

Junio C Hamano· Dec 15, 2011, 22:28 UTC · re: Junio C Hamano · lore

Re: [PATCH] Gitweb: Avoid warnings when a repo does not have a valid HEAD

Junio C Hamano <gitster@pobox.com> writes:

Show 16 quoted lines

> Joe Ratterman <jratt0@gmail.com> writes:
>
>> It is possible that the HEAD reference does not point to an existing
>> branch.  When viewing such a repository in gitweb, a message like this
>> one was sent to the error log:
>>
>>   gitweb.cgi: Use of uninitialized value in string eq at /usr/src/git/gitweb/gitweb.cgi line 5115.
>>
> ..., but in that case a repository with
> a HEAD that points at an unborn branch _and_ have other refs that do point
> at existing commit is already screwed-up, so if we want to be extremely
> pedantic then perhaps ...
>
>     my $curr = ((defined $head && exists $ref{"id"} && defined $ref{"id"})
> 		? ($ref{"id"} eq $head)
>                 : 0);

Just in case, I was not suggesting to update the patch to look like the above by "if we want to be extremely pedantic".

After all, that error message in the log may be a good thing that notifies the site administrators about a suspicious repository so that it can be fixed (even though it was not a designed "feature" but something that happens to work).

Jakub Narebski· Dec 16, 2011, 14:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] Gitweb: Avoid warnings when a repo does not have a valid HEAD

On Thu, 15 Dec 2011, Junio C Hamano wrote:

Show 15 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>> Joe Ratterman <jratt0@gmail.com> writes:
>>
>>> It is possible that the HEAD reference does not point to an existing
>>> branch.  When viewing such a repository in gitweb, a message like this
>>> one was sent to the error log:
>>>
>>>   gitweb.cgi: Use of uninitialized value in string eq at /usr/src/git/gitweb/gitweb.cgi line 5115.
>>>
>> ..., but in that case a repository with
>> a HEAD that points at an unborn branch _and_ have other refs that do point
>> at existing commit is already screwed-up, so if we want to be extremely
>> pedantic then perhaps ...
>>
>>     my $curr = ((defined $head && exists $ref{"id"} && defined $ref{"id"})

       my $curr = ((defined $head && defined $ref{"id"})

is enough, because '!exists $hash{key}' implies '!defined $hash{key}' (though reverse is not true).

Show 10 quoted lines

>> 		? ($ref{"id"} eq $head)
>>                 : 0);
> 
> Just in case, I was not suggesting to update the patch to look like the
> above by "if we want to be extremely pedantic".
> 
> After all, that error message in the log may be a good thing that notifies
> the site administrators about a suspicious repository so that it can be
> fixed (even though it was not a designed "feature" but something that
> happens to work).

Well, but for that those error messages should describe error; the above is quite hard to translate to

  warning: HEAD points to an unborn branch in repository foo.git

BTW. we should probably warn about this situation more clear, perhaps by showing warning instead of empty place where subject of current commit is usually shown.

Anyway git marks up branches (sic!) that _point to_ the same commit as HEAD because we calculate it anyway, at least for 'summary' view.

Better solution would be to examine HEAD (either using Perl, or using "git symbolic-ref") to get _name_ of current branch. I'd rather avoid yet another command call.

-- 
Jakub Narebski
Poland