[PATCH 0/3 (resend)] gitweb: Miscelanous fixes

[PATCH 0/3 (resend)] gitweb: Miscelanous fixes

[Jakub Narebski]

There were a few gitweb patches which were send (and seemed to
be accepted) before 1.5.4-rc0 (and before feature freeze request),
but are not in 'master'; it is not even in 'offcuts' (what is this
branch for by the way?).

All those patches are simple bugfixes (well, perhaps exept last one,
but even it can be treated as bugfix). All passes the gitweb test:
t9500-gitweb-standalone-no-errors (not that it says much).

Patches are independent conceptually, and should be textually
independent. Apply to 'master'

Table of contents:
 [PATCH] gitweb: Make config_to_multi return [] instead of [undef]
 [PATCH] gitweb: disambiguate heads and tags withs the same name
 [PATCH] gitweb: Teach "a=blob" action to be more lenient about
         blob/file mime type

Diffstat:
 gitweb/gitweb.perl                     |   38 ++++++++++++++++---------------
 t/t9500-gitweb-standalone-no-errors.sh |   25 +++++++++++++++++++++
 2 files changed, 45 insertions(+), 18 deletions(-)


P.S. I could not check if gitweb shows correct output as my Apache
has very mysteriously stopped working and refused to restart. It might
have been cause by an incident when runaway program filled whole disk
space with log, leaving no space on device... but it might not.

# /etc/rc.d/init.d/httpd status
httpd is stopped
# /etc/rc.d/init.d/httpd configtest 
Syntax OK
# /etc/rc.d/init.d/httpd start     
Starting httpd:                                            [FAILED]
# cat /var/log/httpd/error_log
Configuration Failed
$ rpm -q httpd
httpd-2.0.54-10.3

-- 
Jakub Narębski
Poland

[PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Jakub Narebski]

From: Petr Baudis <pasky@suse.cz>
Date: Sat, 8 Dec 2007 12:30:59 +0100
Subject: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

This is important for the list of clone urls, where if there are
no per-repository clone URL configured, the default base URLs
are never used for URL construction without this patch.

Add tests for different ways of setting project URLs, just in case.
Note that those tests in current form wouldn't detect breakage fixed
by this patch, as it only checks for errors and not for expected
output.

Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Originally by Petr Baudis, changed to use defined($val). I have also
added parentheses for better readibility.

I have added tests _then_ I have realized that in current form they
cannot detect regression corrected by this patch. So if you want, you
can not apply changes to test (and remove paragraph about test from
commit message).

The fact that patch was not applied might be cause by the lack of Ack
from pasky.

 gitweb/gitweb.perl                     |    2 +-
 t/t9500-gitweb-standalone-no-errors.sh |   25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 24b3158..a746a85 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1511,7 +1511,7 @@ sub config_to_int {
 sub config_to_multi {
        my $val = shift;
 
-       return ref($val) ? $val : [ $val ];
+       return ref($val) ? $val : (defined($val) ? [ $val ] : []);
 }
 
 sub git_get_project_config {
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 35fff3d..2d3d4e8 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -558,6 +558,31 @@ test_expect_success \
 test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
+# testing config_to_multi / cloneurl
+
+test_expect_success \
+       'URL: no project URLs, no base URL' \
+       'gitweb_run "p=.git;a=summary"'
+test_debug 'cat gitweb.log'
+
+test_expect_success \
+       'URL: project URLs via gitweb.url' \
+       'git config --add gitweb.url git://example.com/git/trash.git &&
+        git config --add gitweb.url http://example.com/git/trash.git &&
+        gitweb_run "p=.git;a=summary"'
+test_debug 'cat gitweb.log'
+
+cat >.git/cloneurl <<\EOF
+git://example.com/git/trash.git
+http://example.com/git/trash.git
+EOF
+
+test_expect_success \
+       'URL: project URLs via cloneurl file' \
+       'gitweb_run "p=.git;a=summary"'
+test_debug 'cat gitweb.log'
+
+# ----------------------------------------------------------------------
 # gitweb config and repo config
 
 cat >>gitweb_config.perl <<EOF
-- 
1.5.3.7

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Petr Baudis]

On Sat, Dec 15, 2007 at 03:36:32PM +0100, Jakub Narebski wrote:
> From: Petr Baudis <pasky@suse.cz>
> Date: Sat, 8 Dec 2007 12:30:59 +0100
> Subject: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]
> 
> This is important for the list of clone urls, where if there are
> no per-repository clone URL configured, the default base URLs
> are never used for URL construction without this patch.
> 
> Add tests for different ways of setting project URLs, just in case.
> Note that those tests in current form wouldn't detect breakage fixed
> by this patch, as it only checks for errors and not for expected
> output.
> 
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

Acked-by: Petr Baudis <pasky@suse.cz>

Thank you all for catching the defined($val). :-)

-- 
				Petr "Pasky" Baudis
We don't know who it was that discovered water, but we're pretty sure
that it wasn't a fish.		-- Marshall McLuhan

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Jakub Narebski]

On Sat, 15 Dec 2007, Jakub Narebski wrote:

