{"thread":{"id":"6133","subject":"[PATCH 0/6] gitweb: Some mod_perl specific support (but not only)","startedAt":"2006-12-27T22:55:31Z","lastAt":"2006-12-28T01:28:00Z","messageCount":11,"participants":["Jakub Narebski","Robert Fitzsimons","Junio C Hamano","Shawn Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"30369","messageId":"200612272355.31923.jnareb@gmail.com","threadId":"6133","inReplyTo":null,"subject":"[PATCH 0/6] gitweb: Some mod_perl specific support (but not only)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-27T22:55:31Z","receivedAt":"2006-12-27T22:55:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This is series of gitweb patches to provide better mod_perl\nsupport, for now running gitweb under mod_perl's Registry,\nbut in the future as mod_perl handler.\n\nFirst patch is mod_perl related only in a way that it provides\npath to mod_perl specific support, but has its own advantages\neven when running gitweb simply as CGI script, namely the\ncentralization of stopping output after HTTP header for HEAD\nrequests. Till this patch only git_feed had this feature (originally\nby Andreas Fuchs).\n\nSecond patch was created because further patches make gitweb to\nhave different codepath for mod_perl; so mod_perl version string\nwas added to \"generator\" meta header in HTML header.\n\nI'm not so sure about third patch, namely if I understood what\nis written in CGI(3pm) about compile method.\n\nFourth patch prepares the way for mod_perl specific support.\nPerhaps \"our $r = shift @_;\" instead of \"my $r = shift @_;\"\nwould be better.\n\nFifth patch appears to be unnecessary, at least for now, because\nmod_perl Registry populates %ENV hash (and does not need to set\nenvirionmental variables). Still, it prepares the way for future\nrunning gitweb as mod_perl handler, and not under Registry.\n\nSixth patch is an RFC. It tries to add HTTP headers directly,\nallowing Apache to not need to parse headers, which should speed\nup gitweb a bit. It also makes use of mod_perl meets_expectation\nmethod to respond to If-Modified-Since: and If-None-Match: requests\nfor cache validation. Current state is a bit of mess as it is now.\nComments (and patches) appreciated.\n\nTable of contents (shortlog):\n=============================\n [PATCH 1/6] gitweb: Separate HTTP header output\n [PATCH 2/6] gitweb: Add mod_perl version string to \"generator\" meta header\n [PATCH 3/6] gitweb: Precompile CGI routines for mod_perl\n [PATCH/RFC 4/6] gitweb: Prepare for mod_perl specific support\n [RFC/PATCH 5/6] gitweb: Make possible to run under mod_perl without SetupEnv\n [RFC/PATCH 6/6] gitweb: Make possible to run under mod_perl without ParseHeaders\n\nDiffstat:\n=========\n gitweb/gitweb.perl |  227 +++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 files changed, 205 insertions(+), 22 deletions(-)\n\nBenchmarks:\n===========\n$ ab -n 10 \"http://localhost/perl/gitweb/gitweb.cgi?p=git.git;a=summary\"\n$ ab -n 10 -c 2 \"http://localhost/perl/gitweb/gitweb.cgi?p=git.git;a=summary\"\n(hot cache)\n\n$ ab -n 10 \"http://localhost/perl/gitweb/gitweb.cgi/git.git\"\n$ ab -n 10 -c 2 \"http://localhost/perl/gitweb/gitweb.cgi/git.git\"\n(hot cache)\n\npatch                                         | mean +/- sd     | mean -c 2\n-----------------------------------------------------------------------------\n[before first patch in series]:               | 287 +/-  8.8 ms | 296.049 ms\n (path_info version)                          | 293 +/- 10.6 ms | 314.526 ms\ngitweb-Separate-HTTP-header-output:           | 302 +/- 46.7 ms | 300.305 ms\ngitweb-Add-mod_perl-to-generator:             | 288 +/- 15.6 ms | 306.050 ms\ngitweb-Precompile-CGI-routines-for-mod_perl:  | 291 +/- 10.9 ms | 306.704 ms\ngitweb-Prepare-for-mod_perl-specific-support: | 299 +/- 11.0 ms | 300.879 ms\ngitweb-mod_perl-without-SetupEnv:             | 288 +/- 12.4 ms | 296.809 ms\n (path_info version)                          | 292 +/- 12.7 ms | 307.380 ms\ngitweb-mod_perl-without-ParseHeaders:         | ???             | ???\n\n-- \nJakub Narebski\nPoland\n"},{"id":"30370","messageId":"200612272357.56532.jnareb@gmail.com","threadId":"6133","inReplyTo":"200612272355.31923.jnareb@gmail.com","subject":"[PATCH 1/6] gitweb: Separate HTTP header output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-27T22:57:56Z","receivedAt":"2006-12-27T22:57:56Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Separate output (writing) of HTTP headers into http_header subroutine,\nto centralize setting HTTP header for further mod_perl specific tweaks\n(to be able to run gitweb without PerlOptions +ParseHeaders, which\nwould speed gitweb some), and checking for HEAD request.\n\nAlways return just after HTTP header is sent when asking only about\nheaders (HTTP request method 'HEAD'); first appeared in git_rss.\n\nWhile at it uniquify style of http_header(...) calls, formerly\n\"print $cgi->header(...)\", and remove default HTTP status, '200 OK'.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis one is fairly generic, and if considered worthy, I think\ncan be accepted without much ado.\n\nPerhaps the cleanup part of it should be split into separate patch?\n\n gitweb/gitweb.perl |   40 +++++++++++++++++++++++++++-------------\n 1 files changed, 27 insertions(+), 13 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 65fcdb0..aaee217 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1678,7 +1678,17 @@ sub blob_mimetype {\n }\n \n ## ======================================================================\n-## functions printing HTML: header, footer, error page\n+## functions printing HTTP or HTML: header, footer, error page\n+\n+sub http_header {\n+\tmy @header = @_;\n+\n+\tprint $cgi->header(@header);\n+\n+\t# Optimization: skip generating the body if client asks only\n+\t# for HTTP header (e.g. cache validation).\n+\treturn if ($cgi->request_method() eq 'HEAD');\n+}\n \n sub git_header_html {\n \tmy $status = shift || \"200 OK\";\n@@ -1709,8 +1719,11 @@ sub git_header_html {\n \t} else {\n \t\t$content_type = 'text/html';\n \t}\n-\tprint $cgi->header(-type=>$content_type, -charset => 'utf-8',\n-\t                   -status=> $status, -expires => $expires);\n+\thttp_header(\n+\t\t-type => $content_type,\n+\t\t-charset => 'utf-8',\n+\t\t-status => $status,\n+\t\t-expires => $expires);\n \tprint <<EOF;\n <?xml version=\"1.0\" encoding=\"utf-8\"?>\n <!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n@@ -2983,7 +2996,7 @@ sub git_forks {\n sub git_project_index {\n \tmy @projects = git_get_projects_list($project);\n \n-\tprint $cgi->header(\n+\thttp_header(\n \t\t-type => 'text/plain',\n \t\t-charset => 'utf-8',\n \t\t-content_disposition => 'inline; filename=\"index.aux\"');\n@@ -3375,7 +3388,7 @@ sub git_blob_plain {\n \t\t$save_as .= '.txt';\n \t}\n \n-\tprint $cgi->header(\n+\thttp_header(\n \t\t-type => \"$type\",\n \t\t-expires=>$expires,\n \t\t-content_disposition => 'inline; filename=\"' . \"$save_as\" . '\"');\n@@ -3591,10 +3604,9 @@ sub git_snapshot {\n \n \tmy $filename = basename($project) . \"-$hash.tar.$suffix\";\n \n-\tprint $cgi->header(\n+\thttp_header(\n \t\t-type => \"application/$ctype\",\n-\t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"',\n-\t\t-status => '200 OK');\n+\t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"');\n \n \tmy $git = git_cmd_str();\n \tmy $name = $project;\n@@ -3979,7 +3991,7 @@ sub git_blobdiff {\n \t\t}\n \n \t} elsif ($format eq 'plain') {\n-\t\tprint $cgi->header(\n+\t\thttp_header(\n \t\t\t-type => 'text/plain',\n \t\t\t-charset => 'utf-8',\n \t\t\t-expires => $expires,\n@@ -4128,7 +4140,7 @@ sub git_commitdiff {\n \t\tmy $tagname = git_get_rev_name_tags($hash);\n \t\tmy $filename = basename($project) . \"-$hash.patch\";\n \n-\t\tprint $cgi->header(\n+\t\thttp_header(\n \t\t\t-type => 'text/plain',\n \t\t\t-charset => 'utf-8',\n \t\t\t-expires => $expires,\n@@ -4465,12 +4477,12 @@ sub git_feed {\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n \t\t%latest_date   = parse_date($latest_commit{'author_epoch'});\n-\t\tprint $cgi->header(\n+\t\thttp_header(\n \t\t\t-type => $content_type,\n \t\t\t-charset => 'utf-8',\n \t\t\t-last_modified => $latest_date{'rfc2822'});\n \t} else {\n-\t\tprint $cgi->header(\n+\t\thttp_header(\n \t\t\t-type => $content_type,\n \t\t\t-charset => 'utf-8');\n \t}\n@@ -4670,7 +4682,9 @@ sub git_atom {\n sub git_opml {\n \tmy @list = git_get_projects_list();\n \n-\tprint $cgi->header(-type => 'text/xml', -charset => 'utf-8');\n+\thttp_header(\n+\t\t-type => 'text/xml',\n+\t\t-charset => 'utf-8');\n \tprint <<XML;\n <?xml version=\"1.0\" encoding=\"utf-8\"?>\n <opml version=\"1.0\">\n-- \n1.4.4.3\n"},{"id":"30374","messageId":"200612272359.51960.jnareb@gmail.com","threadId":"6133","inReplyTo":"200612272355.31923.jnareb@gmail.com","subject":"[PATCH 2/6] gitweb: Add mod_perl version string to \"generator\" meta header","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-27T22:59:51Z","receivedAt":"2006-12-27T22:59:51Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add mod_perl version string (the value of $ENV{'MOD_PERL'} if it is\nset) to \"generator\" meta header.\n\nThe purpose of this is to identify version of gitweb, now that\ncodepath may differ for gitweb run as CGI script, run under\nmod_perl 1.0 and run under mod_perl 2.0.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nFor example mod_perl 2.0 sets MOD_PERL to something like\n\"mod_perl/2.0.1\".\n\nThis patch was created because further patches make gitweb to\nhave different codepath for mod_perl; so mod_perl version string\nwas added to \"generator\" meta header in HTML header.\n\n gitweb/gitweb.perl |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex aaee217..bb1d66c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1724,6 +1724,8 @@ sub git_header_html {\n \t\t-charset => 'utf-8',\n \t\t-status => $status,\n \t\t-expires => $expires);\n+\t# the environmental variable MOD_PERL has 'mod_perl/VERSION' value if set\n+\tmy $mod_perl_version = $ENV{'MOD_PERL'} ? \" $ENV{'MOD_PERL'}\" : '';\n \tprint <<EOF;\n <?xml version=\"1.0\" encoding=\"utf-8\"?>\n <!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n@@ -1732,7 +1734,7 @@ sub git_header_html {\n <!-- git core binaries version $git_version -->\n <head>\n <meta http-equiv=\"content-type\" content=\"$content_type; charset=utf-8\"/>\n-<meta name=\"generator\" content=\"gitweb/$version git/$git_version\"/>\n+<meta name=\"generator\" content=\"gitweb/$version git/$git_version$mod_perl_version\"/>\n <meta name=\"robots\" content=\"index, nofollow\"/>\n <title>$title</title>\n EOF\n-- \n1.4.4.3\n"},{"id":"30371","messageId":"200612280000.52586.jnareb@gmail.com","threadId":"6133","inReplyTo":"200612272355.31923.jnareb@gmail.com","subject":"[PATCH 3/6] gitweb: Precompile CGI routines for mod_perl","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-27T23:00:52Z","receivedAt":"2006-12-27T23:00:52Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Following advice from CGI(3pm) man page, precompile all CGI routines\nfor mod_perl, in the BEGIN block.\n\n\n  If you want to compile without importing use the compile() method\n  instead:\n\n    use CGI();\n    CGI->compile();\n\n  This is particularly useful in a mod_perl environment, in which you\n  might want to precompile all CGI routines in a startup script, and then\n  import the functions individually in each mod_perl script.\n\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nCould help. I'm a bit unsure.\n\n gitweb/gitweb.perl |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex bb1d66c..3888563 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -18,6 +18,10 @@ use File::Find qw();\n use File::Basename qw(basename);\n binmode STDOUT, ':utf8';\n \n+BEGIN {\n+\tCGI->compile() if $ENV{MOD_PERL};\n+}\n+\n our $cgi = new CGI;\n our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n-- \n1.4.4.3\n"},{"id":"30373","messageId":"200612280004.32728.jnareb@gmail.com","threadId":"6133","inReplyTo":"200612272355.31923.jnareb@gmail.com","subject":"[PATCH/RFC 4/6] gitweb: Prepare for mod_perl specific support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-27T23:04:32Z","receivedAt":"2006-12-27T23:04:32Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Prepare gitweb for mod_perl specific support in CGI compatibility mode\n(Apache::Registry/ModPerl::Registry or Apache::PerlRun/ModPerl::PerlRun)\nby storing request (an argument to a handler) in $r variable, for later\nuse.\n\nThe idea is to have gitweb run as CGI script, under mod_perl 1.0 and under\nmod_perl 2.0 without modifications, while being able to make use of mod_perl\ncapabilities.\n\nDefine MP_GEN constant and set it to 0 if mod_perl is not available,\nto 1 if running under mod_perl 1.0, and 2 for mod_perl 2.0. It is later used\nin BEGIN block to load appropriate mod_perl modules; for now the one\nin which request is defined, and the one with status and HTTP constants.\nBased on \"Porting Apache:: Perl Modules from mod_perl 1.0 to 2.0\" document\n  http://perl.apache.org/docs/2.0/user/porting/porting.html\nchapter \"Making Code Conditional on Running mod_perl Version\".\n\nUse \"if (MP_GEN)\" for checking if gitweb is run under mod_perl; later\non we will use \"if ($r)\" for that.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nA bit of RFC, because I'm not sure if \"my $r\" or \"our $r\" should be\nused (in script which makes use of subroutines; under Registry those\nwould end as nested subroutines).\n\nPerhaps we should import everything?\n\n gitweb/gitweb.perl |   28 +++++++++++++++++++++++++++-\n 1 files changed, 27 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3888563..9983e9e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -18,10 +18,36 @@ use File::Find qw();\n use File::Basename qw(basename);\n binmode STDOUT, ':utf8';\n \n+# Set the constant MP_GEN to 0 if mod_perl is not available,\n+# to 1 if running under mod_perl 1.0\n+# and 2 for mod_perl 2.0\n+use constant {\n+\tMP_GEN => ($ENV{'MOD_PERL'}\n+\t           ? ( exists $ENV{'MOD_PERL_API_VERSION'} and \n+\t                      $ENV{'MOD_PERL_API_VERSION'} >= 2 ) ? 2 : 1\n+\t           : 0),\n+};\n+\n BEGIN {\n-\tCGI->compile() if $ENV{MOD_PERL};\n+\t# use appropriate mod_perl modules (conditional use)\n+\tif (MP_GEN == 2) {\n+\t\trequire Apache2::RequestRec;\n+\t\trequire Apache2::Const;\n+\t\tApache2::Const->import(-compile => qw(:common :http));\n+\t} elsif (MP_GEN == 1) {\n+\t\trequire Apache;\n+\t\trequire Apache::Constants;\n+\t\tApache::Constants->import(qw(:common :http));\n+\t}\n+\n+\t# precompile CGI for mod_perl\n+\tCGI->compile() if MP_GEN;\n }\n \n+# mod_perl request\n+my $r;\n+$r = shift @_ if MP_GEN;\n+\n our $cgi = new CGI;\n our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n-- \n1.4.4.3\n"},{"id":"30375","messageId":"200612280049.13385.jnareb@gmail.com","threadId":"6133","inReplyTo":"200612272355.31923.jnareb@gmail.com","subject":"[PATCH/RFC 5/6] gitweb: Make possible to run under mod_perl without SetupEnv","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-27T23:49:12Z","receivedAt":"2006-12-27T23:49:12Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Make possible to run gitweb under mod_perl without need to set up CGI\nenvironmental variables (i.e. \"PerlOptions -SetupEnv\" in mod_perl 2.0,\n\"PerlSetupEnv Off\" in mod_perl 1.0).\n\nActually ModPerl::Registry / Apache::Registry populates %ENV hash,\nwithout need to actually set environmental variables.\n\n\nPass the request variable $r to CGI constructor if CGI.pm module is\nnew enough (at least CGI version 2.93, and at least 3.11 for mod_perl\n2.0).\n\nReplace $ENV{'PATH_INFO'} by $r->path_info() if we use mod_perl.\n\nReplace $ENV{'SERVER_NAME'} by $r->server()->server_hostname() if we\nuse mod_perl.\n\nUniquify using of %ENV to $ENV{'NAME'}, while at it.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis patch appears to be unnecessary, at least for now, because\nmod_perl Registry populates %ENV hash (and does not need to set\nenvirionmental variables). Still, it prepares the way for future\nrunning gitweb as mod_perl handler, and not under Registry.\n\nPerhaps the cleanup part of this patch should be put into separate\npatch...\n\n gitweb/gitweb.perl |   20 +++++++++++++++++---\n 1 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 9983e9e..2900ae6 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -32,11 +32,16 @@ BEGIN {\n \t# use appropriate mod_perl modules (conditional use)\n \tif (MP_GEN == 2) {\n \t\trequire Apache2::RequestRec;\n+\t\trequire Apache2::ServerRec;\n+\t\trequire Apache2::Response;\n \t\trequire Apache2::Const;\n+\t\tApache2::RequestRec->import();\n+\t\tApache2::ServerRec->import();\n \t\tApache2::Const->import(-compile => qw(:common :http));\n \t} elsif (MP_GEN == 1) {\n \t\trequire Apache;\n \t\trequire Apache::Constants;\n+\t\timport Apache;\n \t\tApache::Constants->import(qw(:common :http));\n \t}\n \n@@ -48,7 +53,15 @@ BEGIN {\n my $r;\n $r = shift @_ if MP_GEN;\n \n-our $cgi = new CGI;\n+our $cgi;\n+if ((MP_GEN == 1 && $CGI::VERSION >= 2.93) ||\n+    (MP_GEN == 2 && $CGI::VERSION >= 3.11)) {\n+\t# CGI.pm is new enough\n+\t$cgi = new CGI($r);\n+} else {\n+\t$cgi = new CGI;\n+}\n+\n our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n our $my_uri = $cgi->url(-absolute => 1);\n@@ -70,7 +83,8 @@ our $home_link_str = \"++GITWEB_HOME_LINK_STR++\";\n # name of your site or organization to appear in page titles\n # replace this with something more descriptive for clearer bookmarks\n our $site_name = \"++GITWEB_SITENAME++\"\n-                 || ($ENV{'SERVER_NAME'} || \"Untitled\") . \" Git\";\n+                 || (($r ? $r->server()->server_hostname() : $ENV{'SERVER_NAME'})\n+                     || \"Untitled\") . \" Git\";\n \n # filename of html text to include at top of each page\n our $site_header = \"++GITWEB_SITE_HEADER++\";\n@@ -403,7 +417,7 @@ if (defined $searchtype) {\n # now read PATH_INFO and use it as alternative to parameters\n sub evaluate_path_info {\n \treturn if defined $project;\n-\tmy $path_info = $ENV{\"PATH_INFO\"};\n+\tmy $path_info = $r ? $r->path_info() : $ENV{'PATH_INFO'};\n \treturn if !$path_info;\n \t$path_info =~ s,^/+,,;\n \treturn if !$path_info;\n-- \n1.4.4.3\n"},{"id":"30372","messageId":"200612280106.24331.jnareb@gmail.com","threadId":"6133","inReplyTo":"200612272355.31923.jnareb@gmail.com","subject":"[RFC/PATCH 6/6] gitweb: Make possible to run under mod_perl without ParseHeaders","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-28T00:06:24Z","receivedAt":"2006-12-28T00:06:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add mod_perl version of http_header, setting headers directly (both\nfor mod_perl 2.0 and 1.0); bits of code taken from CGI and CGI::Util\nmodules.  While at it add cache validation via $r->meets_conditions()\nin mod_perl code.\n\nSeparate HTTP redirection into http_redirect subroutine and add\nmod_perl version, setting headers directly.\n\nAll this is meant to allow gitweb to run under ModPerl::Registry (for\nmod_perl 2.0) / Apache::Registry (for mod_perl 1.0) without need for\nApache to parse headers (without ParseHeaders), which should speed up\ngitweb a bit.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis patch tries to add HTTP headers directly, allowing Apache to not\nneed to parse headers (without ParseHeaders), which should speed\nup gitweb a bit. It also makes use of mod_perl meets_expectation\nmethod to respond to If-Modified-Since: and If-None-Match: requests\nfor cache validation. Current state is a bit of mess as it is now.\nComments (and patches) appreciated.\n\nIt is not benchmarked because ApacheBench went crazy, showing *negative*\nwaiting time. Probably I did something wrong...\n\nNot checked for warnings, only slightly tested: definitely an RFC.\n\n gitweb/gitweb.perl |  137 +++++++++++++++++++++++++++++++++++++++++++++++++---\n 1 files changed, 130 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 2900ae6..12f1cb2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -35,13 +35,17 @@ BEGIN {\n \t\trequire Apache2::ServerRec;\n \t\trequire Apache2::Response;\n \t\trequire Apache2::Const;\n+\t\trequire APR::Date;\n \t\tApache2::RequestRec->import();\n \t\tApache2::ServerRec->import();\n \t\tApache2::Const->import(-compile => qw(:common :http));\n+\t\tAPR::Date->import();\n \t} elsif (MP_GEN == 1) {\n \t\trequire Apache;\n \t\trequire Apache::Constants;\n+\t\trequire Apache::File;\n \t\timport Apache;\n+\t\timport Apache::File;\n \t\tApache::Constants->import(qw(:common :http));\n \t}\n \n@@ -1727,11 +1731,129 @@ sub blob_mimetype {\n sub http_header {\n \tmy @header = @_;\n \n-\tprint $cgi->header(@header);\n+\tif (MP_GEN) {\n+\t\tmy %header = @header;\n+\t\tmy $cache_validator;\n+\n+\t\t## special cases ##\n+\t\t# -status\n+\t\t$r->status_line($header{-status})\n+\t\t\tif $header{-status};\n+\t\tdelete $header{-status} if exists $header{-status};\n+\t\t# -type and -charset\n+\t\tif ($header{-type} || $header{-charset}) {\n+\t\t\tmy $type = $header{-type} || 'text/html';\n+\t\t\t$type .= \"; charset=$header{-charset}\"\n+\t\t\t\tif $type =~ m!^text/! and $type !~ /\\bcharset\\b/ and $header{-charset};\n+\n+\t\t\t$r->content_type($type);\n+\t\t}\n+\t\tdelete $header{-type} if exists $header{-type};\n+\t\tdelete $header{-charset} if exists $header{-charset};\n+\t\t# -content_encoding\n+\t\t$r->content_encoding($header{-content_encoding})\n+\t\t\tif $header{-content_encoding};\n+\t\tdelete $header{-content_encoding} if exists $header{-content_encoding};\n+\t\t# -expires\n+\t\tif ($header{-expires}) {\n+\t\t\tmy $expires = CGI::Util::expires($header{-expires}, 'http');\n+\t\t\tif (MP_GEN == 1) {\n+\t\t\t\t$r->header_out('Expires', $expires);\n+\t\t\t} else {\n+\t\t\t\t$r->headers_out->add('Expires', $expires);\n+\t\t\t}\n+\t\t}\n+\t\tdelete $header{-expires} if exists $header{-expires};\n+\t\t# -last_modified\n+\t\tif ($header{-last_modified}) {\n+\t\t\t$cache_validator ||= 1;\n+\t\t\tif (MP_GEN == 1) {\n+\t\t\t\t$r->header_out('Last-Modified', $header{-last_modified});\n+\t\t\t} else {\n+\t\t\t\t$r->set_last_modified(APR::Date::parse_http($header{-last_modified}));\n+\t\t\t}\n+\t\t}\n+\t\tdelete $header{-last_modified} if exists $header{-last_modified};\n+\n+\t\t## other headers ##\n+\t\twhile (my ($key, $value) = each %header) {\n+\t\t\t$key =~ s/^-//; # -content_disposition -> content_disposition\n+\t\t\t$key =~ s/_/-/; #  content_disposition -> content-disposition\n+\t\t\t$key =~ s/(\\w)(\\w*)/\\u$1$2/g;\n+\t\t\t                #  content-disposition -> Content-Disposition\n+\n+\t\t\tif (MP_GEN == 1) {\n+\t\t\t\t$r->header_out($key, $value);\n+\t\t\t} else {\n+\t\t\t\t$r->headers_out->add($key, $value);\n+\t\t\t}\n+\t\t}\n+\t\t$cache_validator ||= (exists $header{-ETag} || exists $header{-etag});\n+\n+\t\t## send headers / flush ##\n+\t\tif (MP_GEN == 1) {\n+\t\t\t$r->send_http_headers();\n+\n+\t\t\t## validate cache ##\n+\t\t\tif ($cache_validator &&\n+\t\t\t    (my $rc = $r->meets_conditions()) != Apache::Constant::OK) {\n+\t\t\t\treturn $rc;\n+\t\t\t}\n+\t\t} else {\n+\t\t\t$r->rflush();\n+\n+\t\t\t## validate cache ##\n+\t\t\tif ($cache_validator &&\n+\t\t\t    (my $rc = $r->meets_conditions()) != Apache2::Const::OK) {\n+\t\t\t\treturn $rc;\n+\t\t\t}\n+\t\t}\n+\t} else {\n+\t\tprint $cgi->header(@header);\n+\t}\n \n \t# Optimization: skip generating the body if client asks only\n \t# for HTTP header (e.g. cache validation).\n-\treturn if ($cgi->request_method() eq 'HEAD');\n+\tif (MP_GEN == 2) {\n+\t\treturn Apache2::Const::OK   if $r->header_only();\n+\t} elsif (MP_GEN == 1) {\n+\t\treturn Apache::Constant::OK if $r->header_only();\n+\t} else {\n+\t\treturn if ($cgi->request_method() eq 'HEAD');\n+\t}\n+}\n+\n+sub http_redirect {\n+\tmy @params = @_;\n+\tmy %params;\n+\tif (@params % 2 == 0) {\n+\t\t%params = @params;\n+\t}\n+\tmy $uri = $params{-uri} || $params{-url} || $params{-location}\n+\t\t|| $params[0] || $cgi->self_url;\n+\tmy $status = $params{-status};\n+\n+\tif (MP_GEN == 1) {\n+\t\t$r->header_out('Location', $uri);\n+\t\tif (defined $status) {\n+\t\t\t$r->status_line($status);\n+\t\t} else {\n+\t\t\t$r->status(Apache::Constant::REDIRECT);\n+\t\t}\n+\n+\t\t$r->send_http_headers();\n+\t} elsif (MP_GEN == 2) {\n+\t\t$r->headers_out->add('Location', $uri);\n+\t\tif (defined $status) {\n+\t\t\t$r->status_line($status);\n+\t\t} else {\n+\t\t\t$r->status(Apache2::Const::REDIRECT);\n+\t\t}\n+\n+\t\t$r->rflush();\n+\t} else {\n+\t\tprint $cgi->redirect(@params);\n+\t}\n }\n \n sub git_header_html {\n@@ -1854,7 +1976,8 @@ EOF\n \t\t      $cgi->hidden(-name => \"a\") . \"\\n\" .\n \t\t      $cgi->hidden(-name => \"h\") . \"\\n\" .\n \t\t      $cgi->popup_menu(-name => 'st', -default => 'commit',\n-\t\t\t\t       -values => ['commit', 'author', 'committer', 'pickaxe']) .\n+\t\t                       -values => ['commit', 'author', 'committer',\n+\t\t                       gitweb_check_feature('pickaxe') ? 'pickaxe' : ()]) .\n \t\t      $cgi->sup($cgi->a({-href => href(action=>\"search_help\")}, \"?\")) .\n \t\t      \" search:\\n\",\n \t\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n@@ -3901,10 +4024,10 @@ sub git_object {\n \t\tdie_error('404 Not Found', \"Not enough information to find object\");\n \t}\n \n-\tprint $cgi->redirect(-uri => href(action=>$type, -full=>1,\n-\t                                  hash=>$hash, hash_base=>$hash_base,\n-\t                                  file_name=>$file_name),\n-\t                     -status => '302 Found');\n+\thttp_redirect(-uri => href(action=>$type, -full=>1,\n+\t                           hash=>$hash, hash_base=>$hash_base,\n+\t                           file_name=>$file_name),\n+\t              -status => '302 Found');\n }\n \n sub git_blobdiff {\n-- \n1.4.4.3\n"},{"id":"30378","messageId":"20061228010311.GD6558@localhost","threadId":"6133","inReplyTo":"200612280106.24331.jnareb@gmail.com","subject":"Re: [RFC/PATCH 6/6] gitweb: Make possible to run under mod_perl without ParseHeaders","fromName":"Robert Fitzsimons","fromEmail":"robfitz@273k.net","sentAt":"2006-12-28T01:03:11Z","receivedAt":"2006-12-28T01:03:11Z","isPatch":true,"sender":{"key":"robfitz@273k.net","avatar":null},"body":"> @@ -1854,7 +1976,8 @@ EOF\n>  \t\t      $cgi->hidden(-name => \"a\") . \"\\n\" .\n>  \t\t      $cgi->hidden(-name => \"h\") . \"\\n\" .\n>  \t\t      $cgi->popup_menu(-name => 'st', -default => 'commit',\n> -\t\t\t\t       -values => ['commit', 'author', 'committer', 'pickaxe']) .\n> +\t\t                       -values => ['commit', 'author', 'committer',\n> +\t\t                       gitweb_check_feature('pickaxe') ? 'pickaxe' : ()]) .\n>  \t\t      $cgi->sup($cgi->a({-href => href(action=>\"search_help\")}, \"?\")) .\n>  \t\t      \" search:\\n\",\n>  \t\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n\nThis should be a separate patch.\n\nRobert\n"},{"id":"30379","messageId":"200612280212.45120.jnareb@gmail.com","threadId":"6133","inReplyTo":"20061228010311.GD6558@localhost","subject":"Re: [RFC/PATCH 6/6] gitweb: Make possible to run under mod_perl without ParseHeaders","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-28T01:12:44Z","receivedAt":"2006-12-28T01:12:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Robert Fitzsimons wrote:\n>> @@ -1854,7 +1976,8 @@ EOF\n>>  \t\t      $cgi->hidden(-name => \"a\") . \"\\n\" .\n>>  \t\t      $cgi->hidden(-name => \"h\") . \"\\n\" .\n>>  \t\t      $cgi->popup_menu(-name => 'st', -default => 'commit',\n>> -\t\t\t\t       -values => ['commit', 'author', 'committer', 'pickaxe']) .\n>> +\t\t                       -values => ['commit', 'author', 'committer',\n>> +\t\t                       gitweb_check_feature('pickaxe') ? 'pickaxe' : ()]) .\n>>  \t\t      $cgi->sup($cgi->a({-href => href(action=>\"search_help\")}, \"?\")) .\n>>  \t\t      \" search:\\n\",\n>>  \t\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n> \n> This should be a separate patch.\n\nI'm sorry, somehow I missed this.\n\nBesides, it would be better to assign return value to some\nvariable, to avoid calling gitweb_check_feature again... \n-- \nJakub Narebski\nPoland\n"},{"id":"30380","messageId":"7v7iwc4xu9.fsf@assigned-by-dhcp.cox.net","threadId":"6133","inReplyTo":"200612272357.56532.jnareb@gmail.com","subject":"Re: [PATCH 1/6] gitweb: Separate HTTP header output","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-28T01:23:58Z","receivedAt":"2006-12-28T01:23:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Always return just after HTTP header is sent when asking only about\n> headers (HTTP request method 'HEAD'); first appeared in git_rss.\n>\n> While at it uniquify style of http_header(...) calls, formerly\n> \"print $cgi->header(...)\", and remove default HTTP status, '200 OK'.\n>\n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n> ---\n> This one is fairly generic, and if considered worthy, I think\n> can be accepted without much ado.\n\nMaybe I am missing something fundamental, but I cannot see how\nthis affects anything whatsoever...\n\n> +## functions printing HTTP or HTML: header, footer, error page\n> +\n> +sub http_header {\n> +\tmy @header = @_;\n> +\n> +\tprint $cgi->header(@header);\n> +\n> +\t# Optimization: skip generating the body if client asks only\n> +\t# for HTTP header (e.g. cache validation).\n> +\treturn if ($cgi->request_method() eq 'HEAD');\n> +}\n\nOk, so this explicitly written \"return\" returns when it is a\nHEAD request not GET.  Otherwise the control falls out of the\nend of the function.  Either way you return undef.\n\nThen the caller does...\n\n> @@ -1709,8 +1719,11 @@ sub git_header_html {\n>  \t} else {\n>  \t\t$content_type = 'text/html';\n>  \t}\n> -\tprint $cgi->header(-type=>$content_type, -charset => 'utf-8',\n> -\t                   -status=> $status, -expires => $expires);\n> +\thttp_header(\n> +\t\t-type => $content_type,\n> +\t\t-charset => 'utf-8',\n> +\t\t-status => $status,\n> +\t\t-expires => $expires);\n>  \tprint <<EOF;\n>  <?xml version=\"1.0\" encoding=\"utf-8\"?>\n>  <!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n\nwhich means it does not omit generating the body anyway no\nmatter what \"sub http_header\" did...\n\nOr is there some Perl magic that makes a return from sub named\n*_header magically terminate the execution of the caller?\n\nPuzzled...\n"},{"id":"30381","messageId":"20061228012800.GA16612@spearce.org","threadId":"6133","inReplyTo":"7v7iwc4xu9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/6] gitweb: Separate HTTP header output","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-28T01:28:00Z","receivedAt":"2006-12-28T01:28:00Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> which means it does not omit generating the body anyway no\n> matter what \"sub http_header\" did...\n> \n> Or is there some Perl magic that makes a return from sub named\n> *_header magically terminate the execution of the caller?\n\nNo magic.  Bad patch.  Your assessment of the patch is correct;\nit is not avoiding the body generation for a HEAD request.\n\n-- \nShawn.\n"}]}