[PATCH 0/6] gitweb: Some mod_perl specific support (but not only)

[PATCH 0/6] gitweb: Some mod_perl specific support (but not only)

[Jakub Narebski]

This is series of gitweb patches to provide better mod_perl
support, for now running gitweb under mod_perl's Registry,
but in the future as mod_perl handler.

First patch is mod_perl related only in a way that it provides
path to mod_perl specific support, but has its own advantages
even when running gitweb simply as CGI script, namely the
centralization of stopping output after HTTP header for HEAD
requests. Till this patch only git_feed had this feature (originally
by Andreas Fuchs).

Second patch was created because further patches make gitweb to
have different codepath for mod_perl; so mod_perl version string
was added to "generator" meta header in HTML header.

I'm not so sure about third patch, namely if I understood what
is written in CGI(3pm) about compile method.

Fourth patch prepares the way for mod_perl specific support.
Perhaps "our $r = shift @_;" instead of "my $r = shift @_;"
would be better.

Fifth patch appears to be unnecessary, at least for now, because
mod_perl Registry populates %ENV hash (and does not need to set
envirionmental variables). Still, it prepares the way for future
running gitweb as mod_perl handler, and not under Registry.

Sixth patch is an RFC. It tries to add HTTP headers directly,
allowing Apache to not need to parse headers, which should speed
up gitweb a bit. It also makes use of mod_perl meets_expectation
method to respond to If-Modified-Since: and If-None-Match: requests
for cache validation. Current state is a bit of mess as it is now.
Comments (and patches) appreciated.

Table of contents (shortlog):
=============================
 [PATCH 1/6] gitweb: Separate HTTP header output
 [PATCH 2/6] gitweb: Add mod_perl version string to "generator" meta header
 [PATCH 3/6] gitweb: Precompile CGI routines for mod_perl
 [PATCH/RFC 4/6] gitweb: Prepare for mod_perl specific support
 [RFC/PATCH 5/6] gitweb: Make possible to run under mod_perl without SetupEnv
 [RFC/PATCH 6/6] gitweb: Make possible to run under mod_perl without ParseHeaders

Diffstat:
=========
 gitweb/gitweb.perl |  227 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 205 insertions(+), 22 deletions(-)

Benchmarks:
===========
$ ab -n 10 "http://localhost/perl/gitweb/gitweb.cgi?p=git.git;a=summary"
$ ab -n 10 -c 2 "http://localhost/perl/gitweb/gitweb.cgi?p=git.git;a=summary"
(hot cache)

$ ab -n 10 "http://localhost/perl/gitweb/gitweb.cgi/git.git"
$ ab -n 10 -c 2 "http://localhost/perl/gitweb/gitweb.cgi/git.git"
(hot cache)

patch                                         | mean +/- sd     | mean -c 2
-----------------------------------------------------------------------------
[before first patch in series]:               | 287 +/-  8.8 ms | 296.049 ms
 (path_info version)                          | 293 +/- 10.6 ms | 314.526 ms
gitweb-Separate-HTTP-header-output:           | 302 +/- 46.7 ms | 300.305 ms
gitweb-Add-mod_perl-to-generator:             | 288 +/- 15.6 ms | 306.050 ms
gitweb-Precompile-CGI-routines-for-mod_perl:  | 291 +/- 10.9 ms | 306.704 ms
gitweb-Prepare-for-mod_perl-specific-support: | 299 +/- 11.0 ms | 300.879 ms
gitweb-mod_perl-without-SetupEnv:             | 288 +/- 12.4 ms | 296.809 ms
 (path_info version)                          | 292 +/- 12.7 ms | 307.380 ms
gitweb-mod_perl-without-ParseHeaders:         | ???             | ???

-- 
Jakub Narebski
Poland

[PATCH 1/6] gitweb: Separate HTTP header output

[Jakub Narebski]

Separate output (writing) of HTTP headers into http_header subroutine,
to centralize setting HTTP header for further mod_perl specific tweaks
(to be able to run gitweb without PerlOptions +ParseHeaders, which
would speed gitweb some), and checking for HEAD request.

