Skip to contentSkip to conversation

gitweb: Miscelanous fixes

PATCH In review Jakub Narebski 3 participants 8 messages Jun 15, 2016

lore
Share & tools
lore

Jakub Narebski

author
#0 lore raw

cover gitweb: Miscelanous fixes

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
Signature · 2 lines
Jakub Narębski
Poland
author
#1 lore raw

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

From: Petr Baudis <[email protected]>
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 <[email protected]>
Signed-off-by Jakub Narebski <[email protected]>

---
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(-)
gitweb/gitweb.perl +1 -1
index 24b3158..a746a85 100755
@@ -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 {
t/t9500-gitweb-standalone-no-errors.sh +25 -0
index 35fff3d..2d3d4e8 100755
@@ -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
Signature · 1 line
1.5.3.7
Acked-by on v1 1/1
#2 lore raw

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

Quoted 8 lines · Jakub Narebski

From: Petr Baudis <[email protected]>
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.

Quoted 7 lines

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 <[email protected]>
Signed-off-by Jakub Narebski <[email protected]>
Acked-by Petr Baudis <[email protected]>

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

Signature · 3 lines
				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
author
#3 lore raw

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

In reply to Jakub Narebski
Quoted 8 lines · Jakub Narebski

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.

Quoted 4 lines

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

Quoted 11 lines
gitweb/gitweb.perl +0 -0
index 24b3158..a746a85 100755
@@ -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.

Signature · 2 lines
Jakub Narebski
Poland
#4 lore raw

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

Please send updates if something is not right. Thanks.

author
#5 lore raw

PATCH 1/1 gitweb: disambiguate heads and tags withs the same name

In reply to 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 <[email protected]>
Signed-off-by Jakub Narebski <[email protected]>

---
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(-)
gitweb/gitweb.perl +8 -6
index a746a85..448dca7 100755
@@ -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>";
 	}
Signature · 1 line
1.5.3.7
author
#6 lore raw

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

In reply to 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 <[email protected]>

---
Original version of the patch has a simple bug.

 gitweb/gitweb.perl |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)
gitweb/gitweb.perl +11 -11
index 448dca7..6256641 100755
@@ -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";
Signature · 1 line
1.5.3.7