threads / patch / 11303

patch, 3 partsgitweb: Miscelanous fixes

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

## tl;dr

8 messages between Dec 15, 2007 and Dec 18, 2007. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Jakub Narebski· Dec 15, 2007, 14:34 UTC · lore

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
Jakub Narebski· Dec 15, 2007, 14:36 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to 2 files +26 −1

gitweb/gitweb.perl, t/t9500-gitweb-standalone-no-errors.sh

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
Petr Baudis· Dec 15, 2007, 15:05 UTC · re: Jakub Narebski · lore

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

On Sat, Dec 15, 2007 at 03:36:32PM +0100, Jakub Narebski wrote:
Show 15 quoted lines
> 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
Jakub Narebski· Dec 18, 2007, 00:12 UTC · re: Jakub Narebski · lore

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

On Sat, 15 Dec 2007, Jakub Narebski wrote:
Show 8 quoted lines
> 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...
Show 11 quoted lines
> 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
Jakub Narebski· Dec 15, 2007, 14:40 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to gitweb/gitweb.perl +8 −6
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
Jakub Narebski· Dec 15, 2007, 14:41 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to gitweb/gitweb.perl +11 −11
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

← back to recent threads