> This is important for the list of clone urls, where if there are
> no per-repository clone URL configured, the default base URLs
> are never used for URL construction without this patch.
> 
> Add tests for different ways of setting project URLs, just in case.
> Note that those tests in current form wouldn't detect breakage fixed
> by this patch, as it only checks for errors and not for expected
> output.

The patch applied to master bc8b95ae4a4b21753e84bbfd28cbcbf1b3f6e0a8
does have above paragraph in commit message, but DOES NOT have the
tests itself.
 
> I have added tests _then_ I have realized that in current form they
> cannot detect regression corrected by this patch. So if you want, you
> can not apply changes to test (and remove paragraph about test from
> commit message).

See above...

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 24b3158..a746a85 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1511,7 +1511,7 @@ sub config_to_int {
>  sub config_to_multi {
>         my $val = shift;
>  
> -       return ref($val) ? $val : [ $val ];
> +       return ref($val) ? $val : (defined($val) ? [ $val ] : []);
>  }

And somehow (I don't know how[*1*]) patch got whitespace corrupted,
and now the line is indented with spaces, instead of with tab.

[*1*] Other patches are fine.
-- 
Jakub Narebski
Poland

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Junio C Hamano]

Please send updates if something is not right.  Thanks.

[PATCH] gitweb: disambiguate heads and tags withs the same name

[Jakub Narebski]

Avoid wrong disambiguation that would link logs/trees of tags and
heads which share the same name to the same page, leading to
a disambiguation that would prefer the tag, thus making it impossible
to access the corresponding head log and tree without hacking the url
by hand.

It does it by using full refname (with 'refs/heads/' or 'refs/tags/'
prefix) instead of shortened one in the URLs in 'heads' and 'tags'
tables.  This makes URLs (and refs) provided by gitweb unambiguous.

Signed-off-by: Guillaume Seguin <guillaume@segu.in>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Based on the patch by Guillaume Seguin, which solved the same problem
in slightly different way, by re-adding 'refs/heads/' or 'refs/tags/'.
I think this way is slightly better.

Note that there is companion RFC patch (which is not a part of this
mini-series) which added check if the refname is ambiguous in git_tag:
  "gitweb: Try harder in parse_tag; perhaps it was given ambiguous name"

 gitweb/gitweb.perl |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a746a85..448dca7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2233,6 +2233,7 @@ sub git_get_heads_list {
 		my ($hash, $name, $title) = split(' ', $refinfo, 3);
 		my ($committer, $epoch, $tz) =
 			($committerinfo =~ /^(.*) ([0-9]+) (.*)$/);
+		$ref_item{'fullname'}  = $name;
 		$name =~ s!^refs/heads/!!;
 
 		$ref_item{'name'}  = $name;
@@ -2270,6 +2271,7 @@ sub git_get_tags_list {
 		my ($id, $type, $name, $refid, $reftype, $title) = split(' ', $refinfo, 6);
 		my ($creator, $epoch, $tz) =
 			($creatorinfo =~ /^(.*) ([0-9]+) (.*)$/);
+		$ref_item{'fullname'} = $name;
 		$name =~ s!^refs/tags/!!;
 
 		$ref_item{'type'} = $type;
@@ -3690,8 +3692,8 @@ sub git_tags_body {
 		      "<td class=\"link\">" . " | " .
 		      $cgi->a({-href => href(action=>$tag{'reftype'}, hash=>$tag{'refid'})}, $tag{'reftype'});
 		if ($tag{'reftype'} eq "commit") {
-			print " | " . $cgi->a({-href => href(action=>"shortlog", hash=>$tag{'name'})}, "shortlog") .
-			      " | " . $cgi->a({-href => href(action=>"log", hash=>$tag{'name'})}, "log");
+			print " | " . $cgi->a({-href => href(action=>"shortlog", hash=>$tag{'fullname'})}, "shortlog") .
+			      " | " . $cgi->a({-href => href(action=>"log", hash=>$tag{'fullname'})}, "log");
 		} elsif ($tag{'reftype'} eq "blob") {
 			print " | " . $cgi->a({-href => href(action=>"blob_plain", hash=>$tag{'refid'})}, "raw");
 		}
@@ -3726,13 +3728,13 @@ sub git_heads_body {
 		$alternate ^= 1;
 		print "<td><i>$ref{'age'}</i></td>\n" .
 		      ($curr ? "<td class=\"current_head\">" : "<td>") .
-		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'name'}),
+		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'fullname'}),
 		               -class => "list name"},esc_html($ref{'name'})) .
 		      "</td>\n" .
 		      "<td class=\"link\">" .
-		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'name'})}, "shortlog") . " | " .
-		      $cgi->a({-href => href(action=>"log", hash=>$ref{'name'})}, "log") . " | " .
-		      $cgi->a({-href => href(action=>"tree", hash=>$ref{'name'}, hash_base=>$ref{'name'})}, "tree") .
+		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'fullname'})}, "shortlog") . " | " .
+		      $cgi->a({-href => href(action=>"log", hash=>$ref{'fullname'})}, "log") . " | " .
+		      $cgi->a({-href => href(action=>"tree", hash=>$ref{'fullname'}, hash_base=>$ref{'name'})}, "tree") .
 		      "</td>\n" .
 		      "</tr>";
 	}
