Re: [PATCHv2 1/3] gitweb: Deal with HEAD pointing to unborn branch in "heads" view

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jakub Narebski <jnareb@xxxxxxxxx> writes:

> On Thu, 16 Feb 2012, Junio C Hamano wrote:
>> Jakub Narebski <jnareb@xxxxxxxxx> writes:
>> 
>> > Gitweb has problems if HEAD points to an unborn branch, with no
>> > commits on it yet, but there are other branches present (so it is not
>> > newly initialized repository).
>> 
>> It would be more readable if you rephrase the vague "has problems" with a
>> concrete description of what the problem is.
>
> Sorry about this.
>
> The problem is that gitweb would generate the following warning, writing
> it in web server logs:
>
>   Use of uninitialized value in string eq

When known and easily describable in a short paragraph, let's write both
the cause and the symptom together.

> Should I re-roll this patch with improved commit message?

I was following Peff's 4 step review process, and I was in step #1 (read
the log message---can I understand what this is about without looking at
the patch?), so I haven't reached the diff part of your message ;-)

You added "defined $head_at &&" to protect against the undef comparison
for all uses of $head and I do not see any $head that was left unconverted
by mistake, so the patch looks OK to me (at times I wish gitweb were
written in a language more strict than Perl so that a compiler can catch
such mistakes).

But after trying to write a reroll myself, I have to wonder what would
happen if you have two branches pointing at the same commit as the one at
HEAD.  Why isn't the use of current_head class controlled by comparison
between the name of the ref and the output from "symbolic-ref HEAD"?

-- >8 --
From: Jakub Narebski <jnareb@xxxxxxxxx>
Date: Wed, 15 Feb 2012 16:36:41 +0100
Subject: [PATCH] gitweb: Fix "heads" view when there is no current branch

In a repository whose HEAD points to an unborn branch with no commits,
"heads" view and "summary" view (which shows what is shown in "heads"
view) compared the object names of commits at the tip of branches with the
output from "git rev-parse HEAD", which caused comparison of a string with
undef and resulted in a warning in the server log.

This can happen if non-bare repository (with default 'master' branch)
is updated not via committing but by other means like push to it, or
Gerrit.  It can happen also just after running "git checkout --orphan
<new branch>" but before creating any new commit on this branch.

Rewrite the comparison so that it also works when $head points at nothing;
in such a case, no branch can be "the current branch", add a test for it.

While at it rename local variable $head to $head_at, as it points to
current commit rather than current branch name (HEAD contents).

Reported-by: Rajesh Boyapati
Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 gitweb/gitweb.perl                     |    4 ++--
 t/t9500-gitweb-standalone-no-errors.sh |    9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3fc7380..af154c3 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5633,7 +5633,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);
 
@@ -5642,7 +5642,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 = defined $head_at && $ref{'id'} eq $head_at;
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
 		} else {
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
-- 
1.7.9.1.236.gedc23




--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]