{"thread":{"id":"25573","subject":"[PATCH 0/3] Gitweb caching v7","startedAt":"2010-10-28T00:42:51Z","lastAt":"2010-12-02T19:36:46Z","messageCount":43,"participants":["John 'Warthog9' Hawley","Jakub Narebski","Junio C Hamano","demerphq"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"154600","messageId":"1288226574-19068-1-git-send-email-warthog9@eaglescrag.net","threadId":"25573","inReplyTo":null,"subject":"[PATCH 0/3] Gitweb caching v7","fromName":"John 'Warthog9' Hawley","fromEmail":"warthog9@eaglescrag.net","sentAt":"2010-10-28T00:42:51Z","receivedAt":"2010-10-28T00:42:51Z","isPatch":true,"sender":{"key":"warthog9@kernel.org","avatar":"https://avatars.githubusercontent.com/u/2334704?v=4"},"body":"\nEvening everyone,\n \nThis is the latest incarnation of gitweb w/ caching. Per the general\nconsensus and requests from the recent Gittogether I'm re-submitting \nmy patches.\n\nBiggest changes are bringing it back in line with Junio's tree, couple\nof locking fixes (specifically targeted at binary files), and at the \nnagging behest of Sverre I've cut the old patch down to a much more\nmanageable (and in his words \"reviewable\") series.\n\nThis still differs, slightly, from whats in production on kernel.org.\nIt's missing the index page git:// link, and kernel.org is still running\nthe broken out output buffering but I'm likely to flip over to this just\nso I'm not maintaining quite so much (and the changes are easier to track\nwith upstream)\n\nv7:\n\t- Rework output system, now central STDOUT redirect\n\t- Various fixes to caching brought in from existing\n\t running system\n\nv6:\n\t- Never saw the light of day\n\t- Various testing, and reworks.\n\nv5:\n\t- Missed a couple of things that were in my local tree, and\n\t added them back in.\n\t- Split up the die_error and the version matching patch\n\t- Set version matching to be on by default - otherwise this\n\t really is code that will never get checked, or at best\n\t enabled by default by distributions\n\t- Added a minor code cleanup with respect to $site_header\n\t that was already in my tree\n\t- Applied against a more recent git tree vs. 1.6.6-rc2\n\t- Removed breakout patch for now (did that in v4 actually)\n\t and will deal with that separately \n\n\thttp://git.kernel.org/?p=git/warthog9/gitweb.git;a=shortlog;h=refs/heads/gitweb-ml-v5\n\nv4:\n\t- major re-working of the caching layer to use file handle\n\t redirection instead of buffering output\n\t- other minor improvements\n\n\thttp://git.kernel.org/?p=git/warthog9/gitweb.git;a=shortlog;h=refs/heads/gitweb-ml-v4\nv3:\n\t- various minor re-works based on mailing list feedback,\n\t this series was not sent to the mailing list.\nv2:\n\t- Better breakout\n\t- You can actually disable the cache now\n\n- John 'Warthog9' Hawley \n\n\nJohn 'Warthog9' Hawley (3):\n gitweb: Add option to force version match\n gitweb: add output buffering and associated functions\n gitweb: File based caching layer (from git.kernel.org)\n\n gitweb/README | 4 +\n gitweb/cache.pm | 365 ++++++++++++++++++++++++++++++++++++++++++++++\n gitweb/gitweb.perl | 147 ++++++++++++++++++-\n gitweb/static/gitweb.css | 6 +\n 4 files changed, 514 insertions(+), 8 deletions(-)\n create mode 100644 gitweb/cache.pm\n\n-- \n1.7.2.3\n"},{"id":"154602","messageId":"1288226574-19068-2-git-send-email-warthog9@eaglescrag.net","threadId":"25573","inReplyTo":"1288226574-19068-1-git-send-email-warthog9@eaglescrag.net","subject":"[PATCH 1/3] gitweb: Add option to force version match","fromName":"John 'Warthog9' Hawley","fromEmail":"warthog9@eaglescrag.net","sentAt":"2010-10-28T00:42:52Z","receivedAt":"2010-10-28T00:42:52Z","isPatch":true,"sender":{"key":"warthog9@kernel.org","avatar":"https://avatars.githubusercontent.com/u/2334704?v=4"},"body":"\nThis adds $git_versions_must_match variable, which is set to true,\nchecks that we are running on the same version of git that we\nshipped with, and if not throw '500 Internal Server Error' error.\nWhat is checked is the version of gitweb (embedded in building\ngitweb.cgi), against version of runtime git binary used.\n\nGitweb can usually run with a mismatched git install. This is more\nhere to give an obvious warning as to whats going on vs. silently\nfailing.\n\nBy default this feature is turned on.\n\nSigned-off-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\n gitweb/README | 4 ++++\n gitweb/gitweb.perl | 27 +++++++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 0 deletions(-)\n\n\n\ndiff --git a/gitweb/README b/gitweb/README\nindex bf3664f..7ee8450 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -246,6 +246,10 @@ not include variables usually directly set during build):\n http://www.andre-simon.de due to assumptions about parameters and output).\n Useful if highlight is not installed on your webserver's PATH.\n [Default: highlight]\n+ * $git_versions_must_match\n+ If set, gitweb fails with 500 Internal Server Error if the version of gitweb\n+ doesn't match version of git binary. The default is true.\n+\n \n Projects list file format\n ~~~~~~~~~~~~~~~~~~~~~~~~~\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8d7e4c5..215a4e9 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -232,6 +232,9 @@ our %avatar_size = (\n \t'double' => 32\n );\n \n+# If it is true, exit if gitweb version and git binary version don't match\n+our $git_versions_must_match = 1;\n+\n # Used to set the maximum load that we will still respond to gitweb queries.\n # If server load exceed this value then return \"503 server busy\" error.\n # If gitweb cannot determined server load, it is taken to be 0.\n@@ -649,6 +652,29 @@ sub check_loadavg {\n \t}\n }\n \n+sub check_versionmatch {\n+\t# Throw an error if git versions does not match, if $git_versions_must_match is true.\n+\tif ($git_versions_must_match &&\n+\t $git_version ne $version) {\n+\t\tmy $admin_contact =\n+\t\t\tdefined $ENV{'SERVER_ADMIN'} ? \", $ENV{'SERVER_ADMIN'},\" : '';\n+\t\tmy $err_msg = <*** Warning ***\n+

\n+This version of gitweb was compiled for @{[esc_html($version)]},\n+however git version @{[esc_html($git_version)]} was found on server,\n+and administrator requested strict version checking.\n+

\n+

\n+Please contact the server administrator${admin_contact} to either configure\n+gitweb to allow mismatched versions, or update git or gitweb installation.\n+

\n+EOT\n+\t\tdie_error(500, 'Internal server error', $err_msg);\n+\t}\n+\n+}\n+\n # ======================================================================\n # input validation and dispatch\n \n@@ -1075,6 +1101,7 @@ sub run_request {\n \tevaluate_uri();\n \tevaluate_gitweb_config();\n \tcheck_loadavg();\n+\tcheck_versionmatch();\n \n \t# $projectroot and $projects_list might be set in gitweb config file\n \t$projects_list ||= $projectroot;\n"},{"id":"154601","messageId":"1288226574-19068-3-git-send-email-warthog9@eaglescrag.net","threadId":"25573","inReplyTo":"1288226574-19068-1-git-send-email-warthog9@eaglescrag.net","subject":"[PATCH 2/3] gitweb: add output buffering and associated functions","fromName":"John 'Warthog9' Hawley","fromEmail":"warthog9@eaglescrag.net","sentAt":"2010-10-28T00:42:53Z","receivedAt":"2010-10-28T00:42:53Z","isPatch":true,"sender":{"key":"warthog9@kernel.org","avatar":"https://avatars.githubusercontent.com/u/2334704?v=4"},"body":"\nThis adds output buffering for gitweb, mainly in preparation for\ncaching support. This is a dramatic change to how caching was being\ndone, mainly in passing around the variable manually and such.\n\nThis centrally flips the entire STDOUT to a variable, which after the\ncompletion of the run, flips it back and does a print on the resulting\ndata.\n\nThis should save on the previous 10K line patch (or so) that adds more\nexplicit output passing.\n---\n gitweb/gitweb.perl | 29 +++++++++++++++++++++++++++++\n 1 files changed, 29 insertions(+), 0 deletions(-)\n\n\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 215a4e9..757ef46 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -30,6 +30,9 @@ BEGIN {\n \n our $version = \"++GIT_VERSION++\";\n \n+# Output buffer variable\n+$output = \"\";\n+\n our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n sub evaluate_uri {\n \tour $cgi;\n@@ -1151,6 +1154,25 @@ sub evaluate_argv {\n \t);\n }\n \n+sub change_output {\n+\tour $output;\n+\n+\t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n+\topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n+\n+\t# Close STDOUT, so that it isn't being used anymore.\n+\tclose STDOUT;\n+\n+\t# Trap STDOUT to the $output variable, which is what I was using in the original\n+\t# patch anyway.\n+\topen(STDOUT,\">\", \\$output) || die \"Unable to open STDOUT: $!\"; #open STDOUT handle to use $var\n+}\n+\n+sub reset_output {\n+\t# This basically takes STDOUT_REAL and puts it back as STDOUTl\n+\topen(STDOUT,\">&STDOUT_REAL\");\n+}\n+\n sub run {\n \tevaluate_argv();\n \tevaluate_git_version();\n@@ -1163,7 +1185,10 @@ sub run {\n \t\t$pre_dispatch_hook->()\n \t\t\tif $pre_dispatch_hook;\n \n+\t\tchange_output();\n \t\trun_request();\n+\t\treset_output();\n+\t\tprint $output;\n \n \t\t$post_dispatch_hook->()\n \t\t\tif $post_dispatch_hook;\n@@ -3673,6 +3698,10 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\t# Reset the output so that we are actually going to STDOUT as opposed\n+\t# to buffering the output.\n+\treset_output();\n+\n \tgit_header_html($http_responses{$status}, undef, %opts);\n \tprint <\n"},{"id":"154603","messageId":"1288226574-19068-4-git-send-email-warthog9@eaglescrag.net","threadId":"25573","inReplyTo":"1288226574-19068-1-git-send-email-warthog9@eaglescrag.net","subject":"[PATCH 3/3] gitweb: File based caching layer (from git.kernel.org)","fromName":"John 'Warthog9' Hawley","fromEmail":"warthog9@eaglescrag.net","sentAt":"2010-10-28T00:42:54Z","receivedAt":"2010-10-28T00:42:54Z","isPatch":true,"sender":{"key":"warthog9@kernel.org","avatar":"https://avatars.githubusercontent.com/u/2334704?v=4"},"body":"\nThis is a relatively large patch that implements the file based\ncaching layer that is quite similar to the one used on such large\nsites as kernel.org and soon git.fedoraproject.org. This provides\na simple, and straight forward caching mechanism that scales\ndramatically better than Gitweb by itself.\n\nThe caching layer basically buffers the output that Gitweb would\nnormally return, and saves that output to a cache file on the local\ndisk. When the file is requested it attempts to gain a shared lock\non the cache file and cat it out to the client. Should an exclusive\nlock be on a file (it's being updated) the code has a choice to either\nupdate in the background and go ahead and show the stale page while\nupdate is being performed, or stall the client(s) until the page\nis generated.\n\nThere are two forms of stalling involved here, background building\nand non-background building, both of which are discussed in the\nconfiguration page.\n\nThere are still a few known \"issues\" with respect to this:\n- Code needs to be added to be \"browser\" aware so\n that clients like wget that are trying to get a\n binary blob don't obtain a \"Generating...\" page\n\nCaching is disabled by default with the $cache_enable variable,\nsetting this to 1 will enable file based caching. It is expected\nthat this will be extended to include additional types of caching\n(like memcached) in the future and should not be exclusively\nconsidered a binary value.\n---\n gitweb/cache.pm | 365 ++++++++++++++++++++++++++++++++++++++++++++++\n gitweb/gitweb.perl | 99 +++++++++++--\n gitweb/static/gitweb.css | 6 +\n 3 files changed, 458 insertions(+), 12 deletions(-)\n create mode 100644 gitweb/cache.pm\n\n\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nnew file mode 100644\nindex 0000000..c86265c\n--- /dev/null\n+++ b/gitweb/cache.pm\n@@ -0,0 +1,365 @@\n+# gitweb - simple web interface to track changes in git repositories\n+#\n+# (C) 2006, John 'Warthog9' Hawley \n+#\n+# This program is licensed under the GPLv2\n+\n+#\n+# Gitweb caching engine\n+#\n+\n+#use File::Path qw(make_path remove_tree);\n+use File::Path qw(mkpath rmtree); # Used for compatability reasons\n+use Digest::MD5 qw(md5 md5_hex md5_base64);\n+use Fcntl ':flock';\n+use File::Copy;\n+\n+sub cache_fetch {\n+\tmy ($action) = @_;\n+\tmy $cacheTime = 0;\n+\n+\t# Deal with cache being disabled\n+\tif( $cache_enable == 0 ){\n+\t\tundef $backgroundCache;\n+\t\t#$fullhashpath = \\$nocachedata;\n+\t\t#$fullhashbinpath = \\$nocachedatabin;\n+\t\t$fullhashpath = *STDOUT;\n+\t\t$fullhashbinpath = *STDOUT;\n+\t\t$actions{$action}->();\n+\t\treturn;\n+\t}elsif( $cache_enable == 1 ){\n+\t\t#obviously we are using file based caching\n+\n+\t\tif(! -d $cachedir){\n+\t\t\tprint \"*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\\n\";\n+\t\t\tmkdir (\"cache\", 0665) || die \"Cannot create cache dir - you will need to manually create\";\n+\t\t\tprint \"Cache directory created successfully\\n\";\n+\t\t}\n+\n+\t\tour $full_url = \"$my_url?\". $ENV{'QUERY_STRING'};\n+\t\tour $urlhash = md5_hex($full_url);\n+\t\tour $fullhashdir = \"$cachedir/\". substr( $urlhash, 0, 2) .\"/\";\n+\n+\t\teval { mkpath( $fullhashdir, 0, 0777 ) };\n+\t\tif ($@) {\n+\t\t\tdie_error(500, \"Internal Server Error\", \"Could not create cache directory: $@\");\n+\t\t}\n+\t\t$fullhashpath = \"$fullhashdir/\". substr( $urlhash, 2 );\n+\t\t$fullhashbinpath = \"$fullhashpath.bin.wt\";\n+\t\t$fullhashbinpathfinal = \"$fullhashpath.bin\";\n+\t} # done dealing with cache enabled / disabled\n+\n+\tif(! -e \"$fullhashpath\" ){\n+\t\tif( ! $cache_enable || ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\tcacheUpdate($action,0);\n+\t\t\tcacheDisplay($action);\n+\t\t} elsif ( $childPid == 0 ){\n+\t\t\t#run the updater\n+\t\t\tcacheUpdate($action,1);\n+\t\t}else{\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\t}else{\n+\t\t#if cache is out dated, update\n+\t\t#else displayCache();\n+\t\topen(cacheFile, '<', \"$fullhashpath\");\n+\t\tstat(cacheFile);\n+\t\tclose(cacheFile);\n+\t\tmy $stat_time = (stat(_))[9];\n+\t\tmy $stat_size = (stat(_))[7];\n+\n+\t\t$cacheTime = get_loadavg() * 60;\n+\t\tif( $cacheTime > $maxCacheTime ){\n+\t\t\t$cacheTime = $maxCacheTime;\n+\t\t}\n+\t\tif( $cacheTime < $minCacheTime ){\n+\t\t\t$cacheTime = $minCacheTime;\n+\t\t}\n+\t\tif( $stat_time < (time - $cacheTime) || $stat_size == 0 ){\n+\t\t\tif( ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\t\tcacheUpdate($action,0);\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t} elsif ( $childPid == 0 ){\n+\t\t\t\t#run the updater\n+\t\t\t\t#print \"Running updater\\n\";\n+\t\t\t\tcacheUpdate($action,1);\n+\t\t\t}else{\n+\t\t\t\t#print \"Waiting for update\\n\";\n+\t\t\t\tcacheWaitForUpdate($action);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\n+\n+\t}\n+\n+\t#\n+\t# If all of the caching failes - lets go ahead and press on without it and fall back to 'default'\n+\t# non-caching behavior. This is the softest of the failure conditions.\n+\t#\n+\t#$actions{$action}->();\n+}\n+\n+sub cacheUpdate {\n+\tmy ($action,$areForked) = @_;\n+\tmy $lockingStatus;\n+\tmy $fileData = \"\";\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFileBG, '>:utf8', \"$fullhashpath.bg\");\n+\t\tmy $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStatBG;\n+\t}else{\n+\t\topen(cacheFile, '>:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n+\t\topen(cacheFile, '>:utf8', \\$fullhashpath) if ($cache_enable == 0);\n+\t\tmy $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStat;\n+\t}\n+\t#print \"lock status: $lockStat\\n\";\n+\n+\n+\tif ($cache_enable != 0 && ! $lockStatus ){\n+\t\tif ( $areForked ){\n+\t\t\texit(0);\n+\t\t}else{\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinWT, '>>:utf8', \"$fullhashbinpath\");\n+\t\tmy $lockStatBin = flock(cacheFileBinWT,LOCK_EX|LOCK_NB);\n+\t}\n+\n+\t# Trap all output from the action\n+\tchange_output();\n+\n+\t$actions{$action}->();\n+\n+\t# Reset the outputs as we should be fine now\n+\treset_output();\n+\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFile, '>:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_EX);\n+\n+\t\tif (! $lockStat ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinFINAL, '>:utf8', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBinFINAL,LOCK_EX);\n+\n+\t\tif (! $lockStatBIN ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# Actually dump the output to the proper file handler\n+\tlocal $/ = undef;\n+\t$|++;\n+\tprint cacheFile \"$output\";\n+\t$|--;\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmove(\"$fullhashbinpath\", \"$fullhashbinpathfinal\") or die \"Binary Cache file could not be updated: $!\";\n+\n+\t\tflock(cacheFileBinFINAL,LOCK_UN);\n+\t\tclose(cacheFileBinFINAL);\n+\n+\t\tflock(cacheFileBinWT,LOCK_UN);\n+\t\tclose(cacheFileBinWT);\n+\t}\n+\n+\tflock(cacheFile,LOCK_UN);\n+\tclose(cacheFile);\n+\n+\tif($backgroundCache){\n+\t\tflock(cacheFileBG,LOCK_UN);\n+\t\tclose(cacheFileBG);\n+\t}\n+\n+\tif ( $areForked ){\n+\t\texit(0);\n+\t} else {\n+\t\treturn;\n+\t}\n+}\n+\n+\n+sub cacheWaitForUpdate {\n+\tmy ($action) = @_;\n+\tmy $x = 0;\n+\tmy $max = 10;\n+\tmy $lockStat = 0;\n+\n+\tif( $backgroundCache ){\n+\t\tif( -e \"$fullhashpath\" ){\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n+\t\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tstat(cacheFile);\n+\t\t\tclose(cacheFile);\n+\n+\t\t\tif( $lockStat && ( (stat(_))[9] > (time - $maxCacheLife) ) ){\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"atom\"\n+\t\t||\n+\t\t$action eq \"rss\"\n+\t\t||\n+\t\t$action eq \"opml\"\n+\t){\n+\t\tdo {\n+\t\t\tsleep 2 if $x > 0;\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n+\t\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tclose(cacheFile);\n+\t\t\t$x++;\n+\t\t\t$combinedLockStat = $lockStat;\n+\t\t} while ((! $combinedLockStat) && ($x < $max));\n+\n+\t\tif( $x != $max ){\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\t\treturn;\n+\t}\n+\n+\t$| = 1;\n+\n+\tprint $::cgi->header(\n+\t\t\t\t-type=>'text/html',\n+\t\t\t\t-charset => 'utf-8',\n+\t\t\t\t-status=> 200,\n+\t\t\t\t-expires => 'now',\n+\t\t\t\t# HTTP/1.0\n+\t\t\t\t-Pragma => 'no-cache',\n+\t\t\t\t# HTTP/1.1\n+\t\t\t\t-Cache_Control => join(\n+\t\t\t\t\t\t\t', ',\n+\t\t\t\t\t\t\tqw(\n+\t\t\t\t\t\t\t\tprivate\n+\t\t\t\t\t\t\t\tno-cache\n+\t\t\t\t\t\t\t\tno-store\n+\t\t\t\t\t\t\t\tmust-revalidate\n+\t\t\t\t\t\t\t\tmax-age=0\n+\t\t\t\t\t\t\t\tpre-check=0\n+\t\t\t\t\t\t\t\tpost-check=0\n+\t\t\t\t\t\t\t)\n+\t\t\t\t\t\t)\n+\t\t\t\t);\n+\n+\tprint <\n+\n+\n+\n+\n+\n+\n+\n+$title\n+\n+\n+EOF\n+\n+\tprint \"Generating..\";\n+\tdo {\n+\t\tprint \".\";\n+\t\tsleep 2 if $x > 0;\n+\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n+\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n+\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\tclose(cacheFile);\n+\t\t$x++;\n+\t\t$combinedLockStat = $lockStat;\n+\t} while ((! $combinedLockStat) && ($x < $max));\n+\tprint <\n+\n+EOF\n+\treturn;\n+}\n+\n+sub cacheDisplay {\n+\tlocal $/ = undef;\n+\t$|++;\n+\n+\tmy ($action) = @_;\n+\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n+\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n+\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\n+\tif ($cache_enable != 0 && ! $lockStat ){\n+\t\tclose(cacheFile);\n+\t\tcacheWaitForUpdate($action);\n+\t}\n+\n+\tif(\n+\t\t(\n+\t\t\t$action eq \"snapshot\"\n+\t\t\t||\n+\t\t\t$action eq \"blob_plain\"\n+\t\t)\n+\t\t&&\n+\t\t$cache_enable > 0\n+\t){\n+\t\tmy $openstat = open(cacheFileBin, '<', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBin,LOCK_SH|LOCK_NB);\n+\t\tif ($cache_enable != 0 && ! $lockStatBIN ){\n+\t\t\tsystem (\"echo 'cacheDisplay - bailing due to binary lock failure' >> /tmp/gitweb.log\");\n+\t\t\tclose(cacheFile);\n+\t\t\tclose(cacheFileBin);\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\n+\t\tmy $binfilesize = -s \"$fullhashbinpathfinal\";\n+\t\tprint \"Content-Length: $binfilesize\";\n+\t}\n+\twhile( ){\n+\t\tprint $_;\n+\t}\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tbinmode STDOUT, ':raw';\n+\t\tprint ;\n+\t\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\t\tclose(cacheFileBin);\n+\t}\n+\tclose(cacheFile);\n+\t$|--;\n+}\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 757ef46..eb53075 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -31,7 +31,7 @@ BEGIN {\n our $version = \"++GIT_VERSION++\";\n \n # Output buffer variable\n-$output = \"\";\n+our $output = \"\";\n \n our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n sub evaluate_uri {\n@@ -244,6 +244,57 @@ our $git_versions_must_match = 1;\n # Leave it undefined (or set to 'undef') to turn off load checking.\n our $maxload = 300;\n \n+# This enables/disables the caching layer in gitweb. This currently only supports the\n+# 'dumb' file based caching layer, primarily used on git.kernel.org. this is reasonably\n+# effective but it has the downside of requiring a huge amount of disk space if there\n+# are a number of repositories involved. It is not uncommon for git.kernel.org to have\n+# on the order of 80G - 120G accumulate over the course of a few months. It is recommended\n+# that the cache directory be periodically completely deleted, and this is safe to perform.\n+# Suggested mechanism\n+# mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush\n+# Value is binary. 0 = disabled (default), 1 = enabled.\n+#\n+# Values of caching:\n+# \t1 = 'dumb' file based caching used on git.kernel.org\n+our $cache_enable = 0;\n+\n+# Used to set the minimum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to be under this number of seconds we set the cache timeout\n+# to this minimum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $minCacheTime = 20;\n+\n+# Used to set the maximum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to exceed this number of seconds we set the cache timeout\n+# to this maximum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $maxCacheTime = 1200;\n+\n+# If you need to change the location of the caching directory, override this\n+# otherwise this will probably do fine for you\n+our $cachedir = 'cache';\n+\n+# If this is set (to 1) cache will do it's best to always display something instead\n+# of making someone wait for the cache to update. This will launch the cacheUpdate\n+# into the background and it will lock a .bg file and will only lock the\n+# actual cache file when it needs to write into it. In theory this will make\n+# gitweb seem more responsive at the price of possibly stale data.\n+our $backgroundCache = 1;\n+\n+# Used to set the maximum cache file life. If a cache files last modify time exceeds\n+# this value, it will assume that the data is just too old, and HAS to be regenerated\n+# instead of trying to display the existing cache data.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+# 18000 = 5 hours\n+our $maxCacheLife = 18000;\n+\n+# Used to enable or disable background forking of the gitweb caching. Mainly here for debugging purposes\n+our $cacheDoFork = 1;\n+\n+our $fullhashpath = *STDOUT;\n+our $fullhashbinpath = *STDOUT;\n+our $fullhashbinpathfinal = *STDOUT;\n+\n # configuration for 'highlight' (http://www.andre-simon.de/)\n # match by basename\n our %highlight_basename = (\n@@ -500,6 +551,11 @@ our %feature = (\n \t\t'default' => [0]},\n );\n \n+#\n+# Includes\n+#\n+do 'cache.pm';\n+\n sub gitweb_get_feature {\n \tmy ($name) = @_;\n \treturn unless exists $feature{$name};\n@@ -1089,7 +1145,8 @@ sub dispatch {\n \t !$project) {\n \t\tdie_error(400, \"Project needed\");\n \t}\n-\t$actions{$action}->();\n+\t#$actions{$action}->();\n+\tcache_fetch($action);\n }\n \n sub reset_timer {\n@@ -1159,6 +1216,7 @@ sub change_output {\n \n \t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n \topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n+\tprint STDOUT_REAL \"\";\n \n \t# Close STDOUT, so that it isn't being used anymore.\n \tclose STDOUT;\n@@ -1185,10 +1243,7 @@ sub run {\n \t\t$pre_dispatch_hook->()\n \t\t\tif $pre_dispatch_hook;\n \n-\t\tchange_output();\n \t\trun_request();\n-\t\treset_output();\n-\t\tprint $output;\n \n \t\t$post_dispatch_hook->()\n \t\t\tif $post_dispatch_hook;\n@@ -3465,7 +3520,9 @@ sub git_header_html {\n \t# support xhtml+xml but choking when it gets what it asked for.\n \tif (defined $cgi->http('HTTP_ACCEPT') &&\n \t $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\\s|^)application\\/xhtml\\+xml(,|;|\\s|$)/ &&\n-\t $cgi->Accept('application/xhtml+xml') != 0) {\n+\t $cgi->Accept('application/xhtml+xml') != 0\n+\t &&\n+\t $cache_enable == 0) {\n \t\t$content_type = 'application/xhtml+xml';\n \t} else {\n \t\t$content_type = 'text/html';\n@@ -3610,6 +3667,7 @@ sub git_footer_html {\n \tmy $feed_class = 'rss_logo';\n \n \tprint \"
\\n\";\n+\tprint \"
Cache Last Updated: \". gmtime( time ) .\" GMT
\\n\";\n \tif (defined $project) {\n \t\tmy $descr = git_get_project_description($project);\n \t\tif (defined $descr) {\n@@ -3698,6 +3756,11 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\t# The output handlers for die_error need to be reset to STDOUT\n+\t# so that half the message isn't being output to random and\n+\t# half to STDOUT as expected. This is mainly for the benefit\n+\t# of using git_header_html() and git_footer_html() since\n+\t#\n \t# Reset the output so that we are actually going to STDOUT as opposed\n \t# to buffering the output.\n \treset_output();\n@@ -5610,9 +5673,15 @@ sub git_blob_plain {\n \t\t\t($sandbox ? 'attachment' : 'inline')\n \t\t\t. '; filename=\"' . $save_as . '\"');\n \tlocal $/ = undef;\n-\tbinmode STDOUT, ':raw';\n-\tprint <$fd>;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif( $cache_enable != 0){\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \n@@ -5897,9 +5966,15 @@ sub git_snapshot {\n \n \topen my $fd, \"-|\", $cmd\n \t\tor die_error(500, \"Execute git-archive failed\");\n-\tbinmode STDOUT, ':raw';\n-\tprint <$fd>;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif( $cache_enable != 0){\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \ndiff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\nindex 4132aab..972d32e 100644\n--- a/gitweb/static/gitweb.css\n+++ b/gitweb/static/gitweb.css\n@@ -67,6 +67,12 @@ div.page_path {\n \tborder-width: 0px 0px 1px;\n }\n \n+div.cachetime {\n+\tfloat: left;\n+\tmargin-right: 10px;\n+\tcolor: #555555;\n+}\n+\n div.page_footer {\n \theight: 17px;\n \tpadding: 4px 8px;\n"},{"id":"154618","messageId":"m3fwvqir0o.fsf@localhost.localdomain","threadId":"25573","inReplyTo":"1288226574-19068-2-git-send-email-warthog9@eaglescrag.net","subject":"Re: [PATCH 1/3] gitweb: Add option to force version match","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-10-28T09:52:44Z","receivedAt":"2010-10-28T09:52:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"\"John 'Warthog9' Hawley\" writes:\n\n> This adds $git_versions_must_match variable, which is set to true,\n> checks that we are running on the same version of git that we\n> shipped with, and if not throw '500 Internal Server Error' error.\n> What is checked is the version of gitweb (embedded in building\n> gitweb.cgi), against version of runtime git binary used.\n> \n> Gitweb can usually run with a mismatched git install. This is more\n> here to give an obvious warning as to whats going on vs. silently\n> failing.\n\nWere you bitten by mismatch between git version and gitweb version?\nJust how serious is that (hypothetical?) situation?\n\nShould it be warning, or erroring out?\n\n> \n> By default this feature is turned on.\n\nI think last time this patch stalled on discussion whether this\nfeature should be turned on by default, where it can break some\nsetups; or be turned off by default, where it is much less useful\nprotecting against errors.\n\n> \n> Signed-off-by: John 'Warthog9' Hawley \n> Signed-off-by: Jakub Narebski \n> ---\n> gitweb/README | 4 ++++\n> gitweb/gitweb.perl | 27 +++++++++++++++++++++++++++\n> 2 files changed, 31 insertions(+), 0 deletions(-)\n\nFor me, if $git_versions_must_match is to be on by default, I would\nprefer also update to t/gitweb-lib.sh, so hat I don't have to\nrecompile git to test new version of gitweb... \n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 8d7e4c5..215a4e9 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -232,6 +232,9 @@ our %avatar_size = (\n> \t'double' => 32\n> );\n> \n> +# If it is true, exit if gitweb version and git binary version don't match\n> +our $git_versions_must_match = 1;\n> +\n> # Used to set the maximum load that we will still respond to gitweb queries.\n> # If server load exceed this value then return \"503 server busy\" error.\n> # If gitweb cannot determined server load, it is taken to be 0.\n> @@ -649,6 +652,29 @@ sub check_loadavg {\n> \t}\n> }\n> \n> +sub check_versionmatch {\n> +\t# Throw an error if git versions does not match, if $git_versions_must_match is true.\n> +\tif ($git_versions_must_match &&\n> +\t $git_version ne $version) {\n> +\t\tmy $admin_contact =\n> +\t\t\tdefined $ENV{'SERVER_ADMIN'} ? \", $ENV{'SERVER_ADMIN'},\" : '';\n> +\t\tmy $err_msg = < +

*** Warning ***

\n> +

\n> +This version of gitweb was compiled for @{[esc_html($version)]},\n> +however git version @{[esc_html($git_version)]} was found on server,\n> +and administrator requested strict version checking.\n> +

\n> +

\n> +Please contact the server administrator${admin_contact} to either configure\n> +gitweb to allow mismatched versions, or update git or gitweb installation.\n> +

\n> +EOT\n> +\t\tdie_error(500, 'Internal server error', $err_msg);\n> +\t}\n> +\n> +}\n\nNice.\n\n> +\n> # ======================================================================\n> # input validation and dispatch\n> \n> @@ -1075,6 +1101,7 @@ sub run_request {\n> \tevaluate_uri();\n> \tevaluate_gitweb_config();\n> \tcheck_loadavg();\n> +\tcheck_versionmatch();\n\nShouldn't it be before check_loadavg()?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"154619","messageId":"m3bp6eiqu5.fsf@localhost.localdomain","threadId":"25573","inReplyTo":"1288226574-19068-3-git-send-email-warthog9@eaglescrag.net","subject":"Re: [PATCH 2/3] gitweb: add output buffering and associated functions","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-10-28T09:56:35Z","receivedAt":"2010-10-28T09:56:35Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"\"John 'Warthog9' Hawley\" writes:\n\n> This adds output buffering for gitweb, mainly in preparation for\n> caching support. This is a dramatic change to how caching was being\n> done, mainly in passing around the variable manually and such.\n> \n> This centrally flips the entire STDOUT to a variable, which after the\n> completion of the run, flips it back and does a print on the resulting\n> data.\n> \n> This should save on the previous 10K line patch (or so) that adds more\n> explicit output passing.\n\nSignoff?\n\n> ---\n> gitweb/gitweb.perl | 29 +++++++++++++++++++++++++++++\n> 1 files changed, 29 insertions(+), 0 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 215a4e9..757ef46 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -30,6 +30,9 @@ BEGIN {\n> \n> our $version = \"++GIT_VERSION++\";\n> \n> +# Output buffer variable\n> +$output = \"\";\n> +\n> our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n> sub evaluate_uri {\n> \tour $cgi;\n> @@ -1151,6 +1154,25 @@ sub evaluate_argv {\n> \t);\n> }\n> \n> +sub change_output {\n> +\tour $output;\n> +\n> +\t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n> +\topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n> +\n> +\t# Close STDOUT, so that it isn't being used anymore.\n> +\tclose STDOUT;\n> +\n> +\t# Trap STDOUT to the $output variable, which is what I was using in the original\n> +\t# patch anyway.\n> +\topen(STDOUT,\">\", \\$output) || die \"Unable to open STDOUT: $!\"; #open STDOUT handle to use $var\n> +}\n> +\n> +sub reset_output {\n> +\t# This basically takes STDOUT_REAL and puts it back as STDOUTl\n> +\topen(STDOUT,\">&STDOUT_REAL\");\n> +}\n\nNice solution. I'll steal it for GitwebCache::Capture::Redirect (or\nsomething like that, instead of relying on a bit cryptic select($fh)).\n\n> @@ -1163,7 +1185,10 @@ sub run {\n> \t\t$pre_dispatch_hook->()\n> \t\t\tif $pre_dispatch_hook;\n> \n> +\t\tchange_output();\n> \t\trun_request();\n> +\t\treset_output();\n> +\t\tprint $output;\n\nDoesn't this enable capture unconditionally? \n\nWouldn't that screw-up the blame_data part of blame_interactive Ajax-y\nview? Wouldn't that decrease perceived responsiveness of gitweb?\n\n> \n> \t\t$post_dispatch_hook->()\n> \t\t\tif $post_dispatch_hook;\n> @@ -3673,6 +3698,10 @@ sub die_error {\n> \t\t500 => '500 Internal Server Error',\n> \t\t503 => '503 Service Unavailable',\n> \t);\n> +\t# Reset the output so that we are actually going to STDOUT as opposed\n> +\t# to buffering the output.\n> +\treset_output();\n> +\n\nGood.\n\n> \tgit_header_html($http_responses{$status}, undef, %opts);\n> \tprint <
\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"154637","messageId":"m37hh2i9ir.fsf@localhost.localdomain","threadId":"25573","inReplyTo":"1288226574-19068-4-git-send-email-warthog9@eaglescrag.net","subject":"Re: [PATCH 3/3] gitweb: File based caching layer (from git.kernel.org)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-10-28T16:10:41Z","receivedAt":"2010-10-28T16:10:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"I am not commenting on minor style issues, like using camelCase names\nof variables and subroutines, of 'if( ... )' instead of 'if (...)'\netc. minor issues.\n\n\n\"John 'Warthog9' Hawley\" writes:\n\n> This is a relatively large patch that implements the file based\n> caching layer that is quite similar to the one used on such large\n> sites as kernel.org and soon git.fedoraproject.org. This provides\n> a simple, and straight forward caching mechanism that scales\n> dramatically better than Gitweb by itself.\n\n\"Quite similar\" - what does it mean? In what way it is different from\nthe one used on git.kernel.org?\n\n> \n> The caching layer basically buffers the output that Gitweb would\n> normally return, and saves that output to a cache file on the local\n> disk.\n\nO.K. This means caching HTTP response (caching output of CGI script),\nusing \"simple\" file based caching engine, without any serialization.\n\nBut this doesn't tell us how the cache expiration is handled, i.e. how\ndo you know that cache entry is expired, and should be regenerated.\n\n\n> When the file is requested it attempts to gain a shared lock\n> on the cache file and cat it out to the client. Should an exclusive\n> lock be on a file (it's being updated) the code has a choice to either\n> update in the background and go ahead and show the stale page while\n> update is being performed, or stall the client(s) until the page\n> is generated.\n\nIn my rewrite of earlier version of your gitweb caching patch it is\nresponsibility of caching engine (GitwebCache::FileCacheWithLocking)\nto serialize (via locking) (re)generating cache entry. Gitweb just\npasses page key, function that returns captured output to cache\ninstance; it passes callback for \"staling\" the client to cache\nconstructor.\n\nIt is not the \"file\" that is being requested: on the gitweb level it\nis a page / action / view that is being requested, on the level of\ncaching negine it is cache entry that is requested... only at the low\nlevel of cache driver that a file holding a cached entry is requested.\n\n> \n> There are two forms of stalling involved here, background building\n> and non-background building, both of which are discussed in the\n> configuration page.\n> \n> There are still a few known \"issues\" with respect to this:\n> - Code needs to be added to be \"browser\" aware so\n> that clients like wget that are trying to get a\n> binary blob don't obtain a \"Generating...\" page\n\nRight. This is also missing from my rewrite.\n\nSidenote: why \"binary blob\"? Most downloading utilities do not follow\nhttp-meta in-HTML redirects, do they?\n\n> \n> Caching is disabled by default with the $cache_enable variable,\n> setting this to 1 will enable file based caching. It is expected\n> that this will be extended to include additional types of caching\n> (like memcached) in the future and should not be exclusively\n> considered a binary value.\n\nWhy not do the same as in my rewrite, namely $cache_enabled or\n$caching_enabled *boolean* value (not \"binary\"), and $cache variable\nto select caching engine (undef to use default)?\n\n> ---\n> gitweb/cache.pm | 365 ++++++++++++++++++++++++++++++++++++++++++++++\n> gitweb/gitweb.perl | 99 +++++++++++--\n> gitweb/static/gitweb.css | 6 +\n> 3 files changed, 458 insertions(+), 12 deletions(-)\n> create mode 100644 gitweb/cache.pm\n\nIt would be nice to have at least some minimal testing for caching\nenabled gitweb. Hmmm... I should have split update to t9500-* test\nso you would be able to cherry-pick it.\n\nAlso, it would be good idea to have 'make install-gitweb' to install\n'cache.pm' alongside gitweb, especially that you do not do any error\nchecking for \"do 'cache.pm';\" (see below).\n\n> \n> diff --git a/gitweb/cache.pm b/gitweb/cache.pm\n> new file mode 100644\n> index 0000000..c86265c\n> --- /dev/null\n> +++ b/gitweb/cache.pm\n> @@ -0,0 +1,365 @@\n> +# gitweb - simple web interface to track changes in git repositories\n> +#\n> +# (C) 2006, John 'Warthog9' Hawley \n> +#\n> +# This program is licensed under the GPLv2\n> +\n> +#\n> +# Gitweb caching engine\n> +#\n\nWouldn't it be better to use real Perl module, i.e. start with\n'package' line, like in my rewrite?\n\n> +\n> +#use File::Path qw(make_path remove_tree);\n\nIs above line really needed?\n\n> +use File::Path qw(mkpath rmtree); # Used for compatability reasons\n\nBecause [stable] Perl earlier than 5.10.0 has pre-2.00 version of\nFile::Path in core, which didn't have make_path and remove_tree.\n\nAt least that is compatibility with pre-2.00 File::Path should be\nmentioned in this comment, if we are to mention it at all.\n\nBTW. do you need rmtree?\n\n> +use Digest::MD5 qw(md5 md5_hex md5_base64);\n\nDo you use *all* those subroutines?\n\n> +use Fcntl ':flock';\n\nAll right.\n\n> +use File::Copy;\n\nWhy do you need this for?\n\n> +\n> +sub cache_fetch {\n> +\tmy ($action) = @_;\n> +\tmy $cacheTime = 0;\n> +\n> +\t# Deal with cache being disabled\n> +\tif( $cache_enable == 0 ){\n\nWouldn't it be better to use boolean 'if (!$caching_enabled) {' rather\nthan explicitely test for numeric 0?\n\nWouldn't it be better to have this conditional _outside_ cache_fetch?\n\n> +\t\tundef $backgroundCache;\n> +\t\t#$fullhashpath = \\$nocachedata;\n> +\t\t#$fullhashbinpath = \\$nocachedatabin;\n\nLeftover debugging?\n\n> +\t\t$fullhashpath = *STDOUT;\n> +\t\t$fullhashbinpath = *STDOUT;\n\nWTF is this for? Defensive programming? You shouldn't protect\nyourself against bugs in your program...\n\n> +\t\t$actions{$action}->();\n> +\t\treturn;\n> +\t}elsif( $cache_enable == 1 ){\n> +\t\t#obviously we are using file based caching\n> +\n> +\t\tif(! -d $cachedir){\n> +\t\t\tprint \"*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\\n\";\n\nWhy print this warning? Why not use 'warn' instead, so it would get\nin web server logs?\n\nWouldn't printing this here screw up HTTP headers something fierce?\nI think it is a bug; rarely encountered but still a bug.\n\n> +\t\t\tmkdir (\"cache\", 0665) || die \"Cannot create cache dir - you will need to manually create\";\n> +\t\t\tprint \"Cache directory created successfully\\n\";\n> +\t\t}\n> +\n> +\t\tour $full_url = \"$my_url?\". $ENV{'QUERY_STRING'};\n\nWhy not use 'href(-replay => 1, -full => 1, -path_info => 0)'?\n\n> +\t\tour $urlhash = md5_hex($full_url);\n> +\t\tour $fullhashdir = \"$cachedir/\". substr( $urlhash, 0, 2) .\"/\";\n\nWhy 'our' and not 'my', i.e. why use global variables, and not local\nto function (lexical scoped to be more exact)?\n\nWe use 'our' in gitweb *only* because otherwise mod_perl with\nModPerl::Registry handler wouldn't work, see e.g. dde80d9 (gitweb: Fix\nmod_perl support., 2008-11-06) which fixes one issue of 'my' instead\nof 'our'. But global variables are bad idea...\n\n> +\n> +\t\teval { mkpath( $fullhashdir, 0, 0777 ) };\n> +\t\tif ($@) {\n> +\t\t\tdie_error(500, \"Internal Server Error\", \"Could not create cache directory: $@\");\n> +\t\t}\n\nGood... but wouldn't it leak sensitive information?\n\n> +\t\t$fullhashpath = \"$fullhashdir/\". substr( $urlhash, 2 );\n> +\t\t$fullhashbinpath = \"$fullhashpath.bin.wt\";\n> +\t\t$fullhashbinpathfinal = \"$fullhashpath.bin\";\n\nFirst, why do you treat binary and non-binary case differently,\ni.e. you have $fullhashbinpathfinal (egh, thats a mouthfull ;-), but\nnot $fullhashpathfinal?\n\nSecond, do you really need to have distinct code for binary and\nnon-binary case besides what gitweb already has with \n\"binmode STDOUT, ':raw';\" in those places few that needs it?\n\nIf you are capturing to in-memory file (to scalar), and binmode acts\non said in-memory file, then what gets saved is raw (bytes) data.\nThen you can print it raw to web browser, save it raw to cache file,\nand read it raw from cache file.\n\n> +\t} # done dealing with cache enabled / disabled\n> +\n> +\tif(! -e \"$fullhashpath\" ){\n\nWhy it is not within 'if ($caching_enabled)' branch of conditional?\n\n> +\t\tif( ! $cache_enable || ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n> +\t\t\tcacheUpdate($action,0);\n> +\t\t\tcacheDisplay($action);\n> +\t\t} elsif ( $childPid == 0 ){\n> +\t\t\t#run the updater\n> +\t\t\tcacheUpdate($action,1);\n> +\t\t}else{\n> +\t\t\tcacheWaitForUpdate($action);\n> +\t\t}\n\nWTF do you have forking *here*? If cache entry is not expired, you\ncan just read it, without need for forking. It is only when you need\nto (re)generate cache that forking is needed.\n\n> +\t}else{\n> +\t\t#if cache is out dated, update\n> +\t\t#else displayCache();\n\nLefotover statement.\n\n> +\t\topen(cacheFile, '<', \"$fullhashpath\");\n> +\t\tstat(cacheFile);\n> +\t\tclose(cacheFile);\n\nYou don't need to open file to stat it, you can do\n\n +\t\tstat($fullhashpath);\n\nBTW. what happens if output is binary?\n\n> +\t\tmy $stat_time = (stat(_))[9];\n> +\t\tmy $stat_size = (stat(_))[7];\n> +\n\n\n> +\t\t$cacheTime = get_loadavg() * 60;\n> +\t\tif( $cacheTime > $maxCacheTime ){\n> +\t\t\t$cacheTime = $maxCacheTime;\n> +\t\t}\n> +\t\tif( $cacheTime < $minCacheTime ){\n> +\t\t\t$cacheTime = $minCacheTime;\n> +\t\t}\n\nAll right.\n\n> +\t\tif( $stat_time < (time - $cacheTime) || $stat_size == 0 ){\n\nSome comment about '$stat_size == 0' test would be good idea, I think.\n\n> +\t\t\tif( ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n> +\t\t\t\tcacheUpdate($action,0);\n> +\t\t\t\tcacheDisplay($action);\n> +\t\t\t} elsif ( $childPid == 0 ){\n> +\t\t\t\t#run the updater\n> +\t\t\t\t#print \"Running updater\\n\";\n> +\t\t\t\tcacheUpdate($action,1);\n> +\t\t\t}else{\n> +\t\t\t\t#print \"Waiting for update\\n\";\n> +\t\t\t\tcacheWaitForUpdate($action);\n> +\t\t\t}\n\nThe above block of code is almost verbatim copy of code higher up.\nNot good.\n\n> +\t\t} else {\n> +\t\t\tcacheDisplay($action);\n> +\t\t}\n\nWouldn't it be better to have those two branches of conditional\nreversed, so that else part is larger? Just a thought...\n\n> +\n> +\n> +\t}\n> +\n> +\t#\n> +\t# If all of the caching failes - lets go ahead and press on without it and fall back to 'default'\n> +\t# non-caching behavior. This is the softest of the failure conditions.\n\nThis comment refers to...\n\n> +\t#\n> +\t#$actions{$action}->();\n\n... commented out code? Eh?\n\n> +}\n\nYou don't use persistent web environment[1] for git.kernel.org, do\nyou?\n\nWith persistent web environments, where gitweb script runs for a long\ntime, and doesn't exit after end of request, you really should\n\"daemonize\" (detach) background process which regenerates cache, at\nleast when said process doesn't display web page to server (serve\nstale data + regenerate cache in background). Otherwise you would get\nlots of zombie processes...\n\n[1] Gitweb currently supports FastCGI (just rename it to gitweb.fcgi,\nbut you would need FCGI perl module installed together with\nCGI::Fast), mod_perl via ModPerl::Registry handler, and you can use\nit with PSGI via gitweb.psgi wrapper like the one generated by \n'git instaweb --httpd=plackup'.\n\n> +\n> +sub cacheUpdate {\n> +\tmy ($action,$areForked) = @_;\n\nStrange API.\n\n> +\tmy $lockingStatus;\n> +\tmy $fileData = \"\";\n> +\n> +\tif($backgroundCache){\n> +\t\topen(cacheFileBG, '>:utf8', \"$fullhashpath.bg\");\n> +\t\tmy $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);\n> +\n> +\t\t$lockStatus = $lockStatBG;\n> +\t}else{\n> +\t\topen(cacheFile, '>:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n> +\t\topen(cacheFile, '>:utf8', \\$fullhashpath) if ($cache_enable == 0);\n\nWhy do you save to in-memory file if caching is disabled, instead of\njust printing to STDOUT?\n\n> +\t\tmy $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n> +\n> +\t\t$lockStatus = $lockStat;\n> +\t}\n\nWhy do you need different names for lockfiles for background cache\nentry enabled and for it disabled? Why do you need $lockStat and\n$lockStatBG if what you do is just assign it to $lockStatus?\n\nWhy do you use fileglobs (which are *global* variables!) and not\nlexical filehandles?\n\n> +\t#print \"lock status: $lockStat\\n\";\n\nLeftover debugging? \n\nBTW. didn't you have problems debugging it from withing gitweb code,\nand not as a separate caching engine which can be tested individually?\n\n> +\n> +\n> +\tif ($cache_enable != 0 && ! $lockStatus ){\n> +\t\tif ( $areForked ){\n> +\t\t\texit(0);\n> +\t\t}else{\n> +\t\t\treturn;\n> +\t\t}\n> +\t}\n\nStrange code flow...\n\n> +\n> +\tif(\n> +\t\t$action eq \"snapshot\"\n> +\t\t||\n> +\t\t$action eq \"blob_plain\"\n> +\t){\n> +\t\tmy $openstat = open(cacheFileBinWT, '>>:utf8', \"$fullhashbinpath\");\n> +\t\tmy $lockStatBin = flock(cacheFileBinWT,LOCK_EX|LOCK_NB);\n> +\t}\n\nWhy '>>', and not '>'. Why use ':utf8' on binary file? Shouldn't you\nuse different file for locking than for saving cache output in?\n\n> +\n> +\t# Trap all output from the action\n> +\tchange_output();\n> +\n> +\t$actions{$action}->();\n> +\n> +\t# Reset the outputs as we should be fine now\n> +\treset_output();\n\nAll right.\n\n> +\n> +\n> +\tif($backgroundCache){\n> +\t\topen(cacheFile, '>:utf8', \"$fullhashpath\");\n> +\t\t$lockStat = flock(cacheFile,LOCK_EX);\n> +\n> +\t\tif (! $lockStat ){\n> +\t\t\tif ( $areForked ){\n> +\t\t\t\texit(0);\n> +\t\t\t}else{\n> +\t\t\t\treturn;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\tif(\n> +\t\t$action eq \"snapshot\"\n> +\t\t||\n> +\t\t$action eq \"blob_plain\"\n> +\t){\n> +\t\tmy $openstat = open(cacheFileBinFINAL, '>:utf8', \"$fullhashbinpathfinal\");\n> +\t\t$lockStatBIN = flock(cacheFileBinFINAL,LOCK_EX);\n> +\n> +\t\tif (! $lockStatBIN ){\n> +\t\t\tif ( $areForked ){\n> +\t\t\t\texit(0);\n> +\t\t\t}else{\n> +\t\t\t\treturn;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n\nDidn't I see nearly exactly the same code? Copy'n'paste programming\nisn't good idea...\n\n> +\n> +\t# Actually dump the output to the proper file handler\n> +\tlocal $/ = undef;\n> +\t$|++;\n> +\tprint cacheFile \"$output\";\n> +\t$|--;\n\nWhy stringify $output? Why not 'local $| = 1' too?\n\n> +\tif(\n> +\t\t$action eq \"snapshot\"\n> +\t\t||\n> +\t\t$action eq \"blob_plain\"\n> +\t){\n> +\t\tmove(\"$fullhashbinpath\", \"$fullhashbinpathfinal\") or die \"Binary Cache file could not be updated: $!\";\n\nDo you really need 'File::Copy::move' instead of simple 'rename' here?\nBoth files are in the same directory, so there should be no problems\nwith it.\n\n> +\n> +\t\tflock(cacheFileBinFINAL,LOCK_UN);\n> +\t\tclose(cacheFileBinFINAL);\n> +\n> +\t\tflock(cacheFileBinWT,LOCK_UN);\n> +\t\tclose(cacheFileBinWT);\n\nClosing lockfile unlocks it, there is no need for LOCK_UN; moreover,\nLOCK_Un is unreliable, as I have checked myself, and as I have read\non http://perl.plover.com/yak/flock/ and also on StackOverflow\nhttp://stackoverflow.com/questions/34920/how-do-i-lock-a-file-in-perl\n \n\nWhy do binary files needs _two_ locks for?\n\n> +\t}\n> +\n> +\tflock(cacheFile,LOCK_UN);\n> +\tclose(cacheFile);\n> +\n> +\tif($backgroundCache){\n> +\t\tflock(cacheFileBG,LOCK_UN);\n> +\t\tclose(cacheFileBG);\n> +\t}\n\nWhy do background cache generation needs _two_ (or _four_ in case of\nbinary files!!!) locks?\n\n> +\n> +\tif ( $areForked ){\n> +\t\texit(0);\n> +\t} else {\n> +\t\treturn;\n> +\t}\n\nWhy can't a caller do that?\n\n> +}\n> +\n> +\n\nI think it would be better to describe how cacheWaitForUpdate() works.\n\n> +sub cacheWaitForUpdate {\n> +\tmy ($action) = @_;\n> +\tmy $x = 0;\n> +\tmy $max = 10;\n> +\tmy $lockStat = 0;\n> +\n> +\tif( $backgroundCache ){\n> +\t\tif( -e \"$fullhashpath\" ){\n> +\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n> +\t\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n\nSame af above: when caching is disabled, why do you capture gitweb\noutput for?\n\n> +\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n> +\t\t\tstat(cacheFile);\n> +\t\t\tclose(cacheFile);\n\nHere you unlocked just acquired lock!\n\nSidenote: using real locsk with flock() has the advantage over trick\nof creating *.lock files with O_CREAT in that if process dies (and all\nfiles it opened are closed) the lock is automatically unlocked.\nTherefore you don't have problems of dead lockfiles.\n\n> +\n> +\t\t\tif( $lockStat && ( (stat(_))[9] > (time - $maxCacheLife) ) ){\n> +\t\t\t\tcacheDisplay($action);\n> +\t\t\t\treturn;\n> +\t\t\t}\n\nIs running time() a second time intended, or just a side effect of\narrangement of code?\n\n> +\t\t}\n> +\t}\n> +\n> +\tif(\n> +\t\t$action eq \"atom\"\n> +\t\t||\n> +\t\t$action eq \"rss\"\n> +\t\t||\n> +\t\t$action eq \"opml\"\n> +\t){\n\nSidenote: this is a bit ugly, and that is why in my rewrite I have\ncommit d20e6a4 (gitweb: Introduce %actions_info, gathering information\nabout actions, 2010-10-05).\n\nAlso, in my rewrite I can simply 'return' instead of 'exit()' here,\nand you would get data from caching engine: no need for below code\nblock (which is repeated in very similar way a bit later in same\nsubroutine).\n\n> +\t\tdo {\n> +\t\t\tsleep 2 if $x > 0;\n> +\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n> +\t\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n> +\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n> +\t\t\tclose(cacheFile);\n\nHere you unlock just acquired lock. Is it intended? \n\nIs it safe against other process acquiting writer's lock (LOCK_EX)?\nIf it is, I probably should unlock it earlier and not have exit() do\nit...\n\n> +\t\t\t$x++;\n> +\t\t\t$combinedLockStat = $lockStat;\n> +\t\t} while ((! $combinedLockStat) && ($x < $max));\n\nMy rewrite instead tries to acquire LOCK_SH *without* LOCK_NB, i.e. in\na blocking way... but with a timeout via $SIG{ALRM} and eval { ... }.\n\n> +\n> +\t\tif( $x != $max ){\n> +\t\t\tcacheDisplay($action);\n> +\t\t}\n> +\t\treturn;\n> +\t}\n> +\n> +\t$| = 1;\n\nI have autoflush turned on later. But by all thats's koly, please\nlocalize $| before changing its value, i.e.\n\n +\tlocal $| = 1;\n\n\n> +\n> +\tprint $::cgi->header(\n\nWhy $::cgi->header()?\n\nIn my rewrite git_generating_data_html() subroutine is defined in\ngitweb.perl, and passed as callback to cache constructor instead.\n\n> +\t\t\t\t-type=>'text/html',\n> +\t\t\t\t-charset => 'utf-8',\n> +\t\t\t\t-status=> 200,\n> +\t\t\t\t-expires => 'now',\n\nThat I have.\n\n> +\t\t\t\t# HTTP/1.0\n> +\t\t\t\t-Pragma => 'no-cache',\n> +\t\t\t\t# HTTP/1.1\n> +\t\t\t\t-Cache_Control => join(\n> +\t\t\t\t\t\t\t', ',\n> +\t\t\t\t\t\t\tqw(\n> +\t\t\t\t\t\t\t\tprivate\n> +\t\t\t\t\t\t\t\tno-cache\n> +\t\t\t\t\t\t\t\tno-store\n> +\t\t\t\t\t\t\t\tmust-revalidate\n> +\t\t\t\t\t\t\t\tmax-age=0\n> +\t\t\t\t\t\t\t\tpre-check=0\n> +\t\t\t\t\t\t\t\tpost-check=0\n> +\t\t\t\t\t\t\t)\n> +\t\t\t\t\t\t)\n\nThis I don't, and I probably should have. Good idea.\n\n> +\t\t\t\t);\n> +\n> +\tprint < +\n> +\n> +\n> +\n> +\n> +\n> +\n\nYou meant here \"noindex, nofollow\", isn't it?\n\n> +\n> +$title\n> +\n> +\n> +EOF\n> +\n> +\tprint \"Generating..\";\n> +\tdo {\n> +\t\tprint \".\";\n> +\t\tsleep 2 if $x > 0;\n> +\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n> +\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n> +\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n> +\t\tclose(cacheFile);\n> +\t\t$x++;\n> +\t\t$combinedLockStat = $lockStat;\n> +\t} while ((! $combinedLockStat) && ($x < $max));\n\nA bit of code repetition here.\n\n> +\tprint < +\n> +\n> +EOF\n> +\treturn;\n> +}\n> +\n> +sub cacheDisplay {\n> +\tlocal $/ = undef;\n> +\t$|++;\n> +\n> +\tmy ($action) = @_;\n> +\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n> +\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n> +\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n> +\n> +\tif ($cache_enable != 0 && ! $lockStat ){\n> +\t\tclose(cacheFile);\n> +\t\tcacheWaitForUpdate($action);\n> +\t}\n> +\n> +\tif(\n> +\t\t(\n> +\t\t\t$action eq \"snapshot\"\n> +\t\t\t||\n> +\t\t\t$action eq \"blob_plain\"\n> +\t\t)\n> +\t\t&&\n> +\t\t$cache_enable > 0\n> +\t){\n> +\t\tmy $openstat = open(cacheFileBin, '<', \"$fullhashbinpathfinal\");\n> +\t\t$lockStatBIN = flock(cacheFileBin,LOCK_SH|LOCK_NB);\n> +\t\tif ($cache_enable != 0 && ! $lockStatBIN ){\n> +\t\t\tsystem (\"echo 'cacheDisplay - bailing due to binary lock failure' >> /tmp/gitweb.log\");\n\nErrrrr... what? Using system(\"echo ...\") instead of just writing to a\nfile? Or better, using 'warn' and relying on CGI::Carp module to\ndeliver warning to web server logs?\n\n> +\t\t\tclose(cacheFile);\n\nWhy not close 'cacheFile' earlier, or say, not opening it in first\nplace if it is not necessary... oh, sorry, I see that you are using\nthe same file for cache file for normal case and for lockfile.\n\n> +\t\t\tclose(cacheFileBin);\n> +\t\t\tcacheWaitForUpdate($action);\n> +\t\t}\n> +\n\n> +\t\tmy $binfilesize = -s \"$fullhashbinpathfinal\";\n> +\t\tprint \"Content-Length: $binfilesize\";\n\nGood idea, bad solution. \"Content-Length: \" HTTP header can be only _after_ \n'HTTP/1.1 200 OK' header, isn't it?\n\n> +\t}\n> +\twhile( ){\n> +\t\tprint $_;\n> +\t}\n\nWhy not 'print ;'? besides with $/ undefined you operate\nin slurp / spew mode, and Perl now considers all file to be a single\nline.\n\n> +\tif(\n> +\t\t$action eq \"snapshot\"\n> +\t\t||\n> +\t\t$action eq \"blob_plain\"\n> +\t){\n> +\t\tbinmode STDOUT, ':raw';\n> +\t\tprint ;\n> +\t\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n> +\t\tclose(cacheFileBin);\n> +\t}\n> +\tclose(cacheFile);\n> +\t$|--;\n\nWouldn't it be better to use 'local $| = 1' instead of this \n'$|++ / $|--'?\n\n> +}\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 757ef46..eb53075 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -31,7 +31,7 @@ BEGIN {\n> our $version = \"++GIT_VERSION++\";\n> \n> # Output buffer variable\n> -$output = \"\";\n> +our $output = \"\";\n\nThis belongs to and should be squashed with earlier patch\n\n> \n> our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n> sub evaluate_uri {\n> @@ -244,6 +244,57 @@ our $git_versions_must_match = 1;\n> # Leave it undefined (or set to 'undef') to turn off load checking.\n> our $maxload = 300;\n> \n> +# This enables/disables the caching layer in gitweb. This currently only supports the\n> +# 'dumb' file based caching layer, primarily used on git.kernel.org. this is reasonably\n> +# effective but it has the downside of requiring a huge amount of disk space if there\n> +# are a number of repositories involved. It is not uncommon for git.kernel.org to have\n> +# on the order of 80G - 120G accumulate over the course of a few months. It is recommended\n> +# that the cache directory be periodically completely deleted, and this is safe to perform.\n> +# Suggested mechanism\n> +# mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush\n> +# Value is binary. 0 = disabled (default), 1 = enabled.\n> +#\n> +# Values of caching:\n> +# \t1 = 'dumb' file based caching used on git.kernel.org\n> +our $cache_enable = 0;\n\nAs I wrote earlier, having boolean $caching_enabled and $cache\nspecifying caching engine to use (undef for default) allows to\nseparate act of enabling cache from the cact of selecting (and\nconfiguring) cache. Isn't it?\n\n> +\n> +# Used to set the minimum cache timeout for the dynamic caching algorithm. Basically\n> +# if we calculate the cache to be under this number of seconds we set the cache timeout\n> +# to this minimum.\n> +# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n> +our $minCacheTime = 20;\n> +\n> +# Used to set the maximum cache timeout for the dynamic caching algorithm. Basically\n> +# if we calculate the cache to exceed this number of seconds we set the cache timeout\n> +# to this maximum.\n> +# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n> +our $maxCacheTime = 1200;\n> +\n> +# If you need to change the location of the caching directory, override this\n> +# otherwise this will probably do fine for you\n> +our $cachedir = 'cache';\n\nIf it is relative path, it is relative to location of gitweb.cgi\nscript, isn't it?\n\n> +\n> +# If this is set (to 1) cache will do it's best to always display something instead\n> +# of making someone wait for the cache to update. This will launch the cacheUpdate\n> +# into the background and it will lock a .bg file and will only lock the\n> +# actual cache file when it needs to write into it. In theory this will make\n> +# gitweb seem more responsive at the price of possibly stale data.\n> +our $backgroundCache = 1;\n\nBoolean.\n\n> +\n> +# Used to set the maximum cache file life. If a cache files last modify time exceeds\n> +# this value, it will assume that the data is just too old, and HAS to be regenerated\n> +# instead of trying to display the existing cache data.\n> +# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n> +# 18000 = 5 hours\n> +our $maxCacheLife = 18000;\n> +\n> +# Used to enable or disable background forking of the gitweb caching. Mainly here for debugging purposes\n> +our $cacheDoFork = 1;\n> +\n> +our $fullhashpath = *STDOUT;\n> +our $fullhashbinpath = *STDOUT;\n> +our $fullhashbinpathfinal = *STDOUT;\n> +\n\nIn my rewrite all cache parameters are in %cache_options hash, and\npassed as parameters to cache constructor, instead of littering global\nvariables namespace...\n\n> # configuration for 'highlight' (http://www.andre-simon.de/)\n> # match by basename\n> our %highlight_basename = (\n> @@ -500,6 +551,11 @@ our %feature = (\n> \t\t'default' => [0]},\n> );\n> \n> +#\n> +# Includes\n> +#\n> +do 'cache.pm';\n\nUnsafe! If you have any error in 'cache.pm', you could get a very\nstrange error reports.\n\nAlso what if 'cache.pm' cannot be found because somebody forgot to\ncopy it along with gitweb.cgi?\n\n\nSee how we do it for $GITWEB_CONFIG:\n\n if (-e $GITWEB_CONFIG) {\n do $GITWEB_CONFIG;\n die $@ if $@;\n }\n\nSee also documentation for 'do' (perldoc -f do):\n\n If \"do\" cannot read the file, it returns undef and sets $! to the error.\n If \"do\" can read the file but cannot compile it, it returns undef and sets\n an error message in $@. If the file is successfully compiled, \"do\"\n returns the value of the last expression evaluated.\n\n Note that inclusion of library modules is better done with the \"use\" and\n \"require\" operators, which also do automatic error checking and raise an\n exception if there's a problem.\n\n You might like to use \"do\" to read in a program configuration file. \n Manual error checking can be done this way:\n\n # read in config files: system first, then user\n for $file (\"/share/prog/defaults.rc\",\n \"$ENV{HOME}/.someprogrc\")\n {\n unless ($return = do $file) {\n warn \"couldn't parse $file: $@\" if $@;\n warn \"couldn't do $file: $!\" unless defined $return;\n warn \"couldn't run $file\" unless $return;\n }\n }\n\nIn my rewrite I use simply \"require\", and I check it's output.\n\n> +\n> sub gitweb_get_feature {\n> \tmy ($name) = @_;\n> \treturn unless exists $feature{$name};\n> @@ -1089,7 +1145,8 @@ sub dispatch {\n> \t !$project) {\n> \t\tdie_error(400, \"Project needed\");\n> \t}\n> -\t$actions{$action}->();\n> +\t#$actions{$action}->();\n> +\tcache_fetch($action);\n\nWouldn't a better solution be\n\n -\t$actions{$action}->();\n +\tif ($caching_enabled) {\n +\t\tcache_fetch($action);\n +\t} else {\n +\t\t$actions{$action}->();\n +\t}\n\n> }\n> \n> sub reset_timer {\n> @@ -1159,6 +1216,7 @@ sub change_output {\n> \n> \t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n> \topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n> +\tprint STDOUT_REAL \"\";\n\nWhy it is for?\n\n> \n> \t# Close STDOUT, so that it isn't being used anymore.\n> \tclose STDOUT;\n> @@ -1185,10 +1243,7 @@ sub run {\n> \t\t$pre_dispatch_hook->()\n> \t\t\tif $pre_dispatch_hook;\n> \n> -\t\tchange_output();\n> \t\trun_request();\n> -\t\treset_output();\n> -\t\tprint $output;\n> \n> \t\t$post_dispatch_hook->()\n> \t\t\tif $post_dispatch_hook;\n\nHmmm...\n\n> @@ -3465,7 +3520,9 @@ sub git_header_html {\n> \t# support xhtml+xml but choking when it gets what it asked for.\n> \tif (defined $cgi->http('HTTP_ACCEPT') &&\n> \t $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\\s|^)application\\/xhtml\\+xml(,|;|\\s|$)/ &&\n> -\t $cgi->Accept('application/xhtml+xml') != 0) {\n> +\t $cgi->Accept('application/xhtml+xml') != 0\n> +\t &&\n> +\t $cache_enable == 0) {\n> \t\t$content_type = 'application/xhtml+xml';\n> \t} else {\n> \t\t$content_type = 'text/html';\n\nI also have something similar in my rewrite.\n\n> @@ -3610,6 +3667,7 @@ sub git_footer_html {\n> \tmy $feed_class = 'rss_logo';\n> \n> \tprint \"
\\n\";\n> +\tprint \"
Cache Last Updated: \". gmtime( time ) .\" GMT
\\n\";\n> \tif (defined $project) {\n> \t\tmy $descr = git_get_project_description($project);\n> \t\tif (defined $descr) {\n\nIn my rewrite it replaces \"time to generate page\" info, though\nprobably should be always there.\n\n> @@ -3698,6 +3756,11 @@ sub die_error {\n> \t\t500 => '500 Internal Server Error',\n> \t\t503 => '503 Service Unavailable',\n> \t);\n> +\t# The output handlers for die_error need to be reset to STDOUT\n> +\t# so that half the message isn't being output to random and\n> +\t# half to STDOUT as expected. This is mainly for the benefit\n> +\t# of using git_header_html() and git_footer_html() since\n> +\t#\n> \t# Reset the output so that we are actually going to STDOUT as opposed\n> \t# to buffering the output.\n> \treset_output();\n> @@ -5610,9 +5673,15 @@ sub git_blob_plain {\n> \t\t\t($sandbox ? 'attachment' : 'inline')\n> \t\t\t. '; filename=\"' . $save_as . '\"');\n> \tlocal $/ = undef;\n> -\tbinmode STDOUT, ':raw';\n> -\tprint <$fd>;\n> -\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n> +\tif( $cache_enable != 0){\n> +\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n> +\t}else{\n> +\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n> +\t}\n> +\tbinmode BINOUT, ':raw';\n> +\tprint BINOUT <$fd>;\n> +\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n> +\tclose BINOUT;\n\nWhy it is here? Why caching engine connot take care of that?\n\n> \tclose $fd;\n> }\n> \n> @@ -5897,9 +5966,15 @@ sub git_snapshot {\n> \n> \topen my $fd, \"-|\", $cmd\n> \t\tor die_error(500, \"Execute git-archive failed\");\n> -\tbinmode STDOUT, ':raw';\n> -\tprint <$fd>;\n> -\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n> +\tif( $cache_enable != 0){\n> +\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n> +\t}else{\n> +\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n> +\t}\n> +\tbinmode BINOUT, ':raw';\n> +\tprint BINOUT <$fd>;\n> +\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n> +\tclose BINOUT;\n> \tclose $fd;\n> }\n\nSame as above. I don't think that having to modify gitweb code beside\nenabling caching somewhere at the top shows a good design...\n\n> \n> diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\n> index 4132aab..972d32e 100644\n> --- a/gitweb/static/gitweb.css\n> +++ b/gitweb/static/gitweb.css\n> @@ -67,6 +67,12 @@ div.page_path {\n> \tborder-width: 0px 0px 1px;\n> }\n> \n> +div.cachetime {\n> +\tfloat: left;\n> +\tmargin-right: 10px;\n> +\tcolor: #555555;\n> +}\n\nO.K.\n\n> +\n> div.page_footer {\n> \theight: 17px;\n> \tpadding: 4px 8px;\n\nP.S. Where is my review ;-)???\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"154687","messageId":"7vy69i7yxd.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"1288226574-19068-2-git-send-email-warthog9@eaglescrag.net","subject":"Re: [PATCH 1/3] gitweb: Add option to force version match","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-28T22:08:46Z","receivedAt":"2010-10-28T22:08:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I do not understand why this variable needs to be turned _on_ by default,\nbut more importantly, isn't this more or less independent from what we\ndiscussed at GitTogether, which is to get your battle-tested caching layer\nthat will tremendously help any nontrivial site merged earlier rather than\nlater by reducing load average from 1000 to 3-or-4?\n"},{"id":"154694","messageId":"7vwrp27xzh.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"1288226574-19068-1-git-send-email-warthog9@eaglescrag.net","subject":"Re: [PATCH 0/3] Gitweb caching v7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-28T22:29:06Z","receivedAt":"2010-10-28T22:29:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"John 'Warthog9' Hawley\" writes:\n\n> v7:\n> \t- Rework output system, now central STDOUT redirect\n\nYeah, when I saw your print-to-variable-assignment that was one of the\nthings that came to my mind. Sounds like a reasonable thing to do.\n"},{"id":"154816","messageId":"7vbp6c63gt.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"1288226574-19068-1-git-send-email-warthog9@eaglescrag.net","subject":"Re: [PATCH 0/3] Gitweb caching v7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-29T22:25:54Z","receivedAt":"2010-10-29T22:25:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I am getting this in the gitweb.log:\n\n [Fri Oct 29 22:21:12 2010] gitweb.perl: Undefined subroutine &main::cache_fetch called at .../t/../gitweb/gitweb.perl line 1124.\n\nwhich seems to cause t9500 to fail.\n"},{"id":"154845","messageId":"m3k4l0girs.fsf@localhost.localdomain","threadId":"25573","inReplyTo":"7vbp6c63gt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/3] Gitweb caching v7","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-10-30T08:58:29Z","receivedAt":"2010-10-30T08:58:29Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano writes:\n\n> I am getting this in the gitweb.log:\n> \n> [Fri Oct 29 22:21:12 2010] gitweb.perl: Undefined subroutine \n> &main::cache_fetch called at .../t/../gitweb/gitweb.perl line 1124.\n> \n> which seems to cause t9500 to fail.\n\nThis is caused by three issues (bugs) in v7 caching code.\n\nFirst is the reason why t9500 exhibits this bug. The gitweb caching\nv7 includes file with subroutines related to caching in the following\nway:\n\n do 'cache.pm';\n\nNote that the relative path is used; for t9500 it is relative from\nsomewhere witjin 't/', and not from 'gitweb/', so \"do 'cache.pm';\"\ndoesn't find it.\n\nIn my earlier rewrites I used\n\n do $cache_pm;\n\nand 't/gitweb-lib.sh' set $cache_pm appriopriately.\n\n\nSecond is why this bug is bad. There is no error checking that \n\"do 'cache.pm';\" succeeded. It should be\n\n do $cache_pm;\n die $@ if $@;\n\nat least. Perhaps even better would be to simply turn off caching\nsupport if there is an error in 'cache.pm' (which probably should be\ncalled 'cache.pl', as it is not proper Perl module)... though on the\nother hand side it would could hide the fact that caching is not\nworking.\n\n\nThird is why this matters. In v7 the cache_fetch() subroutine,\ndefined in 'cache.pm', is run *unconditionally*, and has test if the\ncaching is enabled *inside it*, instead of having gitweb (caller) use\nit only if caching is disabled.\n\nThis coupled with the fact that 'make install-gitweb' would *not*\ninstall 'cache.pm' alongside gitweb.cgi means that anybody upgrading\ngitweb, whether he/she wants caching or not, would have gitweb stop\nworking after upgrade... unless he/she knows that he/she has to copy\n'cache.pm' file manually.\n\nOn the other hand having test first if caching is enabled would make\nt9500 tests pass (because they do not even test minimally cache\nenabled / cache disabled cases, like in my rewrite), but would hide\nproblem when caching is enabled and 'cache.pm' is not installed...\n(perhaps also in persistent environments; I don't know where pwd is\nthen).\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"154865","messageId":"7v1v773s7e.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"m3k4l0girs.fsf@localhost.localdomain","subject":"Re: [PATCH 0/3] Gitweb caching v7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-31T04:24:21Z","receivedAt":"2010-10-31T04:24:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n>> I am getting this in the gitweb.log:\n>> \n>> [Fri Oct 29 22:21:12 2010] gitweb.perl: Undefined subroutine \n>> &main::cache_fetch called at .../t/../gitweb/gitweb.perl line 1124.\n>> \n>> which seems to cause t9500 to fail.\n>\n> This is caused by three issues (bugs) in v7 caching code.\n>\n> First is the reason why t9500 exhibits this bug. The gitweb caching\n> v7 includes file with subroutines related to caching in the following\n> way:\n>\n> do 'cache.pm';\n>\n> Note that the relative path is used; for t9500 it is relative from\n> somewhere witjin 't/', and not from 'gitweb/', so \"do 'cache.pm';\"\n> doesn't find it.\n\nJohn, where should cache.pm go in the installed system? Does it typically\ngo next to gitweb.perl?\n\nI think \"do 'that-file'\" honors path specified by the -I option, so I do\nnot think \"do $cache_pm\" is necessary. My preference is to run gitweb\ntests with appropriate -I pointing at the cache.pm in the directory.\n\n> ... It should be\n>\n> do $cache_pm;\n> die $@ if $@;\n>\n> at least.\n\nCatching failure is a good thing to do.\n\n> Perhaps even better would be to simply turn off caching\n> support if there is an error in 'cache.pm'\n\nThat can come later.\n\nJakub, can we have an absolute minimum fix-up, so that we can give\nthis wider exposure? I think there are only\nfour issues:\n\n (1) exclude Ajax-y stuff from caching;\n (2) install cache.pm the same way gitweb.perl is installed via\n the Makefile;\n (3) running tests with appropriate -I so that cache.pm is found; and\n (4) die if 'cache.pm' cannot be \"done\").\n\nI think the change in gitweb-cache v7 is small and safe enough that we\nshould fast-track it to give usability to the real world sites. It may be\na low-risk \"obviously correct\" approach that is quick-and-dirty, but that\nis exactly why this should be fast-tracked. It does not touch the logic\nor formatting in any way, just bypasses the page generation altogether\nwhen it can clearly do so when it can tell the output cannot possibly be\nincorrect (albeit sometimes it might be stale if in certain cases, e.g. it\nis relative to HEAD).\n\nI know you and others were aiming to split things up, but I think the\namount of the effort that is needed for that line of work on top of the\ncurrent codebase is not much different from what is needed on top of\ngitweb-cache v7.\n"},{"id":"154880","messageId":"201010311021.55917.jnareb@gmail.com","threadId":"25573","inReplyTo":"7v1v773s7e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/3] Gitweb caching v7","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-10-31T09:21:54Z","receivedAt":"2010-10-31T09:21:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 31 Oct 2010, Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n>>> I am getting this in the gitweb.log:\n>>> \n>>> [Fri Oct 29 22:21:12 2010] gitweb.perl: Undefined subroutine \n>>> &main::cache_fetch called at .../t/../gitweb/gitweb.perl line 1124.\n>>> \n>>> which seems to cause t9500 to fail.\n>>\n>> This is caused by three issues (bugs) in v7 caching code.\n>>\n>> First is the reason why t9500 exhibits this bug. The gitweb caching\n>> v7 includes file with subroutines related to caching in the following\n>> way:\n>>\n>> do 'cache.pm';\n>>\n>> Note that the relative path is used; for t9500 it is relative from\n>> somewhere witjin 't/', and not from 'gitweb/', so \"do 'cache.pm';\"\n>> doesn't find it.\n> \n> John, where should cache.pm go in the installed system? Does it typically\n> go next to gitweb.perl?\n> \n> I think \"do 'that-file'\" honors path specified by the -I option, so I do\n> not think \"do $cache_pm\" is necessary. My preference is to run gitweb\n> tests with appropriate -I pointing at the cache.pm in the directory.\n\n>From `perldoc -f do`\n\n do 'stat.pl';\n\n is just like\n\n eval 'cat stat.pl';\n\n except that it's more efficient and concise, keeps track of the current\n filename for error messages, searches the @INC libraries, and updates %INC\n if the file is found. ^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\nSo I think it respects '-I' given to perl interpreter.\n\nOn the other hand I don't know how it would work with mod_perl (uwing\nModPerl::Registry handler), whether it wouldn't too require extra\nconfiguration.\n\nSo I think a better solution would be to base 'Gitweb caching v7' plus\nnecessary fixes and improvements on top of \n\n gitweb: Prepare for splitting gitweb\n\nThis means that the directory that gitweb is in would be added to @INC\nvia\n\n use lib __DIR__.'/lib';\n\nThe 'cache.pm' or 'cache.pl' file would be moved to 'gitweb/lib', but\nthat is cosmetic change.\n\n> \n>> ... It should be\n>>\n>> do $cache_pm;\n>> die $@ if $@;\n>>\n>> at least.\n> \n> Catching failure is a good thing to do.\n\nRight. And this is only one-line addition.\n \n>> Perhaps even better would be to simply turn off caching\n>> support if there is an error in 'cache.pm'\n> \n> That can come later.\n> \n> Jakub, can we have an absolute minimum fix-up, so that we can give\n> this wider exposure? I think there are only\n> four issues:\n> \n> (1) exclude Ajax-y stuff from caching;\n\nEasy, but only if check whether to do capturing and caching is moved\nout of cache_fetch to caller, i.e. to gitweb script. See also comments\nbelow about what need and should be done.\n\nAnother solution is to turn off Ajax-y stuff when caching is enabled.\nIt can be done quite easily, just sprinkle some !$caching_enabled\n(see comment below about naming and semantic of this config variable)\nin appropriate place.\n\n> (2) install cache.pm the same way gitweb.perl is installed via\n> the Makefile;\n\nWith \"gitweb: Prepare for splitting gitweb\" as introductory patch this\nwould be just adding\n\n # gitweb output caching\n GITWEB_MODULES += cache.pm\n\n(or cache.pl - it is not a proper Perl package!) e.g. above GITWEB_REPLACE\nsed script in gitweb/Makefile.\n\n> (3) running tests with appropriate -I so that cache.pm is found; and\n\nNo change to test necessary if we use \"use lib __DIR__.'/lib';\" in\ngitweb.perl\n\nI'd like to port from my rewrite change to t/t9500-gitweb-standalone-no-errors.sh\nadding minimal test to check if running gitweb with caching enabled\ndoesn't generate any warnings, and (modified) change to the\nt/t9502-gitweb-standalone-parse-output.sh, adding minimal test that\ngitweb produces the same output with and without caching.\n\n> (4) die if 'cache.pm' cannot be \"done\".\n\nO.K. (4) is one liner.\n\nThere is are also other issues\n\n (5) naming and semantic of gitweb config variables configuring caching;\n at least change $cache_enabled enum to $caching_enabled boolean\n (6) do not change anything in gitweb behavior if caching is disabled;\n move 'if ($caching_enabled)' test to gitweb.perl, and remove code\n from cache_fetch\n\nAbout (5): names and semantics of configuration variables are gitweb API,\nso we should at least try to keep backward compatibility with old names\n(see for example 'backward compatibility: legacy gitweb config support'\nsection in %known_snapshot_format_aliases). \n\nThere is no much problem with $minCacheTime, $maxCacheTime, $backgroundCache\netc. (besides bleh, camelCase ;-) but I would prefer to have *boolean*\n$caching_enabled (true-ish value means cache is enabled, false-y value\nincluding undef means caching is disabled) to currently two-value *\"enum\"*\n$cache_enable called \"binary\" (sic!) in docs and comments. See also\nissue (6).\n\n\nAbout (6): I would prefer to move check if caching should be enabled\nhigher, to gitweb.perl (to its own configure_caching() subroutine, rather\nthan sprinkling it in top scope), and not even include 'cache.pm'\nif caching is disabled (default). But because we use 'do' and not\n'require', we would have to check %INC to not include it twice...\n\nAnyway, whether we include 'cache.pm' conditionally or not, I'd prefer\nto use\n\n if ($caching_enabled) {\n \tcache_fetch($action);\n } else {\n \t$actions{$action}->();\n }\n\nin dispatch() subroutine, and simply remove all code dealing with\n'$cache_enable == 0' case from 'cache.pm'.\n\nRemember, config variables are forever :-)\n\n> \n> I think the change in gitweb-cache v7 is small and safe enough that we\n> should fast-track it to give usability to the real world sites. It may be\n> a low-risk \"obviously correct\" approach that is quick-and-dirty, but that\n> is exactly why this should be fast-tracked. It does not touch the logic\n> or formatting in any way, just bypasses the page generation altogether\n> when it can clearly do so when it can tell the output cannot possibly be\n> incorrect (albeit sometimes it might be stale if in certain cases, e.g. it\n> is relative to HEAD).\n\nMy rewrite also does the same. I have changed it to do the same STDOUT\nredirection that J.H. gitweb caching v7 does (I used select($fh) based\ncapture) - work in progress, not yet pushed to my repository:\n\n git://repo.or.cz/git/jnareb-git.git\n http://repo.or.cz/w/git/jnareb-git.git (gitweb)\n\n> \n> I know you and others were aiming to split things up, but I think the\n> amount of the effort that is needed for that line of work on top of the\n> current codebase is not much different from what is needed on top of\n> gitweb-cache v7.\n\nWell, I'd have to add support for configuration variables used in this\npatch series, but with exception of $cache_enable -> $caching_enabled\nit wouldn't be much extra work.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"154921","messageId":"1288607092-31458-1-git-send-email-jnareb@gmail.com","threadId":"25573","inReplyTo":"201010311021.55917.jnareb@gmail.com","subject":"[PATCHv7.1 0/4] Gitweb caching v7.1","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-01T10:24:48Z","receivedAt":"2010-11-01T10:24:48Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This series is a bit fixed up and a tiny bit cleaned up version of\n\"Gitweb caching v7\" series from John 'Warthog9' Hawley.\n\nThis series is based on top of 8ff76f4 (gitweb: Move call to\nevaluate_git_version after evaluate_gitweb_config, 2010-09-26)\ncurrently in 'pu'. This commit fixes a bug in gitweb which shows in\nrunning testsuite.\n\nThe patch \"gitweb: Add option to force version match\" was removed from\nthis series as it doesn't belong to it, c.f. \n http://article.gmane.org/gmane.comp.version-control.git/160236\n\n [PATCHv7.1 1/4] gitweb: Prepare for splitting gitweb\n [PATCHv7.1 2/4] gitweb: add output buffering and associated functions\n [PATCHv7.1 3/4] gitweb: File based caching layer (from git.kernel.org)\n [PATCHv7.1 4/4] gitweb: Minimal testing of gitweb caching\n\nJakub Narebski (2):\n gitweb: Prepare for splitting gitweb\n gitweb: Minimal testing of gitweb caching\n\nJohn 'Warthog9' Hawley (2):\n gitweb: add output buffering and associated functions\n gitweb: File based caching layer (from git.kernel.org)\n\n gitweb/Makefile | 20 ++-\n gitweb/gitweb.perl | 134 +++++++++++-\n gitweb/lib/cache.pl | 348 +++++++++++++++++++++++++++++\n gitweb/static/gitweb.css | 6 +\n t/t9500-gitweb-standalone-no-errors.sh | 19 ++\n t/t9502-gitweb-standalone-parse-output.sh | 39 ++++\n 6 files changed, 555 insertions(+), 11 deletions(-)\n create mode 100644 gitweb/lib/cache.pl\n\n-- \n1.7.3\n"},{"id":"154922","messageId":"1288607092-31458-2-git-send-email-jnareb@gmail.com","threadId":"25573","inReplyTo":"201010311021.55917.jnareb@gmail.com","subject":"[PATCHv7.1 1/4] gitweb: Prepare for splitting gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-01T10:24:49Z","receivedAt":"2010-11-01T10:24:49Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Prepare gitweb for having been split into modules that are to be\ninstalled alongside gitweb in 'lib/' subdirectory, by adding\n\n use lib __DIR__.'/lib';\n\nto gitweb.perl (to main gitweb script), and preparing for putting\nmodules (relative path) in $(GITWEB_MODULES) in gitweb/Makefile.\n\nThis preparatory work allows to add new module to gitweb by simply\nadding\n\n GITWEB_MODULES += \n\nto gitweb/Makefile (assuming that the module is in 'gitweb/lib/'\ndirectory).\n\nUsing __DIR__ from Dir::Self module (not in core, that's why currently\ngitweb includes excerpt of code from Dir::Self defining __DIR__) was\nchosen over using FindBin-based solution (in core since perl 5.00307,\nwhile gitweb itself requires at least perl 5.8.0) because FindBin uses\nBEGIN block, which is a problem under mod_perl and other persistent\nPerl environments (thought there are workarounds).\n\nAt Pavan Kumar Sankara suggestion gitweb/Makefile uses\n\n install [OPTION]... SOURCE... DIRECTORY\n\nformat (2nd format) with single SOURCE rather than\n\n install [OPTION]... SOURCE DEST\n\nformat (1st format) because of security reasons (race conditions).\nModern GNU install has `-T' / `--no-target-directory' option, but we\ncannot rely that the $(INSTALL) we are using supports this option.\n\nThe install-modules target in gitweb/Makefile uses shell 'for' loop,\ninstead of make's $(foreach) function, to avoid possible problem with\ngenerating a command line that exceeded the maximum argument list\nlength.\n\nSigned-off-by: Jakub Narebski \n---\nThis patch would conflict with the patch 958a846 (gitweb/Makefile: Add\n'test' and 'test-installed' targets, 2010-09-26), which is in 'pu'.\nResolving those conflicts is easy, though non-trivial.\n\nPlease tell me if I should send this series rebased on top of \n'jn/gitweb-test' (which includes 958a846) instead.\n\nThis patch introduces infrastructure which would be required later for\nsplitting gitweb.\n\n gitweb/Makefile | 17 ++++++++++++++---\n gitweb/gitweb.perl | 8 ++++++++\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 2fb7c2d..6fa7625 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -55,6 +55,7 @@ PERL_PATH ?= /usr/bin/perl\n bindir_SQ = $(subst ','\\'',$(bindir))#'\n gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))#'\n gitwebstaticdir_SQ = $(subst ','\\'',$(gitwebdir)/static)#'\n+gitweblibdir_SQ = $(subst ','\\'',$(gitwebdir)/lib)#'\n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))#'\n PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))#'\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))#'\n@@ -145,16 +146,26 @@ gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS\n \n ### Installation rules\n \n-install: all\n+install: all install-modules\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \t$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \n+install-modules:\n+\tinstall_dirs=\"$(sort $(dir $(GITWEB_MODULES)))\" && \\\n+\tfor dir in $$install_dirs; do \\\n+\t\ttest -d '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$dir' || \\\n+\t\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$dir'; \\\n+\tdone\n+\tgitweb_modules=\"$(GITWEB_MODULES)\" && \\\n+\tfor mod in $$gitweb_modules; do \\\n+\t\t$(INSTALL) -m 644 lib/$$mod '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$(dirname $$mod)'; \\\n+\tdone\n+\n ### Cleaning rules\n \n clean:\n \t$(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS\n \n-.PHONY: all clean install .FORCE-GIT-VERSION-FILE FORCE\n-\n+.PHONY: all clean install install-modules .FORCE-GIT-VERSION-FILE FORCE\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex d521b4c..e4c08ba 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -9,6 +9,14 @@\n \n use strict;\n use warnings;\n+\n+use File::Spec;\n+# __DIR__ is taken from Dir::Self __DIR__ fragment\n+sub __DIR__ () {\n+\tFile::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);\n+}\n+use lib __DIR__ . '/lib';\n+\n use CGI qw(:standard :escapeHTML -nosticky);\n use CGI::Util qw(unescape);\n use CGI::Carp qw(fatalsToBrowser set_message);\n-- \n1.7.3\n"},{"id":"154923","messageId":"1288607092-31458-3-git-send-email-jnareb@gmail.com","threadId":"25573","inReplyTo":"201010311021.55917.jnareb@gmail.com","subject":"[PATCHv7.1 2/4] gitweb: add output buffering and associated functions","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-01T10:24:50Z","receivedAt":"2010-11-01T10:24:50Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis adds output buffering for gitweb, mainly in preparation for\ncaching support. This is a dramatic change to how caching was being\ndone, mainly in passing around the variable manually and such.\n\nThis centrally flips the entire STDOUT to a variable, which after the\ncompletion of the run, flips it back and does a print on the resulting\ndata.\n\nThis should save on the previous 10K line patch (or so) that adds more\nexplicit output passing.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Jakub Narebski \n---\nCommit message unchanged.\n\nI have modified reset_output to silence the following warning from t9500:\n\n gitweb.perl: Name \"main::STDOUT_REAL\" used only once: possible typo\n at ../gitweb/gitweb.perl line 1130.\n\nHere is the interdiff for gitweb.perl from 2e5c355 (J.H. patch in 'pu'):\n\n diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n index b2b0a23..91e274f 100755\n --- a/gitweb/gitweb.perl\n +++ b/gitweb/gitweb.perl\n @@ -32,7 +39,7 @@ BEGIN {\n our $version = \"++GIT_VERSION++\";\n \n # Output buffer variable\n -$output = \"\";\n +our $output = \"\";\n \n our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n sub evaluate_uri {\n @@ -1143,13 +1138,12 @@ sub change_output {\n }\n \n sub reset_output {\n -\t# This basically takes STDOUT_REAL and puts it back as STDOUTl\n -\topen(STDOUT,\">&STDOUT_REAL\");\n +\t# This basically takes STDOUT_REAL and puts it back as STDOUT\n +\topen STDOUT, \">&\", \\*STDOUT_REAL;\n }\n \n\n gitweb/gitweb.perl | 29 +++++++++++++++++++++++++++++\n 1 files changed, 29 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e4c08ba..91e274f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -38,6 +38,9 @@ BEGIN {\n \n our $version = \"++GIT_VERSION++\";\n \n+# Output buffer variable\n+our $output = \"\";\n+\n our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n sub evaluate_uri {\n \tour $cgi;\n@@ -1120,6 +1123,25 @@ sub evaluate_argv {\n \t);\n }\n \n+sub change_output {\n+\tour $output;\n+\n+\t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n+\topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n+\n+\t# Close STDOUT, so that it isn't being used anymore.\n+\tclose STDOUT;\n+\n+\t# Trap STDOUT to the $output variable, which is what I was using in the original\n+\t# patch anyway.\n+\topen(STDOUT,\">\", \\$output) || die \"Unable to open STDOUT: $!\"; #open STDOUT handle to use $var\n+}\n+\n+sub reset_output {\n+\t# This basically takes STDOUT_REAL and puts it back as STDOUT\n+\topen STDOUT, \">&\", \\*STDOUT_REAL;\n+}\n+\n sub run {\n \tevaluate_argv();\n \n@@ -1131,7 +1153,10 @@ sub run {\n \t\t$pre_dispatch_hook->()\n \t\t\tif $pre_dispatch_hook;\n \n+\t\tchange_output();\n \t\trun_request();\n+\t\treset_output();\n+\t\tprint $output;\n \n \t\t$post_dispatch_hook->()\n \t\t\tif $post_dispatch_hook;\n@@ -3640,6 +3665,10 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\t# Reset the output so that we are actually going to STDOUT as opposed\n+\t# to buffering the output.\n+\treset_output();\n+\n \tgit_header_html($http_responses{$status}, undef, %opts);\n \tprint <\n-- \n1.7.3\n"},{"id":"154924","messageId":"1288607092-31458-4-git-send-email-jnareb@gmail.com","threadId":"25573","inReplyTo":"201010311021.55917.jnareb@gmail.com","subject":"[PATCHv7.1 3/4] gitweb: File based caching layer (from git.kernel.org)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-01T10:24:51Z","receivedAt":"2010-11-01T10:24:51Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis is a relatively large patch that implements the file based\ncaching layer that is quite similar to the one used on such large\nsites as kernel.org and soon git.fedoraproject.org. This provides\na simple, and straight forward caching mechanism that scales\ndramatically better than Gitweb by itself.\n\nThe caching layer basically buffers the output that Gitweb would\nnormally return, and saves that output to a cache file on the local\ndisk. When the file is requested it attempts to gain a shared lock\non the cache file and cat it out to the client. Should an exclusive\nlock be on a file (it's being updated) the code has a choice to either\nupdate in the background and go ahead and show the stale page while\nupdate is being performed, or stall the client(s) until the page\nis generated.\n\nThere are two forms of stalling involved here, background building\nand non-background building, both of which are discussed in the\nconfiguration page.\n\nThere are still a few known \"issues\" with respect to this:\n- Code needs to be added to be \"browser\" aware so\n that clients like wget that are trying to get a\n binary blob don't obtain a \"Generating...\" page\n\nCaching is disabled by default. You can turn it on by setting\n$caching_enabled variable to true to enable file based caching.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Jakub Narebski \n---\nThis is *minimal* fixup of J.H. patch, addressing only major concerns\nof Junio C Hamano (JH) and me (JN).\n\nJH> Jakub, can we have an absolute minimum fix-up, so that we can give\nJH> this wider exposure? I think there are only\nJH> four issues:\nJH>\nJH> (1) exclude Ajax-y stuff from caching\n\nDone, via \"if ($caching_enabled && is_cacheable($action) { ... }\".\n\nAlternate solution would be to degrade Ajax-y actions to their\nnon-Ajax-y versions (i.e. 'blame_incremental' to just 'blame') when\ncaching is enabled.\n\nJH> (2) install cache.pm the same way gitweb.perl is installed via\nJH> the Makefile\n\nIt is now 'gitweb/lib/cache.pl' (*.pl and not *.pm because it is not a\nproper Perl module: it lacks 'package Foo' for one), and is installed\nvia 'install-modules' target in gitweb/Makefile, which is prereq to\n'install' therein. So 'make install-gitweb' would install cache.pl\nalongside gitweb.\n\nJH> (3) running tests with appropriate -I so that cache.pm is found;\n\nThanks to \"use lib __DIR__.'/lib';\" it is not necessary. The \n'use lib' solution has the advantage that it would work for mod_perl\ntoo.\n\nJH> (4) die if 'cache.pm' cannot be \"done\".\n\n #\n # Includes\n #\n if (!exists $INC{'cache.pl'}) {\n \tmy $return = do 'cache.pl';\n \tdie $@ if $@;\n \tdie \"Couldn't read 'cache.pl': $!\" if (!defined $return);\n }\n\nGitweb now includes 'cache.pl' only once (which is important in\npersistent environments that wrap gitweb, like mod_perl's\nModPerl::Registry, or Plack's Plack::App::WrapCGI which is used for\n--httpd=plackup in git-instaweb), checks if there were any errors\nparsing it, and checks if there were any errors finding it\n(e.g. locating it).\n\nJN> (5) naming and semantic of gitweb config variables configuring caching;\nJN> at least change $cache_enabled enum to $caching_enabled boolean\n\nDone. $cache_enable is not more.\n\nJN> (6) do not change anything in gitweb behavior if caching is disabled;\nJN> move 'if ($caching_enabled)' test to gitweb.perl, and remove code\nJN> from cache_fetch\n\nThis simplified 'cache.pl' by removing all conditionals depending on\nstate of $cache_enable from it (see interdiff below). This required\nadding check if caching is enabled before running reset_output() in\ndie_error() subroutine.\n\n\nFor tests to not hang I had to change permissions of cache directory\nfrom 0665 ('rw-rw-r-x') to 0755 ('rwxr-xr-x'). I think that was a\nbug.\n\nInterdiff (with '--ignore-all-space'):\n\n diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n index 8f748a3..abaeec6 100755\n --- a/gitweb/gitweb.perl\n +++ b/gitweb/gitweb.perl\n @@ -250,11 +251,7 @@ our $maxload = 300;\n # that the cache directory be periodically completely deleted, and this is safe to perform.\n # Suggested mechanism\n # mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush\n -# Value is binary. 0 = disabled (default), 1 = enabled.\n -#\n -# Values of caching:\n -# \t1 = 'dumb' file based caching used on git.kernel.org\n -our $cache_enable = 0;\n +our $caching_enabled = 0;\n \n # Used to set the minimum cache timeout for the dynamic caching algorithm. Basically\n # if we calculate the cache to be under this number of seconds we set the cache timeout\n @@ -552,7 +549,11 @@ our %feature = (\n #\n # Includes\n #\n -do 'cache.pm';\n +if (!exists $INC{'cache.pl'}) {\n +\tmy $return = do 'cache.pl';\n +\tdie $@ if $@;\n +\tdie \"Couldn't read 'cache.pl': $!\" if (!defined $return);\n +}\n \n sub gitweb_get_feature {\n \tmy ($name) = @_;\n @@ -782,6 +783,10 @@ our %actions = (\n \t\"project_list\" => \\&git_project_list,\n \t\"project_index\" => \\&git_project_index,\n );\n +sub is_cacheable {\n +\tmy $action = shift;\n +\treturn !($action eq 'blame_data' || $action eq 'blame_incremental');\n +}\n \n # finally, we have the hash of allowed extra_options for the commands that\n # allow them\n @@ -1120,8 +1118,11 @@ sub dispatch {\n \t !$project) {\n \t\tdie_error(400, \"Project needed\");\n \t}\n -\t#$actions{$action}->();\n +\tif ($caching_enabled && is_cacheable($action)) {\n \tcache_fetch($action);\n +\t} else {\n +\t\t$actions{$action}->();\n +\t}\n }\n \n sub reset_timer {\n @@ -3494,9 +3494,8 @@ sub git_header_html {\n \t# support xhtml+xml but choking when it gets what it asked for.\n \tif (defined $cgi->http('HTTP_ACCEPT') &&\n \t $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\\s|^)application\\/xhtml\\+xml(,|;|\\s|$)/ &&\n -\t $cgi->Accept('application/xhtml+xml') != 0\n -\t &&\n -\t $cache_enable == 0) {\n +\t $cgi->Accept('application/xhtml+xml') != 0 &&\n +\t !$caching_enabled) {\n \t\t$content_type = 'application/xhtml+xml';\n \t} else {\n \t\t$content_type = 'text/html';\n @@ -3737,7 +3736,7 @@ sub die_error {\n \t#\n \t# Reset the output so that we are actually going to STDOUT as opposed\n \t# to buffering the output.\n -\treset_output();\n +\treset_output() if ($caching_enabled);\n \n \tgit_header_html($http_responses{$status}, undef, %opts);\n \tprint <', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n \t}else{\n \t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n @@ -5940,7 +5939,7 @@ sub git_snapshot {\n \n \topen my $fd, \"-|\", $cmd\n \t\tor die_error(500, \"Execute git-archive failed\");\n -\tif( $cache_enable != 0){\n +\tif ($caching_enabled) {\n \t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n \t}else{\n \t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n diff --git a/gitweb/cache.pm b/gitweb/lib/cache.pl\n similarity index 75%\n rename from gitweb/cache.pm\n rename to gitweb/lib/cache.pl\n index c86265c..dd14bfb 100644\n --- a/gitweb/cache.pm\n +++ b/gitweb/lib/cache.pl\n @@ -18,21 +18,9 @@ sub cache_fetch {\n \tmy ($action) = @_;\n \tmy $cacheTime = 0;\n \n -\t# Deal with cache being disabled\n -\tif( $cache_enable == 0 ){\n -\t\tundef $backgroundCache;\n -\t\t#$fullhashpath = \\$nocachedata;\n -\t\t#$fullhashbinpath = \\$nocachedatabin;\n -\t\t$fullhashpath = *STDOUT;\n -\t\t$fullhashbinpath = *STDOUT;\n -\t\t$actions{$action}->();\n -\t\treturn;\n -\t}elsif( $cache_enable == 1 ){\n -\t\t#obviously we are using file based caching\n -\n \t\tif(! -d $cachedir){\n \t\t\tprint \"*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\\n\";\n -\t\t\tmkdir (\"cache\", 0665) || die \"Cannot create cache dir - you will need to manually create\";\n +\t\tmkdir (\"cache\", 0755) || die \"Cannot create cache dir - you will need to manually create\";\n \t\t\tprint \"Cache directory created successfully\\n\";\n \t\t}\n \n @@ -47,10 +35,9 @@ sub cache_fetch {\n \t\t$fullhashpath = \"$fullhashdir/\". substr( $urlhash, 2 );\n \t\t$fullhashbinpath = \"$fullhashpath.bin.wt\";\n \t\t$fullhashbinpathfinal = \"$fullhashpath.bin\";\n -\t} # done dealing with cache enabled / disabled\n \n \tif(! -e \"$fullhashpath\" ){\n -\t\tif( ! $cache_enable || ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n +\t\tif(! $cacheDoFork || ! defined(my $childPid = fork()) ){\n \t\t\tcacheUpdate($action,0);\n \t\t\tcacheDisplay($action);\n \t\t} elsif ( $childPid == 0 ){\n @@ -112,8 +99,7 @@ sub cacheUpdate {\n \n \t\t$lockStatus = $lockStatBG;\n \t}else{\n -\t\topen(cacheFile, '>:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n -\t\topen(cacheFile, '>:utf8', \\$fullhashpath) if ($cache_enable == 0);\n +\t\topen(cacheFile, '>:utf8', \\$fullhashpath);\n \t\tmy $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n \n \t\t$lockStatus = $lockStat;\n @@ -121,7 +107,7 @@ sub cacheUpdate {\n \t#print \"lock status: $lockStat\\n\";\n \n \n -\tif ($cache_enable != 0 && ! $lockStatus ){\n +\tif (! $lockStatus ){\n \t\tif ( $areForked ){\n \t\t\texit(0);\n \t\t}else{\n @@ -220,8 +206,7 @@ sub cacheWaitForUpdate {\n \n \tif( $backgroundCache ){\n \t\tif( -e \"$fullhashpath\" ){\n -\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n -\t\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n +\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n \t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n \t\t\tstat(cacheFile);\n \t\t\tclose(cacheFile);\n @@ -242,8 +227,7 @@ sub cacheWaitForUpdate {\n \t){\n \t\tdo {\n \t\t\tsleep 2 if $x > 0;\n -\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n -\t\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n +\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n \t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n \t\t\tclose(cacheFile);\n \t\t\t$x++;\n @@ -298,8 +282,7 @@ EOF\n \tdo {\n \t\tprint \".\";\n \t\tsleep 2 if $x > 0;\n -\t\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n -\t\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n +\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n \t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n \t\tclose(cacheFile);\n \t\t$x++;\n @@ -317,11 +300,10 @@ sub cacheDisplay {\n \t$|++;\n \n \tmy ($action) = @_;\n -\topen(cacheFile, '<:utf8', \"$fullhashpath\") if ($cache_enable > 0);\n -\topen(cacheFile, '<:utf8', \\$fullhashpath) if ($cache_enable == 0);\n +\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n \t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n \n -\tif ($cache_enable != 0 && ! $lockStat ){\n +\tif (! $lockStat ){\n \t\tclose(cacheFile);\n \t\tcacheWaitForUpdate($action);\n \t}\n @@ -332,12 +314,10 @@ sub cacheDisplay {\n \t\t\t||\n \t\t\t$action eq \"blob_plain\"\n \t\t)\n -\t\t&&\n -\t\t$cache_enable > 0\n \t){\n \t\tmy $openstat = open(cacheFileBin, '<', \"$fullhashbinpathfinal\");\n \t\t$lockStatBIN = flock(cacheFileBin,LOCK_SH|LOCK_NB);\n -\t\tif ($cache_enable != 0 && ! $lockStatBIN ){\n +\t\tif (! $lockStatBIN ){\n \t\t\tsystem (\"echo 'cacheDisplay - bailing due to binary lock failure' >> /tmp/gitweb.log\");\n \t\t\tclose(cacheFile);\n \t\t\tclose(cacheFileBin);\n @@ -363,3 +343,6 @@ sub cacheDisplay {\n \tclose(cacheFile);\n \t$|--;\n }\n +\n +1;\n\n gitweb/Makefile | 3 +\n gitweb/gitweb.perl | 105 ++++++++++++--\n gitweb/lib/cache.pl | 348 ++++++++++++++++++++++++++++++++++++++++++++++\n gitweb/static/gitweb.css | 6 +\n 4 files changed, 450 insertions(+), 12 deletions(-)\n create mode 100644 gitweb/lib/cache.pl\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 6fa7625..315753e 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -111,6 +111,9 @@ endif\n \n GITWEB_FILES += static/git-logo.png static/git-favicon.png\n \n+# Gitweb caching\n+GITWEB_MODULES += cache.pl\n+\n GITWEB_REPLACE = \\\n \t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n \t-e 's|++GIT_BINDIR++|$(bindir)|g' \\\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 91e274f..abaeec6 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -243,6 +243,53 @@ our %avatar_size = (\n # Leave it undefined (or set to 'undef') to turn off load checking.\n our $maxload = 300;\n \n+# This enables/disables the caching layer in gitweb. This currently only supports the\n+# 'dumb' file based caching layer, primarily used on git.kernel.org. this is reasonably\n+# effective but it has the downside of requiring a huge amount of disk space if there\n+# are a number of repositories involved. It is not uncommon for git.kernel.org to have\n+# on the order of 80G - 120G accumulate over the course of a few months. It is recommended\n+# that the cache directory be periodically completely deleted, and this is safe to perform.\n+# Suggested mechanism\n+# mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush\n+our $caching_enabled = 0;\n+\n+# Used to set the minimum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to be under this number of seconds we set the cache timeout\n+# to this minimum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $minCacheTime = 20;\n+\n+# Used to set the maximum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to exceed this number of seconds we set the cache timeout\n+# to this maximum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $maxCacheTime = 1200;\n+\n+# If you need to change the location of the caching directory, override this\n+# otherwise this will probably do fine for you\n+our $cachedir = 'cache';\n+\n+# If this is set (to 1) cache will do it's best to always display something instead\n+# of making someone wait for the cache to update. This will launch the cacheUpdate\n+# into the background and it will lock a .bg file and will only lock the\n+# actual cache file when it needs to write into it. In theory this will make\n+# gitweb seem more responsive at the price of possibly stale data.\n+our $backgroundCache = 1;\n+\n+# Used to set the maximum cache file life. If a cache files last modify time exceeds\n+# this value, it will assume that the data is just too old, and HAS to be regenerated\n+# instead of trying to display the existing cache data.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+# 18000 = 5 hours\n+our $maxCacheLife = 18000;\n+\n+# Used to enable or disable background forking of the gitweb caching. Mainly here for debugging purposes\n+our $cacheDoFork = 1;\n+\n+our $fullhashpath = *STDOUT;\n+our $fullhashbinpath = *STDOUT;\n+our $fullhashbinpathfinal = *STDOUT;\n+\n # configuration for 'highlight' (http://www.andre-simon.de/)\n # match by basename\n our %highlight_basename = (\n@@ -499,6 +546,15 @@ our %feature = (\n \t\t'default' => [0]},\n );\n \n+#\n+# Includes\n+#\n+if (!exists $INC{'cache.pl'}) {\n+\tmy $return = do 'cache.pl';\n+\tdie $@ if $@;\n+\tdie \"Couldn't read 'cache.pl': $!\" if (!defined $return);\n+}\n+\n sub gitweb_get_feature {\n \tmy ($name) = @_;\n \treturn unless exists $feature{$name};\n@@ -727,6 +783,10 @@ our %actions = (\n \t\"project_list\" => \\&git_project_list,\n \t\"project_index\" => \\&git_project_index,\n );\n+sub is_cacheable {\n+\tmy $action = shift;\n+\treturn !($action eq 'blame_data' || $action eq 'blame_incremental');\n+}\n \n # finally, we have the hash of allowed extra_options for the commands that\n # allow them\n@@ -1058,7 +1118,11 @@ sub dispatch {\n \t !$project) {\n \t\tdie_error(400, \"Project needed\");\n \t}\n-\t$actions{$action}->();\n+\tif ($caching_enabled && is_cacheable($action)) {\n+\t\tcache_fetch($action);\n+\t} else {\n+\t\t$actions{$action}->();\n+\t}\n }\n \n sub reset_timer {\n@@ -1128,6 +1192,7 @@ sub change_output {\n \n \t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n \topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n+\tprint STDOUT_REAL \"\";\n \n \t# Close STDOUT, so that it isn't being used anymore.\n \tclose STDOUT;\n@@ -1153,10 +1218,7 @@ sub run {\n \t\t$pre_dispatch_hook->()\n \t\t\tif $pre_dispatch_hook;\n \n-\t\tchange_output();\n \t\trun_request();\n-\t\treset_output();\n-\t\tprint $output;\n \n \t\t$post_dispatch_hook->()\n \t\t\tif $post_dispatch_hook;\n@@ -3432,7 +3494,8 @@ sub git_header_html {\n \t# support xhtml+xml but choking when it gets what it asked for.\n \tif (defined $cgi->http('HTTP_ACCEPT') &&\n \t $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\\s|^)application\\/xhtml\\+xml(,|;|\\s|$)/ &&\n-\t $cgi->Accept('application/xhtml+xml') != 0) {\n+\t $cgi->Accept('application/xhtml+xml') != 0 &&\n+\t !$caching_enabled) {\n \t\t$content_type = 'application/xhtml+xml';\n \t} else {\n \t\t$content_type = 'text/html';\n@@ -3577,6 +3640,7 @@ sub git_footer_html {\n \tmy $feed_class = 'rss_logo';\n \n \tprint \"
\\n\";\n+\tprint \"
Cache Last Updated: \". gmtime( time ) .\" GMT
\\n\";\n \tif (defined $project) {\n \t\tmy $descr = git_get_project_description($project);\n \t\tif (defined $descr) {\n@@ -3665,9 +3729,14 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\t# The output handlers for die_error need to be reset to STDOUT\n+\t# so that half the message isn't being output to random and\n+\t# half to STDOUT as expected. This is mainly for the benefit\n+\t# of using git_header_html() and git_footer_html() since\n+\t#\n \t# Reset the output so that we are actually going to STDOUT as opposed\n \t# to buffering the output.\n-\treset_output();\n+\treset_output() if ($caching_enabled);\n \n \tgit_header_html($http_responses{$status}, undef, %opts);\n \tprint <;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif ($caching_enabled) {\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \n@@ -5864,9 +5939,15 @@ sub git_snapshot {\n \n \topen my $fd, \"-|\", $cmd\n \t\tor die_error(500, \"Execute git-archive failed\");\n-\tbinmode STDOUT, ':raw';\n-\tprint <$fd>;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif ($caching_enabled) {\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \ndiff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl\nnew file mode 100644\nindex 0000000..dd14bfb\n--- /dev/null\n+++ b/gitweb/lib/cache.pl\n@@ -0,0 +1,347 @@\n+# gitweb - simple web interface to track changes in git repositories\n+#\n+# (C) 2006, John 'Warthog9' Hawley \n+#\n+# This program is licensed under the GPLv2\n+\n+#\n+# Gitweb caching engine\n+#\n+\n+#use File::Path qw(make_path remove_tree);\n+use File::Path qw(mkpath rmtree); # Used for compatability reasons\n+use Digest::MD5 qw(md5 md5_hex md5_base64);\n+use Fcntl ':flock';\n+use File::Copy;\n+\n+sub cache_fetch {\n+\tmy ($action) = @_;\n+\tmy $cacheTime = 0;\n+\n+\tif(! -d $cachedir){\n+\t\tprint \"*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\\n\";\n+\t\tmkdir (\"cache\", 0755) || die \"Cannot create cache dir - you will need to manually create\";\n+\t\tprint \"Cache directory created successfully\\n\";\n+\t}\n+\n+\tour $full_url = \"$my_url?\". $ENV{'QUERY_STRING'};\n+\tour $urlhash = md5_hex($full_url);\n+\tour $fullhashdir = \"$cachedir/\". substr( $urlhash, 0, 2) .\"/\";\n+\n+\teval { mkpath( $fullhashdir, 0, 0777 ) };\n+\tif ($@) {\n+\t\tdie_error(500, \"Internal Server Error\", \"Could not create cache directory: $@\");\n+\t}\n+\t$fullhashpath = \"$fullhashdir/\". substr( $urlhash, 2 );\n+\t$fullhashbinpath = \"$fullhashpath.bin.wt\";\n+\t$fullhashbinpathfinal = \"$fullhashpath.bin\";\n+\n+\tif(! -e \"$fullhashpath\" ){\n+\t\tif(! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\tcacheUpdate($action,0);\n+\t\t\tcacheDisplay($action);\n+\t\t} elsif ( $childPid == 0 ){\n+\t\t\t#run the updater\n+\t\t\tcacheUpdate($action,1);\n+\t\t}else{\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\t}else{\n+\t\t#if cache is out dated, update\n+\t\t#else displayCache();\n+\t\topen(cacheFile, '<', \"$fullhashpath\");\n+\t\tstat(cacheFile);\n+\t\tclose(cacheFile);\n+\t\tmy $stat_time = (stat(_))[9];\n+\t\tmy $stat_size = (stat(_))[7];\n+\n+\t\t$cacheTime = get_loadavg() * 60;\n+\t\tif( $cacheTime > $maxCacheTime ){\n+\t\t\t$cacheTime = $maxCacheTime;\n+\t\t}\n+\t\tif( $cacheTime < $minCacheTime ){\n+\t\t\t$cacheTime = $minCacheTime;\n+\t\t}\n+\t\tif( $stat_time < (time - $cacheTime) || $stat_size == 0 ){\n+\t\t\tif( ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\t\tcacheUpdate($action,0);\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t} elsif ( $childPid == 0 ){\n+\t\t\t\t#run the updater\n+\t\t\t\t#print \"Running updater\\n\";\n+\t\t\t\tcacheUpdate($action,1);\n+\t\t\t}else{\n+\t\t\t\t#print \"Waiting for update\\n\";\n+\t\t\t\tcacheWaitForUpdate($action);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\n+\n+\t}\n+\n+\t#\n+\t# If all of the caching failes - lets go ahead and press on without it and fall back to 'default'\n+\t# non-caching behavior. This is the softest of the failure conditions.\n+\t#\n+\t#$actions{$action}->();\n+}\n+\n+sub cacheUpdate {\n+\tmy ($action,$areForked) = @_;\n+\tmy $lockingStatus;\n+\tmy $fileData = \"\";\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFileBG, '>:utf8', \"$fullhashpath.bg\");\n+\t\tmy $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStatBG;\n+\t}else{\n+\t\topen(cacheFile, '>:utf8', \\$fullhashpath);\n+\t\tmy $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStat;\n+\t}\n+\t#print \"lock status: $lockStat\\n\";\n+\n+\n+\tif (! $lockStatus ){\n+\t\tif ( $areForked ){\n+\t\t\texit(0);\n+\t\t}else{\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinWT, '>>:utf8', \"$fullhashbinpath\");\n+\t\tmy $lockStatBin = flock(cacheFileBinWT,LOCK_EX|LOCK_NB);\n+\t}\n+\n+\t# Trap all output from the action\n+\tchange_output();\n+\n+\t$actions{$action}->();\n+\n+\t# Reset the outputs as we should be fine now\n+\treset_output();\n+\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFile, '>:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_EX);\n+\n+\t\tif (! $lockStat ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinFINAL, '>:utf8', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBinFINAL,LOCK_EX);\n+\n+\t\tif (! $lockStatBIN ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# Actually dump the output to the proper file handler\n+\tlocal $/ = undef;\n+\t$|++;\n+\tprint cacheFile \"$output\";\n+\t$|--;\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmove(\"$fullhashbinpath\", \"$fullhashbinpathfinal\") or die \"Binary Cache file could not be updated: $!\";\n+\n+\t\tflock(cacheFileBinFINAL,LOCK_UN);\n+\t\tclose(cacheFileBinFINAL);\n+\n+\t\tflock(cacheFileBinWT,LOCK_UN);\n+\t\tclose(cacheFileBinWT);\n+\t}\n+\n+\tflock(cacheFile,LOCK_UN);\n+\tclose(cacheFile);\n+\n+\tif($backgroundCache){\n+\t\tflock(cacheFileBG,LOCK_UN);\n+\t\tclose(cacheFileBG);\n+\t}\n+\n+\tif ( $areForked ){\n+\t\texit(0);\n+\t} else {\n+\t\treturn;\n+\t}\n+}\n+\n+\n+sub cacheWaitForUpdate {\n+\tmy ($action) = @_;\n+\tmy $x = 0;\n+\tmy $max = 10;\n+\tmy $lockStat = 0;\n+\n+\tif( $backgroundCache ){\n+\t\tif( -e \"$fullhashpath\" ){\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tstat(cacheFile);\n+\t\t\tclose(cacheFile);\n+\n+\t\t\tif( $lockStat && ( (stat(_))[9] > (time - $maxCacheLife) ) ){\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"atom\"\n+\t\t||\n+\t\t$action eq \"rss\"\n+\t\t||\n+\t\t$action eq \"opml\"\n+\t){\n+\t\tdo {\n+\t\t\tsleep 2 if $x > 0;\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tclose(cacheFile);\n+\t\t\t$x++;\n+\t\t\t$combinedLockStat = $lockStat;\n+\t\t} while ((! $combinedLockStat) && ($x < $max));\n+\n+\t\tif( $x != $max ){\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\t\treturn;\n+\t}\n+\n+\t$| = 1;\n+\n+\tprint $::cgi->header(\n+\t\t\t\t-type=>'text/html',\n+\t\t\t\t-charset => 'utf-8',\n+\t\t\t\t-status=> 200,\n+\t\t\t\t-expires => 'now',\n+\t\t\t\t# HTTP/1.0\n+\t\t\t\t-Pragma => 'no-cache',\n+\t\t\t\t# HTTP/1.1\n+\t\t\t\t-Cache_Control => join(\n+\t\t\t\t\t\t\t', ',\n+\t\t\t\t\t\t\tqw(\n+\t\t\t\t\t\t\t\tprivate\n+\t\t\t\t\t\t\t\tno-cache\n+\t\t\t\t\t\t\t\tno-store\n+\t\t\t\t\t\t\t\tmust-revalidate\n+\t\t\t\t\t\t\t\tmax-age=0\n+\t\t\t\t\t\t\t\tpre-check=0\n+\t\t\t\t\t\t\t\tpost-check=0\n+\t\t\t\t\t\t\t)\n+\t\t\t\t\t\t)\n+\t\t\t\t);\n+\n+\tprint <\n+\n+\n+\n+\n+\n+\n+\n+$title\n+\n+\n+EOF\n+\n+\tprint \"Generating..\";\n+\tdo {\n+\t\tprint \".\";\n+\t\tsleep 2 if $x > 0;\n+\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\tclose(cacheFile);\n+\t\t$x++;\n+\t\t$combinedLockStat = $lockStat;\n+\t} while ((! $combinedLockStat) && ($x < $max));\n+\tprint <\n+\n+EOF\n+\treturn;\n+}\n+\n+sub cacheDisplay {\n+\tlocal $/ = undef;\n+\t$|++;\n+\n+\tmy ($action) = @_;\n+\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\n+\tif (! $lockStat ){\n+\t\tclose(cacheFile);\n+\t\tcacheWaitForUpdate($action);\n+\t}\n+\n+\tif(\n+\t\t(\n+\t\t\t$action eq \"snapshot\"\n+\t\t\t||\n+\t\t\t$action eq \"blob_plain\"\n+\t\t)\n+\t){\n+\t\tmy $openstat = open(cacheFileBin, '<', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBin,LOCK_SH|LOCK_NB);\n+\t\tif (! $lockStatBIN ){\n+\t\t\tsystem (\"echo 'cacheDisplay - bailing due to binary lock failure' >> /tmp/gitweb.log\");\n+\t\t\tclose(cacheFile);\n+\t\t\tclose(cacheFileBin);\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\n+\t\tmy $binfilesize = -s \"$fullhashbinpathfinal\";\n+\t\tprint \"Content-Length: $binfilesize\";\n+\t}\n+\twhile( ){\n+\t\tprint $_;\n+\t}\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tbinmode STDOUT, ':raw';\n+\t\tprint ;\n+\t\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\t\tclose(cacheFileBin);\n+\t}\n+\tclose(cacheFile);\n+\t$|--;\n+}\n+\n+1;\ndiff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\nindex 4132aab..972d32e 100644\n--- a/gitweb/static/gitweb.css\n+++ b/gitweb/static/gitweb.css\n@@ -67,6 +67,12 @@ div.page_path {\n \tborder-width: 0px 0px 1px;\n }\n \n+div.cachetime {\n+\tfloat: left;\n+\tmargin-right: 10px;\n+\tcolor: #555555;\n+}\n+\n div.page_footer {\n \theight: 17px;\n \tpadding: 4px 8px;\n-- \n1.7.3\n"},{"id":"154925","messageId":"1288607092-31458-5-git-send-email-jnareb@gmail.com","threadId":"25573","inReplyTo":"201010311021.55917.jnareb@gmail.com","subject":"[PATCHv7.1 4/4] gitweb: Minimal testing of gitweb caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-01T10:24:52Z","receivedAt":"2010-11-01T10:24:52Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add basic tests of caching support to t9500-gitweb-standalone-no-errors\ntest: set $caching_enabled to true and check for errors for first time\nrun (generating cache) and second time run (retrieving from cache) for a\nsingle view - summary view for a project. Check in the\nt9502-gitweb-standalone-parse-output test that gitweb produces the same\noutput with and without caching, for first and second run, with binary\nor text output.\n\nSigned-off-by: Jakub Narebski \n---\nAll tests, including those added here, passes.\n\n t/t9500-gitweb-standalone-no-errors.sh | 19 ++++++++++++++\n t/t9502-gitweb-standalone-parse-output.sh | 39 +++++++++++++++++++++++++++++\n 2 files changed, 58 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 4f2b9b0..0ad5fc8 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -676,4 +676,23 @@ test_expect_success HIGHLIGHT \\\n \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n test_debug 'cat gitweb.log'\n \n+# ----------------------------------------------------------------------\n+# caching\n+\n+cat >>gitweb_config.perl <<\\EOF\n+$caching_enabled = 1;\n+$cachedir = 'cache'; # to clear right thing\n+EOF\n+rm -rf cache\n+\n+test_expect_success \\\n+\t'caching enabled (project summary, first run, generating cache)' \\\n+\t'gitweb_run \"p=.git;a=summary\"'\n+test_debug 'cat gitweb.log'\n+\n+test_expect_success \\\n+\t'caching enabled (project summary, second run, cached version)' \\\n+\t'gitweb_run \"p=.git;a=summary\"'\n+test_debug 'cat gitweb.log'\n+\n test_done\ndiff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh\nindex dd83890..f3d0cf0 100755\n--- a/t/t9502-gitweb-standalone-parse-output.sh\n+++ b/t/t9502-gitweb-standalone-parse-output.sh\n@@ -112,4 +112,43 @@ test_expect_success 'snapshot: hierarchical branch name (xx/test)' '\n '\n test_debug 'cat gitweb.headers'\n \n+\n+# ----------------------------------------------------------------------\n+# whether gitweb with caching enabled produces the same output\n+\n+test_expect_success 'setup for caching tests (utf8 commit, binary file)' '\n+\t. \"$TEST_DIRECTORY\"/t3901-utf8.txt &&\n+\tcp \"$TEST_DIRECTORY\"/test9200a.png image.png &&\n+\tgit add image.png &&\n+\tgit commit -F \"$TEST_DIRECTORY\"/t3900/1-UTF-8.txt &&\n+\tgitweb_run \"p=.git;a=patch\" &&\n+\tmv gitweb.body no_cache.html &&\n+\tgitweb_run \"p=.git;a=blob_plain;f=image.png\" &&\n+\tmv gitweb.body no_cache.png\n+'\n+\n+test_expect_success 'enable caching' '\n+\tcat >>gitweb_config.perl <<-\\EOF &&\n+\t$caching_enabled = 1;\n+\t$cachedir = \"cache\"; # to clear right thing\n+\tEOF\n+\trm -rf cache\n+'\n+\n+for desc in 'generating cache' 'cached version'; do\n+\ttest_expect_success \"caching enabled, HTML output, $desc\" '\n+\t\tgitweb_run \"p=.git;a=patch\" &&\n+\t\tmv gitweb.body cache.html &&\n+\t\ttest_cmp no_cache.html cache.html\n+\t'\n+done\n+\n+for desc in 'generating cache' 'cached version'; do\n+\ttest_expect_success \"caching enabled, binary output, $desc\" '\n+\t\tgitweb_run \"p=.git;a=blob_plain;f=image.png\" &&\n+\t\tmv gitweb.body cache.png &&\n+\t\tcmp no_cache.png cache.png\n+\t'\n+done\n+\n test_done\n-- \n1.7.3\n"},{"id":"154939","messageId":"201011011950.18730.jnareb@gmail.com","threadId":"25573","inReplyTo":"1288607092-31458-2-git-send-email-jnareb@gmail.com","subject":"[PATCHv7.1b 1/4] gitweb: Prepare for splitting gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-01T18:50:10Z","receivedAt":"2010-11-01T18:50:10Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Prepare gitweb for having been split into modules that are to be\ninstalled alongside gitweb in 'lib/' subdirectory, by adding\n\n use lib __DIR__.'/lib';\n\nto gitweb.perl (to main gitweb script), and preparing for putting\nmodules (relative path) in $(GITWEB_MODULES) in gitweb/Makefile.\n\nThis preparatory work allows to add new module to gitweb by simply\nadding\n\n GITWEB_MODULES += \n\nto gitweb/Makefile (assuming that the module is in 'gitweb/lib/'\ndirectory).\n\nWhile at it pass GITWEBLIBDIR in addition to GITWEB_TEST_INSTALLED\nto test instaleed version of gitweb and installed version of modules\n(for tests which check individual (sub)modules).\n\n\nUsing __DIR__ from Dir::Self module (not in core, that's why currently\ngitweb includes excerpt of code from Dir::Self defining __DIR__) was\nchosen over using FindBin-based solution (in core since perl 5.00307,\nwhile gitweb itself requires at least perl 5.8.0) because FindBin uses\nBEGIN block, which is a problem under mod_perl and other persistent\nPerl environments (thought there are workarounds).\n\nAt Pavan Kumar Sankara suggestion gitweb/Makefile uses\n\n install [OPTION]... SOURCE... DIRECTORY\n\nformat (2nd format) with single SOURCE rather than\n\n install [OPTION]... SOURCE DEST\n\nformat (1st format) because of security reasons (race conditions).\nModern GNU install has `-T' / `--no-target-directory' option, but we\ncannot rely that the $(INSTALL) we are using supports this option.\n\nThe install-modules target in gitweb/Makefile uses shell 'for' loop,\ninstead of make's $(foreach) function, to avoid possible problem with\ngenerating a command line that exceeded the maximum argument list\nlength.\n\nSigned-off-by: Jakub Narebski \n---\nThis version requires and applies cleanly on top of patch 958a846 \n(gitweb/Makefile: Add 'test' and 'test-installed' targets, 2010-09-26)\nin 'pu'.\n\n gitweb/Makefile | 17 +++++++++++++++--\n gitweb/gitweb.perl | 8 ++++++++\n 2 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex df908a1..c2d72e4 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -56,6 +56,7 @@ PERL_PATH ?= /usr/bin/perl\n bindir_SQ = $(subst ','\\'',$(bindir))#'\n gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))#'\n gitwebstaticdir_SQ = $(subst ','\\'',$(gitwebdir)/static)#'\n+gitweblibdir_SQ = $(subst ','\\'',$(gitwebdir)/lib)#'\n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))#'\n PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))#'\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))#'\n@@ -151,20 +152,32 @@ test:\n \n test-installed:\n \tGITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \\\n+\tGITWEBLIBDIR='$(DESTDIR_SQ)$(gitweblibdir_SQ)' \\\n \t\t$(MAKE) -C ../t gitweb-test\n \n ### Installation rules\n \n-install: all\n+install: all install-modules\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \t$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \n+install-modules:\n+\tinstall_dirs=\"$(sort $(dir $(GITWEB_MODULES)))\" && \\\n+\tfor dir in $$install_dirs; do \\\n+\t\ttest -d '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$dir' || \\\n+\t\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$dir'; \\\n+\tdone\n+\tgitweb_modules=\"$(GITWEB_MODULES)\" && \\\n+\tfor mod in $$gitweb_modules; do \\\n+\t\t$(INSTALL) -m 644 lib/$$mod \n'$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$(dirname $$mod)'; \\\n+\tdone\n+\n ### Cleaning rules\n \n clean:\n \t$(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css \nGITWEB-BUILD-OPTIONS\n \n-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE \nFORCE\n+.PHONY: all clean install install-modules test \ntest-installed .FORCE-GIT-VERSION-FILE FORCE\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex d521b4c..e4c08ba 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -9,6 +9,14 @@\n \n use strict;\n use warnings;\n+\n+use File::Spec;\n+# __DIR__ is taken from Dir::Self __DIR__ fragment\n+sub __DIR__ () {\n+\tFile::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);\n+}\n+use lib __DIR__ . '/lib';\n+\n use CGI qw(:standard :escapeHTML -nosticky);\n use CGI::Util qw(unescape);\n use CGI::Carp qw(fatalsToBrowser set_message);\n-- \n1.7.3\n"},{"id":"155805","messageId":"201011130035.50229.jnareb@gmail.com","threadId":"25573","inReplyTo":"1288607092-31458-1-git-send-email-jnareb@gmail.com","subject":"[RFC/PATCHv7.2 0/4] Gitweb caching v7.2","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-12T23:35:46Z","receivedAt":"2010-11-12T23:35:46Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This series is a bit fixed up and a tiny bit cleaned up version of\n\"Gitweb caching v7\" series from John 'Warthog9' Hawley:\n http://thread.gmane.org/gmane.comp.version-control.git/160147\n\nThis series is based on top of 'next', because it contains \n'jn/gitweb-test' branch.\n\nThe difference from v7.2 is that it takes into account 'test-installed'\ntarget in gitweb/Makefile in first patch of its series, and that testing\nof caching support is slightly extended. Note that some of those tests\nfail currently, not because of error in gitweb caching code, but because\nI was not able to disable calling cacheWaitForUpdate(), which hinders\ntesting.\n\nThose differences were also described in \"Re: What's cooking in git.git\n(Nov 2010, #01; Tue, 9)\":\n http://article.gmane.org/gmane.comp.version-control.git/161309\n\n\nTable of contents:\n==================\n [PATCHv7.2 1/4] gitweb: Prepare for splitting gitweb\n [PATCHv7.1 2/4] gitweb: add output buffering and associated functions\n [PATCHv7.1 3/4] gitweb: File based caching layer (from git.kernel.org)\n [PATCHv7.2 4/4] gitweb: Minimal testing of gitweb caching\n\nShortlog:\n=========\nJakub Narebski (2):\n gitweb: Prepare for splitting gitweb\n gitweb: Minimal testing of gitweb caching\n\nJohn 'Warthog9' Hawley (2):\n gitweb: add output buffering and associated functions\n gitweb: File based caching layer (from git.kernel.org)\n\nDiffstat:\n=========\n gitweb/Makefile | 20 ++-\n gitweb/gitweb.perl | 134 +++++++++++-\n gitweb/lib/cache.pl | 348 +++++++++++++++++++++++++++++\n gitweb/static/gitweb.css | 6 +\n t/gitweb-lib.sh | 15 ++\n t/t9500-gitweb-standalone-no-errors.sh | 20 ++\n t/t9501-gitweb-standalone-http-status.sh | 13 +\n t/t9502-gitweb-standalone-parse-output.sh | 33 +++\n 8 files changed, 579 insertions(+), 10 deletions(-)\n create mode 100644 gitweb/lib/cache.pl\n\n-- \n1.7.3\n"},{"id":"155806","messageId":"201011130041.22918.jnareb@gmail.com","threadId":"25573","inReplyTo":"201011130035.50229.jnareb@gmail.com","subject":"[PATCHv7.2 1/4] gitweb: Prepare for splitting gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-12T23:41:22Z","receivedAt":"2010-11-12T23:41:22Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Prepare gitweb for having been split into modules that are to be\ninstalled alongside gitweb in 'lib/' subdirectory, by adding\n\n use lib __DIR__.'/lib';\n\nto gitweb.perl (to main gitweb script), and preparing for putting\nmodules (relative path) in $(GITWEB_MODULES) in gitweb/Makefile.\n\nThis preparatory work allows to add new module to gitweb by simply\nadding\n\n GITWEB_MODULES += \n\nto gitweb/Makefile (assuming that the module is in 'gitweb/lib/'\ndirectory).\n\nWhile at it pass GITWEBLIBDIR in addition to GITWEB_TEST_INSTALLED\nto test instaleed version of gitweb and installed version of modules\n(for tests which check individual (sub)modules).\n\n\nUsing __DIR__ from Dir::Self module (not in core, that's why currently\ngitweb includes excerpt of code from Dir::Self defining __DIR__) was\nchosen over using FindBin-based solution (in core since perl 5.00307,\nwhile gitweb itself requires at least perl 5.8.0) because FindBin uses\nBEGIN block, which is a problem under mod_perl and other persistent\nPerl environments (thought there are workarounds).\n\nAt Pavan Kumar Sankara suggestion gitweb/Makefile uses\n\n install [OPTION]... SOURCE... DIRECTORY\n\nformat (2nd format) with single SOURCE rather than\n\n install [OPTION]... SOURCE DEST\n\nformat (1st format) because of security reasons (race conditions).\nModern GNU install has `-T' / `--no-target-directory' option, but we\ncannot rely that the $(INSTALL) we are using supports this option.\n\nThe install-modules target in gitweb/Makefile uses shell 'for' loop,\ninstead of make's $(foreach) function, to avoid possible problem with\ngenerating a command line that exceeded the maximum argument list\nlength.\n\nSigned-off-by: Jakub Narebski \n---\nThis version is rebased on top of 'next', which means that it takes\n'jn/gitweb-test' into account, in particular 958a846 (gitweb/Makefile:\nAdd 'test' and 'test-installed' targets, 2010-09-26).\n\nThat means adding GITWEBLIBDIR for future testing of individual\nmodules (it was not present in v7.1).\n\n gitweb/Makefile | 17 +++++++++++++++--\n gitweb/gitweb.perl | 8 ++++++++\n 2 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 0a6ac00..f9e32eb 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -57,6 +57,7 @@ PERL_PATH ?= /usr/bin/perl\n bindir_SQ = $(subst ','\\'',$(bindir))#'\n gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))#'\n gitwebstaticdir_SQ = $(subst ','\\'',$(gitwebdir)/static)#'\n+gitweblibdir_SQ = $(subst ','\\'',$(gitwebdir)/lib)#'\n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))#'\n PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))#'\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))#'\n@@ -153,20 +154,32 @@ test:\n \n test-installed:\n \tGITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \\\n+\tGITWEBLIBDIR='$(DESTDIR_SQ)$(gitweblibdir_SQ)' \\\n \t\t$(MAKE) -C ../t gitweb-test\n \n ### Installation rules\n \n-install: all\n+install: all install-modules\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \t$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \n+install-modules:\n+\tinstall_dirs=\"$(sort $(dir $(GITWEB_MODULES)))\" && \\\n+\tfor dir in $$install_dirs; do \\\n+\t\ttest -d '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$dir' || \\\n+\t\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$dir'; \\\n+\tdone\n+\tgitweb_modules=\"$(GITWEB_MODULES)\" && \\\n+\tfor mod in $$gitweb_modules; do \\\n+\t\t$(INSTALL) -m 644 lib/$$mod '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$(dirname $$mod)'; \\\n+\tdone\n+\n ### Cleaning rules\n \n clean:\n \t$(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS\n \n-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE\n+.PHONY: all clean install install-modules test test-installed .FORCE-GIT-VERSION-FILE FORCE\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 679f2da..cfa511c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -10,6 +10,14 @@\n use 5.008;\n use strict;\n use warnings;\n+\n+use File::Spec;\n+# __DIR__ is taken from Dir::Self __DIR__ fragment\n+sub __DIR__ () {\n+\tFile::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);\n+}\n+use lib __DIR__ . '/lib';\n+\n use CGI qw(:standard :escapeHTML -nosticky);\n use CGI::Util qw(unescape);\n use CGI::Carp qw(fatalsToBrowser set_message);\n-- \n1.7.3\n"},{"id":"155807","messageId":"201011130044.49142.jnareb@gmail.com","threadId":"25573","inReplyTo":"201011130035.50229.jnareb@gmail.com","subject":"[PATCHv7.1 2/4] gitweb: add output buffering and associated functions","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-12T23:44:48Z","receivedAt":"2010-11-12T23:44:48Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis adds output buffering for gitweb, mainly in preparation for\ncaching support. This is a dramatic change to how caching was being\ndone, mainly in passing around the variable manually and such.\n\nThis centrally flips the entire STDOUT to a variable, which after the\ncompletion of the run, flips it back and does a print on the resulting\ndata.\n\nThis should save on the previous 10K line patch (or so) that adds more\nexplicit output passing.\n\n[jn: modified reset_output to silence 'gitweb.perl: Name \"main::STDOUT_REAL\"\n used only once: possible typo at ../gitweb/gitweb.perl line 1130.' warning]\n\nSigned-off-by: Junio C Hamano \n---\nThis is unchanged from previous version.\n\n\nReminder: it uses\n\n open STDOUT, \">&\", \\*STDOUT_REAL;\n\nrather than\n\n open(STDOUT,\">&STDOUT_REAL\");\n\nto silence spurious (invalid) warning\n\n gitweb.perl: Name \"main::STDOUT_REAL\" used only once: possible typo\n at ../gitweb/gitweb.perl line 1130.\n\n\n gitweb/gitweb.perl | 29 +++++++++++++++++++++++++++++\n 1 files changed, 29 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex cfa511c..cae0e34 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -39,6 +39,9 @@ BEGIN {\n \n our $version = \"++GIT_VERSION++\";\n \n+# Output buffer variable\n+our $output = \"\";\n+\n our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n sub evaluate_uri {\n \tour $cgi;\n@@ -1134,6 +1137,25 @@ sub evaluate_argv {\n \t);\n }\n \n+sub change_output {\n+\tour $output;\n+\n+\t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n+\topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n+\n+\t# Close STDOUT, so that it isn't being used anymore.\n+\tclose STDOUT;\n+\n+\t# Trap STDOUT to the $output variable, which is what I was using in the original\n+\t# patch anyway.\n+\topen(STDOUT,\">\", \\$output) || die \"Unable to open STDOUT: $!\"; #open STDOUT handle to use $var\n+}\n+\n+sub reset_output {\n+\t# This basically takes STDOUT_REAL and puts it back as STDOUT\n+\topen STDOUT, \">&\", \\*STDOUT_REAL;\n+}\n+\n sub run {\n \tevaluate_argv();\n \n@@ -1145,7 +1167,10 @@ sub run {\n \t\t$pre_dispatch_hook->()\n \t\t\tif $pre_dispatch_hook;\n \n+\t\tchange_output();\n \t\trun_request();\n+\t\treset_output();\n+\t\tprint $output;\n \n \t\t$post_dispatch_hook->()\n \t\t\tif $post_dispatch_hook;\n@@ -3655,6 +3680,10 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\t# Reset the output so that we are actually going to STDOUT as opposed\n+\t# to buffering the output.\n+\treset_output();\n+\n \tgit_header_html($http_responses{$status}, undef, %opts);\n \tprint <\n-- \n1.7.3\n"},{"id":"155808","messageId":"201011130056.24222.jnareb@gmail.com","threadId":"25573","inReplyTo":"201011130035.50229.jnareb@gmail.com","subject":"[PATCHv7.1 3/4] gitweb: File based caching layer (from git.kernel.org)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-12T23:56:23Z","receivedAt":"2010-11-12T23:56:23Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis is a relatively large patch that implements the file based\ncaching layer that is quite similar to the one used on such large\nsites as kernel.org and soon git.fedoraproject.org. This provides\na simple, and straight forward caching mechanism that scales\ndramatically better than Gitweb by itself.\n\nThe caching layer basically buffers the output that Gitweb would\nnormally return, and saves that output to a cache file on the local\ndisk. When the file is requested it attempts to gain a shared lock\non the cache file and cat it out to the client. Should an exclusive\nlock be on a file (it's being updated) the code has a choice to either\nupdate in the background and go ahead and show the stale page while\nupdate is being performed, or stall the client(s) until the page\nis generated.\n\nThere are two forms of stalling involved here, background building\nand non-background building, both of which are discussed in the\nconfiguration page.\n\nThere are still a few known \"issues\" with respect to this:\n- Code needs to be added to be \"browser\" aware so\n that clients like wget that are trying to get a\n binary blob don't obtain a \"Generating...\" page\n\nCaching is disabled by default. You can turn it on by setting\n$caching_enabled variable to true to enable file based caching.\n\n[jn: added error checking to loading 'cache.pl'; moved check\n for $caching_enabled outside out of cache_fetch, which required\n update to die_error()]\n\nSigned-off-by: Junio C Hamano \n---\nThis patch is unchanged from previous version.\n\nReminder: the main difference from J.H. patch was moving check for\n$caching_enabled out of cache_fetch, so that when caching is disabled\nthen gitweb works (almost) the same as before this patch.\n\nAlso, error checking for loading 'cache.pl' file.\n\nInterdiff in previous series.\n\n gitweb/Makefile | 3 +\n gitweb/gitweb.perl | 105 ++++++++++++--\n gitweb/lib/cache.pl | 348 ++++++++++++++++++++++++++++++++++++++++++++++\n gitweb/static/gitweb.css | 6 +\n 4 files changed, 450 insertions(+), 12 deletions(-)\n create mode 100644 gitweb/lib/cache.pl\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex f9e32eb..6ddd4f1 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -113,6 +113,9 @@ endif\n \n GITWEB_FILES += static/git-logo.png static/git-favicon.png\n \n+# Gitweb caching\n+GITWEB_MODULES += cache.pl\n+\n GITWEB_REPLACE = \\\n \t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n \t-e 's|++GIT_BINDIR++|$(bindir)|g' \\\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex cae0e34..3c3ff08 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -250,6 +250,53 @@ our %avatar_size = (\n # Leave it undefined (or set to 'undef') to turn off load checking.\n our $maxload = 300;\n \n+# This enables/disables the caching layer in gitweb. This currently only supports the\n+# 'dumb' file based caching layer, primarily used on git.kernel.org. this is reasonably\n+# effective but it has the downside of requiring a huge amount of disk space if there\n+# are a number of repositories involved. It is not uncommon for git.kernel.org to have\n+# on the order of 80G - 120G accumulate over the course of a few months. It is recommended\n+# that the cache directory be periodically completely deleted, and this is safe to perform.\n+# Suggested mechanism\n+# mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush\n+our $caching_enabled = 0;\n+\n+# Used to set the minimum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to be under this number of seconds we set the cache timeout\n+# to this minimum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $minCacheTime = 20;\n+\n+# Used to set the maximum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to exceed this number of seconds we set the cache timeout\n+# to this maximum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $maxCacheTime = 1200;\n+\n+# If you need to change the location of the caching directory, override this\n+# otherwise this will probably do fine for you\n+our $cachedir = 'cache';\n+\n+# If this is set (to 1) cache will do it's best to always display something instead\n+# of making someone wait for the cache to update. This will launch the cacheUpdate\n+# into the background and it will lock a .bg file and will only lock the\n+# actual cache file when it needs to write into it. In theory this will make\n+# gitweb seem more responsive at the price of possibly stale data.\n+our $backgroundCache = 1;\n+\n+# Used to set the maximum cache file life. If a cache files last modify time exceeds\n+# this value, it will assume that the data is just too old, and HAS to be regenerated\n+# instead of trying to display the existing cache data.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+# 18000 = 5 hours\n+our $maxCacheLife = 18000;\n+\n+# Used to enable or disable background forking of the gitweb caching. Mainly here for debugging purposes\n+our $cacheDoFork = 1;\n+\n+our $fullhashpath = *STDOUT;\n+our $fullhashbinpath = *STDOUT;\n+our $fullhashbinpathfinal = *STDOUT;\n+\n # configuration for 'highlight' (http://www.andre-simon.de/)\n # match by basename\n our %highlight_basename = (\n@@ -506,6 +553,15 @@ our %feature = (\n \t\t'default' => [0]},\n );\n \n+#\n+# Includes\n+#\n+if (!exists $INC{'cache.pl'}) {\n+\tmy $return = do 'cache.pl';\n+\tdie $@ if $@;\n+\tdie \"Couldn't read 'cache.pl': $!\" if (!defined $return);\n+}\n+\n sub gitweb_get_feature {\n \tmy ($name) = @_;\n \treturn unless exists $feature{$name};\n@@ -734,6 +790,10 @@ our %actions = (\n \t\"project_list\" => \\&git_project_list,\n \t\"project_index\" => \\&git_project_index,\n );\n+sub is_cacheable {\n+\tmy $action = shift;\n+\treturn !($action eq 'blame_data' || $action eq 'blame_incremental');\n+}\n \n # finally, we have the hash of allowed extra_options for the commands that\n # allow them\n@@ -1072,7 +1132,11 @@ sub dispatch {\n \t !$project) {\n \t\tdie_error(400, \"Project needed\");\n \t}\n-\t$actions{$action}->();\n+\tif ($caching_enabled && is_cacheable($action)) {\n+\t\tcache_fetch($action);\n+\t} else {\n+\t\t$actions{$action}->();\n+\t}\n }\n \n sub reset_timer {\n@@ -1142,6 +1206,7 @@ sub change_output {\n \n \t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n \topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n+\tprint STDOUT_REAL \"\";\n \n \t# Close STDOUT, so that it isn't being used anymore.\n \tclose STDOUT;\n@@ -1167,10 +1232,7 @@ sub run {\n \t\t$pre_dispatch_hook->()\n \t\t\tif $pre_dispatch_hook;\n \n-\t\tchange_output();\n \t\trun_request();\n-\t\treset_output();\n-\t\tprint $output;\n \n \t\t$post_dispatch_hook->()\n \t\t\tif $post_dispatch_hook;\n@@ -3447,7 +3509,8 @@ sub git_header_html {\n \t# support xhtml+xml but choking when it gets what it asked for.\n \tif (defined $cgi->http('HTTP_ACCEPT') &&\n \t $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\\s|^)application\\/xhtml\\+xml(,|;|\\s|$)/ &&\n-\t $cgi->Accept('application/xhtml+xml') != 0) {\n+\t $cgi->Accept('application/xhtml+xml') != 0 &&\n+\t !$caching_enabled) {\n \t\t$content_type = 'application/xhtml+xml';\n \t} else {\n \t\t$content_type = 'text/html';\n@@ -3592,6 +3655,7 @@ sub git_footer_html {\n \tmy $feed_class = 'rss_logo';\n \n \tprint \"
\\n\";\n+\tprint \"
Cache Last Updated: \". gmtime( time ) .\" GMT
\\n\";\n \tif (defined $project) {\n \t\tmy $descr = git_get_project_description($project);\n \t\tif (defined $descr) {\n@@ -3680,9 +3744,14 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\t# The output handlers for die_error need to be reset to STDOUT\n+\t# so that half the message isn't being output to random and\n+\t# half to STDOUT as expected. This is mainly for the benefit\n+\t# of using git_header_html() and git_footer_html() since\n+\t#\n \t# Reset the output so that we are actually going to STDOUT as opposed\n \t# to buffering the output.\n-\treset_output();\n+\treset_output() if ($caching_enabled);\n \n \tgit_header_html($http_responses{$status}, undef, %opts);\n \tprint <;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif ($caching_enabled) {\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \n@@ -5879,9 +5954,15 @@ sub git_snapshot {\n \n \topen my $fd, \"-|\", $cmd\n \t\tor die_error(500, \"Execute git-archive failed\");\n-\tbinmode STDOUT, ':raw';\n-\tprint <$fd>;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif ($caching_enabled) {\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \ndiff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl\nnew file mode 100644\nindex 0000000..dd14bfb\n--- /dev/null\n+++ b/gitweb/lib/cache.pl\n@@ -0,0 +1,348 @@\n+# gitweb - simple web interface to track changes in git repositories\n+#\n+# (C) 2006, John 'Warthog9' Hawley \n+#\n+# This program is licensed under the GPLv2\n+\n+#\n+# Gitweb caching engine\n+#\n+\n+#use File::Path qw(make_path remove_tree);\n+use File::Path qw(mkpath rmtree); # Used for compatability reasons\n+use Digest::MD5 qw(md5 md5_hex md5_base64);\n+use Fcntl ':flock';\n+use File::Copy;\n+\n+sub cache_fetch {\n+\tmy ($action) = @_;\n+\tmy $cacheTime = 0;\n+\n+\tif(! -d $cachedir){\n+\t\tprint \"*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\\n\";\n+\t\tmkdir (\"cache\", 0755) || die \"Cannot create cache dir - you will need to manually create\";\n+\t\tprint \"Cache directory created successfully\\n\";\n+\t}\n+\n+\tour $full_url = \"$my_url?\". $ENV{'QUERY_STRING'};\n+\tour $urlhash = md5_hex($full_url);\n+\tour $fullhashdir = \"$cachedir/\". substr( $urlhash, 0, 2) .\"/\";\n+\n+\teval { mkpath( $fullhashdir, 0, 0777 ) };\n+\tif ($@) {\n+\t\tdie_error(500, \"Internal Server Error\", \"Could not create cache directory: $@\");\n+\t}\n+\t$fullhashpath = \"$fullhashdir/\". substr( $urlhash, 2 );\n+\t$fullhashbinpath = \"$fullhashpath.bin.wt\";\n+\t$fullhashbinpathfinal = \"$fullhashpath.bin\";\n+\n+\tif(! -e \"$fullhashpath\" ){\n+\t\tif(! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\tcacheUpdate($action,0);\n+\t\t\tcacheDisplay($action);\n+\t\t} elsif ( $childPid == 0 ){\n+\t\t\t#run the updater\n+\t\t\tcacheUpdate($action,1);\n+\t\t}else{\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\t}else{\n+\t\t#if cache is out dated, update\n+\t\t#else displayCache();\n+\t\topen(cacheFile, '<', \"$fullhashpath\");\n+\t\tstat(cacheFile);\n+\t\tclose(cacheFile);\n+\t\tmy $stat_time = (stat(_))[9];\n+\t\tmy $stat_size = (stat(_))[7];\n+\n+\t\t$cacheTime = get_loadavg() * 60;\n+\t\tif( $cacheTime > $maxCacheTime ){\n+\t\t\t$cacheTime = $maxCacheTime;\n+\t\t}\n+\t\tif( $cacheTime < $minCacheTime ){\n+\t\t\t$cacheTime = $minCacheTime;\n+\t\t}\n+\t\tif( $stat_time < (time - $cacheTime) || $stat_size == 0 ){\n+\t\t\tif( ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\t\tcacheUpdate($action,0);\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t} elsif ( $childPid == 0 ){\n+\t\t\t\t#run the updater\n+\t\t\t\t#print \"Running updater\\n\";\n+\t\t\t\tcacheUpdate($action,1);\n+\t\t\t}else{\n+\t\t\t\t#print \"Waiting for update\\n\";\n+\t\t\t\tcacheWaitForUpdate($action);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\n+\n+\t}\n+\n+\t#\n+\t# If all of the caching failes - lets go ahead and press on without it and fall back to 'default'\n+\t# non-caching behavior. This is the softest of the failure conditions.\n+\t#\n+\t#$actions{$action}->();\n+}\n+\n+sub cacheUpdate {\n+\tmy ($action,$areForked) = @_;\n+\tmy $lockingStatus;\n+\tmy $fileData = \"\";\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFileBG, '>:utf8', \"$fullhashpath.bg\");\n+\t\tmy $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStatBG;\n+\t}else{\n+\t\topen(cacheFile, '>:utf8', \\$fullhashpath);\n+\t\tmy $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStat;\n+\t}\n+\t#print \"lock status: $lockStat\\n\";\n+\n+\n+\tif (! $lockStatus ){\n+\t\tif ( $areForked ){\n+\t\t\texit(0);\n+\t\t}else{\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinWT, '>>:utf8', \"$fullhashbinpath\");\n+\t\tmy $lockStatBin = flock(cacheFileBinWT,LOCK_EX|LOCK_NB);\n+\t}\n+\n+\t# Trap all output from the action\n+\tchange_output();\n+\n+\t$actions{$action}->();\n+\n+\t# Reset the outputs as we should be fine now\n+\treset_output();\n+\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFile, '>:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_EX);\n+\n+\t\tif (! $lockStat ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinFINAL, '>:utf8', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBinFINAL,LOCK_EX);\n+\n+\t\tif (! $lockStatBIN ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# Actually dump the output to the proper file handler\n+\tlocal $/ = undef;\n+\t$|++;\n+\tprint cacheFile \"$output\";\n+\t$|--;\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmove(\"$fullhashbinpath\", \"$fullhashbinpathfinal\") or die \"Binary Cache file could not be updated: $!\";\n+\n+\t\tflock(cacheFileBinFINAL,LOCK_UN);\n+\t\tclose(cacheFileBinFINAL);\n+\n+\t\tflock(cacheFileBinWT,LOCK_UN);\n+\t\tclose(cacheFileBinWT);\n+\t}\n+\n+\tflock(cacheFile,LOCK_UN);\n+\tclose(cacheFile);\n+\n+\tif($backgroundCache){\n+\t\tflock(cacheFileBG,LOCK_UN);\n+\t\tclose(cacheFileBG);\n+\t}\n+\n+\tif ( $areForked ){\n+\t\texit(0);\n+\t} else {\n+\t\treturn;\n+\t}\n+}\n+\n+\n+sub cacheWaitForUpdate {\n+\tmy ($action) = @_;\n+\tmy $x = 0;\n+\tmy $max = 10;\n+\tmy $lockStat = 0;\n+\n+\tif( $backgroundCache ){\n+\t\tif( -e \"$fullhashpath\" ){\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tstat(cacheFile);\n+\t\t\tclose(cacheFile);\n+\n+\t\t\tif( $lockStat && ( (stat(_))[9] > (time - $maxCacheLife) ) ){\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"atom\"\n+\t\t||\n+\t\t$action eq \"rss\"\n+\t\t||\n+\t\t$action eq \"opml\"\n+\t){\n+\t\tdo {\n+\t\t\tsleep 2 if $x > 0;\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tclose(cacheFile);\n+\t\t\t$x++;\n+\t\t\t$combinedLockStat = $lockStat;\n+\t\t} while ((! $combinedLockStat) && ($x < $max));\n+\n+\t\tif( $x != $max ){\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\t\treturn;\n+\t}\n+\n+\t$| = 1;\n+\n+\tprint $::cgi->header(\n+\t\t\t\t-type=>'text/html',\n+\t\t\t\t-charset => 'utf-8',\n+\t\t\t\t-status=> 200,\n+\t\t\t\t-expires => 'now',\n+\t\t\t\t# HTTP/1.0\n+\t\t\t\t-Pragma => 'no-cache',\n+\t\t\t\t# HTTP/1.1\n+\t\t\t\t-Cache_Control => join(\n+\t\t\t\t\t\t\t', ',\n+\t\t\t\t\t\t\tqw(\n+\t\t\t\t\t\t\t\tprivate\n+\t\t\t\t\t\t\t\tno-cache\n+\t\t\t\t\t\t\t\tno-store\n+\t\t\t\t\t\t\t\tmust-revalidate\n+\t\t\t\t\t\t\t\tmax-age=0\n+\t\t\t\t\t\t\t\tpre-check=0\n+\t\t\t\t\t\t\t\tpost-check=0\n+\t\t\t\t\t\t\t)\n+\t\t\t\t\t\t)\n+\t\t\t\t);\n+\n+\tprint <\n+\n+\n+\n+\n+\n+\n+\n+$title\n+\n+\n+EOF\n+\n+\tprint \"Generating..\";\n+\tdo {\n+\t\tprint \".\";\n+\t\tsleep 2 if $x > 0;\n+\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\tclose(cacheFile);\n+\t\t$x++;\n+\t\t$combinedLockStat = $lockStat;\n+\t} while ((! $combinedLockStat) && ($x < $max));\n+\tprint <\n+\n+EOF\n+\treturn;\n+}\n+\n+sub cacheDisplay {\n+\tlocal $/ = undef;\n+\t$|++;\n+\n+\tmy ($action) = @_;\n+\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\n+\tif (! $lockStat ){\n+\t\tclose(cacheFile);\n+\t\tcacheWaitForUpdate($action);\n+\t}\n+\n+\tif(\n+\t\t(\n+\t\t\t$action eq \"snapshot\"\n+\t\t\t||\n+\t\t\t$action eq \"blob_plain\"\n+\t\t)\n+\t){\n+\t\tmy $openstat = open(cacheFileBin, '<', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBin,LOCK_SH|LOCK_NB);\n+\t\tif (! $lockStatBIN ){\n+\t\t\tsystem (\"echo 'cacheDisplay - bailing due to binary lock failure' >> /tmp/gitweb.log\");\n+\t\t\tclose(cacheFile);\n+\t\t\tclose(cacheFileBin);\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\n+\t\tmy $binfilesize = -s \"$fullhashbinpathfinal\";\n+\t\tprint \"Content-Length: $binfilesize\";\n+\t}\n+\twhile( ){\n+\t\tprint $_;\n+\t}\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tbinmode STDOUT, ':raw';\n+\t\tprint ;\n+\t\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\t\tclose(cacheFileBin);\n+\t}\n+\tclose(cacheFile);\n+\t$|--;\n+}\n+\n+1;\n+__END__\ndiff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\nindex 4132aab..972d32e 100644\n--- a/gitweb/static/gitweb.css\n+++ b/gitweb/static/gitweb.css\n@@ -67,6 +67,12 @@ div.page_path {\n \tborder-width: 0px 0px 1px;\n }\n \n+div.cachetime {\n+\tfloat: left;\n+\tmargin-right: 10px;\n+\tcolor: #555555;\n+}\n+\n div.page_footer {\n \theight: 17px;\n \tpadding: 4px 8px;\n-- \n1.7.3\n"},{"id":"155809","messageId":"201011130101.51116.jnareb@gmail.com","threadId":"25573","inReplyTo":"201011130035.50229.jnareb@gmail.com","subject":"[PATCHv7.2 4/4] gitweb: Minimal testing of gitweb caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-13T00:01:50Z","receivedAt":"2010-11-13T00:01:50Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add basic tests of caching support to t9500-gitweb-standalone-no-errors\ntest: set $caching_enabled to true and check for errors for first time\nrun (generating cache) and second time run (retrieving from cache) for a\nsingle view - summary view for a project. Check also that request for\nnon-existent object (which results in die_error() codepath to be called)\ndoesn't produce errors.\n\nCheck in t9501-gitweb-standalone-http-status that request for\nnon-existent object produces correct output (HTTP headers and HTML\noutput) also when caching is enabled.\n\nCheck in the t9502-gitweb-standalone-parse-output test that gitweb\nproduces the same output with and without caching, for first and\nsecond run, with binary (raw) or plain text (utf8) output.\n\nThe common routine that enables cache, gitweb_enable_caching, is\ndefined in t/gitweb-lib.sh\n\nSigned-off-by: Jakub Narebski \n---\nAs I wrote in the cover letter, because of cacheWaitForUpdate() generating\nextra output some of those newly introduced tests fail, some\nintermittently.\n\nJohn, could you take a look at it and check if the problem is: in tests,\nin my simplification, or in caching code...\n\n t/gitweb-lib.sh | 15 +++++++++++++\n t/t9500-gitweb-standalone-no-errors.sh | 20 +++++++++++++++++\n t/t9501-gitweb-standalone-http-status.sh | 13 +++++++++++\n t/t9502-gitweb-standalone-parse-output.sh | 33 +++++++++++++++++++++++++++++\n 4 files changed, 81 insertions(+), 0 deletions(-)\n\ndiff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\nindex b9bb95f..16ce811 100644\n--- a/t/gitweb-lib.sh\n+++ b/t/gitweb-lib.sh\n@@ -52,6 +52,21 @@ EOF\n \texport SCRIPT_NAME\n }\n \n+gitweb_enable_caching () {\n+\ttest_expect_success 'enable caching' '\n+\t\tcat >>gitweb_config.perl <<-\\EOF &&\n+\t\tour $caching_enabled = 1;\n+\t\tour $minCacheTime = 60*60*24*7*30; # very long expiration time for tests (a month)\n+\t\tour $maxCacheTime = 60*60*24*7*30*365; # upper bound for dynamic (adaptive) caching\n+\t\tour $cachedir = \"cache\"; # for testsuite to clear the right thing\n+\t\t# required, because otherwise some tests might intermittently not pass\n+\t\tour $backgroundCache = 0; # should turn off cacheWaitForUpdate() / \"Generating...\"\n+\t\t#our $cacheDoFork = 0;\n+\t\tEOF\n+\t\trm -rf cache/\n+\t'\n+}\n+\n gitweb_run () {\n \tGATEWAY_INTERFACE='CGI/1.1'\n \tHTTP_ACCEPT='*/*'\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 21cd286..86c1b50 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -677,4 +677,24 @@ test_expect_success HIGHLIGHT \\\n \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n test_debug 'cat gitweb.log'\n \n+# ----------------------------------------------------------------------\n+# caching\n+\n+gitweb_enable_caching\n+\n+test_expect_success \\\n+\t'caching enabled (project summary, first run, generating cache)' \\\n+\t'gitweb_run \"p=.git;a=summary\"'\n+test_debug 'cat gitweb.log'\n+\n+test_expect_success \\\n+\t'caching enabled (project summary, second run, cached version)' \\\n+\t'gitweb_run \"p=.git;a=summary\"'\n+test_debug 'cat gitweb.log'\n+\n+test_expect_success \\\n+\t'caching enabled (non-existent commit, non-cache error page)' \\\n+\t'gitweb_run \"p=.git;a=commit;h=non-existent\"'\n+test_debug 'cat gitweb.log'\n+\n test_done\ndiff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh\nindex 2487da1..5b1df3f 100755\n--- a/t/t9501-gitweb-standalone-http-status.sh\n+++ b/t/t9501-gitweb-standalone-http-status.sh\n@@ -134,5 +134,18 @@ cat >>gitweb_config.perl <<\\EOF\n our $maxload = undef;\n EOF\n \n+# ----------------------------------------------------------------------\n+# output caching\n+\n+gitweb_enable_caching\n+\n+test_expect_success 'caching enabled (non-existent commit, 404 error)' '\n+\tgitweb_run \"p=.git;a=commit;h=non-existent\" &&\n+\tgrep \"Status: 404 Not Found\" gitweb.headers &&\n+\tgrep \"404 - Unknown commit object\" gitweb.body\n+'\n+test_debug 'echo \"log\" && cat gitweb.log'\n+test_debug 'echo \"headers\" && cat gitweb.headers'\n+test_debug 'echo \"body\" && cat gitweb.body'\n \n test_done\ndiff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh\nindex dd83890..bc8eb01 100755\n--- a/t/t9502-gitweb-standalone-parse-output.sh\n+++ b/t/t9502-gitweb-standalone-parse-output.sh\n@@ -112,4 +112,37 @@ test_expect_success 'snapshot: hierarchical branch name (xx/test)' '\n '\n test_debug 'cat gitweb.headers'\n \n+\n+# ----------------------------------------------------------------------\n+# whether gitweb with caching enabled produces the same output\n+\n+test_expect_success 'setup for caching tests (utf8 patch, binary file)' '\n+\t. \"$TEST_DIRECTORY\"/t3901-utf8.txt &&\n+\tcp \"$TEST_DIRECTORY\"/test9200a.png image.png &&\n+\tgit add image.png &&\n+\tgit commit -F \"$TEST_DIRECTORY\"/t3900/1-UTF-8.txt &&\n+\tgitweb_run \"p=.git;a=patch\" &&\n+\tmv gitweb.body no_cache.txt &&\n+\tgitweb_run \"p=.git;a=blob_plain;f=image.png\" &&\n+\tmv gitweb.body no_cache.png\n+'\n+\n+gitweb_enable_caching\n+\n+for desc in 'generating cache' 'cached version'; do\n+\ttest_expect_success \"caching enabled, plain text (utf8) output, $desc\" '\n+\t\tgitweb_run \"p=.git;a=patch\" &&\n+\t\tmv gitweb.body cache.txt &&\n+\t\ttest_cmp no_cache.txt cache.txt\n+\t'\n+done\n+\n+for desc in 'generating cache' 'cached version'; do\n+\ttest_expect_success \"caching enabled, binary output (raw), $desc\" '\n+\t\tgitweb_run \"p=.git;a=blob_plain;f=image.png\" &&\n+\t\tmv gitweb.body cache.png &&\n+\t\tcmp no_cache.png cache.png\n+\t'\n+done\n+\n test_done\n-- \n1.7.3\n"},{"id":"156067","messageId":"7vk4kbefv8.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"201011130101.51116.jnareb@gmail.com","subject":"Re: [PATCHv7.2 4/4] gitweb: Minimal testing of gitweb caching","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-17T22:37:31Z","receivedAt":"2010-11-17T22:37:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> +gitweb_enable_caching () {\n> +\ttest_expect_success 'enable caching' '\n> +\t\tcat >>gitweb_config.perl <<-\\EOF &&\n> +\t\tour $caching_enabled = 1;\n> +\t\tour $minCacheTime = 60*60*24*7*30; # very long expiration time for tests (a month)\n\nDoes your month have 210 days in it?\n\n> +\t\tour $maxCacheTime = 60*60*24*7*30*365; # upper bound for dynamic (adaptive) caching\n\nLikewise, is this 210 years? Does this bust 32-bit time_t somewhere?\n\n> +\t\tour $cachedir = \"cache\"; # for testsuite to clear the right thing\n> +\t\t# required, because otherwise some tests might intermittently not pass\n> +\t\tour $backgroundCache = 0; # should turn off cacheWaitForUpdate() / \"Generating...\"\n> +\t\t#our $cacheDoFork = 0;\n\nleftover....\n"},{"id":"156068","messageId":"7vd3q3eecm.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"201011130041.22918.jnareb@gmail.com","subject":"Re: [PATCHv7.2 1/4] gitweb: Prepare for splitting gitweb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-17T23:10:17Z","receivedAt":"2010-11-17T23:10:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> That means adding GITWEBLIBDIR for future testing of individual\n> modules (it was not present in v7.1).\n\nYou confused me. \"git grep\" after test-applying this 4-patch series on\ntop of next shows only one GITWEBLIBDIR, which means that the symbol is\ndefined but nobody pays attention to it.\n\n> diff --git a/gitweb/Makefile b/gitweb/Makefile\n> index 0a6ac00..f9e32eb 100644\n> --- a/gitweb/Makefile\n> +++ b/gitweb/Makefile\n> @@ -57,6 +57,7 @@ PERL_PATH ?= /usr/bin/perl\n> ...\n> -.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE\n> +.PHONY: all clean install install-modules test test-installed .FORCE-GIT-VERSION-FILE FORCE\n> \n\nIn the earlier series, 478da52 (gitweb: Prepare for splitting gitweb,\n2010-10-30) removed the trailing blank line from here but this does not do\nso anymore, which is a micronit regression.\n"},{"id":"156070","messageId":"201011180012.23294.jnareb@gmail.com","threadId":"25573","inReplyTo":"7vk4kbefv8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv7.2 4/4] gitweb: Minimal testing of gitweb caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-17T23:12:22Z","receivedAt":"2010-11-17T23:12:22Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia środa 17. listopada 2010 23:37, Junio C Hamano napisał:\n> Jakub Narebski writes:\n> \n> > +gitweb_enable_caching () {\n> > +\ttest_expect_success 'enable caching' '\n> > +\t\tcat >>gitweb_config.perl <<-\\EOF &&\n> > +\t\tour $caching_enabled = 1;\n> > +\t\tour $minCacheTime = 60*60*24*7*30; # very long expiration time for tests (a month)\n> \n> Does your month have 210 days in it?\n> \n> > +\t\tour $maxCacheTime = 60*60*24*7*30*365; # upper bound for dynamic (adaptive) caching\n> \n> Likewise, is this 210 years? Does this bust 32-bit time_t somewhere?\n\nOoops.\n\nStill, fixing this doesn't fix tests. I think I'd have to make another\nextra commit, adding configure knob to turn off cacheWaitForUpdate()...\n\n\nOr perhaps I did something wrong when simplifying, but I have checked\ndiff between full and simplified version (in comments in v7.1 series),\nand it doesn't look like it...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"156094","messageId":"201011181437.18499.jnareb@gmail.com","threadId":"25573","inReplyTo":"7vd3q3eecm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv7.2 1/4] gitweb: Prepare for splitting gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-18T13:37:17Z","receivedAt":"2010-11-18T13:37:17Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 18 Nov 2010, Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n> > That means adding GITWEBLIBDIR for future testing of individual\n> > modules (it was not present in v7.1).\n> \n> You confused me. \"git grep\" after test-applying this 4-patch series on\n> top of next shows only one GITWEBLIBDIR, which means that the symbol is\n> defined but nobody pays attention to it.\n\nThat's why I wrote \"for _future_ testing\"... but perhaps it is not worth\nintroducing it now that it is not yet used by any test.\n\n> > diff --git a/gitweb/Makefile b/gitweb/Makefile\n> > index 0a6ac00..f9e32eb 100644\n> > --- a/gitweb/Makefile\n> > +++ b/gitweb/Makefile\n> > @@ -57,6 +57,7 @@ PERL_PATH ?= /usr/bin/perl\n> > ...\n> > -.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE\n> > +.PHONY: all clean install install-modules test test-installed .FORCE-GIT-VERSION-FILE FORCE\n> > \n> \n> In the earlier series, 478da52 (gitweb: Prepare for splitting gitweb,\n> 2010-10-30) removed the trailing blank line from here but this does not do\n> so anymore, which is a micronit regression.\n\nI was not sure if such \"while at it\" style improvements are encouraged\nor not. Will remember it for v7.3.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"156753","messageId":"201011281223.03141.jnareb@gmail.com","threadId":"25573","inReplyTo":"201011130056.24222.jnareb@gmail.com","subject":"[PATCHv7.1 3/4 (amend)] gitweb: File based caching layer (from git.kernel.org)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-28T11:22:58Z","receivedAt":"2010-11-28T11:22:58Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis is a relatively large patch that implements the file based\ncaching layer that is quite similar to the one used on such large\nsites as kernel.org and soon git.fedoraproject.org. This provides\na simple, and straight forward caching mechanism that scales\ndramatically better than Gitweb by itself.\n\nThe caching layer basically buffers the output that Gitweb would\nnormally return, and saves that output to a cache file on the local\ndisk. When the file is requested it attempts to gain a shared lock\non the cache file and cat it out to the client. Should an exclusive\nlock be on a file (it's being updated) the code has a choice to either\nupdate in the background and go ahead and show the stale page while\nupdate is being performed, or stall the client(s) until the page\nis generated.\n\nThere are two forms of stalling involved here, background building\nand non-background building, both of which are discussed in the\nconfiguration page.\n\nThere are still a few known \"issues\" with respect to this:\n- Code needs to be added to be \"browser\" aware so\n that clients like wget that are trying to get a\n binary blob don't obtain a \"Generating...\" page\n\nCaching is disabled by default. You can turn it on by setting\n$caching_enabled variable to true to enable file based caching.\n\n[jn: added error checking to loading 'cache.pl'; moved check\n for $caching_enabled outside out of cache_fetch, which required\n update to die_error()]\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Jakub Narebski \n---\n\nOn Sat, 13 Nov 2010, Jakub Narebski wrote:\n\n> +sub cacheUpdate {\n> +       my ($action,$areForked) = @_;\n> +       my $lockingStatus;\n> +       my $fileData = \"\";\n> +\n> +       if($backgroundCache){\n> +               open(cacheFileBG, '>:utf8', \"$fullhashpath.bg\");\n> +               my $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);\n> +\n> +               $lockStatus = $lockStatBG;\n> +       }else{\n> +               open(cacheFile, '>:utf8', \\$fullhashpath);\n ^^^^^^^^^^^^^^\n\nI have made mistake with this line when moving $caching_enabled check\nout of cache_fetch to its caller.\n\nReusing $fullhashpath variable as a *capture buffer* (it has nothing\nto do with path; it is not a filename no longer) wouldn't help there.\n\n> +               my $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n\nA note about original code: you can't flock in-memory files.\n\n> +\n> +               $lockStatus = $lockStat;\n> +       }\n> +       #print \"lock status: $lockStat\\n\";\n[...]\n\n gitweb/Makefile | 3 +\n gitweb/gitweb.perl | 105 ++++++++++++--\n gitweb/lib/cache.pl | 348 ++++++++++++++++++++++++++++++++++++++++++++++\n gitweb/static/gitweb.css | 6 +\n 4 files changed, 450 insertions(+), 12 deletions(-)\n create mode 100644 gitweb/lib/cache.pl\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex f9e32eb..6ddd4f1 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -113,6 +113,9 @@ endif\n \n GITWEB_FILES += static/git-logo.png static/git-favicon.png\n \n+# Gitweb caching\n+GITWEB_MODULES += cache.pl\n+\n GITWEB_REPLACE = \\\n \t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n \t-e 's|++GIT_BINDIR++|$(bindir)|g' \\\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex cae0e34..3c3ff08 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -250,6 +250,53 @@ our %avatar_size = (\n # Leave it undefined (or set to 'undef') to turn off load checking.\n our $maxload = 300;\n \n+# This enables/disables the caching layer in gitweb. This currently only supports the\n+# 'dumb' file based caching layer, primarily used on git.kernel.org. this is reasonably\n+# effective but it has the downside of requiring a huge amount of disk space if there\n+# are a number of repositories involved. It is not uncommon for git.kernel.org to have\n+# on the order of 80G - 120G accumulate over the course of a few months. It is recommended\n+# that the cache directory be periodically completely deleted, and this is safe to perform.\n+# Suggested mechanism\n+# mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush\n+our $caching_enabled = 0;\n+\n+# Used to set the minimum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to be under this number of seconds we set the cache timeout\n+# to this minimum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $minCacheTime = 20;\n+\n+# Used to set the maximum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to exceed this number of seconds we set the cache timeout\n+# to this maximum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $maxCacheTime = 1200;\n+\n+# If you need to change the location of the caching directory, override this\n+# otherwise this will probably do fine for you\n+our $cachedir = 'cache';\n+\n+# If this is set (to 1) cache will do it's best to always display something instead\n+# of making someone wait for the cache to update. This will launch the cacheUpdate\n+# into the background and it will lock a .bg file and will only lock the\n+# actual cache file when it needs to write into it. In theory this will make\n+# gitweb seem more responsive at the price of possibly stale data.\n+our $backgroundCache = 1;\n+\n+# Used to set the maximum cache file life. If a cache files last modify time exceeds\n+# this value, it will assume that the data is just too old, and HAS to be regenerated\n+# instead of trying to display the existing cache data.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+# 18000 = 5 hours\n+our $maxCacheLife = 18000;\n+\n+# Used to enable or disable background forking of the gitweb caching. Mainly here for debugging purposes\n+our $cacheDoFork = 1;\n+\n+our $fullhashpath = *STDOUT;\n+our $fullhashbinpath = *STDOUT;\n+our $fullhashbinpathfinal = *STDOUT;\n+\n # configuration for 'highlight' (http://www.andre-simon.de/)\n # match by basename\n our %highlight_basename = (\n@@ -506,6 +553,15 @@ our %feature = (\n \t\t'default' => [0]},\n );\n \n+#\n+# Includes\n+#\n+if (!exists $INC{'cache.pl'}) {\n+\tmy $return = do 'cache.pl';\n+\tdie $@ if $@;\n+\tdie \"Couldn't read 'cache.pl': $!\" if (!defined $return);\n+}\n+\n sub gitweb_get_feature {\n \tmy ($name) = @_;\n \treturn unless exists $feature{$name};\n@@ -734,6 +790,10 @@ our %actions = (\n \t\"project_list\" => \\&git_project_list,\n \t\"project_index\" => \\&git_project_index,\n );\n+sub is_cacheable {\n+\tmy $action = shift;\n+\treturn !($action eq 'blame_data' || $action eq 'blame_incremental');\n+}\n \n # finally, we have the hash of allowed extra_options for the commands that\n # allow them\n@@ -1072,7 +1132,11 @@ sub dispatch {\n \t !$project) {\n \t\tdie_error(400, \"Project needed\");\n \t}\n-\t$actions{$action}->();\n+\tif ($caching_enabled && is_cacheable($action)) {\n+\t\tcache_fetch($action);\n+\t} else {\n+\t\t$actions{$action}->();\n+\t}\n }\n \n sub reset_timer {\n@@ -1142,6 +1206,7 @@ sub change_output {\n \n \t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n \topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n+\tprint STDOUT_REAL \"\";\n \n \t# Close STDOUT, so that it isn't being used anymore.\n \tclose STDOUT;\n@@ -1167,10 +1232,7 @@ sub run {\n \t\t$pre_dispatch_hook->()\n \t\t\tif $pre_dispatch_hook;\n \n-\t\tchange_output();\n \t\trun_request();\n-\t\treset_output();\n-\t\tprint $output;\n \n \t\t$post_dispatch_hook->()\n \t\t\tif $post_dispatch_hook;\n@@ -3447,7 +3509,8 @@ sub git_header_html {\n \t# support xhtml+xml but choking when it gets what it asked for.\n \tif (defined $cgi->http('HTTP_ACCEPT') &&\n \t $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\\s|^)application\\/xhtml\\+xml(,|;|\\s|$)/ &&\n-\t $cgi->Accept('application/xhtml+xml') != 0) {\n+\t $cgi->Accept('application/xhtml+xml') != 0 &&\n+\t !$caching_enabled) {\n \t\t$content_type = 'application/xhtml+xml';\n \t} else {\n \t\t$content_type = 'text/html';\n@@ -3592,6 +3655,7 @@ sub git_footer_html {\n \tmy $feed_class = 'rss_logo';\n \n \tprint \"
\\n\";\n+\tprint \"
Cache Last Updated: \". gmtime( time ) .\" GMT
\\n\";\n \tif (defined $project) {\n \t\tmy $descr = git_get_project_description($project);\n \t\tif (defined $descr) {\n@@ -3680,9 +3744,14 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\t# The output handlers for die_error need to be reset to STDOUT\n+\t# so that half the message isn't being output to random and\n+\t# half to STDOUT as expected. This is mainly for the benefit\n+\t# of using git_header_html() and git_footer_html() since\n+\t#\n \t# Reset the output so that we are actually going to STDOUT as opposed\n \t# to buffering the output.\n-\treset_output();\n+\treset_output() if ($caching_enabled);\n \n \tgit_header_html($http_responses{$status}, undef, %opts);\n \tprint <;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif ($caching_enabled) {\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \n@@ -5879,9 +5954,15 @@ sub git_snapshot {\n \n \topen my $fd, \"-|\", $cmd\n \t\tor die_error(500, \"Execute git-archive failed\");\n-\tbinmode STDOUT, ':raw';\n-\tprint <$fd>;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif ($caching_enabled) {\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \ndiff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl\nnew file mode 100644\nindex 0000000..dd14bfb\n--- /dev/null\n+++ b/gitweb/lib/cache.pl\n@@ -0,0 +1,348 @@\n+# gitweb - simple web interface to track changes in git repositories\n+#\n+# (C) 2006, John 'Warthog9' Hawley \n+#\n+# This program is licensed under the GPLv2\n+\n+#\n+# Gitweb caching engine\n+#\n+\n+#use File::Path qw(make_path remove_tree);\n+use File::Path qw(mkpath rmtree); # Used for compatability reasons\n+use Digest::MD5 qw(md5 md5_hex md5_base64);\n+use Fcntl ':flock';\n+use File::Copy;\n+\n+sub cache_fetch {\n+\tmy ($action) = @_;\n+\tmy $cacheTime = 0;\n+\n+\tif(! -d $cachedir){\n+\t\tprint \"*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\\n\";\n+\t\tmkdir (\"cache\", 0755) || die \"Cannot create cache dir - you will need to manually create\";\n+\t\tprint \"Cache directory created successfully\\n\";\n+\t}\n+\n+\tour $full_url = \"$my_url?\". $ENV{'QUERY_STRING'};\n+\tour $urlhash = md5_hex($full_url);\n+\tour $fullhashdir = \"$cachedir/\". substr( $urlhash, 0, 2) .\"/\";\n+\n+\teval { mkpath( $fullhashdir, 0, 0777 ) };\n+\tif ($@) {\n+\t\tdie_error(500, \"Internal Server Error\", \"Could not create cache directory: $@\");\n+\t}\n+\t$fullhashpath = \"$fullhashdir/\". substr( $urlhash, 2 );\n+\t$fullhashbinpath = \"$fullhashpath.bin.wt\";\n+\t$fullhashbinpathfinal = \"$fullhashpath.bin\";\n+\n+\tif(! -e \"$fullhashpath\" ){\n+\t\tif(! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\tcacheUpdate($action,0);\n+\t\t\tcacheDisplay($action);\n+\t\t} elsif ( $childPid == 0 ){\n+\t\t\t#run the updater\n+\t\t\tcacheUpdate($action,1);\n+\t\t}else{\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\t}else{\n+\t\t#if cache is out dated, update\n+\t\t#else displayCache();\n+\t\topen(cacheFile, '<', \"$fullhashpath\");\n+\t\tstat(cacheFile);\n+\t\tclose(cacheFile);\n+\t\tmy $stat_time = (stat(_))[9];\n+\t\tmy $stat_size = (stat(_))[7];\n+\n+\t\t$cacheTime = get_loadavg() * 60;\n+\t\tif( $cacheTime > $maxCacheTime ){\n+\t\t\t$cacheTime = $maxCacheTime;\n+\t\t}\n+\t\tif( $cacheTime < $minCacheTime ){\n+\t\t\t$cacheTime = $minCacheTime;\n+\t\t}\n+\t\tif( $stat_time < (time - $cacheTime) || $stat_size == 0 ){\n+\t\t\tif( ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\t\tcacheUpdate($action,0);\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t} elsif ( $childPid == 0 ){\n+\t\t\t\t#run the updater\n+\t\t\t\t#print \"Running updater\\n\";\n+\t\t\t\tcacheUpdate($action,1);\n+\t\t\t}else{\n+\t\t\t\t#print \"Waiting for update\\n\";\n+\t\t\t\tcacheWaitForUpdate($action);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\n+\n+\t}\n+\n+\t#\n+\t# If all of the caching failes - lets go ahead and press on without it and fall back to 'default'\n+\t# non-caching behavior. This is the softest of the failure conditions.\n+\t#\n+\t#$actions{$action}->();\n+}\n+\n+sub cacheUpdate {\n+\tmy ($action,$areForked) = @_;\n+\tmy $lockingStatus;\n+\tmy $fileData = \"\";\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFileBG, '>:utf8', \"$fullhashpath.bg\");\n+\t\tmy $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStatBG;\n+\t}else{\n+\t\topen(cacheFile, '>:utf8', \\$fullhashpath);\n+\t\tmy $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStat;\n+\t}\n+\t#print \"lock status: $lockStat\\n\";\n+\n+\n+\tif (! $lockStatus ){\n+\t\tif ( $areForked ){\n+\t\t\texit(0);\n+\t\t}else{\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinWT, '>>:utf8', \"$fullhashbinpath\");\n+\t\tmy $lockStatBin = flock(cacheFileBinWT,LOCK_EX|LOCK_NB);\n+\t}\n+\n+\t# Trap all output from the action\n+\tchange_output();\n+\n+\t$actions{$action}->();\n+\n+\t# Reset the outputs as we should be fine now\n+\treset_output();\n+\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFile, '>:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_EX);\n+\n+\t\tif (! $lockStat ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinFINAL, '>:utf8', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBinFINAL,LOCK_EX);\n+\n+\t\tif (! $lockStatBIN ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# Actually dump the output to the proper file handler\n+\tlocal $/ = undef;\n+\t$|++;\n+\tprint cacheFile \"$output\";\n+\t$|--;\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmove(\"$fullhashbinpath\", \"$fullhashbinpathfinal\") or die \"Binary Cache file could not be updated: $!\";\n+\n+\t\tflock(cacheFileBinFINAL,LOCK_UN);\n+\t\tclose(cacheFileBinFINAL);\n+\n+\t\tflock(cacheFileBinWT,LOCK_UN);\n+\t\tclose(cacheFileBinWT);\n+\t}\n+\n+\tflock(cacheFile,LOCK_UN);\n+\tclose(cacheFile);\n+\n+\tif($backgroundCache){\n+\t\tflock(cacheFileBG,LOCK_UN);\n+\t\tclose(cacheFileBG);\n+\t}\n+\n+\tif ( $areForked ){\n+\t\texit(0);\n+\t} else {\n+\t\treturn;\n+\t}\n+}\n+\n+\n+sub cacheWaitForUpdate {\n+\tmy ($action) = @_;\n+\tmy $x = 0;\n+\tmy $max = 10;\n+\tmy $lockStat = 0;\n+\n+\tif( $backgroundCache ){\n+\t\tif( -e \"$fullhashpath\" ){\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tstat(cacheFile);\n+\t\t\tclose(cacheFile);\n+\n+\t\t\tif( $lockStat && ( (stat(_))[9] > (time - $maxCacheLife) ) ){\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"atom\"\n+\t\t||\n+\t\t$action eq \"rss\"\n+\t\t||\n+\t\t$action eq \"opml\"\n+\t){\n+\t\tdo {\n+\t\t\tsleep 2 if $x > 0;\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tclose(cacheFile);\n+\t\t\t$x++;\n+\t\t\t$combinedLockStat = $lockStat;\n+\t\t} while ((! $combinedLockStat) && ($x < $max));\n+\n+\t\tif( $x != $max ){\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\t\treturn;\n+\t}\n+\n+\t$| = 1;\n+\n+\tprint $::cgi->header(\n+\t\t\t\t-type=>'text/html',\n+\t\t\t\t-charset => 'utf-8',\n+\t\t\t\t-status=> 200,\n+\t\t\t\t-expires => 'now',\n+\t\t\t\t# HTTP/1.0\n+\t\t\t\t-Pragma => 'no-cache',\n+\t\t\t\t# HTTP/1.1\n+\t\t\t\t-Cache_Control => join(\n+\t\t\t\t\t\t\t', ',\n+\t\t\t\t\t\t\tqw(\n+\t\t\t\t\t\t\t\tprivate\n+\t\t\t\t\t\t\t\tno-cache\n+\t\t\t\t\t\t\t\tno-store\n+\t\t\t\t\t\t\t\tmust-revalidate\n+\t\t\t\t\t\t\t\tmax-age=0\n+\t\t\t\t\t\t\t\tpre-check=0\n+\t\t\t\t\t\t\t\tpost-check=0\n+\t\t\t\t\t\t\t)\n+\t\t\t\t\t\t)\n+\t\t\t\t);\n+\n+\tprint <\n+\n+\n+\n+\n+\n+\n+\n+$title\n+\n+\n+EOF\n+\n+\tprint \"Generating..\";\n+\tdo {\n+\t\tprint \".\";\n+\t\tsleep 2 if $x > 0;\n+\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\tclose(cacheFile);\n+\t\t$x++;\n+\t\t$combinedLockStat = $lockStat;\n+\t} while ((! $combinedLockStat) && ($x < $max));\n+\tprint <\n+\n+EOF\n+\treturn;\n+}\n+\n+sub cacheDisplay {\n+\tlocal $/ = undef;\n+\t$|++;\n+\n+\tmy ($action) = @_;\n+\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\n+\tif (! $lockStat ){\n+\t\tclose(cacheFile);\n+\t\tcacheWaitForUpdate($action);\n+\t}\n+\n+\tif(\n+\t\t(\n+\t\t\t$action eq \"snapshot\"\n+\t\t\t||\n+\t\t\t$action eq \"blob_plain\"\n+\t\t)\n+\t){\n+\t\tmy $openstat = open(cacheFileBin, '<', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBin,LOCK_SH|LOCK_NB);\n+\t\tif (! $lockStatBIN ){\n+\t\t\tsystem (\"echo 'cacheDisplay - bailing due to binary lock failure' >> /tmp/gitweb.log\");\n+\t\t\tclose(cacheFile);\n+\t\t\tclose(cacheFileBin);\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\n+\t\tmy $binfilesize = -s \"$fullhashbinpathfinal\";\n+\t\tprint \"Content-Length: $binfilesize\";\n+\t}\n+\twhile( ){\n+\t\tprint $_;\n+\t}\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tbinmode STDOUT, ':raw';\n+\t\tprint ;\n+\t\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\t\tclose(cacheFileBin);\n+\t}\n+\tclose(cacheFile);\n+\t$|--;\n+}\n+\n+1;\n+__END__\ndiff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\nindex 4132aab..972d32e 100644\n--- a/gitweb/static/gitweb.css\n+++ b/gitweb/static/gitweb.css\n@@ -67,6 +67,12 @@ div.page_path {\n \tborder-width: 0px 0px 1px;\n }\n \n+div.cachetime {\n+\tfloat: left;\n+\tmargin-right: 10px;\n+\tcolor: #555555;\n+}\n+\n div.page_footer {\n \theight: 17px;\n \tpadding: 4px 8px;\n-- \n1.7.3\n"},{"id":"156754","messageId":"201011281231.46786.jnareb@gmail.com","threadId":"25573","inReplyTo":"201011281223.03141.jnareb@gmail.com","subject":"Re: [PATCHv7.1 3/4 (amend)] gitweb: File based caching layer (from git.kernel.org)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-28T11:31:46Z","receivedAt":"2010-11-28T11:31:46Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 28 Nov 2010, Jakub Narebski wrote:\n> On Sat, 13 Nov 2010, Jakub Narebski wrote:\n> \n> > +sub cacheUpdate {\n> > +       my ($action,$areForked) = @_;\n> > +       my $lockingStatus;\n> > +       my $fileData = \"\";\n> > +\n> > +       if($backgroundCache){\n> > +               open(cacheFileBG, '>:utf8', \"$fullhashpath.bg\");\n> > +               my $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);\n> > +\n> > +               $lockStatus = $lockStatBG;\n> > +       }else{\n> > +               open(cacheFile, '>:utf8', \\$fullhashpath);\n>                                             ^^^^^^^^^^^^^^\n> \n> I have made mistake with this line when moving $caching_enabled check\n> out of cache_fetch to its caller.\n> \n> Reusing $fullhashpath variable as a *capture buffer* (it has nothing\n> to do with path; it is not a filename no longer) wouldn't help there.\n\nErrr... I meant that this abuse didn't help avoiding my mistake.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"156845","messageId":"7vvd3fzskv.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"201011281231.46786.jnareb@gmail.com","subject":"Re: [PATCHv7.1 3/4 (amend)] gitweb: File based caching layer (from git.kernel.org)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-29T22:13:36Z","receivedAt":"2010-11-29T22:13:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n>> I have made mistake with this line when moving $caching_enabled check\n>> out of cache_fetch to its caller.\n>> \n>> Reusing $fullhashpath variable as a *capture buffer* (it has nothing\n>> to do with path; it is not a filename no longer) wouldn't help there.\n>\n> Errr... I meant that this abuse didn't help avoiding my mistake.\n\nThanks.\n"},{"id":"156846","messageId":"7vr5e3zs9u.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"7vvd3fzskv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv7.1 3/4 (amend)] gitweb: File based caching layer (from git.kernel.org)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-29T22:20:13Z","receivedAt":"2010-11-29T22:20:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jakub Narebski writes:\n>\n>>> I have made mistake with this line when moving $caching_enabled check\n>>> out of cache_fetch to its caller.\n>>> \n>>> Reusing $fullhashpath variable as a *capture buffer* (it has nothing\n>>> to do with path; it is not a filename no longer) wouldn't help there.\n>>\n>> Errr... I meant that this abuse didn't help avoiding my mistake.\n>\n> Thanks.\n\nWait a bit.\n\nThis seems to match what I have already queued on 'pu', no? Am I\nhallucinating?\n"},{"id":"156859","messageId":"AANLkTinX9Phu+SuOyEWtAsMz182VkfVkJva1ep==V-zQ@mail.gmail.com","threadId":"25573","inReplyTo":"201011130056.24222.jnareb@gmail.com","subject":"Re: [PATCHv7.1 3/4] gitweb: File based caching layer (from git.kernel.org)","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2010-11-29T23:07:39Z","receivedAt":"2010-11-29T23:07:39Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"2010/11/13 Jakub Narebski :\n> -       binmode STDOUT, ':raw';\n> -       print <$fd>;\n> -       binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n> +       if ($caching_enabled) {\n> +               open BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n> +       }else{\n> +               open BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n> +       }\n> +       binmode BINOUT, ':raw';\n> +       print BINOUT <$fd>;\n> +       binmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n> +       close BINOUT;\n\nWhy do you use dynamically scoped file handles here as opposed to\nlexically scoped ones?\n\nAnd why do you change the output discipline on BINOUT immediately\nbefore closing the file, and after you print data to it?\n\nDoing so sortof makes sense when the filedhandle is STDOUT, but not\nwhen it is BINOUT.\n\ncheers,\nYves\n\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"156861","messageId":"201011300009.05437.jnareb@gmail.com","threadId":"25573","inReplyTo":"7vr5e3zs9u.fsf@alter.siamese.dyndns.org","subject":"[PATCHv7.1 3/4 (amend v2)] gitweb: File based caching layer (from git.kernel.org)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-29T23:09:03Z","receivedAt":"2010-11-29T23:09:03Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 29 Nov 2010, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>> Jakub Narebski writes:\n>>\n>>>> I have made mistake with this line when moving $caching_enabled check\n>>>> out of cache_fetch to its caller.\n>>>> \n>>>> Reusing $fullhashpath variable as a *capture buffer* (it has nothing\n>>>> to do with path; it is not a filename no longer) wouldn't help there.\n>>>\n>>> Errr... I meant that this abuse didn't help avoiding my mistake.\n>>\n>> Thanks.\n> \n> Wait a bit.\n> \n> This seems to match what I have already queued on 'pu', no? Am I\n> hallucinating?\n \nDamn, it looks like my mailer included wrong (older) version of a file.\nI'm sorry. Below there is interdiff:\n\n diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl\n index dd14bfb..4360b3d 100644\n --- a/gitweb/lib/cache.pl\n +++ b/gitweb/lib/cache.pl\n @@ -99,7 +99,7 @@ sub cacheUpdate {\n \n \t\t$lockStatus = $lockStatBG;\n \t}else{\n -\t\topen(cacheFile, '>:utf8', \\$fullhashpath);\n +\t\topen(cacheFile, '>:utf8', $fullhashpath);\n \t\tmy $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n \n \t\t$lockStatus = $lockStat;\n\n-- >8 --\nFrom: John 'Warthog9' Hawley \n\nThis is a relatively large patch that implements the file based\ncaching layer that is quite similar to the one used on such large\nsites as kernel.org and soon git.fedoraproject.org. This provides\na simple, and straight forward caching mechanism that scales\ndramatically better than Gitweb by itself.\n\nThe caching layer basically buffers the output that Gitweb would\nnormally return, and saves that output to a cache file on the local\ndisk. When the file is requested it attempts to gain a shared lock\non the cache file and cat it out to the client. Should an exclusive\nlock be on a file (it's being updated) the code has a choice to either\nupdate in the background and go ahead and show the stale page while\nupdate is being performed, or stall the client(s) until the page\nis generated.\n\nThere are two forms of stalling involved here, background building\nand non-background building, both of which are discussed in the\nconfiguration page.\n\nThere are still a few known \"issues\" with respect to this:\n- Code needs to be added to be \"browser\" aware so\n that clients like wget that are trying to get a\n binary blob don't obtain a \"Generating...\" page\n\nCaching is disabled by default. You can turn it on by setting\n$caching_enabled variable to true to enable file based caching.\n\n[jn: added error checking to loading 'cache.pl'; moved check\n for $caching_enabled outside out of cache_fetch, which required\n update to die_error()]\n\nSigned-off-by: Junio C Hamano \n---\n gitweb/Makefile | 3 +\n gitweb/gitweb.perl | 105 ++++++++++++--\n gitweb/lib/cache.pl | 348 ++++++++++++++++++++++++++++++++++++++++++++++\n gitweb/static/gitweb.css | 6 +\n 4 files changed, 450 insertions(+), 12 deletions(-)\n create mode 100644 gitweb/lib/cache.pl\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex f9e32eb..6ddd4f1 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -113,6 +113,9 @@ endif\n \n GITWEB_FILES += static/git-logo.png static/git-favicon.png\n \n+# Gitweb caching\n+GITWEB_MODULES += cache.pl\n+\n GITWEB_REPLACE = \\\n \t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n \t-e 's|++GIT_BINDIR++|$(bindir)|g' \\\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex cae0e34..3c3ff08 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -250,6 +250,53 @@ our %avatar_size = (\n # Leave it undefined (or set to 'undef') to turn off load checking.\n our $maxload = 300;\n \n+# This enables/disables the caching layer in gitweb. This currently only supports the\n+# 'dumb' file based caching layer, primarily used on git.kernel.org. this is reasonably\n+# effective but it has the downside of requiring a huge amount of disk space if there\n+# are a number of repositories involved. It is not uncommon for git.kernel.org to have\n+# on the order of 80G - 120G accumulate over the course of a few months. It is recommended\n+# that the cache directory be periodically completely deleted, and this is safe to perform.\n+# Suggested mechanism\n+# mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush\n+our $caching_enabled = 0;\n+\n+# Used to set the minimum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to be under this number of seconds we set the cache timeout\n+# to this minimum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $minCacheTime = 20;\n+\n+# Used to set the maximum cache timeout for the dynamic caching algorithm. Basically\n+# if we calculate the cache to exceed this number of seconds we set the cache timeout\n+# to this maximum.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+our $maxCacheTime = 1200;\n+\n+# If you need to change the location of the caching directory, override this\n+# otherwise this will probably do fine for you\n+our $cachedir = 'cache';\n+\n+# If this is set (to 1) cache will do it's best to always display something instead\n+# of making someone wait for the cache to update. This will launch the cacheUpdate\n+# into the background and it will lock a .bg file and will only lock the\n+# actual cache file when it needs to write into it. In theory this will make\n+# gitweb seem more responsive at the price of possibly stale data.\n+our $backgroundCache = 1;\n+\n+# Used to set the maximum cache file life. If a cache files last modify time exceeds\n+# this value, it will assume that the data is just too old, and HAS to be regenerated\n+# instead of trying to display the existing cache data.\n+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour\n+# 18000 = 5 hours\n+our $maxCacheLife = 18000;\n+\n+# Used to enable or disable background forking of the gitweb caching. Mainly here for debugging purposes\n+our $cacheDoFork = 1;\n+\n+our $fullhashpath = *STDOUT;\n+our $fullhashbinpath = *STDOUT;\n+our $fullhashbinpathfinal = *STDOUT;\n+\n # configuration for 'highlight' (http://www.andre-simon.de/)\n # match by basename\n our %highlight_basename = (\n@@ -506,6 +553,15 @@ our %feature = (\n \t\t'default' => [0]},\n );\n \n+#\n+# Includes\n+#\n+if (!exists $INC{'cache.pl'}) {\n+\tmy $return = do 'cache.pl';\n+\tdie $@ if $@;\n+\tdie \"Couldn't read 'cache.pl': $!\" if (!defined $return);\n+}\n+\n sub gitweb_get_feature {\n \tmy ($name) = @_;\n \treturn unless exists $feature{$name};\n@@ -734,6 +790,10 @@ our %actions = (\n \t\"project_list\" => \\&git_project_list,\n \t\"project_index\" => \\&git_project_index,\n );\n+sub is_cacheable {\n+\tmy $action = shift;\n+\treturn !($action eq 'blame_data' || $action eq 'blame_incremental');\n+}\n \n # finally, we have the hash of allowed extra_options for the commands that\n # allow them\n@@ -1072,7 +1132,11 @@ sub dispatch {\n \t !$project) {\n \t\tdie_error(400, \"Project needed\");\n \t}\n-\t$actions{$action}->();\n+\tif ($caching_enabled && is_cacheable($action)) {\n+\t\tcache_fetch($action);\n+\t} else {\n+\t\t$actions{$action}->();\n+\t}\n }\n \n sub reset_timer {\n@@ -1142,6 +1206,7 @@ sub change_output {\n \n \t# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such\n \topen(STDOUT_REAL,\">&STDOUT\") or die \"Unable to capture STDOUT $!\\n\";\n+\tprint STDOUT_REAL \"\";\n \n \t# Close STDOUT, so that it isn't being used anymore.\n \tclose STDOUT;\n@@ -1167,10 +1232,7 @@ sub run {\n \t\t$pre_dispatch_hook->()\n \t\t\tif $pre_dispatch_hook;\n \n-\t\tchange_output();\n \t\trun_request();\n-\t\treset_output();\n-\t\tprint $output;\n \n \t\t$post_dispatch_hook->()\n \t\t\tif $post_dispatch_hook;\n@@ -3447,7 +3509,8 @@ sub git_header_html {\n \t# support xhtml+xml but choking when it gets what it asked for.\n \tif (defined $cgi->http('HTTP_ACCEPT') &&\n \t $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\\s|^)application\\/xhtml\\+xml(,|;|\\s|$)/ &&\n-\t $cgi->Accept('application/xhtml+xml') != 0) {\n+\t $cgi->Accept('application/xhtml+xml') != 0 &&\n+\t !$caching_enabled) {\n \t\t$content_type = 'application/xhtml+xml';\n \t} else {\n \t\t$content_type = 'text/html';\n@@ -3592,6 +3655,7 @@ sub git_footer_html {\n \tmy $feed_class = 'rss_logo';\n \n \tprint \"
\\n\";\n+\tprint \"
Cache Last Updated: \". gmtime( time ) .\" GMT
\\n\";\n \tif (defined $project) {\n \t\tmy $descr = git_get_project_description($project);\n \t\tif (defined $descr) {\n@@ -3680,9 +3744,14 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\t# The output handlers for die_error need to be reset to STDOUT\n+\t# so that half the message isn't being output to random and\n+\t# half to STDOUT as expected. This is mainly for the benefit\n+\t# of using git_header_html() and git_footer_html() since\n+\t#\n \t# Reset the output so that we are actually going to STDOUT as opposed\n \t# to buffering the output.\n-\treset_output();\n+\treset_output() if ($caching_enabled);\n \n \tgit_header_html($http_responses{$status}, undef, %opts);\n \tprint <;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif ($caching_enabled) {\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \n@@ -5879,9 +5954,15 @@ sub git_snapshot {\n \n \topen my $fd, \"-|\", $cmd\n \t\tor die_error(500, \"Execute git-archive failed\");\n-\tbinmode STDOUT, ':raw';\n-\tprint <$fd>;\n-\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tif ($caching_enabled) {\n+\t\topen BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}else{\n+\t\topen BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n+\t}\n+\tbinmode BINOUT, ':raw';\n+\tprint BINOUT <$fd>;\n+\tbinmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\tclose BINOUT;\n \tclose $fd;\n }\n \ndiff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl\nnew file mode 100644\nindex 0000000..4360b3d\n--- /dev/null\n+++ b/gitweb/lib/cache.pl\n@@ -0,0 +1,348 @@\n+# gitweb - simple web interface to track changes in git repositories\n+#\n+# (C) 2006, John 'Warthog9' Hawley \n+#\n+# This program is licensed under the GPLv2\n+\n+#\n+# Gitweb caching engine\n+#\n+\n+#use File::Path qw(make_path remove_tree);\n+use File::Path qw(mkpath rmtree); # Used for compatability reasons\n+use Digest::MD5 qw(md5 md5_hex md5_base64);\n+use Fcntl ':flock';\n+use File::Copy;\n+\n+sub cache_fetch {\n+\tmy ($action) = @_;\n+\tmy $cacheTime = 0;\n+\n+\tif(! -d $cachedir){\n+\t\tprint \"*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\\n\";\n+\t\tmkdir (\"cache\", 0755) || die \"Cannot create cache dir - you will need to manually create\";\n+\t\tprint \"Cache directory created successfully\\n\";\n+\t}\n+\n+\tour $full_url = \"$my_url?\". $ENV{'QUERY_STRING'};\n+\tour $urlhash = md5_hex($full_url);\n+\tour $fullhashdir = \"$cachedir/\". substr( $urlhash, 0, 2) .\"/\";\n+\n+\teval { mkpath( $fullhashdir, 0, 0777 ) };\n+\tif ($@) {\n+\t\tdie_error(500, \"Internal Server Error\", \"Could not create cache directory: $@\");\n+\t}\n+\t$fullhashpath = \"$fullhashdir/\". substr( $urlhash, 2 );\n+\t$fullhashbinpath = \"$fullhashpath.bin.wt\";\n+\t$fullhashbinpathfinal = \"$fullhashpath.bin\";\n+\n+\tif(! -e \"$fullhashpath\" ){\n+\t\tif(! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\tcacheUpdate($action,0);\n+\t\t\tcacheDisplay($action);\n+\t\t} elsif ( $childPid == 0 ){\n+\t\t\t#run the updater\n+\t\t\tcacheUpdate($action,1);\n+\t\t}else{\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\t}else{\n+\t\t#if cache is out dated, update\n+\t\t#else displayCache();\n+\t\topen(cacheFile, '<', \"$fullhashpath\");\n+\t\tstat(cacheFile);\n+\t\tclose(cacheFile);\n+\t\tmy $stat_time = (stat(_))[9];\n+\t\tmy $stat_size = (stat(_))[7];\n+\n+\t\t$cacheTime = get_loadavg() * 60;\n+\t\tif( $cacheTime > $maxCacheTime ){\n+\t\t\t$cacheTime = $maxCacheTime;\n+\t\t}\n+\t\tif( $cacheTime < $minCacheTime ){\n+\t\t\t$cacheTime = $minCacheTime;\n+\t\t}\n+\t\tif( $stat_time < (time - $cacheTime) || $stat_size == 0 ){\n+\t\t\tif( ! $cacheDoFork || ! defined(my $childPid = fork()) ){\n+\t\t\t\tcacheUpdate($action,0);\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t} elsif ( $childPid == 0 ){\n+\t\t\t\t#run the updater\n+\t\t\t\t#print \"Running updater\\n\";\n+\t\t\t\tcacheUpdate($action,1);\n+\t\t\t}else{\n+\t\t\t\t#print \"Waiting for update\\n\";\n+\t\t\t\tcacheWaitForUpdate($action);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\n+\n+\t}\n+\n+\t#\n+\t# If all of the caching failes - lets go ahead and press on without it and fall back to 'default'\n+\t# non-caching behavior. This is the softest of the failure conditions.\n+\t#\n+\t#$actions{$action}->();\n+}\n+\n+sub cacheUpdate {\n+\tmy ($action,$areForked) = @_;\n+\tmy $lockingStatus;\n+\tmy $fileData = \"\";\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFileBG, '>:utf8', \"$fullhashpath.bg\");\n+\t\tmy $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStatBG;\n+\t}else{\n+\t\topen(cacheFile, '>:utf8', $fullhashpath);\n+\t\tmy $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);\n+\n+\t\t$lockStatus = $lockStat;\n+\t}\n+\t#print \"lock status: $lockStat\\n\";\n+\n+\n+\tif (! $lockStatus ){\n+\t\tif ( $areForked ){\n+\t\t\texit(0);\n+\t\t}else{\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinWT, '>>:utf8', \"$fullhashbinpath\");\n+\t\tmy $lockStatBin = flock(cacheFileBinWT,LOCK_EX|LOCK_NB);\n+\t}\n+\n+\t# Trap all output from the action\n+\tchange_output();\n+\n+\t$actions{$action}->();\n+\n+\t# Reset the outputs as we should be fine now\n+\treset_output();\n+\n+\n+\tif($backgroundCache){\n+\t\topen(cacheFile, '>:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_EX);\n+\n+\t\tif (! $lockStat ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmy $openstat = open(cacheFileBinFINAL, '>:utf8', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBinFINAL,LOCK_EX);\n+\n+\t\tif (! $lockStatBIN ){\n+\t\t\tif ( $areForked ){\n+\t\t\t\texit(0);\n+\t\t\t}else{\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# Actually dump the output to the proper file handler\n+\tlocal $/ = undef;\n+\t$|++;\n+\tprint cacheFile \"$output\";\n+\t$|--;\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tmove(\"$fullhashbinpath\", \"$fullhashbinpathfinal\") or die \"Binary Cache file could not be updated: $!\";\n+\n+\t\tflock(cacheFileBinFINAL,LOCK_UN);\n+\t\tclose(cacheFileBinFINAL);\n+\n+\t\tflock(cacheFileBinWT,LOCK_UN);\n+\t\tclose(cacheFileBinWT);\n+\t}\n+\n+\tflock(cacheFile,LOCK_UN);\n+\tclose(cacheFile);\n+\n+\tif($backgroundCache){\n+\t\tflock(cacheFileBG,LOCK_UN);\n+\t\tclose(cacheFileBG);\n+\t}\n+\n+\tif ( $areForked ){\n+\t\texit(0);\n+\t} else {\n+\t\treturn;\n+\t}\n+}\n+\n+\n+sub cacheWaitForUpdate {\n+\tmy ($action) = @_;\n+\tmy $x = 0;\n+\tmy $max = 10;\n+\tmy $lockStat = 0;\n+\n+\tif( $backgroundCache ){\n+\t\tif( -e \"$fullhashpath\" ){\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tstat(cacheFile);\n+\t\t\tclose(cacheFile);\n+\n+\t\t\tif( $lockStat && ( (stat(_))[9] > (time - $maxCacheLife) ) ){\n+\t\t\t\tcacheDisplay($action);\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif(\n+\t\t$action eq \"atom\"\n+\t\t||\n+\t\t$action eq \"rss\"\n+\t\t||\n+\t\t$action eq \"opml\"\n+\t){\n+\t\tdo {\n+\t\t\tsleep 2 if $x > 0;\n+\t\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\t\tclose(cacheFile);\n+\t\t\t$x++;\n+\t\t\t$combinedLockStat = $lockStat;\n+\t\t} while ((! $combinedLockStat) && ($x < $max));\n+\n+\t\tif( $x != $max ){\n+\t\t\tcacheDisplay($action);\n+\t\t}\n+\t\treturn;\n+\t}\n+\n+\t$| = 1;\n+\n+\tprint $::cgi->header(\n+\t\t\t\t-type=>'text/html',\n+\t\t\t\t-charset => 'utf-8',\n+\t\t\t\t-status=> 200,\n+\t\t\t\t-expires => 'now',\n+\t\t\t\t# HTTP/1.0\n+\t\t\t\t-Pragma => 'no-cache',\n+\t\t\t\t# HTTP/1.1\n+\t\t\t\t-Cache_Control => join(\n+\t\t\t\t\t\t\t', ',\n+\t\t\t\t\t\t\tqw(\n+\t\t\t\t\t\t\t\tprivate\n+\t\t\t\t\t\t\t\tno-cache\n+\t\t\t\t\t\t\t\tno-store\n+\t\t\t\t\t\t\t\tmust-revalidate\n+\t\t\t\t\t\t\t\tmax-age=0\n+\t\t\t\t\t\t\t\tpre-check=0\n+\t\t\t\t\t\t\t\tpost-check=0\n+\t\t\t\t\t\t\t)\n+\t\t\t\t\t\t)\n+\t\t\t\t);\n+\n+\tprint <\n+\n+\n+\n+\n+\n+\n+\n+$title\n+\n+\n+EOF\n+\n+\tprint \"Generating..\";\n+\tdo {\n+\t\tprint \".\";\n+\t\tsleep 2 if $x > 0;\n+\t\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\t\tclose(cacheFile);\n+\t\t$x++;\n+\t\t$combinedLockStat = $lockStat;\n+\t} while ((! $combinedLockStat) && ($x < $max));\n+\tprint <\n+\n+EOF\n+\treturn;\n+}\n+\n+sub cacheDisplay {\n+\tlocal $/ = undef;\n+\t$|++;\n+\n+\tmy ($action) = @_;\n+\topen(cacheFile, '<:utf8', \"$fullhashpath\");\n+\t$lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);\n+\n+\tif (! $lockStat ){\n+\t\tclose(cacheFile);\n+\t\tcacheWaitForUpdate($action);\n+\t}\n+\n+\tif(\n+\t\t(\n+\t\t\t$action eq \"snapshot\"\n+\t\t\t||\n+\t\t\t$action eq \"blob_plain\"\n+\t\t)\n+\t){\n+\t\tmy $openstat = open(cacheFileBin, '<', \"$fullhashbinpathfinal\");\n+\t\t$lockStatBIN = flock(cacheFileBin,LOCK_SH|LOCK_NB);\n+\t\tif (! $lockStatBIN ){\n+\t\t\tsystem (\"echo 'cacheDisplay - bailing due to binary lock failure' >> /tmp/gitweb.log\");\n+\t\t\tclose(cacheFile);\n+\t\t\tclose(cacheFileBin);\n+\t\t\tcacheWaitForUpdate($action);\n+\t\t}\n+\n+\t\tmy $binfilesize = -s \"$fullhashbinpathfinal\";\n+\t\tprint \"Content-Length: $binfilesize\";\n+\t}\n+\twhile( ){\n+\t\tprint $_;\n+\t}\n+\tif(\n+\t\t$action eq \"snapshot\"\n+\t\t||\n+\t\t$action eq \"blob_plain\"\n+\t){\n+\t\tbinmode STDOUT, ':raw';\n+\t\tprint ;\n+\t\tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n+\t\tclose(cacheFileBin);\n+\t}\n+\tclose(cacheFile);\n+\t$|--;\n+}\n+\n+1;\n+__END__\ndiff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\nindex 4132aab..972d32e 100644\n--- a/gitweb/static/gitweb.css\n+++ b/gitweb/static/gitweb.css\n@@ -67,6 +67,12 @@ div.page_path {\n \tborder-width: 0px 0px 1px;\n }\n \n+div.cachetime {\n+\tfloat: left;\n+\tmargin-right: 10px;\n+\tcolor: #555555;\n+}\n+\n div.page_footer {\n \theight: 17px;\n \tpadding: 4px 8px;\n-- \n1.7.3\n"},{"id":"156863","messageId":"AANLkTim5V_50Er8OiNTWDHNzvqJ3J+biOwuqnBdnR4jH@mail.gmail.com","threadId":"25573","inReplyTo":"AANLkTinX9Phu+SuOyEWtAsMz182VkfVkJva1ep==V-zQ@mail.gmail.com","subject":"Re: [PATCHv7.1 3/4] gitweb: File based caching layer (from git.kernel.org)","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2010-11-29T23:26:56Z","receivedAt":"2010-11-29T23:26:56Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 30 November 2010 00:07, demerphq wrote:\n> 2010/11/13 Jakub Narebski :\n>> -       binmode STDOUT, ':raw';\n>> -       print <$fd>;\n>> -       binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n>> +       if ($caching_enabled) {\n>> +               open BINOUT, '>', $fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n>> +       }else{\n>> +               open BINOUT, '>', \\$fullhashbinpath or die_error(500, \"Could not open bin dump file\");\n>> +       }\n>> +       binmode BINOUT, ':raw';\n>> +       print BINOUT <$fd>;\n>> +       binmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n>> +       close BINOUT;\n>\n> Why do you use dynamically scoped file handles here as opposed to\n> lexically scoped ones?\n>\n> And why do you change the output discipline on BINOUT immediately\n> before closing the file, and after you print data to it?\n>\n> Doing so sortof makes sense when the filedhandle is STDOUT, but not\n> when it is BINOUT.\n\nAlso in this code:\n\n2010/11/28 Jakub Narebski :\n> +#\n> +# Includes\n> +#\n> +if (!exists $INC{'cache.pl'}) {\n> + my $return = do 'cache.pl';\n> + die $@ if $@;\n> + die \"Couldn't read 'cache.pl': $!\" if (!defined $return);\n> +}\n\nWhy is that preferred to:\n\n require 'cache.pl';\n\nAnd why is this thing even a .pl file? Why isnt it called\nlib/GitWeb/Cache.pm or something like that?\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"156864","messageId":"201011300054.47312.jnareb@gmail.com","threadId":"25573","inReplyTo":"AANLkTim5V_50Er8OiNTWDHNzvqJ3J+biOwuqnBdnR4jH@mail.gmail.com","subject":"Re: [PATCHv7.1 3/4] gitweb: File based caching layer (from git.kernel.org)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-29T23:54:45Z","receivedAt":"2010-11-29T23:54:45Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 30 Nov 2010, demerphq wrote:\n> On 30 November 2010 00:07, demerphq wrote:\n> > 2010/11/13 Jakub Narebski :\n\n> \n> Also in this code:\n> \n> 2010/11/28 Jakub Narebski :\n> > +#\n> > +# Includes\n> > +#\n> > +if (!exists $INC{'cache.pl'}) {\n> > + my $return = do 'cache.pl';\n> > + die $@ if $@;\n> > + die \"Couldn't read 'cache.pl': $!\" if (!defined $return);\n> > +}\n> \n> Why is that preferred to:\n> \n> require 'cache.pl';\n> \n> And why is this thing even a .pl file? Why isnt it called\n> lib/GitWeb/Cache.pm or something like that?\n\nBecause it is not a Perl module; in particular 'cache.pl' uses global \nvariables from gitweb.perl (like $my_url, or $cachedir, or %action)\nand subroutines from gitweb.perl (like change_output() and \nreset_output()). That is why it needs to be injected via do, rather\nthan included in its owne namespace with package/require.\n\n\nP.S. This is not my code, this is patch by J.H. (John Hawley); I did\nonly *minimal* fixups.\n\nP.P.S. My rewrite can be found in 'gitweb/cache-kernel-pu' branch in\nmy repository (links are to web interface)\n http://repo.or.cz/w/git/jnareb-git.git\n https://github.com/jnareb/git\nSent in http://thread.gmane.org/gmane.comp.version-control.git/158313\n\n-- \nJakub Narebski\nPoland\n"},{"id":"156867","messageId":"7v4oazy6pw.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"201011300009.05437.jnareb@gmail.com","subject":"Re: [PATCHv7.1 3/4 (amend v2)] gitweb: File based caching layer (from git.kernel.org)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-30T00:51:07Z","receivedAt":"2010-11-30T00:51:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> On Mon, 29 Nov 2010, Junio C Hamano wrote:\n>\n>> Wait a bit.\n>> \n>> This seems to match what I have already queued on 'pu', no? Am I\n>> hallucinating?\n> \n> Damn, it looks like my mailer included wrong (older) version of a file.\n> I'm sorry. Below there is interdiff:\n\nThanks. What I had was what I pulled from you. I take it that you want\nme to rebase the two tip commits on the branch?\n"},{"id":"156895","messageId":"201011301121.03586.jnareb@gmail.com","threadId":"25573","inReplyTo":"7v4oazy6pw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv7.1 3/4 (amend v2)] gitweb: File based caching layer (from git.kernel.org)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-11-30T10:21:02Z","receivedAt":"2010-11-30T10:21:02Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 30 Nov 2010, Junio C Hamano wrote:\n> Jakub Narebski writes:\n>> On Mon, 29 Nov 2010, Junio C Hamano wrote:\n>>\n>>> Wait a bit.\n>>> \n>>> This seems to match what I have already queued on 'pu', no? Am I\n>>> hallucinating?\n>> \n>> Damn, it looks like my mailer included wrong (older) version of a file.\n>> I'm sorry. Below there is interdiff:\n> \n> Thanks. What I had was what I pulled from you. I take it that you want\n> me to rebase the two tip commits on the branch?\n\nYes, please.\n\nThough it still doesn't pass new tests in t9501 and t9502; the problem\nis more with setting up tests rather than with caching code, though.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"157099","messageId":"201012021117.16183.jnareb@gmail.com","threadId":"25573","inReplyTo":"201011130041.22918.jnareb@gmail.com","subject":"[PATCHv7.3 1/4 (bugfix)] gitweb: Prepare for splitting gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-12-02T10:17:14Z","receivedAt":"2010-12-02T10:17:14Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Prepare gitweb for having been split into modules that are to be\ninstalled alongside gitweb in 'lib/' subdirectory, by adding\n\n use lib __DIR__.'/lib';\n\nto gitweb.perl (to main gitweb script), and preparing for putting\nmodules (relative path) in $(GITWEB_MODULES) in gitweb/Makefile.\n\nThis preparatory work allows to add new module to gitweb by simply\nadding\n\n GITWEB_MODULES += \n\nto gitweb/Makefile (assuming that the module is in 'gitweb/lib/'\ndirectory).\n\nWhile at it pass GITWEBLIBDIR in addition to GITWEB_TEST_INSTALLED to\nallow testing installed version of gitweb and installed version of\nmodules (for future tests which would check individual (sub)modules).\n\n\nUsing __DIR__ from Dir::Self module (not in core, that's why currently\ngitweb includes excerpt of code from Dir::Self defining __DIR__) was\nchosen over using FindBin-based solution (in core since perl 5.00307,\nwhile gitweb itself requires at least perl 5.8.0) because FindBin uses\nBEGIN block, which is a problem under mod_perl and other persistent\nPerl environments (thought there are workarounds).\n\nAt Pavan Kumar Sankara suggestion gitweb/Makefile uses\n\n install [OPTION]... SOURCE... DIRECTORY\n\nformat (2nd format) with single SOURCE rather than\n\n install [OPTION]... SOURCE DEST\n\nformat (1st format) because of security reasons (race conditions).\nModern GNU install has `-T' / `--no-target-directory' option, but we\ncannot rely that the $(INSTALL) we are using supports this option.\n\nThe install-modules target in gitweb/Makefile uses shell 'for' loop,\ninstead of make's $(foreach) function, to avoid possible problem with\ngenerating a command line that exceeded the maximum argument list\nlength.\n\nSigned-off-by: Jakub Narebski \n---\nBUGFIX: shell variables should not be in single quotes\n\nI am very sorry about this bug...\n\n gitweb/Makefile | 18 +++++++++++++++---\n gitweb/gitweb.perl | 8 ++++++++\n 2 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 0a6ac00..ad5a282 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -57,6 +57,7 @@ PERL_PATH ?= /usr/bin/perl\n bindir_SQ = $(subst ','\\'',$(bindir))#'\n gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))#'\n gitwebstaticdir_SQ = $(subst ','\\'',$(gitwebdir)/static)#'\n+gitweblibdir_SQ = $(subst ','\\'',$(gitwebdir)/lib)#'\n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))#'\n PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))#'\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))#'\n@@ -153,20 +154,31 @@ test:\n \n test-installed:\n \tGITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \\\n+\tGITWEBLIBDIR='$(DESTDIR_SQ)$(gitweblibdir_SQ)' \\\n \t\t$(MAKE) -C ../t gitweb-test\n \n ### Installation rules\n \n-install: all\n+install: all install-modules\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \t$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \n+install-modules:\n+\tinstall_dirs=\"$(sort $(dir $(GITWEB_MODULES)))\" && \\\n+\tfor dir in $$install_dirs; do \\\n+\t\ttest -d '$(DESTDIR_SQ)$(gitweblibdir_SQ)'/\"$$dir\" || \\\n+\t\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)'/\"$$dir\"; \\\n+\tdone\n+\tgitweb_modules=\"$(GITWEB_MODULES)\" && \\\n+\tfor mod in $$gitweb_modules; do \\\n+\t\t$(INSTALL) -m 644 \"lib/$$mod\" '$(DESTDIR_SQ)$(gitweblibdir_SQ)'/\"$$(dirname $$mod)\"; \\\n+\tdone\n+\n ### Cleaning rules\n \n clean:\n \t$(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS\n \n-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE\n-\n+.PHONY: all clean install install-modules test test-installed .FORCE-GIT-VERSION-FILE FORCE\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 679f2da..cfa511c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -10,6 +10,14 @@\n use 5.008;\n use strict;\n use warnings;\n+\n+use File::Spec;\n+# __DIR__ is taken from Dir::Self __DIR__ fragment\n+sub __DIR__ () {\n+\tFile::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);\n+}\n+use lib __DIR__ . '/lib';\n+\n use CGI qw(:standard :escapeHTML -nosticky);\n use CGI::Util qw(unescape);\n use CGI::Carp qw(fatalsToBrowser set_message);\n-- \n1.7.3\n"},{"id":"157122","messageId":"7vtyiwrs8e.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"201012021117.16183.jnareb@gmail.com","subject":"Re: [PATCHv7.3 1/4 (bugfix)] gitweb: Prepare for splitting gitweb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-02T17:37:21Z","receivedAt":"2010-12-02T17:37:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hmm, how did you find the issue, and more importantly, how did I or other\npeople who saw this patch so easily fail to notice it?\n\nFWIW, I do run \"make install\" from the toplevel as part of my pre-push\ntest, so I _should_ have noticed it.\n\nAh, I don't run the install step for a revision that does not pass its\nselftest, so I haven't run \"make install\" on 'pu' for some time. That may\nexplain it.\n\nAnyway thanks for a fixup.\n"},{"id":"157141","messageId":"201012022001.31739.jnareb@gmail.com","threadId":"25573","inReplyTo":"7vtyiwrs8e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv7.3 1/4 (bugfix)] gitweb: Prepare for splitting gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-12-02T19:01:31Z","receivedAt":"2010-12-02T19:01:31Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 2 Dec 2010, Junio C Hamano wrote:\n\n> Hmm, how did you find the issue, and more importantly, how did I or other\n> people who saw this patch so easily fail to notice it?\n\nWhen working on my total rewrite of J.H. gitweb caching series, available\nas 'gitweb/cache-kernel-pu' branch in git://repo.or.cz/git/jnareb-git.git\nand git://github.com/jnareb/git.git repositories I finally did a *clean*\nreinstal (i.e. remove whole directory, then run \"make install-gitweb\").\nThis branch uses the same \"gitweb: Prepare for splitting gitweb\" patch\n(cherry-picked back and forth).\n \n> FWIW, I do run \"make install\" from the toplevel as part of my pre-push\n> test, so I _should_ have noticed it.\n> \n> Ah, I don't run the install step for a revision that does not pass its\n> selftest, so I haven't run \"make install\" on 'pu' for some time. That may\n> explain it.\n\nHmmm... I thought that \"make install\" doesn't install gitweb, but it does\nwith \"$(MAKE) -C gitweb install\"... though I am not sure if \"make all\"\nbuilds gitweb (runs \"make gitweb\").\n \n> Anyway thanks for a fixup.\n> \n\nSorry for the bug.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"157146","messageId":"7vmxooq8uf.fsf@alter.siamese.dyndns.org","threadId":"25573","inReplyTo":"201012022001.31739.jnareb@gmail.com","subject":"Re: [PATCHv7.3 1/4 (bugfix)] gitweb: Prepare for splitting gitweb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-02T19:21:28Z","receivedAt":"2010-12-02T19:21:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n>> Ah, I don't run the install step for a revision that does not pass its\n>> selftest, so I haven't run \"make install\" on 'pu' for some time. That may\n>> explain it.\n>\n> Hmmm... I thought that \"make install\" doesn't install gitweb, but it does\n> with \"$(MAKE) -C gitweb install\"... though I am not sure if \"make all\"\n> builds gitweb (runs \"make gitweb\").\n\nI think it does, and it should if it doesn't.\n"},{"id":"157150","messageId":"201012022036.46664.jnareb@gmail.com","threadId":"25573","inReplyTo":"7vmxooq8uf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv7.3 1/4 (bugfix)] gitweb: Prepare for splitting gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-12-02T19:36:46Z","receivedAt":"2010-12-02T19:36:46Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n> > > Ah, I don't run the install step for a revision that does not pass its\n> > > selftest, so I haven't run \"make install\" on 'pu' for some time. That may\n> > > explain it.\n> >\n> > Hmmm... I thought that \"make install\" doesn't install gitweb, but it does\n> > with \"$(MAKE) -C gitweb install\"... though I am not sure if \"make all\"\n> > builds gitweb (runs \"make gitweb\").\n> \n> I think it does, and it should if it doesn't.\n\nAnyway \"install\" target in gitweb/Makefile runs \"all\" target in\ngitweb/Makefile, so \n \"make install\" -> \"make -C gitweb install\" -> \"make -C gitweb all\"\n -> gitweb/gitweb.cgi is generated\n\ngitweb/gitweb.cgi is in $(OTHER_PROGRAMS), and we have\n\n all:: [...] $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n\n\nHmmm... shouldn't it be 'gitweb', not 'gitweb/gitweb.cgi'? Just wondering\n(we have gitweb/gitweb.cgi target in main Makefile, which just proxies\nto gitweb/Makefile).\n\n-- \nJakub Narebski\nPoland\n"}]}