-- 
1.5.3.7

[PATCH] gitweb: Teach "a=blob" action to be more lenient about blob/file mime type

[Jakub Narebski]

Since 930cf7dd7cc6b87d173f182230763e1f1913d319 'blob' action knows the
file type; if the file type is not "text/*" or one of common network
image formats/mimetypes (gif, png, jpeg) then the action "blob"
defaulted to "blob_plain".  This caused the problem if mimetypes file
was not well suited for web, for example returning "application/x-sh"
for "*.sh" shell scripts, instead of "text/plain" (or other "text/*").

Now "blob" action defaults to "blob_plain" ('raw' view) only if file
is of type which is neither "text/*" nor "image/{gif,png,jpeg}"
AND it is binary file.  Otherwise it assumes that it can be displayed
either in <img> tag ("image/*" mimetype), or can be displayed line by
line (otherwise).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Original version of the patch has a simple bug.

 gitweb/gitweb.perl |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 448dca7..6256641 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4290,7 +4290,7 @@ sub git_blob {
 	open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
 		or die_error(undef, "Couldn't cat $file_name, $hash");
 	my $mimetype = blob_mimetype($fd, $file_name);
-	if ($mimetype !~ m!^(?:text/|image/(?:gif|png|jpeg)$)!) {
+	if ($mimetype !~ m!^(?:text/|image/(?:gif|png|jpeg)$)! && -B $fd) {
 		close $fd;
 		return git_blob_plain($mimetype);
 	}
@@ -4331,16 +4331,7 @@ sub git_blob {
 	}
 	git_print_page_path($file_name, "blob", $hash_base);
 	print "<div class=\"page_body\">\n";
-	if ($mimetype =~ m!^text/!) {
-		my $nr;
-		while (my $line = <$fd>) {
-			chomp $line;
-			$nr++;
-			$line = untabify($line);
-			printf "<div class=\"pre\"><a id=\"l%i\" href=\"#l%i\" class=\"linenr\">%4i</a> %s</div>\n",
-			       $nr, $nr, $nr, esc_html($line, -nbsp=>1);
-		}
-	} elsif ($mimetype =~ m!^image/!) {
+	if ($mimetype =~ m!^image/!) {
 		print qq!<img type="$mimetype"!;
 		if ($file_name) {
 			print qq! alt="$file_name" title="$file_name"!;
@@ -4349,6 +4340,15 @@ sub git_blob {
 		      href(action=>"blob_plain", hash=>$hash,
 		           hash_base=>$hash_base, file_name=>$file_name) .
 		      qq!" />\n!;
+	} else {
+		my $nr;
+		while (my $line = <$fd>) {
+			chomp $line;
+			$nr++;
+			$line = untabify($line);
+			printf "<div class=\"pre\"><a id=\"l%i\" href=\"#l%i\" class=\"linenr\">%4i</a> %s</div>\n",
+			       $nr, $nr, $nr, esc_html($line, -nbsp=>1);
+		}
 	}
 	close $fd
 		or print "Reading blob failed.\n";
-- 
1.5.3.7

Re: [PATCH 0/3 (resend)] gitweb: Miscelanous fixes

[Junio C Hamano]

Thanks for a reminder.  Will look at it but I'll be offline for several
hours today.

[PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Petr Baudis]

This is important for the list of clone urls, where if there are
no per-repository clone URL configured, the default base URLs
are never used for URL construction with this patch.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 491a3f4..d5505a4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1512,7 +1512,7 @@ sub config_to_int {
 sub config_to_multi {
 	my $val = shift;
 
-	return ref($val) ? $val : [ $val ];
+	return ref($val) ? $val : $val ? [ $val ] : [];
 }
 
 sub git_get_project_config {

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Jakub Narebski]

Petr Baudis <pasky@suse.cz> writes:

> This is important for the list of clone urls, where if there are
> no per-repository clone URL configured, the default base URLs
> are never used for URL construction with this patch.

Thanks.
 
> @@ -1512,7 +1512,7 @@ sub config_to_int {
>  sub config_to_multi {
>  	my $val = shift;
>  
> -	return ref($val) ? $val : [ $val ];
> +	return ref($val) ? $val : $val ? [ $val ] : [];
>  }

Shouldn't it be

  +	return ref($val) ? $val : defined($val) ? [ $val ] : [];

-- 
Jakub Narebski

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Petr Baudis <pasky@suse.cz> writes:
>
>> This is important for the list of clone urls, where if there are
>> no per-repository clone URL configured, the default base URLs
>> are never used for URL construction with this patch.
>
> Thanks.
>  
>> @@ -1512,7 +1512,7 @@ sub config_to_int {
>>  sub config_to_multi {
>>  	my $val = shift;
>>  
>> -	return ref($val) ? $val : [ $val ];
>> +	return ref($val) ? $val : $val ? [ $val ] : [];
>>  }
>
> Shouldn't it be
>
>   +	return ref($val) ? $val : defined($val) ? [ $val ] : [];

I think so.  Pasky?