Always return just after HTTP header is sent when asking only about
headers (HTTP request method 'HEAD'); first appeared in git_rss.

While at it uniquify style of http_header(...) calls, formerly
"print $cgi->header(...)", and remove default HTTP status, '200 OK'.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This one is fairly generic, and if considered worthy, I think
can be accepted without much ado.

Perhaps the cleanup part of it should be split into separate patch?

 gitweb/gitweb.perl |   40 +++++++++++++++++++++++++++-------------
 1 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 65fcdb0..aaee217 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1678,7 +1678,17 @@ sub blob_mimetype {
 }
 
 ## ======================================================================
-## functions printing HTML: header, footer, error page
+## functions printing HTTP or HTML: header, footer, error page
+
+sub http_header {
+	my @header = @_;
+
+	print $cgi->header(@header);
+
+	# Optimization: skip generating the body if client asks only
+	# for HTTP header (e.g. cache validation).
+	return if ($cgi->request_method() eq 'HEAD');
+}
 
 sub git_header_html {
 	my $status = shift || "200 OK";
@@ -1709,8 +1719,11 @@ sub git_header_html {
 	} else {
 		$content_type = 'text/html';
 	}
-	print $cgi->header(-type=>$content_type, -charset => 'utf-8',
-	                   -status=> $status, -expires => $expires);
+	http_header(
+		-type => $content_type,
+		-charset => 'utf-8',
+		-status => $status,
+		-expires => $expires);
 	print <<EOF;
 <?xml version="1.0" encoding="utf-8"?>
 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
@@ -2983,7 +2996,7 @@ sub git_forks {
 sub git_project_index {
 	my @projects = git_get_projects_list($project);
 
-	print $cgi->header(
+	http_header(
 		-type => 'text/plain',
 		-charset => 'utf-8',
 		-content_disposition => 'inline; filename="index.aux"');
@@ -3375,7 +3388,7 @@ sub git_blob_plain {
 		$save_as .= '.txt';
 	}
 
-	print $cgi->header(
+	http_header(
 		-type => "$type",
 		-expires=>$expires,
 		-content_disposition => 'inline; filename="' . "$save_as" . '"');
@@ -3591,10 +3604,9 @@ sub git_snapshot {
 
 	my $filename = basename($project) . "-$hash.tar.$suffix";
 
-	print $cgi->header(
+	http_header(
 		-type => "application/$ctype",
-		-content_disposition => 'inline; filename="' . "$filename" . '"',
-		-status => '200 OK');
+		-content_disposition => 'inline; filename="' . "$filename" . '"');
 
 	my $git = git_cmd_str();
 	my $name = $project;
@@ -3979,7 +3991,7 @@ sub git_blobdiff {
 		}
 
 	} elsif ($format eq 'plain') {
-		print $cgi->header(
+		http_header(
 			-type => 'text/plain',
 			-charset => 'utf-8',
 			-expires => $expires,
@@ -4128,7 +4140,7 @@ sub git_commitdiff {
 		my $tagname = git_get_rev_name_tags($hash);
 		my $filename = basename($project) . "-$hash.patch";
 
-		print $cgi->header(
+		http_header(
 			-type => 'text/plain',
 			-charset => 'utf-8',
 			-expires => $expires,
@@ -4465,12 +4477,12 @@ sub git_feed {
 	if (defined($commitlist[0])) {
 		%latest_commit = %{$commitlist[0]};
 		%latest_date   = parse_date($latest_commit{'author_epoch'});
-		print $cgi->header(
+		http_header(
 			-type => $content_type,
 			-charset => 'utf-8',
 			-last_modified => $latest_date{'rfc2822'});
 	} else {
-		print $cgi->header(
+		http_header(
 			-type => $content_type,
 			-charset => 'utf-8');
 	}
@@ -4670,7 +4682,9 @@ sub git_atom {
 sub git_opml {
 	my @list = git_get_projects_list();
 
-	print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
+	http_header(
+		-type => 'text/xml',
+		-charset => 'utf-8');
 	print <<XML;
 <?xml version="1.0" encoding="utf-8"?>
 <opml version="1.0">
-- 
1.4.4.3

[PATCH 2/6] gitweb: Add mod_perl version string to "generator" meta header

[Jakub Narebski]

Add mod_perl version string (the value of $ENV{'MOD_PERL'} if it is
set) to "generator" meta header.

The purpose of this is to identify version of gitweb, now that
codepath may differ for gitweb run as CGI script, run under
mod_perl 1.0 and run under mod_perl 2.0.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
For example mod_perl 2.0 sets MOD_PERL to something like
"mod_perl/2.0.1".

This patch was created because further patches make gitweb to
have different codepath for mod_perl; so mod_perl version string
was added to "generator" meta header in HTML header.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index aaee217..bb1d66c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1724,6 +1724,8 @@ sub git_header_html {
 		-charset => 'utf-8',
 		-status => $status,
 		-expires => $expires);
+	# the environmental variable MOD_PERL has 'mod_perl/VERSION' value if set
+	my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : '';
 	print <<EOF;
 <?xml version="1.0" encoding="utf-8"?>
 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
@@ -1732,7 +1734,7 @@ sub git_header_html {
 <!-- git core binaries version $git_version -->
 <head>
 <meta http-equiv="content-type" content="$content_type; charset=utf-8"/>
-<meta name="generator" content="gitweb/$version git/$git_version"/>
+<meta name="generator" content="gitweb/$version git/$git_version$mod_perl_version"/>
 <meta name="robots" content="index, nofollow"/>
 <title>$title</title>
 EOF
-- 
1.4.4.3

[PATCH 3/6] gitweb: Precompile CGI routines for mod_perl

[Jakub Narebski]

Following advice from CGI(3pm) man page, precompile all CGI routines
for mod_perl, in the BEGIN block.


  If you want to compile without importing use the compile() method
  instead:

    use CGI();
    CGI->compile();

  This is particularly useful in a mod_perl environment, in which you
  might want to precompile all CGI routines in a startup script, and then
  import the functions individually in each mod_perl script.


Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Could help. I'm a bit unsure.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index bb1d66c..3888563 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -18,6 +18,10 @@ use File::Find qw();
 use File::Basename qw(basename);
 binmode STDOUT, ':utf8';
 
+BEGIN {
+	CGI->compile() if $ENV{MOD_PERL};
+}
+
 our $cgi = new CGI;
 our $version = "++GIT_VERSION++";
 our $my_url = $cgi->url();
-- 
1.4.4.3

[PATCH/RFC 4/6] gitweb: Prepare for mod_perl specific support

[Jakub Narebski]

Prepare gitweb for mod_perl specific support in CGI compatibility mode
(Apache::Registry/ModPerl::Registry or Apache::PerlRun/ModPerl::PerlRun)
by storing request (an argument to a handler) in $r variable, for later
use.

The idea is to have gitweb run as CGI script, under mod_perl 1.0 and under
mod_perl 2.0 without modifications, while being able to make use of mod_perl
capabilities.

Define MP_GEN constant and set it to 0 if mod_perl is not available,
to 1 if running under mod_perl 1.0, and 2 for mod_perl 2.0. It is later used
in BEGIN block to load appropriate mod_perl modules; for now the one
in which request is defined, and the one with status and HTTP constants.
Based on "Porting Apache:: Perl Modules from mod_perl 1.0 to 2.0" document
  http://perl.apache.org/docs/2.0/user/porting/porting.html
chapter "Making Code Conditional on Running mod_perl Version".

Use "if (MP_GEN)" for checking if gitweb is run under mod_perl; later
on we will use "if ($r)" for that.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
A bit of RFC, because I'm not sure if "my $r" or "our $r" should be
used (in script which makes use of subroutines; under Registry those
would end as nested subroutines).

Perhaps we should import everything?

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3888563..9983e9e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -18,10 +18,36 @@ use File::Find qw();
 use File::Basename qw(basename);
 binmode STDOUT, ':utf8';
 
+# Set the constant MP_GEN to 0 if mod_perl is not available,
+# to 1 if running under mod_perl 1.0
+# and 2 for mod_perl 2.0
+use constant {
+	MP_GEN => ($ENV{'MOD_PERL'}
+	           ? ( exists $ENV{'MOD_PERL_API_VERSION'} and 
+	                      $ENV{'MOD_PERL_API_VERSION'} >= 2 ) ? 2 : 1
+	           : 0),
+};
+
 BEGIN {
-	CGI->compile() if $ENV{MOD_PERL};
+	# use appropriate mod_perl modules (conditional use)
+	if (MP_GEN == 2) {
+		require Apache2::RequestRec;
+		require Apache2::Const;
+		Apache2::Const->import(-compile => qw(:common :http));
+	} elsif (MP_GEN == 1) {
+		require Apache;
+		require Apache::Constants;
+		Apache::Constants->import(qw(:common :http));
+	}
+
+	# precompile CGI for mod_perl
+	CGI->compile() if MP_GEN;
 }
 
+# mod_perl request
+my $r;
+$r = shift @_ if MP_GEN;
+
 our $cgi = new CGI;
 our $version = "++GIT_VERSION++";
 our $my_url = $cgi->url();
-- 
1.4.4.3

[PATCH/RFC 5/6] gitweb: Make possible to run under mod_perl without SetupEnv

[Jakub Narebski]

Make possible to run gitweb under mod_perl without need to set up CGI
environmental variables (i.e. "PerlOptions -SetupEnv" in mod_perl 2.0,
"PerlSetupEnv Off" in mod_perl 1.0).

Actually ModPerl::Registry / Apache::Registry populates %ENV hash,
without need to actually set environmental variables.


Pass the request variable $r to CGI constructor if CGI.pm module is
new enough (at least CGI version 2.93, and at least 3.11 for mod_perl
2.0).

Replace $ENV{'PATH_INFO'} by $r->path_info() if we use mod_perl.

Replace $ENV{'SERVER_NAME'} by $r->server()->server_hostname() if we
use mod_perl.

Uniquify using of %ENV to $ENV{'NAME'}, while at it.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch appears to be unnecessary, at least for now, because
mod_perl Registry populates %ENV hash (and does not need to set
envirionmental variables). Still, it prepares the way for future
running gitweb as mod_perl handler, and not under Registry.

Perhaps the cleanup part of this patch should be put into separate
patch...

 gitweb/gitweb.perl |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9983e9e..2900ae6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -32,11 +32,16 @@ BEGIN {
 	# use appropriate mod_perl modules (conditional use)
 	if (MP_GEN == 2) {
 		require Apache2::RequestRec;
+		require Apache2::ServerRec;
+		require Apache2::Response;
 		require Apache2::Const;
+		Apache2::RequestRec->import();
+		Apache2::ServerRec->import();
 		Apache2::Const->import(-compile => qw(:common :http));
 	} elsif (MP_GEN == 1) {
 		require Apache;
 		require Apache::Constants;
+		import Apache;
 		Apache::Constants->import(qw(:common :http));
 	}
 
@@ -48,7 +53,15 @@ BEGIN {
 my $r;
 $r = shift @_ if MP_GEN;
 
-our $cgi = new CGI;
+our $cgi;
+if ((MP_GEN == 1 && $CGI::VERSION >= 2.93) ||
+    (MP_GEN == 2 && $CGI::VERSION >= 3.11)) {
+	# CGI.pm is new enough
+	$cgi = new CGI($r);
+} else {
+	$cgi = new CGI;
+}
+
 our $version = "++GIT_VERSION++";
 our $my_url = $cgi->url();
 our $my_uri = $cgi->url(-absolute => 1);
@@ -70,7 +83,8 @@ our $home_link_str = "++GITWEB_HOME_LINK_STR++";
 # name of your site or organization to appear in page titles
 # replace this with something more descriptive for clearer bookmarks
 our $site_name = "++GITWEB_SITENAME++"
-                 || ($ENV{'SERVER_NAME'} || "Untitled") . " Git";
+                 || (($r ? $r->server()->server_hostname() : $ENV{'SERVER_NAME'})
+                     || "Untitled") . " Git";
 
 # filename of html text to include at top of each page
 our $site_header = "++GITWEB_SITE_HEADER++";
@@ -403,7 +417,7 @@ if (defined $searchtype) {
 # now read PATH_INFO and use it as alternative to parameters
 sub evaluate_path_info {
 	return if defined $project;
-	my $path_info = $ENV{"PATH_INFO"};
+	my $path_info = $r ? $r->path_info() : $ENV{'PATH_INFO'};
 	return if !$path_info;
 	$path_info =~ s,^/+,,;
 	return if !$path_info;
-- 
1.4.4.3

[RFC/PATCH 6/6] gitweb: Make possible to run under mod_perl without ParseHeaders

[Jakub Narebski]

Add mod_perl version of http_header, setting headers directly (both
for mod_perl 2.0 and 1.0); bits of code taken from CGI and CGI::Util
modules.  While at it add cache validation via $r->meets_conditions()
in mod_perl code.

Separate HTTP redirection into http_redirect subroutine and add
mod_perl version, setting headers directly.

All this is meant to allow gitweb to run under ModPerl::Registry (for
mod_perl 2.0) / Apache::Registry (for mod_perl 1.0) without need for
Apache to parse headers (without ParseHeaders), which should speed up
gitweb a bit.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch tries to add HTTP headers directly, allowing Apache to not
need to parse headers (without ParseHeaders), which should speed
up gitweb a bit. It also makes use of mod_perl meets_expectation
method to respond to If-Modified-Since: and If-None-Match: requests
for cache validation. Current state is a bit of mess as it is now.
Comments (and patches) appreciated.

It is not benchmarked because ApacheBench went crazy, showing *negative*
waiting time. Probably I did something wrong...

Not checked for warnings, only slightly tested: definitely an RFC.

 gitweb/gitweb.perl |  137 +++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 130 insertions(+), 7 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2900ae6..12f1cb2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -35,13 +35,17 @@ BEGIN {
 		require Apache2::ServerRec;
 		require Apache2::Response;
 		require Apache2::Const;
+		require APR::Date;
 		Apache2::RequestRec->import();
 		Apache2::ServerRec->import();
 		Apache2::Const->import(-compile => qw(:common :http));
+		APR::Date->import();
 	} elsif (MP_GEN == 1) {
 		require Apache;
 		require Apache::Constants;
+		require Apache::File;
 		import Apache;
+		import Apache::File;
 		Apache::Constants->import(qw(:common :http));
 	}
 
@@ -1727,11 +1731,129 @@ sub blob_mimetype {
 sub http_header {
 	my @header = @_;
 
-	print $cgi->header(@header);
+	if (MP_GEN) {
+		my %header = @header;
+		my $cache_validator;
+
+		## special cases ##
+		# -status
+		$r->status_line($header{-status})
+			if $header{-status};
+		delete $header{-status} if exists $header{-status};
+		# -type and -charset
+		if ($header{-type} || $header{-charset}) {
+			my $type = $header{-type} || 'text/html';
+			$type .= "; charset=$header{-charset}"
+				if $type =~ m!^text/! and $type !~ /\bcharset\b/ and $header{-charset};
+
+			$r->content_type($type);
+		}
+		delete $header{-type} if exists $header{-type};
+		delete $header{-charset} if exists $header{-charset};
+		# -content_encoding
+		$r->content_encoding($header{-content_encoding})
+			if $header{-content_encoding};
+		delete $header{-content_encoding} if exists $header{-content_encoding};
+		# -expires
+		if ($header{-expires}) {
+			my $expires = CGI::Util::expires($header{-expires}, 'http');
+			if (MP_GEN == 1) {
+				$r->header_out('Expires', $expires);
+			} else {
+				$r->headers_out->add('Expires', $expires);
+			}
+		}
+		delete $header{-expires} if exists $header{-expires};
+		# -last_modified
+		if ($header{-last_modified}) {
+			$cache_validator ||= 1;
+			if (MP_GEN == 1) {
+				$r->header_out('Last-Modified', $header{-last_modified});
+			} else {
+				$r->set_last_modified(APR::Date::parse_http($header{-last_modified}));
+			}
+		}
+		delete $header{-last_modified} if exists $header{-last_modified};
+
+		## other headers ##
+		while (my ($key, $value) = each %header) {
+			$key =~ s/^-//; # -content_disposition -> content_disposition
+			$key =~ s/_/-/; #  content_disposition -> content-disposition
+			$key =~ s/(\w)(\w*)/\u$1$2/g;
+			                #  content-disposition -> Content-Disposition
+
+			if (MP_GEN == 1) {
+				$r->header_out($key, $value);
+			} else {
+				$r->headers_out->add($key, $value);
+			}
+		}
+		$cache_validator ||= (exists $header{-ETag} || exists $header{-etag});
+
+		## send headers / flush ##
+		if (MP_GEN == 1) {
+			$r->send_http_headers();
+
+			## validate cache ##
+			if ($cache_validator &&
+			    (my $rc = $r->meets_conditions()) != Apache::Constant::OK) {
+				return $rc;
+			}
+		} else {
+			$r->rflush();
+
+			## validate cache ##
+			if ($cache_validator &&
+			    (my $rc = $r->meets_conditions()) != Apache2::Const::OK) {
+				return $rc;
+			}
+		}
+	} else {
+		print $cgi->header(@header);
+	}
 
 	# Optimization: skip generating the body if client asks only
 	# for HTTP header (e.g. cache validation).
-	return if ($cgi->request_method() eq 'HEAD');
+	if (MP_GEN == 2) {
+		return Apache2::Const::OK   if $r->header_only();
+	} elsif (MP_GEN == 1) {
+		return Apache::Constant::OK if $r->header_only();
+	} else {
+		return if ($cgi->request_method() eq 'HEAD');
+	}
+}
+
+sub http_redirect {
+	my @params = @_;
+	my %params;
+	if (@params % 2 == 0) {
+		%params = @params;
+	}
+	my $uri = $params{-uri} || $params{-url} || $params{-location}
+		|| $params[0] || $cgi->self_url;
+	my $status = $params{-status};
+
+	if (MP_GEN == 1) {
+		$r->header_out('Location', $uri);
+		if (defined $status) {
+			$r->status_line($status);
+		} else {
+			$r->status(Apache::Constant::REDIRECT);
+		}
+
+		$r->send_http_headers();
+	} elsif (MP_GEN == 2) {
+		$r->headers_out->add('Location', $uri);
+		if (defined $status) {
+			$r->status_line($status);
+		} else {
+			$r->status(Apache2::Const::REDIRECT);
+		}
+
+		$r->rflush();
+	} else {
+		print $cgi->redirect(@params);
+	}
 }
 
 sub git_header_html {
@@ -1854,7 +1976,8 @@ EOF
 		      $cgi->hidden(-name => "a") . "\n" .
 		      $cgi->hidden(-name => "h") . "\n" .
 		      $cgi->popup_menu(-name => 'st', -default => 'commit',
-				       -values => ['commit', 'author', 'committer', 'pickaxe']) .
+		                       -values => ['commit', 'author', 'committer',
+		                       gitweb_check_feature('pickaxe') ? 'pickaxe' : ()]) .
 		      $cgi->sup($cgi->a({-href => href(action=>"search_help")}, "?")) .
 		      " search:\n",
 		      $cgi->textfield(-name => "s", -value => $searchtext) . "\n" .
@@ -3901,10 +4024,10 @@ sub git_object {
 		die_error('404 Not Found', "Not enough information to find object");
 	}
 
-	print $cgi->redirect(-uri => href(action=>$type, -full=>1,
-	                                  hash=>$hash, hash_base=>$hash_base,
-	                                  file_name=>$file_name),
-	                     -status => '302 Found');
+	http_redirect(-uri => href(action=>$type, -full=>1,
+	                           hash=>$hash, hash_base=>$hash_base,
+	                           file_name=>$file_name),
+	              -status => '302 Found');
 }
 
 sub git_blobdiff {
-- 
1.4.4.3

Re: [RFC/PATCH 6/6] gitweb: Make possible to run under mod_perl without ParseHeaders

[Robert Fitzsimons]

> @@ -1854,7 +1976,8 @@ EOF
>  		      $cgi->hidden(-name => "a") . "\n" .
>  		      $cgi->hidden(-name => "h") . "\n" .
>  		      $cgi->popup_menu(-name => 'st', -default => 'commit',
> -				       -values => ['commit', 'author', 'committer', 'pickaxe']) .
> +		                       -values => ['commit', 'author', 'committer',
> +		                       gitweb_check_feature('pickaxe') ? 'pickaxe' : ()]) .
>  		      $cgi->sup($cgi->a({-href => href(action=>"search_help")}, "?")) .
>  		      " search:\n",
>  		      $cgi->textfield(-name => "s", -value => $searchtext) . "\n" .

This should be a separate patch.

Robert

Re: [RFC/PATCH 6/6] gitweb: Make possible to run under mod_perl without ParseHeaders

[Jakub Narebski]

Robert Fitzsimons wrote:
>> @@ -1854,7 +1976,8 @@ EOF
>>  		      $cgi->hidden(-name => "a") . "\n" .
>>  		      $cgi->hidden(-name => "h") . "\n" .
>>  		      $cgi->popup_menu(-name => 'st', -default => 'commit',
>> -				       -values => ['commit', 'author', 'committer', 'pickaxe']) .
>> +		                       -values => ['commit', 'author', 'committer',
>> +		                       gitweb_check_feature('pickaxe') ? 'pickaxe' : ()]) .
>>  		      $cgi->sup($cgi->a({-href => href(action=>"search_help")}, "?")) .
>>  		      " search:\n",
>>  		      $cgi->textfield(-name => "s", -value => $searchtext) . "\n" .
> 
> This should be a separate patch.

I'm sorry, somehow I missed this.

Besides, it would be better to assign return value to some
variable, to avoid calling gitweb_check_feature again... 
-- 
Jakub Narebski
Poland

Re: [PATCH 1/6] gitweb: Separate HTTP header output

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Always return just after HTTP header is sent when asking only about
> headers (HTTP request method 'HEAD'); first appeared in git_rss.
>
> While at it uniquify style of http_header(...) calls, formerly
> "print $cgi->header(...)", and remove default HTTP status, '200 OK'.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This one is fairly generic, and if considered worthy, I think
> can be accepted without much ado.

Maybe I am missing something fundamental, but I cannot see how
this affects anything whatsoever...

> +## functions printing HTTP or HTML: header, footer, error page
> +
> +sub http_header {
> +	my @header = @_;
> +
> +	print $cgi->header(@header);
> +
> +	# Optimization: skip generating the body if client asks only
> +	# for HTTP header (e.g. cache validation).
> +	return if ($cgi->request_method() eq 'HEAD');
> +}

Ok, so this explicitly written "return" returns when it is a
HEAD request not GET.  Otherwise the control falls out of the
end of the function.  Either way you return undef.

Then the caller does...

> @@ -1709,8 +1719,11 @@ sub git_header_html {
>  	} else {
>  		$content_type = 'text/html';
>  	}
> -	print $cgi->header(-type=>$content_type, -charset => 'utf-8',
> -	                   -status=> $status, -expires => $expires);
> +	http_header(
> +		-type => $content_type,
> +		-charset => 'utf-8',
> +		-status => $status,
> +		-expires => $expires);
>  	print <<EOF;
>  <?xml version="1.0" encoding="utf-8"?>
>  <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">

which means it does not omit generating the body anyway no
matter what "sub http_header" did...

Or is there some Perl magic that makes a return from sub named
*_header magically terminate the execution of the caller?

Puzzled...

Re: [PATCH 1/6] gitweb: Separate HTTP header output

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> which means it does not omit generating the body anyway no
> matter what "sub http_header" did...
> 
> Or is there some Perl magic that makes a return from sub named
> *_header magically terminate the execution of the caller?

No magic.  Bad patch.  Your assessment of the patch is correct;
it is not avoiding the body generation for a HEAD request.

-- 
Shawn.