{"thread":{"id":"22085","subject":"[RFC/PATCHv2 2/4 (resent)] gitweb: Add option to force version match","startedAt":"2010-01-03T16:07:26Z","lastAt":"2010-01-12T13:05:22Z","messageCount":10,"participants":["Jakub Narebski","J.H."],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"130731","messageId":"1262534850-24572-1-git-send-email-jnareb@gmail.com","threadId":"22085","inReplyTo":null,"subject":"[PATCHv3 0/4 (resent)] Miscelanous gitweb improvements from J.H.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-01-03T16:07:26Z","receivedAt":"2010-01-03T16:07:26Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This is resend of early part of \"[PATCH 0/6] Gitweb caching changes v2\"\nthread by John 'Warthog9' Hawley (J.H.),\n Message-ID: <1260488743-25855-1-git-send-email-warthog9@kernel.org>\n http://thread.gmane.org/gmane.comp.version-control.git/135052\n\nor alternatively\n git://git.kernel.org/pub/scm/git/warthog9/gitweb.git gitweb-ml-v2\n\nwith a few modifications of my own. Those patches were send originally as\nresponses in the mentioned thread, for further comments from original\nauthor. As the discussion didn't pick up (because of kernel.org upgrade, and\nperhaps due to end-of-year stuff), I am resending those patches in a\nseparate thread for a better visibility; only comments are changed.\n\nThis series of patches is rebased on top of commit 37bae10\n(Merge branch 'maint', 2009-12-31) in 'master' branch.\n\n\nChange that apply to all patches in series: \n* moving from \"GITWEB - \" to \"gitweb: \" as subsystem prefix\n* changing author to John 'Warthog9' Hawley \n (it was John 'Warthog9' Hawley )\n* add signoff or change it to John 'Warthog9' Hawley ,\n and of course add my own signoff.\n\n\nI have included reply to neither \"GITWEB - File based caching layer\"\nnor \"GITWEB - Separate defaults from main file\" in this thread/series.\n\nI haven't included the main point of the whole series, namely adding\nresponse caching layer in the form that is used in git.kernel.org, because\nI think this patch should be split into smaller parts, and unit-tested.\nAs it is now it is a bit of mess. I have done patch which makes gitweb\nalways use explicit filehandle when printing (simplifying a bit it\nreplaces 'print ' by 'print {$out} ', with $out set\nto \\*STDOUT), as a patch that prepares for (optional) gitweb caching, while\nnot affecting throughput, latency and memory consumption when caching is\ndisabled, as opposed to original solution by J.H. of always storing whole\nresponse in scalar and writing it at the end. \n\nI haven't included splitting of gitweb_defaults.perl off gitweb.perl, as it\nwas after large and invasive gitweb caching patch, it would require\nsubstantial changes to gitweb tests upfront (by testing built gitweb.cgi and\nnot source gitweb.perl), and needs fixing of Makefile to actually work\nreliably (we could have to process both gitweb.perl and\ngitweb_defaults.perl, while provided Makefile process only the file which\ntriggered the rule... I think).\n\n\nI am not sure if 'gitweb: Add option to force version match' is a good\nsolution to the problem it tires to address, i.e. if it is worth having, \nand I am not sure if I did 'gitweb: Makefile improvements' correctly.\n\nJohn 'Warthog9' Hawley (4):\n gitweb: Load checking\n gitweb: Add option to force version match\n gitweb: Optionally add \"git\" links in project list page\n gitweb: Makefile improvements\n\n Makefile | 65 +++++---------------------\n gitweb/Makefile | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n gitweb/README | 14 +++++-\n gitweb/gitweb.perl | 80 ++++++++++++++++++++++++++++++--\n 4 files changed, 230 insertions(+), 58 deletions(-)\n create mode 100644 gitweb/Makefile\n\nJohn, any comments?\n\n-- \nJakub Narebski\nShadeHawk or jnareb on #git\nPoland\n"},{"id":"130733","messageId":"1262534850-24572-2-git-send-email-jnareb@gmail.com","threadId":"22085","inReplyTo":"1262534850-24572-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv2 1/4 (resent)] gitweb: Load checking","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-01-03T16:07:27Z","receivedAt":"2010-01-03T16:07:27Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis changes slightly the behavior of gitweb, so that it verifies\nthat the box isn't inundated with before attempting to serve gitweb.\nIf the box is overloaded, it basically returns a 503 Server Unavailable\nuntil the load falls below the defined threshold. This helps dramatically\nif you have a box that's I/O bound, reaches a certain load and you\ndon't want gitweb, the I/O hog that it is, increasing the pain the\nserver is already undergoing.\n\nThis behavior is controlled by $maxload configuration variable.\nDefault is a load of 300, which for most cases should never be hit.\nUnset it (set it to undefined value, i.e. undef) to turn off checking.\n\nCurrently it requires that '/proc/loadavg' file exists, otherwise the\nload check is bypassed (load is taken to be 0). So platforms that do\nnot implement '/proc/loadavg' currently cannot use this feature.\n\nSigned-off-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nOriginally appeared in\n http://permalink.gmane.org/gmane.comp.version-control.git/135418\n\nDifferences to original version by John 'Warthog9' Hawley (J.H.):\n* Slightly improved wording in commit message and in comments\n* $maxload is described in \"Gitweb config file variables section \n in gitweb/README \n* You can use '$maxload = undef;' to turn off load checking\n* Error page for too high load is generated using die_error, which had\n to be extended to handle 503 Service Unavailable HTTP error code\n\nOne complaints that didn't get addressed was describing alternate\napproaches to just reading '/proc/loadavg', like using BSD::getloadavg\nmodule from CPAN, either in comments in gitweb.perl or in commit\nmessage. But I have changed my mind since then about this issue,\nand now I think it is not strictly necessary. I think that\nlimitations of current approach are described in sufficient detail,\nand we have fall back for when used method cannot work.\n\n gitweb/README | 7 ++++++-\n gitweb/gitweb.perl | 39 +++++++++++++++++++++++++++++++++++----\n 2 files changed, 41 insertions(+), 5 deletions(-)\n\ndiff --git a/gitweb/README b/gitweb/README\nindex e34ee79..6c2c8e1 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -174,7 +174,7 @@ not include variables usually directly set during build):\n Base URL for relative URLs in pages generated by gitweb,\n (e.g. $logo, $favicon, @stylesheets if they are relative URLs),\n needed and used only for URLs with nonempty PATH_INFO via\n- . Usually gitweb sets its value correctly,\n+ . Usually gitweb sets its value correctly,\n and there is no need to set this variable, e.g. to $my_uri or \"/\".\n * $home_link\n Target of the home link on top of all pages (the first part of view\n@@ -228,6 +228,11 @@ not include variables usually directly set during build):\n repositories from launching cross-site scripting (XSS) attacks. Set this\n to true if you don't trust the content of your repositories. The default\n is false.\n+ * $maxload\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 Service Unavailable\" error.\n+ Server load is taken to be 0 if gitweb cannot determine its value. Set it to\n+ undefined value to turn it off. The default is 300.\n \n \n Projects list file format\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7e477af..3222131 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -221,6 +221,12 @@ our %avatar_size = (\n \t'double' => 32\n );\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+# Leave it undefined (or set to 'undef') to turn off load checking.\n+our $maxload = 300;\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -551,6 +557,26 @@ if (-e $GITWEB_CONFIG) {\n \tdo $GITWEB_CONFIG_SYSTEM if -e $GITWEB_CONFIG_SYSTEM;\n }\n \n+# Get loadavg of system, to compare against $maxload.\n+# Currently it requires '/proc/loadavg' present to get loadavg;\n+# if it is not present it returns 0, which means no load checking.\n+sub get_loadavg {\n+\topen my $fd, '<', '/proc/loadavg'\n+\t\tor return 0;\n+\tmy @load = split(/\\s+/, scalar <$fd>);\n+\tclose $fd;\n+\n+\t# The first three columns measure CPU and IO utilization of the last one,\n+\t# five, and 10 minute periods. The fourth column shows the number of\n+\t# currently running processes and the total number of processes in the m/n\n+\t# format. The last column displays the last process ID used.\n+\treturn $load[0] || 0;\n+}\n+\n+if (defined $maxload && get_loadavg() > $maxload) {\n+\tdie_error(503, \"The load average on the server is too high\");\n+}\n+\n # version of the core git binary\n our $git_version = qx(\"$GIT\" --version) =~ m/git version (.*)$/ ? $1 : \"unknown\";\n $number_of_git_cmds++;\n@@ -3354,14 +3380,19 @@ sub git_footer_html {\n # 500: The server isn't configured properly, or\n # an internal error occurred (e.g. failed assertions caused by bugs), or\n # an unknown error occurred (e.g. the git binary died unexpectedly).\n+# 503: The server is currently unavailable (because it is overloaded,\n+# or down for maintenance). Generally, this is a temporary state.\n sub die_error {\n \tmy $status = shift || 500;\n \tmy $error = shift || \"Internal server error\";\n \n-\tmy %http_responses = (400 => '400 Bad Request',\n-\t\t\t 403 => '403 Forbidden',\n-\t\t\t 404 => '404 Not Found',\n-\t\t\t 500 => '500 Internal Server Error');\n+\tmy %http_responses = (\n+\t\t400 => '400 Bad Request',\n+\t\t403 => '403 Forbidden',\n+\t\t404 => '404 Not Found',\n+\t\t500 => '500 Internal Server Error',\n+\t\t503 => '503 Service Unavailable',\n+\t);\n \tgit_header_html($http_responses{$status});\n \tprint <\n-- \n1.6.5.3\n"},{"id":"130729","messageId":"1262534850-24572-3-git-send-email-jnareb@gmail.com","threadId":"22085","inReplyTo":"1262534850-24572-1-git-send-email-jnareb@gmail.com","subject":"[RFC/PATCHv2 2/4 (resent)] gitweb: Add option to force version match","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-01-03T16:07:28Z","receivedAt":"2010-01-03T16:07:28Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis adds $git_versions_must_match variable, which is set to true\nvalue checks 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 off.\n\nSigned-off-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nI don't quite see the reason behind such option, and I think that\nerror (instead of for example warning) on version mismatch is too much.\n\nIn short: this change is meant to avoid situation where version\nmismatch results in silent error, but it would work only if it would\nbe default on; on the other hand having it default on is I think too\ninconvenient, so I changed it to default off, which perhaps defeats\nthe stated purpose of this patch.\n\nThis is an RFC because formatting of error page is a bit rough, and\n(ab)uses exist CSS classes instead of creating new classnames for\nsemantic markup.\n\nDifferences from original version, by J.H.:\n* Changed name and flipped meaning of config variable, from\n $missmatch_git to $git_versions_must_match\n* $git_versions_must_match is boolean flag - do not compare with an\n empty string.\n* Changed error message a bit, fixed style, added entry in README\n\n gitweb/README | 3 +++\n gitweb/gitweb.perl | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 36 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/README b/gitweb/README\nindex 6c2c8e1..608b0f8 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -233,6 +233,9 @@ not include variables usually directly set during build):\n If server load exceed this value then return \"503 Service Unavailable\" error.\n Server load is taken to be 0 if gitweb cannot determine its value. Set it to\n undefined value to turn it off. The default is 300.\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 false.\n \n \n Projects list file format\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3222131..b9bd865 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -221,6 +221,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 = 0;\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@@ -581,6 +584,36 @@ if (defined $maxload && get_loadavg() > $maxload) {\n our $git_version = qx(\"$GIT\" --version) =~ m/git version (.*)$/ ? $1 : \"unknown\";\n $number_of_git_cmds++;\n \n+# Throw an error if git versions does not match, if $git_versions_must_match is true.\n+if ($git_versions_must_match &&\n+ $git_version ne $version) {\n+\tgit_header_html('500 - Internal Server Error');\n+\tmy $admin_contact =\n+\t\tdefined $ENV{'SERVER_ADMIN'} ? \", $ENV{'SERVER_ADMIN'},\" : '';\n+\tprint <<\"EOT\";\n+
\n+

\n+500 - Internal Server Error\n+
\n+
\n+
\n+
\n+

*** 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+
\n+EOT\n+\tgit_footer_html();\n+\texit;\n+}\n+\n $projects_list ||= $projectroot;\n \n # ======================================================================\n-- \n1.6.5.3\n"},{"id":"130732","messageId":"1262534850-24572-4-git-send-email-jnareb@gmail.com","threadId":"22085","inReplyTo":"1262534850-24572-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv3 3/4 (resent)] gitweb: Optionally add \"git\" links in project list page","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-01-03T16:07:29Z","receivedAt":"2010-01-03T16:07:29Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis adds a \"git\" link for each project in the project list page,\nshould a common $gitlinkurl_base be defined and not empty. The full\nURL of each link is composed of $gitlinkurl_base and project name.\nIt is intended for git:// links, and in fact GITWEB_BASE_URL build\nvariable is used as its default value only if it starts with git://\n\nThis does make the assumption that the git repositories share a common\npath. Nothing to date is known to actually make use of introduced\nlink.\n\nCreated \"git\" link follows rel=vcs-* microformat specification:\n http://kitenet.net/~joey/rfc/rel-vcs/\n\nSigned-off-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nI think it might be good idea... but for the fact \"Nothing to date is\nknown to actually make use of introduced link\". What's its intended\nuse?\n\nDifferences to original version by John 'Warthog9' Hawley (J.H.):\n* It doesn't cause syntax error ;-)\n* Escaping of attribute value is left to CGI.pm (avoid double escaping)\n* $gitlinkurl got renamed to $gitlinkurl_base, now includes git://\n prefix, and defaults to GITWEB_BASE_URL if it begins with git://\n* Added description of $gitlinkurl_base to gitweb/README\n* Uses rel=vcs-* microformat by Joey Hess\n\nI assume that nobody sane would define $gitlinkurl_base to \"0\";\nthe code assumes that is enough to check that $gitlinkurl_base\nis true-ish.\n\n gitweb/README | 4 ++++\n gitweb/gitweb.perl | 8 ++++++++\n 2 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/README b/gitweb/README\nindex 608b0f8..36fb059 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -71,6 +71,7 @@ You can specify the following configuration variables when building GIT:\n * GITWEB_BASE_URL\n Git base URLs used for URL to where fetch project from, i.e. full\n URL is \"$git_base_url/$project\". Shown on projects summary page.\n+ If it begins with \"git://\" it is also used for $gitlinkurl_base, see below.\n Repository URL for project can be also configured per repository; this\n takes precedence over URLs composed from base URL and a project name.\n Note that you can setup multiple base URLs (for example one for\n@@ -204,6 +205,9 @@ not include variables usually directly set during build):\n access, and one for http:// \"dumb\" protocol access). Note that per\n repository configuration in 'cloneurl' file, or as values of gitweb.url\n project config.\n+ * $gitlinkurl_base\n+ Git base URL used (if it is defined and not empty) for \"git\" link in\n+ projects list, for each project. Full URL is \"$gitlinkurl_base/$project\".\n * $default_blob_plain_mimetype\n Default mimetype for blob_plain (raw) view, if mimetype checking\n doesn't result in some other type; by default 'text/plain'.\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex b9bd865..efb6471 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -224,6 +224,10 @@ our %avatar_size = (\n # If it is true, exit if gitweb version and git binary version don't match\n our $git_versions_must_match = 0;\n \n+# If this variable is set and not empty, add an extra link called \"git\"\n+# for each project in project list. Full URL is \"$gitlinkurl_base/$project\".\n+our $gitlinkurl_base = (\"++GITWEB_BASE_URL++\" =~ m!^(git://.*)$!) ? $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@@ -4472,6 +4476,10 @@ sub git_project_list_body {\n \t\t $cgi->a({-href => href(project=>$pr->{'path'}, action=>\"log\")}, \"log\") . \" | \" .\n \t\t $cgi->a({-href => href(project=>$pr->{'path'}, action=>\"tree\")}, \"tree\") .\n \t\t ($pr->{'forks'} ? \" | \" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>\"forks\")}, \"forks\") : '') .\n+\t\t ($gitlinkurl_base ?\n+\t\t \" | \" . $cgi->a({-href=>\"$gitlinkurl_base/$pr->{'path'}\",\n+\t\t -rel=>\"vcs-git\"}, \"git\")\n+\t\t : '') .\n \t\t \"\\n\" .\n \t\t \"\\n\";\n \t}\n-- \n1.6.5.3\n"},{"id":"130730","messageId":"1262534850-24572-5-git-send-email-jnareb@gmail.com","threadId":"22085","inReplyTo":"1262534850-24572-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv2/RFC 4/4 (resent)] gitweb: Makefile improvements","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-01-03T16:07:30Z","receivedAt":"2010-01-03T16:07:30Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: John 'Warthog9' Hawley \n\nThis commit adjust the main Makefile so you can simply run\n\n make gitweb\n\nwhich in turn calls gitweb/Makefile. This means that in order to\ngenerate gitweb, you can simply run 'make' from gitweb subdirectory:\n\n cd gitweb\n make\n\nTargets gitweb/gitweb.cgi and (dependent on JSMIN being defined)\ngitweb/gitweb.min.js in main Makefile are preserved for backward\ncompatibility.\n\nSigned-off-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nThis implements separate Makefile for gitweb, with main Makefile calling\nit, like for gitk-git/, git-gui/, Documentation/, t/, and templates/\ndirectories. The original patch by J.H. did it the opposite way, with\ngitweb/Makefile calling main Makefile.\n\nIt is marked as RFC because I don't feel that my make-fu is strong enough\nto be sure that there are no errors / mistakes. Very slightly tested.\n\n Makefile | 65 +++++----------------------\n gitweb/Makefile | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 141 insertions(+), 53 deletions(-)\n create mode 100644 gitweb/Makefile\n\ndiff --git a/Makefile b/Makefile\nindex c11719c..9c6d2b2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -282,29 +282,6 @@ pathsep = :\n # JavaScript minifier invocation that can function as filter\n JSMIN =\n \n-# default configuration for gitweb\n-GITWEB_CONFIG = gitweb_config.perl\n-GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf\n-GITWEB_HOME_LINK_STR = projects\n-GITWEB_SITENAME =\n-GITWEB_PROJECTROOT = /pub/git\n-GITWEB_PROJECT_MAXDEPTH = 2007\n-GITWEB_EXPORT_OK =\n-GITWEB_STRICT_EXPORT =\n-GITWEB_BASE_URL =\n-GITWEB_LIST =\n-GITWEB_HOMETEXT = indextext.html\n-GITWEB_CSS = gitweb.css\n-GITWEB_LOGO = git-logo.png\n-GITWEB_FAVICON = git-favicon.png\n-ifdef JSMIN\n-GITWEB_JS = gitweb.min.js\n-else\n-GITWEB_JS = gitweb.js\n-endif\n-GITWEB_SITE_HEADER =\n-GITWEB_SITE_FOOTER =\n-\n export prefix bindir sharedir sysconfdir\n \n CC = gcc\n@@ -1526,6 +1503,11 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n+\n+.PHONY: gitweb\n+gitweb:\n+\t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n+\n ifdef JSMIN\n OTHER_PROGRAMS += gitweb/gitweb.cgi gitweb/gitweb.min.js\n gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js\n@@ -1533,30 +1515,13 @@ else\n OTHER_PROGRAMS += gitweb/gitweb.cgi\n gitweb/gitweb.cgi: gitweb/gitweb.perl\n endif\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n-\t -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n-\t -e 's|++GIT_BINDIR++|$(bindir)|g' \\\n-\t -e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \\\n-\t -e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \\\n-\t -e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \\\n-\t -e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \\\n-\t -e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \\\n-\t -e 's|\"++GITWEB_PROJECT_MAXDEPTH++\"|$(GITWEB_PROJECT_MAXDEPTH)|g' \\\n-\t -e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \\\n-\t -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \\\n-\t -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \\\n-\t -e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \\\n-\t -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \\\n-\t -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \\\n-\t -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \\\n-\t -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \\\n-\t -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \\\n-\t -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \\\n-\t -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \\\n-\t $< >$@+ && \\\n-\tchmod +x $@+ && \\\n-\tmv $@+ $@\n+\t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)\n+\n+ifdef JSMIN\n+gitweb/gitweb.min.js: gitweb/gitweb.js\n+\t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)\n+endif # JSMIN\n+\n \n git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css gitweb/gitweb.js\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n@@ -1583,12 +1548,6 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n \tmv $@+ $@\n endif # NO_PERL\n \n-\n-ifdef JSMIN\n-gitweb/gitweb.min.js: gitweb/gitweb.js\n-\t$(QUIET_GEN)$(JSMIN) <$< >$@\n-endif # JSMIN\n-\n ifndef NO_PYTHON\n $(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS\n $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nnew file mode 100644\nindex 0000000..c9eb1ee\n--- /dev/null\n+++ b/gitweb/Makefile\n@@ -0,0 +1,129 @@\n+# The default target of this Makefile is...\n+all::\n+\n+# Define V=1 to have a more verbose compile.\n+#\n+# Define JSMIN to point to JavaScript minifier that functions as\n+# a filter to have gitweb.js minified.\n+#\n+\n+prefix ?= $(HOME)\n+bindir ?= $(prefix)/bin\n+RM ?= rm -f\n+\n+# JavaScript minifier invocation that can function as filter\n+JSMIN ?=\n+\n+# default configuration for gitweb\n+GITWEB_CONFIG = gitweb_config.perl\n+GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf\n+GITWEB_HOME_LINK_STR = projects\n+GITWEB_SITENAME =\n+GITWEB_PROJECTROOT = /pub/git\n+GITWEB_PROJECT_MAXDEPTH = 2007\n+GITWEB_EXPORT_OK =\n+GITWEB_STRICT_EXPORT =\n+GITWEB_BASE_URL =\n+GITWEB_LIST =\n+GITWEB_HOMETEXT = indextext.html\n+GITWEB_CSS = gitweb.css\n+GITWEB_LOGO = git-logo.png\n+GITWEB_FAVICON = git-favicon.png\n+ifdef JSMIN\n+GITWEB_JS = gitweb.min.js\n+else\n+GITWEB_JS = gitweb.js\n+endif\n+GITWEB_SITE_HEADER =\n+GITWEB_SITE_FOOTER =\n+\n+# include user config\n+-include ../config.mak.autogen\n+-include ../config.mak\n+\n+# determine version\n+../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n+\t$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE\n+\n+-include ../GIT-VERSION-FILE\n+\n+### Build rules\n+\n+SHELL_PATH ?= $(SHELL)\n+PERL_PATH ?= /usr/bin/perl\n+\n+# Shell quote;\n+bindir_SQ = $(subst ','\\'',$(bindir)) #'\n+SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH)) #'\n+PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH)) #'\n+\n+# Quiet generation (unless V=1)\n+QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n+QUIET_SUBDIR1 =\n+\n+ifneq ($(findstring $(MAKEFLAGS),w),w)\n+PRINT_DIR = --no-print-directory\n+else # \"make -w\"\n+NO_SUBDIR = :\n+endif\n+\n+ifneq ($(findstring $(MAKEFLAGS),s),s)\n+ifndef V\n+\tQUIET = @\n+\tQUIET_GEN = $(QUIET)echo ' ' GEN $@;\n+\tQUIET_SUBDIR0 = +@subdir=\n+\tQUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \\\n+\t $(MAKE) $(PRINT_DIR) -C $$subdir\n+\texport V\n+\texport QUIET\n+\texport QUIET_GEN\n+\texport QUIET_SUBDIR0\n+\texport QUIET_SUBDIR1\n+endif\n+endif\n+\n+all:: gitweb.cgi\n+\n+ifdef JSMIN\n+FILES=gitweb.cgi gitweb.min.js\n+gitweb.cgi: gitweb.perl gitweb.min.js\n+else # !JSMIN\n+FILES=gitweb.cgi\n+gitweb.cgi: gitweb.perl\n+endif # JSMIN\n+\n+gitweb.cgi:\n+\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n+\tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n+\t -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n+\t -e 's|++GIT_BINDIR++|$(bindir)|g' \\\n+\t -e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \\\n+\t -e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \\\n+\t -e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \\\n+\t -e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \\\n+\t -e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \\\n+\t -e 's|\"++GITWEB_PROJECT_MAXDEPTH++\"|$(GITWEB_PROJECT_MAXDEPTH)|g' \\\n+\t -e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \\\n+\t -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \\\n+\t -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \\\n+\t -e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \\\n+\t -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \\\n+\t -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \\\n+\t -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \\\n+\t -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \\\n+\t -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \\\n+\t -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \\\n+\t -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \\\n+\t $< >$@+ && \\\n+\tchmod +x $@+ && \\\n+\tmv $@+ $@\n+\n+ifdef JSMIN\n+gitweb.min.js: gitweb.js\n+\t$(QUIET_GEN)$(JSMIN) <$< >$@\n+endif # JSMIN\n+\n+clean:\n+\t$(RM) $(FILES)\n+\n+.PHONY: all clean .FORCE-GIT-VERSION-FILE\n-- \n1.6.5.3\n"},{"id":"130959","messageId":"4B450EA2.5020104@kernel.org","threadId":"22085","inReplyTo":"1262534850-24572-1-git-send-email-jnareb@gmail.com","subject":"Re: [PATCHv3 0/4 (resent)] Miscelanous gitweb improvements from J.H.","fromName":"J.H.","fromEmail":"warthog9@kernel.org","sentAt":"2010-01-06T22:28:50Z","receivedAt":"2010-01-06T22:28:50Z","isPatch":false,"sender":{"key":"warthog9@kernel.org","avatar":"https://avatars.githubusercontent.com/u/2334704?v=4"},"body":"Just a heads up I've been on vacation for two weeks and hadn't gotten\naround to dealing with these e-mails. I'm going to get them imported\ninto my tree here and take a look over them.\n\n- John 'Warthog9' Hawley\n\nOn 01/03/2010 08:07 AM, Jakub Narebski wrote:\n> This is resend of early part of \"[PATCH 0/6] Gitweb caching changes v2\"\n> thread by John 'Warthog9' Hawley (J.H.),\n> Message-ID: <1260488743-25855-1-git-send-email-warthog9@kernel.org>\n> http://thread.gmane.org/gmane.comp.version-control.git/135052\n> \n> or alternatively\n> git://git.kernel.org/pub/scm/git/warthog9/gitweb.git gitweb-ml-v2\n> \n> with a few modifications of my own. Those patches were send originally as\n> responses in the mentioned thread, for further comments from original\n> author. As the discussion didn't pick up (because of kernel.org upgrade, and\n> perhaps due to end-of-year stuff), I am resending those patches in a\n> separate thread for a better visibility; only comments are changed.\n> \n> This series of patches is rebased on top of commit 37bae10\n> (Merge branch 'maint', 2009-12-31) in 'master' branch.\n> \n> \n> Change that apply to all patches in series: \n> * moving from \"GITWEB - \" to \"gitweb: \" as subsystem prefix\n> * changing author to John 'Warthog9' Hawley \n> (it was John 'Warthog9' Hawley )\n> * add signoff or change it to John 'Warthog9' Hawley ,\n> and of course add my own signoff.\n> \n> \n> I have included reply to neither \"GITWEB - File based caching layer\"\n> nor \"GITWEB - Separate defaults from main file\" in this thread/series.\n> \n> I haven't included the main point of the whole series, namely adding\n> response caching layer in the form that is used in git.kernel.org, because\n> I think this patch should be split into smaller parts, and unit-tested.\n> As it is now it is a bit of mess. I have done patch which makes gitweb\n> always use explicit filehandle when printing (simplifying a bit it\n> replaces 'print ' by 'print {$out} ', with $out set\n> to \\*STDOUT), as a patch that prepares for (optional) gitweb caching, while\n> not affecting throughput, latency and memory consumption when caching is\n> disabled, as opposed to original solution by J.H. of always storing whole\n> response in scalar and writing it at the end. \n> \n> I haven't included splitting of gitweb_defaults.perl off gitweb.perl, as it\n> was after large and invasive gitweb caching patch, it would require\n> substantial changes to gitweb tests upfront (by testing built gitweb.cgi and\n> not source gitweb.perl), and needs fixing of Makefile to actually work\n> reliably (we could have to process both gitweb.perl and\n> gitweb_defaults.perl, while provided Makefile process only the file which\n> triggered the rule... I think).\n> \n> \n> I am not sure if 'gitweb: Add option to force version match' is a good\n> solution to the problem it tires to address, i.e. if it is worth having, \n> and I am not sure if I did 'gitweb: Makefile improvements' correctly.\n> \n> John 'Warthog9' Hawley (4):\n> gitweb: Load checking\n> gitweb: Add option to force version match\n> gitweb: Optionally add \"git\" links in project list page\n> gitweb: Makefile improvements\n> \n> Makefile | 65 +++++---------------------\n> gitweb/Makefile | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n> gitweb/README | 14 +++++-\n> gitweb/gitweb.perl | 80 ++++++++++++++++++++++++++++++--\n> 4 files changed, 230 insertions(+), 58 deletions(-)\n> create mode 100644 gitweb/Makefile\n> \n> John, any comments?\n> \n"},{"id":"130962","messageId":"201001070022.31531.jnareb@gmail.com","threadId":"22085","inReplyTo":"4B450EA2.5020104@kernel.org","subject":"Re: [PATCHv3 0/4 (resent)] Miscelanous gitweb improvements from J.H.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-01-06T23:22:29Z","receivedAt":"2010-01-06T23:22:29Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 6 Jan 2010, J.H. wrote:\n\n> Just a heads up I've been on vacation for two weeks and hadn't gotten\n> around to dealing with these e-mails.  I'm going to get them imported\n> into my tree here and take a look over them.\n\nThey are also available from 'gitweb/cache-kernel' branch of my \ngit/jnareb-git.git repository at repo.or.cz... and are available from\n'pu' branch in main git repository, as fbd43a8^2 (Merge branch \n'jh/gitweb-cached' into pu, 2010-01-04).\n\nIt might be easier to import (fetch) them from that repository, and just \ndrop top two commits / merge jnareb-git/gitweb/cache-kernel~2\n\n\n git://repo.or.cz/git/jnareb-git.git gitweb/cache-kernel\n\nJohn 'Warthog9' Hawley (4):\n gitweb: Load checking\n gitweb: Add option to force version match\n gitweb: Optionally add \"git\" links in project list page\n gitweb: Makefile improvements\n\nJakub Narebski (2):\n gitweb: Print to explicit filehandle (preparing for caching)\n gitweb: href(..., -path_info => 0|1)\n\nThe two top commits on that branch are work in progress in preparation\nfor response caching for gitweb from J.H. (the one used on \ngit.kernel.org), and are not ready yet, I think; at least there would \nbe one more commit in \"preparing for gitweb caching\" (sub)series.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"131179","messageId":"201001091220.20446.jnareb@gmail.com","threadId":"22085","inReplyTo":"4B47E06C.9070503@eaglescrag.net","subject":"Re: [PATCHv3 3/4 (resent)] gitweb: Optionally add \"git\" links in project list page","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-01-09T11:20:19Z","receivedAt":"2010-01-09T11:20:19Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 9 Jan 2010, J.H. wrote:\n> On 01/03/2010 08:07 AM, Jakub Narebski wrote:\n> > From: John 'Warthog9' Hawley \n> > \n> > This adds a \"git\" link for each project in the project list page,\n> > should a common $gitlinkurl_base be defined and not empty. The full\n> > URL of each link is composed of $gitlinkurl_base and project name.\n> > It is intended for git:// links, and in fact GITWEB_BASE_URL build\n> > variable is used as its default value only if it starts with git://\n> > \n> > This does make the assumption that the git repositories share a common\n> > path. Nothing to date is known to actually make use of introduced\n> > link.\n> > \n> > Created \"git\" link follows rel=vcs-* microformat specification:\n> > http://kitenet.net/~joey/rfc/rel-vcs/\n> > \n> > Signed-off-by: John 'Warthog9' Hawley \n> > Signed-off-by: Jakub Narebski \n> > ---\n> > I think it might be good idea... but for the fact \"Nothing to date is\n> > known to actually make use of introduced link\". What's its intended\n> > use?\n> > \n> > Differences to original version by John 'Warthog9' Hawley (J.H.):\n> > * It doesn't cause syntax error ;-)\n> > * Escaping of attribute value is left to CGI.pm (avoid double escaping)\n> > * $gitlinkurl got renamed to $gitlinkurl_base, now includes git://\n> > prefix, and defaults to GITWEB_BASE_URL if it begins with git://\n> > * Added description of $gitlinkurl_base to gitweb/README\n> > * Uses rel=vcs-* microformat by Joey Hess\n\n> > gitweb/README | 4 ++++\n> > gitweb/gitweb.perl | 8 ++++++++\n> > 2 files changed, 12 insertions(+), 0 deletions(-)\n\nA reminder - this patch series consists of the following patches:\n [PATCHv2 1/4 (resent)] gitweb: Load checking\n [RFC/PATCHv2 2/4 (resent)] gitweb: Add option to force version match\n [PATCHv3 3/4 (resent)] gitweb: Optionally add \"git\" links in project list page\n [PATCHv2/RFC 4/4 (resent)] gitweb: Makefile improvements\n\n> Ok I've been debating this as I've been going through the patches, I've\n> got small modifications on top of your patches Jakub for 1 and 2,\n> haven't pushed them yet but they are relatively trivial. The changes to\n> the first patch sets things up for additional load checkers to be added\n> later on. \n\nGood idea, although I think that such addition can be left for a separate\npatch. \n\nBy the way, are you doing if-elsif fallback chain, trying different\nmechanisms (like '/proc/loadavg', BSD::getloadavg, etc.), or did\nyou made get_loadavg() into code reference, i.e. run it with \n$get_loadavg->(), which has the advantage that the gitweb admin can \noverride it in gitweb config file (including such thing like simply \nusing load average over last 5 minutes, and not over last minute)?\n\n> The second changes the error message to use/abuse die_error() \n> vs. doing it's own thing (though I still think this should be on by\n> default). \n\nTrue, the error message could use improvement (and not only using\nits own class instead of abusing 'readme' class, or renaming 'readme'\nclass to something more generic). The problem with error message for\nthis is who is the target of this message: is it gitweb administrator\n(who can change gitweb configuration), or is it gitweb user (who need\nto contact web admin).\n\nThe problem with this patch is that for it to be useful for protecting\nagainst silent errors it should be on by default, but OTOH having it on\nby default is quite inconvenient.\n\nBest solution would be to treat core of this issue, namely eliminate\nsilent errors and always provide some message in case of error.\n\n> Patch 4 I don't have anything to add or change at this point. \n>\n> This patch has me pondering and I'm unsure of what I'd suggest, mainly\n> because of the addition of the smart http support meaning that git://\n> and http:// are legitimate and useful links for supporting full git\n> transactions.\n> \n> I may withdraw the patch entirely since the link on kernel.org has been\n> around for years, and I'm unsure if anything actually uses it (though I\n> can see it being useful still). If it stays I think there's got to be a\n> way to specifically mark a url as being the one to link to vs.\n> defaulting to git:// (or allow for a marking to override the git://) and\n> I need to ponder that.\n\nAlso, it has to be _fast_, I think, i.e. no reading cloneurl and repo \nconfig (for gitweb.url) for each repository.\n\nYou can always remove the check for \"git://\" prefix, and/or take first\nbase in @git_base_url_list.\n\n> \n> I have given some initial thought to converting the $output options I'm\n> currently using to a print that Jakub is suggesting & exploring.\n\nIt's 'print {$fh}', i.e. use indirect filehandle, not global filehandle.\n\n> I think all told it's going to be a similarly sized patch, since all\n> output still has to get adjusted (including the things that directly\n> output but don't print). \n\nprint -> print {$fh} can be separate patch, and it can be checked that\nit produces the same results. Well print -> $output .= could also be\nseparate patch...\n\n> I'm unsure if there's a real advantage to \n> either way, other than design preference. My patch (forcing the output\n> to get passed around) moves towards more of a modal style design\n> separating data & layout vs. it's combined nature now, well it's a step\n> in that direction anyway.\n\nErrr... what? Forcing buffering (you need to read whole output into\nmemory, including for snapshots (uncompressed in case of .tar.gz))\nis IMVHO orthogonal to the issue of separating data & layout.\nBTW. Modern web server interfaces like Rack, PSGI/Plack etc. explicitly\ninclude streaming support.\n\nThe advantage of doing 'print {$fh}' is that $fh can be \\*STDOUT, can\nbe \\$buffer, but can be filehandle to (temporary) file on disk, and\nyou can even \"tee\" it, i.e. both write to memory/file, and to STDOUT.\nThe number of possible choices / possible improvements is much larger.\n\nAnd what is also important it means that people who do not use caching\ndo not suffer latency penalty and memory pressure from caching \ninfrastructure they do not use.\n\n\nP.S. \nSubject: [Virus] Exploit.PDF-9669\nX-Virus-Scanned: ClamAV 0.88.7/10275/Fri Jan 8 17:06:46 2010 on shards.monkeyblade.net\nX-Virus-Status: Infected with Exploit.PDF-9669\nX-Original-Subject: Re: [PATCHv3 3/4 (resent)] gitweb: Optionally add \"git\"\n links in project list page\n\nA message sent from to\n \n \n \ncontained Exploit.PDF-9669 and has not been delivered.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"131332","messageId":"4B4BC4CB.2030409@eaglescrag.net","threadId":"22085","inReplyTo":"201001091220.20446.jnareb@gmail.com","subject":"Re: [PATCHv3 3/4 (resent)] gitweb: Optionally add \"git\" links in project list page","fromName":"J.H.","fromEmail":"warthog9@eaglescrag.net","sentAt":"2010-01-12T00:39:39Z","receivedAt":"2010-01-12T00:39:39Z","isPatch":false,"sender":{"key":"warthog9@kernel.org","avatar":"https://avatars.githubusercontent.com/u/2334704?v=4"},"body":"On 01/09/2010 03:20 AM, Jakub Narebski wrote:\n> On Sat, 9 Jan 2010, J.H. wrote:\n>> On 01/03/2010 08:07 AM, Jakub Narebski wrote:\n>>> From: John 'Warthog9' Hawley \n>>>\n>>> This adds a \"git\" link for each project in the project list page,\n>>> should a common $gitlinkurl_base be defined and not empty. The full\n>>> URL of each link is composed of $gitlinkurl_base and project name.\n>>> It is intended for git:// links, and in fact GITWEB_BASE_URL build\n>>> variable is used as its default value only if it starts with git://\n>>>\n>>> This does make the assumption that the git repositories share a common\n>>> path. Nothing to date is known to actually make use of introduced\n>>> link.\n>>>\n>>> Created \"git\" link follows rel=vcs-* microformat specification:\n>>> http://kitenet.net/~joey/rfc/rel-vcs/\n>>>\n>>> Signed-off-by: John 'Warthog9' Hawley \n>>> Signed-off-by: Jakub Narebski \n>>> ---\n>>> I think it might be good idea... but for the fact \"Nothing to date is\n>>> known to actually make use of introduced link\". What's its intended\n>>> use?\n>>>\n>>> Differences to original version by John 'Warthog9' Hawley (J.H.):\n>>> * It doesn't cause syntax error ;-)\n>>> * Escaping of attribute value is left to CGI.pm (avoid double escaping)\n>>> * $gitlinkurl got renamed to $gitlinkurl_base, now includes git://\n>>> prefix, and defaults to GITWEB_BASE_URL if it begins with git://\n>>> * Added description of $gitlinkurl_base to gitweb/README\n>>> * Uses rel=vcs-* microformat by Joey Hess\n> \n>>> gitweb/README | 4 ++++\n>>> gitweb/gitweb.perl | 8 ++++++++\n>>> 2 files changed, 12 insertions(+), 0 deletions(-)\n> \n> A reminder - this patch series consists of the following patches:\n> [PATCHv2 1/4 (resent)] gitweb: Load checking\n> [RFC/PATCHv2 2/4 (resent)] gitweb: Add option to force version match\n> [PATCHv3 3/4 (resent)] gitweb: Optionally add \"git\" links in project list page\n> [PATCHv2/RFC 4/4 (resent)] gitweb: Makefile improvements\n> \n>> Ok I've been debating this as I've been going through the patches, I've\n>> got small modifications on top of your patches Jakub for 1 and 2,\n>> haven't pushed them yet but they are relatively trivial. The changes to\n>> the first patch sets things up for additional load checkers to be added\n>> later on. \n> \n> Good idea, although I think that such addition can be left for a separate\n> patch. \n\nIt only set things up for additional checkers, adding a framework for\nthem to be added. I would agree that other load checkers should be in\nadditional patches, just trying to make it easy for other later on.\n\n> By the way, are you doing if-elsif fallback chain, trying different\n> mechanisms (like '/proc/loadavg', BSD::getloadavg, etc.), or did\n> you made get_loadavg() into code reference, i.e. run it with \n> $get_loadavg->(), which has the advantage that the gitweb admin can \n> override it in gitweb config file (including such thing like simply \n> using load average over last 5 minutes, and not over last minute)?\n\nI had set it up using the if-elsif fallback chain. Not sure that an\nadmin would be overriding it for the most part, but it probably wouldn't\nbe hard to do both. Decent defaults for the general case, and if anyone\nwants they can override it afterwards for special cases.\n\n> \n>> The second changes the error message to use/abuse die_error() \n>> vs. doing it's own thing (though I still think this should be on by\n>> default). \n> \n> True, the error message could use improvement (and not only using\n> its own class instead of abusing 'readme' class, or renaming 'readme'\n> class to something more generic). The problem with error message for\n> this is who is the target of this message: is it gitweb administrator\n> (who can change gitweb configuration), or is it gitweb user (who need\n> to contact web admin).\n\nAdmin for sure, this is intended more to be a speed bump that forces the\nadmin to stop and consider what's happening. I originally added it\nbecause I ended up in a situation where gitweb didn't match the\ninstalled git and gitweb was dying silently. I wanted to just give the\nadmin a better guess at what was going on mainly.\n\n> The problem with this patch is that for it to be useful for protecting\n> against silent errors it should be on by default, but OTOH having it on\n> by default is quite inconvenient.\n\nUpside is once it's been disabled it should stay disabled. I'll admit\nit's not ideal but it's at least a stop gap in the interim till the\nother silent errors get filled in.\n\n> Best solution would be to treat core of this issue, namely eliminate\n> silent errors and always provide some message in case of error.\n\nAgreed, this is really just a stop-gap measure. I would imagine *most*\ninstalations of gitweb are installed from the distro's package\nmanagement so that would likely be upgraded at the same time as git itself.\n\n>> Patch 4 I don't have anything to add or change at this point. \n>>\n>> This patch has me pondering and I'm unsure of what I'd suggest, mainly\n>> because of the addition of the smart http support meaning that git://\n>> and http:// are legitimate and useful links for supporting full git\n>> transactions.\n>>\n>> I may withdraw the patch entirely since the link on kernel.org has been\n>> around for years, and I'm unsure if anything actually uses it (though I\n>> can see it being useful still). If it stays I think there's got to be a\n>> way to specifically mark a url as being the one to link to vs.\n>> defaulting to git:// (or allow for a marking to override the git://) and\n>> I need to ponder that.\n> \n> Also, it has to be _fast_, I think, i.e. no reading cloneurl and repo \n> config (for gitweb.url) for each repository.\n> \n> You can always remove the check for \"git://\" prefix, and/or take first\n> base in @git_base_url_list.\n\nTaking the first one by default may make more sense - I'll try that.\n\n>> I have given some initial thought to converting the $output options I'm\n>> currently using to a print that Jakub is suggesting & exploring.\n> \n> It's 'print {$fh}', i.e. use indirect filehandle, not global filehandle.\n> \n>> I think all told it's going to be a similarly sized patch, since all\n>> output still has to get adjusted (including the things that directly\n>> output but don't print). \n> \n> print -> print {$fh} can be separate patch, and it can be checked that\n> it produces the same results. Well print -> $output .= could also be\n> separate patch...\n> \n>> I'm unsure if there's a real advantage to \n>> either way, other than design preference. My patch (forcing the output\n>> to get passed around) moves towards more of a modal style design\n>> separating data & layout vs. it's combined nature now, well it's a step\n>> in that direction anyway.\n> \n> Errr... what? Forcing buffering (you need to read whole output into\n> memory, including for snapshots (uncompressed in case of .tar.gz))\n> is IMVHO orthogonal to the issue of separating data & layout.\n> BTW. Modern web server interfaces like Rack, PSGI/Plack etc. explicitly\n> include streaming support.\n\nThe inbuilt streaming support does change things, and I don't think it\nultimately changes my caching engine really anyway - I should have that\nchange done shortly.\n\n> \n> The advantage of doing 'print {$fh}' is that $fh can be \\*STDOUT, can\n> be \\$buffer, but can be filehandle to (temporary) file on disk, and\n> you can even \"tee\" it, i.e. both write to memory/file, and to STDOUT.\n> The number of possible choices / possible improvements is much larger.\n> \n> And what is also important it means that people who do not use caching\n> do not suffer latency penalty and memory pressure from caching \n> infrastructure they do not use.\n> \n> \n> P.S. \n> Subject: [Virus] Exploit.PDF-9669\n> X-Virus-Scanned: ClamAV 0.88.7/10275/Fri Jan 8 17:06:46 2010 on shards.monkeyblade.net\n> X-Virus-Status: Infected with Exploit.PDF-9669\n> X-Original-Subject: Re: [PATCHv3 3/4 (resent)] gitweb: Optionally add \"git\"\n> links in project list page\n> \n> A message sent from to\n> \n> \n> \n> contained Exploit.PDF-9669 and has not been delivered.\n\nClamav on my personal server wigging out and deciding that all kinds of\nstuff was PDF viruses, *NO* idea why. I've had a different problem but\nstill involving Clamav on kernel.org. Both had been working fine for\nquite some time and the only thing I can think of is that either an\nupdate from clamav went awry, or a date dependent thing triggered and is\ncausing havoc.\n\nShould have a patch series out in an hour or so.\n\n- John 'Warthog9' Hawley\n"},{"id":"131369","messageId":"201001121405.24365.jnareb@gmail.com","threadId":"22085","inReplyTo":"4B4BC4CB.2030409@eaglescrag.net","subject":"Re: [PATCHv3 3/4 (resent)] gitweb: Optionally add \"git\" links in project list page","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-01-12T13:05:22Z","receivedAt":"2010-01-12T13:05:22Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 12 Jan 2010, J.H. wrote:\n> On 01/09/2010 03:20 AM, Jakub Narebski wrote:\n>> On Sat, 9 Jan 2010, J.H. wrote:\n\n>>> I have given some initial thought to converting the $output options I'm\n>>> currently using to a print that Jakub is suggesting & exploring.\n>> \n>> It's 'print {$fh}', i.e. use indirect filehandle, not global filehandle.\n>> \n>>> I think all told it's going to be a similarly sized patch, since all\n>>> output still has to get adjusted (including the things that directly\n>>> output but don't print). \n>> \n>> print -> print {$fh} can be separate patch, and it can be checked that\n>> it produces the same results. Well print -> $output .= could also be\n>> separate patch...\n>> \n>>> I'm unsure if there's a real advantage to \n>>> either way, other than design preference. My patch (forcing the output\n>>> to get passed around) moves towards more of a modal style design\n>>> separating data & layout vs. it's combined nature now, well it's a step\n>>> in that direction anyway.\n>> \n>> Errr... what? Forcing buffering (you need to read whole output into\n>> memory, including for snapshots (uncompressed in case of .tar.gz))\n>> is IMVHO orthogonal to the issue of separating data & layout.\n>> BTW. Modern web server interfaces like Rack, PSGI/Plack etc. explicitly\n>> include streaming support.\n> \n> The inbuilt streaming support does change things, and I don't think it\n> ultimately changes my caching engine really anyway - I should have that\n> change done shortly.\n\nIt doesn't change caching engine much, especially if you encapsulate this\ndetail in the caching engine. With 'print {$fh}' (and in a few places\n'printf {$fh}' (!)) you can just do something like:\n\n $fh = $cache_fh;\n $actions{$action}->();\n show_cached($fh);\n\n\nAbout 'should have that change done shortly': I am working now, time \npermitting, on splitting your caching patch in smaller parts, wrapping\nit a bit differently (and hopefully more clear).\n\n>> The advantage of doing 'print {$fh}' is that $fh can be \\*STDOUT, can\n>> be \\$buffer, but can be filehandle to (temporary) file on disk, and\n>> you can even \"tee\" it, i.e. both write to memory/file, and to STDOUT.\n>> The number of possible choices / possible improvements is much larger.\n>> \n>> And what is also important it means that people who do not use caching\n>> do not suffer latency penalty and memory pressure from caching \n>> infrastructure they do not use.\n\n-- \nJakub Narebski\nPoland\n"}]}