{"thread":{"id":"23991","subject":"[PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","startedAt":"2010-06-03T13:55:54Z","lastAt":"2010-06-03T18:50:01Z","messageCount":13,"participants":["Pavan Kumar Sunkara","Petr Baudis","Ævar Arnfjörð Bjarmason","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"142882","messageId":"1275573356-21466-1-git-send-email-pavan.sss1991@gmail.com","threadId":"23991","inReplyTo":null,"subject":"[PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-03T13:55:54Z","receivedAt":"2010-06-03T13:55:54Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Create Gitweb::Config module in 'gitweb/lib/Gitweb/Config.pm'\nto store all the configuration variables of the gitweb.perl\nscript.\n\nSubroutines moved:\n\tevaluate_gitweb_config\n\tevaluate_git_version\n\nSubroutines yet to move: (Contains calls to not yet packaged subs)\n\tgitweb_get_feature\n\tgitweb_check_feature\n\nUpdate gitweb/Makefile to install gitweb modules alongside gitweb\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n\nThis patch is based on branch 'pu' of alt-git.git\n\n gitweb/Makefile             |    6 +\n gitweb/gitweb.perl          |  421 +++----------------------------------------\n gitweb/lib/Gitweb/Config.pm |  413 ++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 447 insertions(+), 393 deletions(-)\n create mode 100644 gitweb/lib/Gitweb/Config.pm\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex d2584fe..45e176e 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@@ -110,6 +111,9 @@ endif\n \n GITWEB_FILES += static/git-logo.png static/git-favicon.png\n \n+# Files: gitweb/lib/Gitweb\n+GITWEB_LIB_GITWEB += lib/Gitweb/Config.pm\n+\n GITWEB_REPLACE = \\\n \t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n \t-e 's|++GIT_BINDIR++|$(bindir)|g' \\\n@@ -150,6 +154,8 @@ install: all\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+\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)/Gitweb'\n+\t$(INSTALL) -m 644 $(GITWEB_LIB_GITWEB) '$(DESTDIR_SQ)$(gitweblibdir_SQ)/Gitweb'\n \n ### Cleaning rules\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 673e7a3..ef71656 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -16,19 +16,22 @@ use Encode;\n use Fcntl ':mode';\n use File::Find qw();\n use File::Basename qw(basename);\n+use File::Spec;\n binmode STDOUT, ':utf8';\n \n-our $t0;\n-if (eval { require Time::HiRes; 1; }) {\n-\t$t0 = [Time::HiRes::gettimeofday()];\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-our $number_of_git_cmds = 0;\n+use lib __DIR__ . \"/lib\";\n+\n+use Gitweb::Config;\n \n BEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\n }\n \n-our $version = \"++GIT_VERSION++\";\n+$version = \"++GIT_VERSION++\";\n \n our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n sub evaluate_uri {\n@@ -68,402 +71,58 @@ sub evaluate_uri {\n \n # core git executable to use\n # this can just be \"git\" if your webserver has a sensible PATH\n-our $GIT = \"++GIT_BINDIR++/git\";\n+$GIT = \"++GIT_BINDIR++/git\";\n \n # absolute fs-path which will be prepended to the project path\n #our $projectroot = \"/pub/scm\";\n-our $projectroot = \"++GITWEB_PROJECTROOT++\";\n+$projectroot = \"++GITWEB_PROJECTROOT++\";\n \n # fs traversing limit for getting project list\n # the number is relative to the projectroot\n-our $project_maxdepth = \"++GITWEB_PROJECT_MAXDEPTH++\";\n+$project_maxdepth = \"++GITWEB_PROJECT_MAXDEPTH++\";\n \n # string of the home link on top of all pages\n-our $home_link_str = \"++GITWEB_HOME_LINK_STR++\";\n+$home_link_str = \"++GITWEB_HOME_LINK_STR++\";\n \n # name of your site or organization to appear in page titles\n # replace this with something more descriptive for clearer bookmarks\n-our $site_name = \"++GITWEB_SITENAME++\"\n+$site_name = \"++GITWEB_SITENAME++\"\n                  || ($ENV{'SERVER_NAME'} || \"Untitled\") . \" Git\";\n \n # filename of html text to include at top of each page\n-our $site_header = \"++GITWEB_SITE_HEADER++\";\n+$site_header = \"++GITWEB_SITE_HEADER++\";\n # html text to include at home page\n-our $home_text = \"++GITWEB_HOMETEXT++\";\n+$home_text = \"++GITWEB_HOMETEXT++\";\n # filename of html text to include at bottom of each page\n-our $site_footer = \"++GITWEB_SITE_FOOTER++\";\n+$site_footer = \"++GITWEB_SITE_FOOTER++\";\n \n # URI of stylesheets\n-our @stylesheets = (\"++GITWEB_CSS++\");\n+@stylesheets = (\"++GITWEB_CSS++\");\n # URI of a single stylesheet, which can be overridden in GITWEB_CONFIG.\n-our $stylesheet = undef;\n+$stylesheet = undef;\n # URI of GIT logo (72x27 size)\n-our $logo = \"++GITWEB_LOGO++\";\n+$logo = \"++GITWEB_LOGO++\";\n # URI of GIT favicon, assumed to be image/png type\n-our $favicon = \"++GITWEB_FAVICON++\";\n+$favicon = \"++GITWEB_FAVICON++\";\n # URI of gitweb.js (JavaScript code for gitweb)\n-our $javascript = \"++GITWEB_JS++\";\n-\n-# URI and label (title) of GIT logo link\n-#our $logo_url = \"http://www.kernel.org/pub/software/scm/git/docs/\";\n-#our $logo_label = \"git documentation\";\n-our $logo_url = \"http://git-scm.com/\";\n-our $logo_label = \"git homepage\";\n+$javascript = \"++GITWEB_JS++\";\n \n # source of projects list\n-our $projects_list = \"++GITWEB_LIST++\";\n-\n-# the width (in characters) of the projects list \"Description\" column\n-our $projects_list_description_width = 25;\n-\n-# default order of projects list\n-# valid values are none, project, descr, owner, and age\n-our $default_projects_order = \"project\";\n+$projects_list = \"++GITWEB_LIST++\";\n \n # show repository only if this file exists\n # (only effective if this variable evaluates to true)\n-our $export_ok = \"++GITWEB_EXPORT_OK++\";\n-\n-# show repository only if this subroutine returns true\n-# when given the path to the project, for example:\n-#    sub { return -e \"$_[0]/git-daemon-export-ok\"; }\n-our $export_auth_hook = undef;\n+$export_ok = \"++GITWEB_EXPORT_OK++\";\n \n # only allow viewing of repositories also shown on the overview page\n-our $strict_export = \"++GITWEB_STRICT_EXPORT++\";\n+$strict_export = \"++GITWEB_STRICT_EXPORT++\";\n \n # list of git base URLs used for URL to where fetch project from,\n # i.e. full URL is \"$git_base_url/$project\"\n-our @git_base_url_list = grep { $_ ne '' } (\"++GITWEB_BASE_URL++\");\n-\n-# default blob_plain mimetype and default charset for text/plain blob\n-our $default_blob_plain_mimetype = 'text/plain';\n-our $default_text_plain_charset  = undef;\n-\n-# file to use for guessing MIME types before trying /etc/mime.types\n-# (relative to the current git repository)\n-our $mimetypes_file = undef;\n-\n-# assume this charset if line contains non-UTF-8 characters;\n-# it should be valid encoding (see Encoding::Supported(3pm) for list),\n-# for which encoding all byte sequences are valid, for example\n-# 'iso-8859-1' aka 'latin1' (it is decoded without checking, so it\n-# could be even 'utf-8' for the old behavior)\n-our $fallback_encoding = 'latin1';\n-\n-# rename detection options for git-diff and git-diff-tree\n-# - default is '-M', with the cost proportional to\n-#   (number of removed files) * (number of new files).\n-# - more costly is '-C' (which implies '-M'), with the cost proportional to\n-#   (number of changed files + number of removed files) * (number of new files)\n-# - even more costly is '-C', '--find-copies-harder' with cost\n-#   (number of files in the original tree) * (number of new files)\n-# - one might want to include '-B' option, e.g. '-B', '-M'\n-our @diff_opts = ('-M'); # taken from git_commit\n-\n-# Disables features that would allow repository owners to inject script into\n-# the gitweb domain.\n-our $prevent_xss = 0;\n-\n-# information about snapshot formats that gitweb is capable of serving\n-our %known_snapshot_formats = (\n-\t# name => {\n-\t# \t'display' => display name,\n-\t# \t'type' => mime type,\n-\t# \t'suffix' => filename suffix,\n-\t# \t'format' => --format for git-archive,\n-\t# \t'compressor' => [compressor command and arguments]\n-\t# \t                (array reference, optional)\n-\t# \t'disabled' => boolean (optional)}\n-\t#\n-\t'tgz' => {\n-\t\t'display' => 'tar.gz',\n-\t\t'type' => 'application/x-gzip',\n-\t\t'suffix' => '.tar.gz',\n-\t\t'format' => 'tar',\n-\t\t'compressor' => ['gzip']},\n-\n-\t'tbz2' => {\n-\t\t'display' => 'tar.bz2',\n-\t\t'type' => 'application/x-bzip2',\n-\t\t'suffix' => '.tar.bz2',\n-\t\t'format' => 'tar',\n-\t\t'compressor' => ['bzip2']},\n-\n-\t'txz' => {\n-\t\t'display' => 'tar.xz',\n-\t\t'type' => 'application/x-xz',\n-\t\t'suffix' => '.tar.xz',\n-\t\t'format' => 'tar',\n-\t\t'compressor' => ['xz'],\n-\t\t'disabled' => 1},\n-\n-\t'zip' => {\n-\t\t'display' => 'zip',\n-\t\t'type' => 'application/x-zip',\n-\t\t'suffix' => '.zip',\n-\t\t'format' => 'zip'},\n-);\n+@git_base_url_list = grep { $_ ne '' } (\"++GITWEB_BASE_URL++\");\n \n-# Aliases so we understand old gitweb.snapshot values in repository\n-# configuration.\n-our %known_snapshot_format_aliases = (\n-\t'gzip'  => 'tgz',\n-\t'bzip2' => 'tbz2',\n-\t'xz'    => 'txz',\n-\n-\t# backward compatibility: legacy gitweb config support\n-\t'x-gzip' => undef, 'gz' => undef,\n-\t'x-bzip2' => undef, 'bz2' => undef,\n-\t'x-zip' => undef, '' => undef,\n-);\n-\n-# Pixel sizes for icons and avatars. If the default font sizes or lineheights\n-# are changed, it may be appropriate to change these values too via\n-# $GITWEB_CONFIG.\n-our %avatar_size = (\n-\t'default' => 16,\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-\t# feature => {\n-\t# \t'sub' => feature-sub (subroutine),\n-\t# \t'override' => allow-override (boolean),\n-\t# \t'default' => [ default options...] (array reference)}\n-\t#\n-\t# if feature is overridable (it means that allow-override has true value),\n-\t# then feature-sub will be called with default options as parameters;\n-\t# return value of feature-sub indicates if to enable specified feature\n-\t#\n-\t# if there is no 'sub' key (no feature-sub), then feature cannot be\n-\t# overriden\n-\t#\n-\t# use gitweb_get_feature(<feature>) to retrieve the <feature> value\n-\t# (an array) or gitweb_check_feature(<feature>) to check if <feature>\n-\t# is enabled\n-\n-\t# Enable the 'blame' blob view, showing the last commit that modified\n-\t# each line in the file. This can be very CPU-intensive.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG\n-\t# $feature{'blame'}{'default'} = [1];\n-\t# To have project specific config enable override in $GITWEB_CONFIG\n-\t# $feature{'blame'}{'override'} = 1;\n-\t# and in project config gitweb.blame = 0|1;\n-\t'blame' => {\n-\t\t'sub' => sub { feature_bool('blame', @_) },\n-\t\t'override' => 0,\n-\t\t'default' => [0]},\n-\n-\t# Enable the 'snapshot' link, providing a compressed archive of any\n-\t# tree. This can potentially generate high traffic if you have large\n-\t# project.\n-\n-\t# Value is a list of formats defined in %known_snapshot_formats that\n-\t# you wish to offer.\n-\t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'snapshot'}{'default'} = [];\n-\t# To have project specific config enable override in $GITWEB_CONFIG\n-\t# $feature{'snapshot'}{'override'} = 1;\n-\t# and in project config, a comma-separated list of formats or \"none\"\n-\t# to disable.  Example: gitweb.snapshot = tbz2,zip;\n-\t'snapshot' => {\n-\t\t'sub' => \\&feature_snapshot,\n-\t\t'override' => 0,\n-\t\t'default' => ['tgz']},\n-\n-\t# Enable text search, which will list the commits which match author,\n-\t# committer or commit text to a given string.  Enabled by default.\n-\t# Project specific override is not supported.\n-\t'search' => {\n-\t\t'override' => 0,\n-\t\t'default' => [1]},\n-\n-\t# Enable grep search, which will list the files in currently selected\n-\t# tree containing the given string. Enabled by default. This can be\n-\t# potentially CPU-intensive, of course.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG\n-\t# $feature{'grep'}{'default'} = [1];\n-\t# To have project specific config enable override in $GITWEB_CONFIG\n-\t# $feature{'grep'}{'override'} = 1;\n-\t# and in project config gitweb.grep = 0|1;\n-\t'grep' => {\n-\t\t'sub' => sub { feature_bool('grep', @_) },\n-\t\t'override' => 0,\n-\t\t'default' => [1]},\n-\n-\t# Enable the pickaxe search, which will list the commits that modified\n-\t# a given string in a file. This can be practical and quite faster\n-\t# alternative to 'blame', but still potentially CPU-intensive.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG\n-\t# $feature{'pickaxe'}{'default'} = [1];\n-\t# To have project specific config enable override in $GITWEB_CONFIG\n-\t# $feature{'pickaxe'}{'override'} = 1;\n-\t# and in project config gitweb.pickaxe = 0|1;\n-\t'pickaxe' => {\n-\t\t'sub' => sub { feature_bool('pickaxe', @_) },\n-\t\t'override' => 0,\n-\t\t'default' => [1]},\n-\n-\t# Enable showing size of blobs in a 'tree' view, in a separate\n-\t# column, similar to what 'ls -l' does.  This cost a bit of IO.\n-\n-\t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'show-sizes'}{'default'} = [0];\n-\t# To have project specific config enable override in $GITWEB_CONFIG\n-\t# $feature{'show-sizes'}{'override'} = 1;\n-\t# and in project config gitweb.showsizes = 0|1;\n-\t'show-sizes' => {\n-\t\t'sub' => sub { feature_bool('showsizes', @_) },\n-\t\t'override' => 0,\n-\t\t'default' => [1]},\n-\n-\t# Make gitweb use an alternative format of the URLs which can be\n-\t# more readable and natural-looking: project name is embedded\n-\t# directly in the path and the query string contains other\n-\t# auxiliary information. All gitweb installations recognize\n-\t# URL in either format; this configures in which formats gitweb\n-\t# generates links.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG\n-\t# $feature{'pathinfo'}{'default'} = [1];\n-\t# Project specific override is not supported.\n-\n-\t# Note that you will need to change the default location of CSS,\n-\t# favicon, logo and possibly other files to an absolute URL. Also,\n-\t# if gitweb.cgi serves as your indexfile, you will need to force\n-\t# $my_uri to contain the script name in your $GITWEB_CONFIG.\n-\t'pathinfo' => {\n-\t\t'override' => 0,\n-\t\t'default' => [0]},\n-\n-\t# Make gitweb consider projects in project root subdirectories\n-\t# to be forks of existing projects. Given project $projname.git,\n-\t# projects matching $projname/*.git will not be shown in the main\n-\t# projects list, instead a '+' mark will be added to $projname\n-\t# there and a 'forks' view will be enabled for the project, listing\n-\t# all the forks. If project list is taken from a file, forks have\n-\t# to be listed after the main project.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG\n-\t# $feature{'forks'}{'default'} = [1];\n-\t# Project specific override is not supported.\n-\t'forks' => {\n-\t\t'override' => 0,\n-\t\t'default' => [0]},\n-\n-\t# Insert custom links to the action bar of all project pages.\n-\t# This enables you mainly to link to third-party scripts integrating\n-\t# into gitweb; e.g. git-browser for graphical history representation\n-\t# or custom web-based repository administration interface.\n-\n-\t# The 'default' value consists of a list of triplets in the form\n-\t# (label, link, position) where position is the label after which\n-\t# to insert the link and link is a format string where %n expands\n-\t# to the project name, %f to the project path within the filesystem,\n-\t# %h to the current hash (h gitweb parameter) and %b to the current\n-\t# hash base (hb gitweb parameter); %% expands to %.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG e.g.\n-\t# $feature{'actions'}{'default'} = [('graphiclog',\n-\t# \t'/git-browser/by-commit.html?r=%n', 'summary')];\n-\t# Project specific override is not supported.\n-\t'actions' => {\n-\t\t'override' => 0,\n-\t\t'default' => []},\n-\n-\t# Allow gitweb scan project content tags described in ctags/\n-\t# of project repository, and display the popular Web 2.0-ish\n-\t# \"tag cloud\" near the project list. Note that this is something\n-\t# COMPLETELY different from the normal Git tags.\n-\n-\t# gitweb by itself can show existing tags, but it does not handle\n-\t# tagging itself; you need an external application for that.\n-\t# For an example script, check Girocco's cgi/tagproj.cgi.\n-\t# You may want to install the HTML::TagCloud Perl module to get\n-\t# a pretty tag cloud instead of just a list of tags.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG\n-\t# $feature{'ctags'}{'default'} = ['path_to_tag_script'];\n-\t# Project specific override is not supported.\n-\t'ctags' => {\n-\t\t'override' => 0,\n-\t\t'default' => [0]},\n-\n-\t# The maximum number of patches in a patchset generated in patch\n-\t# view. Set this to 0 or undef to disable patch view, or to a\n-\t# negative number to remove any limit.\n-\n-\t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'patches'}{'default'} = [0];\n-\t# To have project specific config enable override in $GITWEB_CONFIG\n-\t# $feature{'patches'}{'override'} = 1;\n-\t# and in project config gitweb.patches = 0|n;\n-\t# where n is the maximum number of patches allowed in a patchset.\n-\t'patches' => {\n-\t\t'sub' => \\&feature_patches,\n-\t\t'override' => 0,\n-\t\t'default' => [16]},\n-\n-\t# Avatar support. When this feature is enabled, views such as\n-\t# shortlog or commit will display an avatar associated with\n-\t# the email of the committer(s) and/or author(s).\n-\n-\t# Currently available providers are gravatar and picon.\n-\t# If an unknown provider is specified, the feature is disabled.\n-\n-\t# Gravatar depends on Digest::MD5.\n-\t# Picon currently relies on the indiana.edu database.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG\n-\t# $feature{'avatar'}{'default'} = ['<provider>'];\n-\t# where <provider> is either gravatar or picon.\n-\t# To have project specific config enable override in $GITWEB_CONFIG\n-\t# $feature{'avatar'}{'override'} = 1;\n-\t# and in project config gitweb.avatar = <provider>;\n-\t'avatar' => {\n-\t\t'sub' => \\&feature_avatar,\n-\t\t'override' => 0,\n-\t\t'default' => ['']},\n-\n-\t# Enable displaying how much time and how many git commands\n-\t# it took to generate and display page.  Disabled by default.\n-\t# Project specific override is not supported.\n-\t'timed' => {\n-\t\t'override' => 0,\n-\t\t'default' => [0]},\n-\n-\t# Enable turning some links into links to actions which require\n-\t# JavaScript to run (like 'blame_incremental').  Not enabled by\n-\t# default.  Project specific override is currently not supported.\n-\t'javascript-actions' => {\n-\t\t'override' => 0,\n-\t\t'default' => [0]},\n-\n-\t# Syntax highlighting support. This is based on Daniel Svensson's\n-\t# and Sham Chukoury's work in gitweb-xmms2.git.\n-\t# It requires the 'highlight' program present in $PATH,\n-\t# and therefore is disabled by default.\n-\n-\t# To enable system wide have in $GITWEB_CONFIG\n-\t# $feature{'highlight'}{'default'} = [1];\n-\n-\t'highlight' => {\n-\t\t'sub' => sub { feature_bool('highlight', @_) },\n-\t\t'override' => 0,\n-\t\t'default' => [0]},\n-);\n+$GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n+$GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || \"++GITWEB_CONFIG_SYSTEM++\";\n \n sub gitweb_get_feature {\n \tmy ($name) = @_;\n@@ -571,20 +230,6 @@ sub filter_snapshot_fmts {\n \t\t!$known_snapshot_formats{$_}{'disabled'}} @fmts;\n }\n \n-our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM);\n-sub evaluate_gitweb_config {\n-\tour $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n-\tour $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || \"++GITWEB_CONFIG_SYSTEM++\";\n-\t# die if there are errors parsing config file\n-\tif (-e $GITWEB_CONFIG) {\n-\t\tdo $GITWEB_CONFIG;\n-\t\tdie $@ if $@;\n-\t} elsif (-e $GITWEB_CONFIG_SYSTEM) {\n-\t\tdo $GITWEB_CONFIG_SYSTEM;\n-\t\tdie $@ if $@;\n-\t}\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@@ -607,13 +252,6 @@ sub get_loadavg {\n \treturn 0;\n }\n \n-# version of the core git binary\n-our $git_version;\n-sub evaluate_git_version {\n-\tour $git_version = qx(\"$GIT\" --version) =~ m/git version (.*)$/ ? $1 : \"unknown\";\n-\t$number_of_git_cmds++;\n-}\n-\n sub check_loadavg {\n \tif (defined $maxload && get_loadavg() > $maxload) {\n \t\tdie_error(503, \"The load average on the server is too high\");\n@@ -1028,7 +666,7 @@ sub dispatch {\n }\n \n sub run_request {\n-\tour $t0 = [Time::HiRes::gettimeofday()]\n+\t$t0 = [Time::HiRes::gettimeofday()]\n \t\tif defined $t0;\n \n \tevaluate_uri();\n@@ -2555,9 +2193,6 @@ sub git_get_projects_list {\n \t\t\tfollow_skip => 2, # ignore duplicates\n \t\t\tdangling_symlinks => 0, # ignore dangling symlinks, silently\n \t\t\twanted => sub {\n-\t\t\t\t# global variables\n-\t\t\t\tour $project_maxdepth;\n-\t\t\t\tour $projectroot;\n \t\t\t\t# skip project-list toplevel, if we get it.\n \t\t\t\treturn if (m!^[/.]$!);\n \t\t\t\t# only directories can be git repositories\ndiff --git a/gitweb/lib/Gitweb/Config.pm b/gitweb/lib/Gitweb/Config.pm\nnew file mode 100644\nindex 0000000..cb8efa5\n--- /dev/null\n+++ b/gitweb/lib/Gitweb/Config.pm\n@@ -0,0 +1,413 @@\n+#!/usr/bin/perl\n+#\n+# Gitweb::Config -- gitweb configuration package\n+#\n+# This program is licensed under the GPLv2\n+\n+package Gitweb::Config;\n+\n+use strict;\n+use warnings;\n+use Exporter qw(import);\n+\n+our @ISA = qw(Exporter);\n+our @EXPORT = qw($t0 $number_of_git_cmds $GIT $version $git_version $projectroot $project_maxdepth\n+                 $projects_list @git_base_url_list $export_ok $strict_export $home_link_str $site_name\n+                 $site_header $site_footer $home_text @stylesheets $stylesheet $logo $favicon $javascript\n+                 $GITWEB_CONFIG $GITWEB_CONFIG_SYSTEM $logo_url $logo_label $default_projects_order\n+                 $projects_list_description_width $export_auth_hook $default_blob_plain_mimetype $mimetypes_file\n+                 $default_text_plain_charset $fallback_encoding @diff_opts $prevent_xss $maxload %avatar_size\n+                 %known_snapshot_formats %known_snapshot_format_aliases %feature &evaluate_gitweb_config\n+                 &evaluate_git_version);\n+\n+# Runtime variables\n+our $t0;\n+if (eval { require Time::HiRes; 1; }) {\n+\t$t0 = [Time::HiRes::gettimeofday()];\n+}\n+our $number_of_git_cmds = 0;\n+\n+# The following variables are affected by build-time configuration\n+# and hence their initialisation is put in gitweb.perl script\n+\n+# core git executable path and versions\n+our ($GIT, $version, $git_version);\n+\n+# fs-path, fs traversing limit, source of projects list and git base URL\n+our ($projectroot, $project_maxdepth, $projects_list, @git_base_url_list);\n+\n+our ($export_ok, $strict_export);\n+\n+# Site configurations\n+our ($home_link_str, $site_name, $site_header, $site_footer, $home_text);\n+\n+# Stylesheet, logo and javascript\n+our (@stylesheets, $stylesheet, $logo, $favicon, $javascript);\n+\n+# gitweb config\n+our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM);\n+\n+# URI and label (title) of GIT logo link\n+#our $logo_url = \"http://www.kernel.org/pub/software/scm/git/docs/\";\n+#our $logo_label = \"git documentation\";\n+our $logo_url = \"http://git-scm.com/\";\n+our $logo_label = \"git homepage\";\n+\n+# the width (in characters) of the projects list \"Description\" column\n+our $projects_list_description_width = 25;\n+\n+# default order of projects list\n+# valid values are none, project, descr, owner, and age\n+our $default_projects_order = \"project\";\n+\n+# show repository only if this subroutine returns true\n+# when given the path to the project, for example:\n+#    sub { return -e \"$_[0]/git-daemon-export-ok\"; }\n+our $export_auth_hook = undef;\n+\n+# default blob_plain mimetype and default charset for text/plain blob\n+our $default_blob_plain_mimetype = 'text/plain';\n+our $default_text_plain_charset  = undef;\n+\n+# file to use for guessing MIME types before trying /etc/mime.types\n+# (relative to the current git repository)\n+our $mimetypes_file = undef;\n+\n+# assume this charset if line contains non-UTF-8 characters;\n+# it should be valid encoding (see Encoding::Supported(3pm) for list),\n+# for which encoding all byte sequences are valid, for example\n+# 'iso-8859-1' aka 'latin1' (it is decoded without checking, so it\n+# could be even 'utf-8' for the old behavior)\n+our $fallback_encoding = 'latin1';\n+\n+# rename detection options for git-diff and git-diff-tree\n+# - default is '-M', with the cost proportional to\n+#   (number of removed files) * (number of new files).\n+# - more costly is '-C' (which implies '-M'), with the cost proportional to\n+#   (number of changed files + number of removed files) * (number of new files)\n+# - even more costly is '-C', '--find-copies-harder' with cost\n+#   (number of files in the original tree) * (number of new files)\n+# - one might want to include '-B' option, e.g. '-B', '-M'\n+our @diff_opts = ('-M'); # taken from git_commit\n+\n+# Disables features that would allow repository owners to inject script into\n+# the gitweb domain.\n+our $prevent_xss = 0;\n+\n+# information about snapshot formats that gitweb is capable of serving\n+our %known_snapshot_formats = (\n+\t# name => {\n+\t# \t'display' => display name,\n+\t# \t'type' => mime type,\n+\t# \t'suffix' => filename suffix,\n+\t# \t'format' => --format for git-archive,\n+\t# \t'compressor' => [compressor command and arguments]\n+\t# \t                (array reference, optional)\n+\t# \t'disabled' => boolean (optional)}\n+\t#\n+\t'tgz' => {\n+\t\t'display' => 'tar.gz',\n+\t\t'type' => 'application/x-gzip',\n+\t\t'suffix' => '.tar.gz',\n+\t\t'format' => 'tar',\n+\t\t'compressor' => ['gzip']},\n+\n+\t'tbz2' => {\n+\t\t'display' => 'tar.bz2',\n+\t\t'type' => 'application/x-bzip2',\n+\t\t'suffix' => '.tar.bz2',\n+\t\t'format' => 'tar',\n+\t\t'compressor' => ['bzip2']},\n+\n+\t'txz' => {\n+\t\t'display' => 'tar.xz',\n+\t\t'type' => 'application/x-xz',\n+\t\t'suffix' => '.tar.xz',\n+\t\t'format' => 'tar',\n+\t\t'compressor' => ['xz'],\n+\t\t'disabled' => 1},\n+\n+\t'zip' => {\n+\t\t'display' => 'zip',\n+\t\t'type' => 'application/x-zip',\n+\t\t'suffix' => '.zip',\n+\t\t'format' => 'zip'},\n+);\n+\n+# Aliases so we understand old gitweb.snapshot values in repository\n+# configuration.\n+our %known_snapshot_format_aliases = (\n+\t'gzip'  => 'tgz',\n+\t'bzip2' => 'tbz2',\n+\t'xz'    => 'txz',\n+\n+\t# backward compatibility: legacy gitweb config support\n+\t'x-gzip' => undef, 'gz' => undef,\n+\t'x-bzip2' => undef, 'bz2' => undef,\n+\t'x-zip' => undef, '' => undef,\n+);\n+\n+# Pixel sizes for icons and avatars. If the default font sizes or lineheights\n+# are changed, it may be appropriate to change these values too via\n+# $GITWEB_CONFIG.\n+our %avatar_size = (\n+\t'default' => 16,\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+\t# feature => {\n+\t# \t'sub' => feature-sub (subroutine),\n+\t# \t'override' => allow-override (boolean),\n+\t# \t'default' => [ default options...] (array reference)}\n+\t#\n+\t# if feature is overridable (it means that allow-override has true value),\n+\t# then feature-sub will be called with default options as parameters;\n+\t# return value of feature-sub indicates if to enable specified feature\n+\t#\n+\t# if there is no 'sub' key (no feature-sub), then feature cannot be\n+\t# overriden\n+\t#\n+\t# use gitweb_get_feature(<feature>) to retrieve the <feature> value\n+\t# (an array) or gitweb_check_feature(<feature>) to check if <feature>\n+\t# is enabled\n+\n+\t# Enable the 'blame' blob view, showing the last commit that modified\n+\t# each line in the file. This can be very CPU-intensive.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'blame'}{'default'} = [1];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'blame'}{'override'} = 1;\n+\t# and in project config gitweb.blame = 0|1;\n+\t'blame' => {\n+\t\t'sub' => sub { feature_bool('blame', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n+\n+\t# Enable the 'snapshot' link, providing a compressed archive of any\n+\t# tree. This can potentially generate high traffic if you have large\n+\t# project.\n+\n+\t# Value is a list of formats defined in %Gitweb::Config::known_snapshot_formats that\n+\t# you wish to offer.\n+\t# To disable system wide have in $GITWEB_CONFIG\n+\t# $feature{'snapshot'}{'default'} = [];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'snapshot'}{'override'} = 1;\n+\t# and in project config, a comma-separated list of formats or \"none\"\n+\t# to disable.  Example: gitweb.snapshot = tbz2,zip;\n+\t'snapshot' => {\n+\t\t'sub' => \\&feature_snapshot,\n+\t\t'override' => 0,\n+\t\t'default' => ['tgz']},\n+\n+\t# Enable text search, which will list the commits which match author,\n+\t# committer or commit text to a given string.  Enabled by default.\n+\t# Project specific override is not supported.\n+\t'search' => {\n+\t\t'override' => 0,\n+\t\t'default' => [1]},\n+\n+\t# Enable grep search, which will list the files in currently selected\n+\t# tree containing the given string. Enabled by default. This can be\n+\t# potentially CPU-intensive, of course.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'grep'}{'default'} = [1];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'grep'}{'override'} = 1;\n+\t# and in project config gitweb.grep = 0|1;\n+\t'grep' => {\n+\t\t'sub' => sub { feature_bool('grep', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [1]},\n+\n+\t# Enable the pickaxe search, which will list the commits that modified\n+\t# a given string in a file. This can be practical and quite faster\n+\t# alternative to 'blame', but still potentially CPU-intensive.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'pickaxe'}{'default'} = [1];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'pickaxe'}{'override'} = 1;\n+\t# and in project config gitweb.pickaxe = 0|1;\n+\t'pickaxe' => {\n+\t\t'sub' => sub { feature_bool('pickaxe', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [1]},\n+\n+\t# Enable showing size of blobs in a 'tree' view, in a separate\n+\t# column, similar to what 'ls -l' does.  This cost a bit of IO.\n+\n+\t# To disable system wide have in $GITWEB_CONFIG\n+\t# $feature{'show-sizes'}{'default'} = [0];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'show-sizes'}{'override'} = 1;\n+\t# and in project config gitweb.showsizes = 0|1;\n+\t'show-sizes' => {\n+\t\t'sub' => sub { feature_bool('showsizes', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [1]},\n+\n+\t# Make gitweb use an alternative format of the URLs which can be\n+\t# more readable and natural-looking: project name is embedded\n+\t# directly in the path and the query string contains other\n+\t# auxiliary information. All gitweb installations recognize\n+\t# URL in either format; this configures in which formats gitweb\n+\t# generates links.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'pathinfo'}{'default'} = [1];\n+\t# Project specific override is not supported.\n+\n+\t# Note that you will need to change the default location of CSS,\n+\t# favicon, logo and possibly other files to an absolute URL. Also,\n+\t# if gitweb.cgi serves as your indexfile, you will need to force\n+\t# $my_uri to contain the script name in your $GITWEB_CONFIG.\n+\t'pathinfo' => {\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n+\n+\t# Make gitweb consider projects in project root subdirectories\n+\t# to be forks of existing projects. Given project $projname.git,\n+\t# projects matching $projname/*.git will not be shown in the main\n+\t# projects list, instead a '+' mark will be added to $projname\n+\t# there and a 'forks' view will be enabled for the project, listing\n+\t# all the forks. If project list is taken from a file, forks have\n+\t# to be listed after the main project.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'forks'}{'default'} = [1];\n+\t# Project specific override is not supported.\n+\t'forks' => {\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n+\n+\t# Insert custom links to the action bar of all project pages.\n+\t# This enables you mainly to link to third-party scripts integrating\n+\t# into gitweb; e.g. git-browser for graphical history representation\n+\t# or custom web-based repository administration interface.\n+\n+\t# The 'default' value consists of a list of triplets in the form\n+\t# (label, link, position) where position is the label after which\n+\t# to insert the link and link is a format string where %n expands\n+\t# to the project name, %f to the project path within the filesystem,\n+\t# %h to the current hash (h gitweb parameter) and %b to the current\n+\t# hash base (hb gitweb parameter); %% expands to %.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG e.g.\n+\t# $feature{'actions'}{'default'} = [('graphiclog',\n+\t# \t'/git-browser/by-commit.html?r=%n', 'summary')];\n+\t# Project specific override is not supported.\n+\t'actions' => {\n+\t\t'override' => 0,\n+\t\t'default' => []},\n+\n+\t# Allow gitweb scan project content tags described in ctags/\n+\t# of project repository, and display the popular Web 2.0-ish\n+\t# \"tag cloud\" near the project list. Note that this is something\n+\t# COMPLETELY different from the normal Git tags.\n+\n+\t# gitweb by itself can show existing tags, but it does not handle\n+\t# tagging itself; you need an external application for that.\n+\t# For an example script, check Girocco's cgi/tagproj.cgi.\n+\t# You may want to install the HTML::TagCloud Perl module to get\n+\t# a pretty tag cloud instead of just a list of tags.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'ctags'}{'default'} = ['path_to_tag_script'];\n+\t# Project specific override is not supported.\n+\t'ctags' => {\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n+\n+\t# The maximum number of patches in a patchset generated in patch\n+\t# view. Set this to 0 or undef to disable patch view, or to a\n+\t# negative number to remove any limit.\n+\n+\t# To disable system wide have in $GITWEB_CONFIG\n+\t# $feature{'patches'}{'default'} = [0];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'patches'}{'override'} = 1;\n+\t# and in project config gitweb.patches = 0|n;\n+\t# where n is the maximum number of patches allowed in a patchset.\n+\t'patches' => {\n+\t\t'sub' => \\&feature_patches,\n+\t\t'override' => 0,\n+\t\t'default' => [16]},\n+\n+\t# Avatar support. When this feature is enabled, views such as\n+\t# shortlog or commit will display an avatar associated with\n+\t# the email of the committer(s) and/or author(s).\n+\n+\t# Currently available providers are gravatar and picon.\n+\t# If an unknown provider is specified, the feature is disabled.\n+\n+\t# Gravatar depends on Digest::MD5.\n+\t# Picon currently relies on the indiana.edu database.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'avatar'}{'default'} = ['<provider>'];\n+\t# where <provider> is either gravatar or picon.\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'avatar'}{'override'} = 1;\n+\t# and in project config gitweb.avatar = <provider>;\n+\t'avatar' => {\n+\t\t'sub' => \\&feature_avatar,\n+\t\t'override' => 0,\n+\t\t'default' => ['']},\n+\n+\t# Enable displaying how much time and how many git commands\n+\t# it took to generate and display page.  Disabled by default.\n+\t# Project specific override is not supported.\n+\t'timed' => {\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n+\n+\t# Enable turning some links into links to actions which require\n+\t# JavaScript to run (like 'blame_incremental').  Not enabled by\n+\t# default.  Project specific override is currently not supported.\n+\t'javascript-actions' => {\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n+\n+\t# Syntax highlighting support. This is based on Daniel Svensson's\n+\t# and Sham Chukoury's work in gitweb-xmms2.git.\n+\t# It requires the 'highlight' program present in $PATH,\n+\t# and therefore is disabled by default.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'highlight'}{'default'} = [1];\n+\n+\t'highlight' => {\n+\t\t'sub' => sub { feature_bool('highlight', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n+);\n+\n+sub evaluate_gitweb_config {\n+\t# die if there are errors parsing config file\n+\tif (-e $GITWEB_CONFIG) {\n+\t\tdo $GITWEB_CONFIG;\n+\t\tdie $@ if $@;\n+\t} elsif (-e $GITWEB_CONFIG_SYSTEM) {\n+\t\tdo $GITWEB_CONFIG_SYSTEM;\n+\t\tdie $@ if $@;\n+\t}\n+}\n+\n+sub evaluate_git_version {\n+\t$git_version = qx(\"$GIT\" --version) =~ m/git version (.*)$/ ? $1 : \"unknown\";\n+\t$number_of_git_cmds++;\n+}\n+\n+1;\n-- \n1.7.1.447.gfddfb\n"},{"id":"142883","messageId":"1275573356-21466-2-git-send-email-pavan.sss1991@gmail.com","threadId":"23991","inReplyTo":"1275573356-21466-1-git-send-email-pavan.sss1991@gmail.com","subject":"[PATCH GSoC 2/3] gitweb: Create Gitweb::Request module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-03T13:55:55Z","receivedAt":"2010-06-03T13:55:55Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Create a Gitweb::Request module in 'gitweb/lib/Gitweb/Request.pm'\nto store and handle all the cgi params and related variables\nregarding the gitweb.perl script.\n\nSubroutines moved:\n\tevaluate_uri\n\tevaluate_query_params\n\tevaluate_git_dir\n\tvalidate_pathname\n\tvalidate_refname\n\nSubroutines yet to move: (Contains calls to not yet packages subs)\n\tevaluate_path_info\n\tevaluate_and _validate_params\n\tvalidate_action\n\tvalidate_project\n\nUpdate gitweb/Makefile to install gitweb modules alongside gitweb\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n\nThis patch is based on branch 'pu' and previous patch.\n\n gitweb/Makefile              |    1 +\n gitweb/gitweb.perl           |  165 ++++--------------------------------------\n gitweb/lib/Gitweb/Request.pm |  156 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 171 insertions(+), 151 deletions(-)\n create mode 100644 gitweb/lib/Gitweb/Request.pm\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 45e176e..15646b2 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -113,6 +113,7 @@ GITWEB_FILES += static/git-logo.png static/git-favicon.png\n \n # Files: gitweb/lib/Gitweb\n GITWEB_LIB_GITWEB += lib/Gitweb/Config.pm\n+GITWEB_LIB_GITWEB += lib/Gitweb/Request.pm\n \n GITWEB_REPLACE = \\\n \t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex ef71656..2514b7a 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -26,6 +26,7 @@ sub __DIR__ () {\n use lib __DIR__ . \"/lib\";\n \n use Gitweb::Config;\n+use Gitweb::Request;\n \n BEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\n@@ -33,42 +34,6 @@ BEGIN {\n \n $version = \"++GIT_VERSION++\";\n \n-our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n-sub evaluate_uri {\n-\tour $cgi;\n-\n-\tour $my_url = $cgi->url();\n-\tour $my_uri = $cgi->url(-absolute => 1);\n-\n-\t# Base URL for relative URLs in gitweb ($logo, $favicon, ...),\n-\t# needed and used only for URLs with nonempty PATH_INFO\n-\tour $base_url = $my_url;\n-\n-\t# When the script is used as DirectoryIndex, the URL does not contain the name\n-\t# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we\n-\t# have to do it ourselves. We make $path_info global because it's also used\n-\t# later on.\n-\t#\n-\t# Another issue with the script being the DirectoryIndex is that the resulting\n-\t# $my_url data is not the full script URL: this is good, because we want\n-\t# generated links to keep implying the script name if it wasn't explicitly\n-\t# indicated in the URL we're handling, but it means that $my_url cannot be used\n-\t# as base URL.\n-\t# Therefore, if we needed to strip PATH_INFO, then we know that we have\n-\t# to build the base URL ourselves:\n-\tour $path_info = $ENV{\"PATH_INFO\"};\n-\tif ($path_info) {\n-\t\tif ($my_url =~ s,\\Q$path_info\\E$,, &&\n-\t\t    $my_uri =~ s,\\Q$path_info\\E$,, &&\n-\t\t    defined $ENV{'SCRIPT_NAME'}) {\n-\t\t\t$base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n-\t\t}\n-\t}\n-\n-\t# target of the home link on top of all pages\n-\tour $home_link = $my_uri || \"/\";\n-}\n-\n # core git executable to use\n # this can just be \"git\" if your webserver has a sensible PATH\n $GIT = \"++GIT_BINDIR++/git\";\n@@ -132,7 +97,6 @@ sub gitweb_get_feature {\n \t\t$feature{$name}{'override'},\n \t\t@{$feature{$name}{'default'}});\n \t# project specific override is possible only if we have project\n-\tour $git_dir; # global variable, declared later\n \tif (!$override || !defined $git_dir) {\n \t\treturn @defaults;\n \t}\n@@ -261,42 +225,6 @@ sub check_loadavg {\n # ======================================================================\n # input validation and dispatch\n \n-# input parameters can be collected from a variety of sources (presently, CGI\n-# and PATH_INFO), so we define an %input_params hash that collects them all\n-# together during validation: this allows subsequent uses (e.g. href()) to be\n-# agnostic of the parameter origin\n-\n-our %input_params = ();\n-\n-# input parameters are stored with the long parameter name as key. This will\n-# also be used in the href subroutine to convert parameters to their CGI\n-# equivalent, and since the href() usage is the most frequent one, we store\n-# the name -> CGI key mapping here, instead of the reverse.\n-#\n-# XXX: Warning: If you touch this, check the search form for updating,\n-# too.\n-\n-our @cgi_param_mapping = (\n-\tproject => \"p\",\n-\taction => \"a\",\n-\tfile_name => \"f\",\n-\tfile_parent => \"fp\",\n-\thash => \"h\",\n-\thash_parent => \"hp\",\n-\thash_base => \"hb\",\n-\thash_parent_base => \"hpb\",\n-\tpage => \"pg\",\n-\torder => \"o\",\n-\tsearchtext => \"s\",\n-\tsearchtype => \"st\",\n-\tsnapshot_format => \"sf\",\n-\textra_options => \"opt\",\n-\tsearch_use_regexp => \"sr\",\n-\t# this must be last entry (for manipulation from JavaScript)\n-\tjavascript => \"js\"\n-);\n-our %cgi_param_mapping = @cgi_param_mapping;\n-\n # we will also need to know the possible actions, for validation\n our %actions = (\n \t\"blame\" => \\&git_blame,\n@@ -332,27 +260,6 @@ our %actions = (\n \t\"project_index\" => \\&git_project_index,\n );\n \n-# finally, we have the hash of allowed extra_options for the commands that\n-# allow them\n-our %allowed_options = (\n-\t\"--no-merges\" => [ qw(rss atom log shortlog history) ],\n-);\n-\n-# fill %input_params with the CGI parameters. All values except for 'opt'\n-# should be single values, but opt can be an array. We should probably\n-# build an array of parameters that can be multi-valued, but since for the time\n-# being it's only this one, we just single it out\n-sub evaluate_query_params {\n-\tour $cgi;\n-\n-\twhile (my ($name, $symbol) = each %cgi_param_mapping) {\n-\t\tif ($symbol eq 'opt') {\n-\t\t\t$input_params{$name} = [ $cgi->param($symbol) ];\n-\t\t} else {\n-\t\t\t$input_params{$name} = $cgi->param($symbol);\n-\t\t}\n-\t}\n-}\n \n # now read PATH_INFO and update the parameter list for missing parameters\n sub evaluate_path_info {\n@@ -498,11 +405,8 @@ sub evaluate_path_info {\n \t}\n }\n \n-our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, $hash_base,\n-     $hash_parent_base, @extra_options, $page, $searchtype, $search_use_regexp,\n-     $searchtext, $search_regexp);\n sub evaluate_and_validate_params {\n-\tour $action = $input_params{'action'};\n+\t$action = $input_params{'action'};\n \tif (defined $action) {\n \t\tif (!validate_action($action)) {\n \t\t\tdie_error(400, \"Invalid action parameter\");\n@@ -510,7 +414,7 @@ sub evaluate_and_validate_params {\n \t}\n \n \t# parameters which are pathnames\n-\tour $project = $input_params{'project'};\n+\t$project = $input_params{'project'};\n \tif (defined $project) {\n \t\tif (!validate_project($project)) {\n \t\t\tundef $project;\n@@ -518,14 +422,14 @@ sub evaluate_and_validate_params {\n \t\t}\n \t}\n \n-\tour $file_name = $input_params{'file_name'};\n+\t$file_name = $input_params{'file_name'};\n \tif (defined $file_name) {\n \t\tif (!validate_pathname($file_name)) {\n \t\t\tdie_error(400, \"Invalid file parameter\");\n \t\t}\n \t}\n \n-\tour $file_parent = $input_params{'file_parent'};\n+\t$file_parent = $input_params{'file_parent'};\n \tif (defined $file_parent) {\n \t\tif (!validate_pathname($file_parent)) {\n \t\t\tdie_error(400, \"Invalid file parent parameter\");\n@@ -533,28 +437,28 @@ sub evaluate_and_validate_params {\n \t}\n \n \t# parameters which are refnames\n-\tour $hash = $input_params{'hash'};\n+\t$hash = $input_params{'hash'};\n \tif (defined $hash) {\n \t\tif (!validate_refname($hash)) {\n \t\t\tdie_error(400, \"Invalid hash parameter\");\n \t\t}\n \t}\n \n-\tour $hash_parent = $input_params{'hash_parent'};\n+\t$hash_parent = $input_params{'hash_parent'};\n \tif (defined $hash_parent) {\n \t\tif (!validate_refname($hash_parent)) {\n \t\t\tdie_error(400, \"Invalid hash parent parameter\");\n \t\t}\n \t}\n \n-\tour $hash_base = $input_params{'hash_base'};\n+\t$hash_base = $input_params{'hash_base'};\n \tif (defined $hash_base) {\n \t\tif (!validate_refname($hash_base)) {\n \t\t\tdie_error(400, \"Invalid hash base parameter\");\n \t\t}\n \t}\n \n-\tour @extra_options = @{$input_params{'extra_options'}};\n+\t@extra_options = @{$input_params{'extra_options'}};\n \t# @extra_options is always defined, since it can only be (currently) set from\n \t# CGI, and $cgi->param() returns the empty array in array context if the param\n \t# is not set\n@@ -567,7 +471,7 @@ sub evaluate_and_validate_params {\n \t\t}\n \t}\n \n-\tour $hash_parent_base = $input_params{'hash_parent_base'};\n+\t$hash_parent_base = $input_params{'hash_parent_base'};\n \tif (defined $hash_parent_base) {\n \t\tif (!validate_refname($hash_parent_base)) {\n \t\t\tdie_error(400, \"Invalid hash parent base parameter\");\n@@ -575,24 +479,23 @@ sub evaluate_and_validate_params {\n \t}\n \n \t# other parameters\n-\tour $page = $input_params{'page'};\n+\t$page = $input_params{'page'};\n \tif (defined $page) {\n \t\tif ($page =~ m/[^0-9]/) {\n \t\t\tdie_error(400, \"Invalid page parameter\");\n \t\t}\n \t}\n \n-\tour $searchtype = $input_params{'searchtype'};\n+\t$searchtype = $input_params{'searchtype'};\n \tif (defined $searchtype) {\n \t\tif ($searchtype =~ m/[^a-z]/) {\n \t\t\tdie_error(400, \"Invalid searchtype parameter\");\n \t\t}\n \t}\n \n-\tour $search_use_regexp = $input_params{'search_use_regexp'};\n+\t$search_use_regexp = $input_params{'search_use_regexp'};\n \n-\tour $searchtext = $input_params{'searchtext'};\n-\tour $search_regexp;\n+\t$searchtext = $input_params{'searchtext'};\n \tif (defined $searchtext) {\n \t\tif (length($searchtext) < 2) {\n \t\t\tdie_error(403, \"At least two characters are required for search parameter\");\n@@ -601,12 +504,6 @@ sub evaluate_and_validate_params {\n \t}\n }\n \n-# path to the current git repository\n-our $git_dir;\n-sub evaluate_git_dir {\n-\tour $git_dir = \"$projectroot/$project\" if $project;\n-}\n-\n our (@snapshot_fmts, $git_avatar);\n sub configure_gitweb_features {\n \t# list of supported snapshot formats\n@@ -690,7 +587,6 @@ sub run_request {\n our $is_last_request = sub { 1 };\n our ($pre_dispatch_hook, $post_dispatch_hook, $pre_listen_hook);\n our $CGI = 'CGI';\n-our $cgi;\n sub evaluate_argv {\n \treturn unless (@ARGV);\n \n@@ -885,39 +781,6 @@ sub validate_project {\n \t}\n }\n \n-sub validate_pathname {\n-\tmy $input = shift || return undef;\n-\n-\t# no '.' or '..' as elements of path, i.e. no '.' nor '..'\n-\t# at the beginning, at the end, and between slashes.\n-\t# also this catches doubled slashes\n-\tif ($input =~ m!(^|/)(|\\.|\\.\\.)(/|$)!) {\n-\t\treturn undef;\n-\t}\n-\t# no null characters\n-\tif ($input =~ m!\\0!) {\n-\t\treturn undef;\n-\t}\n-\treturn $input;\n-}\n-\n-sub validate_refname {\n-\tmy $input = shift || return undef;\n-\n-\t# textual hashes are O.K.\n-\tif ($input =~ m/^[0-9a-fA-F]{40}$/) {\n-\t\treturn $input;\n-\t}\n-\t# it must be correct pathname\n-\t$input = validate_pathname($input)\n-\t\tor return undef;\n-\t# restrictions on ref name according to git-check-ref-format\n-\tif ($input =~ m!(/\\.|\\.\\.|[\\000-\\040\\177 ~^:?*\\[]|/$)!) {\n-\t\treturn undef;\n-\t}\n-\treturn $input;\n-}\n-\n # decode sequences of octets in utf8 into Perl's internal form,\n # which is utf-8 with utf8 flag set if needed.  gitweb writes out\n # in utf-8 thanks to \"binmode STDOUT, ':utf8'\" at beginning\ndiff --git a/gitweb/lib/Gitweb/Request.pm b/gitweb/lib/Gitweb/Request.pm\nnew file mode 100644\nindex 0000000..474e89d\n--- /dev/null\n+++ b/gitweb/lib/Gitweb/Request.pm\n@@ -0,0 +1,156 @@\n+#!/usr/bin/perl\n+#\n+# Gitweb::Request -- gitweb request(cgi) package\n+#\n+# This program is licensed under the GPLv2\n+\n+package Gitweb::Request;\n+\n+use strict;\n+use warnings;\n+use Exporter qw(import);\n+\n+our @ISA = qw(Exporter);\n+our @EXPORT = qw($cgi $my_url $my_uri $base_url $path_info $home_link $action $project $file_name\n+                 $file_parent $hash $hash_parent $hash_base $hash_parent_base @extra_options $page\n+                 $git_dir $searchtype $search_use_regexp $searchtext $search_regexp %input_params\n+                 @cgi_param_mapping %cgi_param_mapping %allowed_options &evaluate_query_params\n+                 &evaluate_uri &evaluate_git_dir &validate_pathname &validate_refname);\n+\n+use Gitweb::Config qw($projectroot);\n+\n+our ($cgi, $my_url, $my_uri, $base_url, $path_info, $home_link);\n+our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, $hash_base,\n+     $hash_parent_base, @extra_options, $page, $git_dir);\n+our ($searchtype, $search_use_regexp, $searchtext, $search_regexp);\n+\n+# input parameters can be collected from a variety of sources (presently, CGI\n+# and PATH_INFO), so we define an %input_params hash that collects them all\n+# together during validation: this allows subsequent uses (e.g. href()) to be\n+# agnostic of the parameter origin\n+\n+our %input_params = ();\n+\n+# input parameters are stored with the long parameter name as key. This will\n+# also be used in the href subroutine to convert parameters to their CGI\n+# equivalent, and since the href() usage is the most frequent one, we store\n+# the name -> CGI key mapping here, instead of the reverse.\n+#\n+# XXX: Warning: If you touch this, check the search form for updating,\n+# too.\n+\n+our @cgi_param_mapping = (\n+\tproject => \"p\",\n+\taction => \"a\",\n+\tfile_name => \"f\",\n+\tfile_parent => \"fp\",\n+\thash => \"h\",\n+\thash_parent => \"hp\",\n+\thash_base => \"hb\",\n+\thash_parent_base => \"hpb\",\n+\tpage => \"pg\",\n+\torder => \"o\",\n+\tsearchtext => \"s\",\n+\tsearchtype => \"st\",\n+\tsnapshot_format => \"sf\",\n+\textra_options => \"opt\",\n+\tsearch_use_regexp => \"sr\",\n+\t# this must be last entry (for manipulation from JavaScript)\n+\tjavascript => \"js\"\n+);\n+our %cgi_param_mapping = @cgi_param_mapping;\n+\n+# finally, we have the hash of allowed extra_options for the commands that\n+# allow them\n+our %allowed_options = (\n+\t\"--no-merges\" => [ qw(rss atom log shortlog history) ],\n+);\n+\n+# fill %input_params with the CGI parameters. All values except for 'opt'\n+# should be single values, but opt can be an array. We should probably\n+# build an array of parameters that can be multi-valued, but since for the time\n+# being it's only this one, we just single it out\n+sub evaluate_query_params {\n+\twhile (my ($name, $symbol) = each %cgi_param_mapping) {\n+\t\tif ($symbol eq 'opt') {\n+\t\t\t$input_params{$name} = [ $cgi->param($symbol) ];\n+\t\t} else {\n+\t\t\t$input_params{$name} = $cgi->param($symbol);\n+\t\t}\n+\t}\n+}\n+\n+sub evaluate_uri {\n+\tour $cgi;\n+\n+\tour $my_url = $cgi->url();\n+\tour $my_uri = $cgi->url(-absolute => 1);\n+\n+\t# Base URL for relative URLs in gitweb ($Gitweb::Config::logo, $Gitweb::Config::favicon, ...),\n+\t# needed and used only for URLs with nonempty PATH_INFO\n+\tour $base_url = $my_url;\n+\n+\t# When the script is used as DirectoryIndex, the URL does not contain the name\n+\t# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we\n+\t# have to do it ourselves. We make $path_info global because it's also used\n+\t# later on.\n+\t#\n+\t# Another issue with the script being the DirectoryIndex is that the resulting\n+\t# $my_url data is not the full script URL: this is good, because we want\n+\t# generated links to keep implying the script name if it wasn't explicitly\n+\t# indicated in the URL we're handling, but it means that $my_url cannot be used\n+\t# as base URL.\n+\t# Therefore, if we needed to strip PATH_INFO, then we know that we have\n+\t# to build the base URL ourselves:\n+\tour $path_info = $ENV{\"PATH_INFO\"};\n+\tif ($path_info) {\n+\t\tif ($my_url =~ s,\\Q$path_info\\E$,, &&\n+\t\t    $my_uri =~ s,\\Q$path_info\\E$,, &&\n+\t\t    defined $ENV{'SCRIPT_NAME'}) {\n+\t\t\t$base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n+\t\t}\n+\t}\n+\n+\t# target of the home link on top of all pages\n+\tour $home_link = $my_uri || \"/\";\n+}\n+\n+sub evaluate_git_dir {\n+\t$git_dir = \"$projectroot/$project\" if $project;\n+}\n+\n+\n+sub validate_pathname {\n+\tmy $input = shift || return undef;\n+\n+\t# no '.' or '..' as elements of path, i.e. no '.' nor '..'\n+\t# at the beginning, at the end, and between slashes.\n+\t# also this catches doubled slashes\n+\tif ($input =~ m!(^|/)(|\\.|\\.\\.)(/|$)!) {\n+\t\treturn undef;\n+\t}\n+\t# no null characters\n+\tif ($input =~ m!\\0!) {\n+\t\treturn undef;\n+\t}\n+\treturn $input;\n+}\n+\n+sub validate_refname {\n+\tmy $input = shift || return undef;\n+\n+\t# textual hashes are O.K.\n+\tif ($input =~ m/^[0-9a-fA-F]{40}$/) {\n+\t\treturn $input;\n+\t}\n+\t# it must be correct pathname\n+\t$input = validate_pathname($input)\n+\t\tor return undef;\n+\t# restrictions on ref name according to git-check-ref-format\n+\tif ($input =~ m!(/\\.|\\.\\.|[\\000-\\040\\177 ~^:?*\\[]|/$)!) {\n+\t\treturn undef;\n+\t}\n+\treturn $input;\n+}\n+\n+1;\n-- \n1.7.1.447.gfddfb\n"},{"id":"142884","messageId":"1275573356-21466-3-git-send-email-pavan.sss1991@gmail.com","threadId":"23991","inReplyTo":"1275573356-21466-1-git-send-email-pavan.sss1991@gmail.com","subject":"[PATCH 3/3] git-instaweb: Add support for --reuse-config using gitconfig","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-03T13:55:56Z","receivedAt":"2010-06-03T13:55:56Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Support instaweb's --reuse-config using instaweb.overwrite\nvariable in gitconfig.\n\nTo use --reuse-config always\n\t[instaweb]\n\t\toverwrite = false\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n\nThis patch is based on branch 'pu' and my previous patch\nhttp://thread.gmane.org/gmane.comp.version-control.git/148161\n\n Documentation/git-instaweb.txt |    2 ++\n git-instaweb.sh                |    5 ++++-\n 2 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-instaweb.txt b/Documentation/git-instaweb.txt\nindex 0e7e20b..12cbe1d 100644\n--- a/Documentation/git-instaweb.txt\n+++ b/Documentation/git-instaweb.txt\n@@ -77,6 +77,8 @@ You may specify configuration in your .git/config\n \tport = 4321\n \tbrowser = konqueror\n \tmodulepath = /usr/lib/apache2/modules\n+\tgitwebdir = /usr/share/gitweb\n+\toverwrite = false\n \n -----------------------------------------------------------------------\n \ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 1c704a3..3635974 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -28,7 +28,7 @@ httpd=\"$(git config --get instaweb.httpd)\"\n root=\"$(git config --get instaweb.gitwebdir)\"\n port=$(git config --get instaweb.port)\n module_path=\"$(git config --get instaweb.modulepath)\"\n-no_reuse=true\n+no_reuse=\"$(git config --bool --get instaweb.overwrite)\"\n \n conf=\"$GIT_DIR/gitweb/httpd.conf\"\n \n@@ -43,6 +43,9 @@ test -z \"$root\" && root='@@GITWEBDIR@@'\n # any untaken local port will do...\n test -z \"$port\" && port=1234\n \n+# Default is true -> overwrite gitweb_config.perl\n+test -z \"$no_reuse\" && no_reuse=true\n+\n resolve_full_httpd () {\n \tcase \"$httpd\" in\n \t*apache2*|*lighttpd*)\n-- \n1.7.1.447.gfddfb\n"},{"id":"142897","messageId":"20100603152030.GD20775@machine.or.cz","threadId":"23991","inReplyTo":"1275573356-21466-1-git-send-email-pavan.sss1991@gmail.com","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-06-03T15:20:30Z","receivedAt":"2010-06-03T15:20:30Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"  Hi!\n\n  I think this is a good start!\n\n  I have couple of concerns; maybe they were addressed in the previous\ndiscussion which I admit I did not read completely, but in that case\nthey ought to be addressed in the commit message as well.\n\nOn Thu, Jun 03, 2010 at 07:25:54PM +0530, Pavan Kumar Sunkara wrote:\n> -our $t0;\n> -if (eval { require Time::HiRes; 1; }) {\n> -\t$t0 = [Time::HiRes::gettimeofday()];\n\nWhy is this moved to Gitweb::Config? Shouldn't this be rather part of\nGitweb::Request?\n\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> -our $number_of_git_cmds = 0;\n> +use lib __DIR__ . \"/lib\";\n\nWouldn't it be more elegant to use FindBin? I'm just not sure how long\nis it part of core Perl.\n\n> +\n> +use Gitweb::Config;\n>  \n>  BEGIN {\n>  \tCGI->compile() if $ENV{'MOD_PERL'};\n>  }\n>  \n> -our $version = \"++GIT_VERSION++\";\n> +$version = \"++GIT_VERSION++\";\n>  \n>  our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n>  sub evaluate_uri {\n> @@ -68,402 +71,58 @@ sub evaluate_uri {\n>  \n>  # core git executable to use\n>  # this can just be \"git\" if your webserver has a sensible PATH\n> -our $GIT = \"++GIT_BINDIR++/git\";\n> +$GIT = \"++GIT_BINDIR++/git\";\n\nI dislike the new schema in one aspect - the list of configuration\nvariables together with their description is not at a single place\nanymore: the build-time overridable variables have their descriptions\nstill in gitweb.pl and only very brief mentions in Gitweb::Config, while\nthe rest has moved fully to Gitweb::Config. I think it would be best to\nmove all descriptions to Gitweb::Config and keep only the override\nassignments in gitweb.pl. So, Gitweb::Config would have\n\n\t# core git executable to use\n\t# this can just be \"git\" if your webserver has a sensible PATH\n\tour $GIT;\n\nand gitweb.pl would have _just_\n\n\t$GIT = \"++GIT_BINDIR++/git\";\n\nHow does that sound?\n\n\nI think you ought to add a comment in front of this section explaining\nthat not all configuration variables are listed here anymore. Something\nlike\n\n\t# Only configuration variables with build-time overridable\n\t# defaults are listed below. The complete set of variables\n\t# with their descriptions is listed in Gitweb::Config.\n\n>  # name of your site or organization to appear in page titles\n>  # replace this with something more descriptive for clearer bookmarks\n> -our $site_name = \"++GITWEB_SITENAME++\"\n> +$site_name = \"++GITWEB_SITENAME++\"\n>                   || ($ENV{'SERVER_NAME'} || \"Untitled\") . \" Git\";\n\nThis looks like some new feature; please do that in a separate patch.\n(BTW, I assume that there are no other changes like this in the rest of\nthe moved code blocks!)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe true meaning of life is to plant a tree under whose shade\nyou will never sit.\n"},{"id":"142902","messageId":"AANLkTikUmFA658jzd27cu1NmjJsV8T9Hkrd7z2WNY3R7@mail.gmail.com","threadId":"23991","inReplyTo":"20100603152030.GD20775@machine.or.cz","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-03T15:54:34Z","receivedAt":"2010-06-03T15:54:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jun 3, 2010 at 15:20, Petr Baudis <pasky@suse.cz> wrote:\n> On Thu, Jun 03, 2010 at 07:25:54PM +0530, Pavan Kumar Sunkara wrote:\n\n>> -our $t0;\n>> -if (eval { require Time::HiRes; 1; }) {\n>> -     $t0 = [Time::HiRes::gettimeofday()];\n>\n> Why is this moved to Gitweb::Config? Shouldn't this be rather part of\n> Gitweb::Request?\n\n>> +our @ISA = qw(Exporter);\n\nThis is also re-arranging deck chairs on the Titanic, but 'use base\nqw(Exporter)' is nicer.\n\n>> +# __DIR__ is taken from Dir::Self __DIR__ fragment\n>> +sub __DIR__ () {\n>> +     File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);\n>>  }\n>> -our $number_of_git_cmds = 0;\n>> +use lib __DIR__ . \"/lib\";\n>\n> Wouldn't it be more elegant to use FindBin? I'm just not sure how long\n> is it part of core Perl.\n\nNo, those don't do the same thing as discussed in previous\nreviews. FindBin finds the invoked binary, Dir::Self finds the the\ncurrent file.\n"},{"id":"142903","messageId":"201006031755.29814.jnareb@gmail.com","threadId":"23991","inReplyTo":"20100603152030.GD20775@machine.or.cz","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-03T15:55:28Z","receivedAt":"2010-06-03T15:55:28Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 3 Jun 2010, Petr Baudis wrote:\n>\n>   I have couple of concerns; maybe they were addressed in the previous\n> discussion which I admit I did not read completely, but in that case\n> they ought to be addressed in the commit message as well.\n> \n> On Thu, Jun 03, 2010 at 07:25:54PM +0530, Pavan Kumar Sunkara wrote:\n> > -our $t0;\n> > -if (eval { require Time::HiRes; 1; }) {\n> > -\t$t0 = [Time::HiRes::gettimeofday()];\n> \n> Why is this moved to Gitweb::Config? Shouldn't this be rather part of\n> Gitweb::Request?\n\nI also think that this should be either part of Gitweb::Request, oe\neven be left in gitweb.perl.  I think having it in Gitweb::Request\nwould be a better idea, because it is about time (and number of git\ncommands) it took to process request.\n\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> > -our $number_of_git_cmds = 0;\n> > +use lib __DIR__ . \"/lib\";\n> \n> Wouldn't it be more elegant to use FindBin? I'm just not sure how long\n> is it part of core Perl.\n\nActually FindBin is in core since 5.4, and we require Perl 5.8+ for\ngitweb anyway (for Encode and proper Unicode support).  \n\nBut from what I've heard FindBin is not recommended anymore, although\nperhaps the disadvantages of FindBin doesn't matter in our situation.\n\n\n>From FindBin(3pm) manpage:\n\n  KNOWN ISSUES\n\n  If there are two modules using `FindBin` from different directories under\n  the same interpreter, this won't work. Since `FindBin` uses a `BEGIN` block,\n  it'll be executed only once, and only the first caller will get it\n  right.  **This is a problem under mod_perl** and other persistent Perl\n  environments, where you shouldn't use this module.\n\n>From what I remember there might also be problems with symlinks.\n\n\n>From Dir::Self(3pm) manpage:\n\n  DESCRIPTION\n\n  Perl has two pseudo-constants describing the current location in your\n  source code, __FILE__ and __LINE__.  This module adds __DIR__, which\n  expands to the directory your source file is in, as an absolute pathname.\n\n  This is useful if your code wants to access files in the same directory,\n  like helper modules or configuration data.  This is a bit like `FindBin`\n  except it's not limited to the main program, i.e. you can also use it in\n  modules.  **And it actually works.**\n\n\nIs there on git mailing list a Perl hacker that can tell us one way or\nanother?  (Not that this matter much, if it works).\n\n> \n> > +\n> > +use Gitweb::Config;\n> >  \n> >  BEGIN {\n> >  \tCGI->compile() if $ENV{'MOD_PERL'};\n> >  }\n> >  \n> > -our $version = \"++GIT_VERSION++\";\n> > +$version = \"++GIT_VERSION++\";\n\nThis change is not necessary.\n\n  our $version = \"++GIT_VERSION++\";\n\nwould keep working even if '$version' is declared in other module and\nexported by this module (is imported into current scope).\n\n> >  \n> >  our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n> >  sub evaluate_uri {\n> > @@ -68,402 +71,58 @@ sub evaluate_uri {\n> >  \n> >  # core git executable to use\n> >  # this can just be \"git\" if your webserver has a sensible PATH\n> > -our $GIT = \"++GIT_BINDIR++/git\";\n> > +$GIT = \"++GIT_BINDIR++/git\";\n> \n> I dislike the new schema in one aspect - the list of configuration\n> variables together with their description is not at a single place\n> anymore: the build-time overridable variables have their descriptions\n> still in gitweb.pl and only very brief mentions in Gitweb::Config, while\n> the rest has moved fully to Gitweb::Config. I think it would be best to\n> move all descriptions to Gitweb::Config and keep only the override\n> assignments in gitweb.pl. So, Gitweb::Config would have\n> \n> \t# core git executable to use\n> \t# this can just be \"git\" if your webserver has a sensible PATH\n> \tour $GIT;\n\nGood idea.\n\nPerhaps we should provide some sane default fallback values, like for\nexample\n\n  \tour $GIT = \"git\";\n\n> \n> and gitweb.pl would have _just_\n> \n> \t$GIT = \"++GIT_BINDIR++/git\";\n\nI would say\n\n  \tour $GIT = \"++GIT_BINDIR++/git\";\n\n> I think you ought to add a comment in front of this section explaining\n> that not all configuration variables are listed here anymore. Something\n> like\n> \n> \t# Only configuration variables with build-time overridable\n> \t# defaults are listed below. The complete set of variables\n> \t# with their descriptions is listed in Gitweb::Config.\n\nRight.  I wholehartily agree.\n\n> >  # name of your site or organization to appear in page titles\n> >  # replace this with something more descriptive for clearer bookmarks\n> > -our $site_name = \"++GITWEB_SITENAME++\"\n> > +$site_name = \"++GITWEB_SITENAME++\"\n> >                   || ($ENV{'SERVER_NAME'} || \"Untitled\") . \" Git\";\n> \n> This looks like some new feature; please do that in a separate patch.\n> (BTW, I assume that there are no other changes like this in the rest of\n> the moved code blocks!)\n\nNo, it isn't.  And without 'our $var = VALUE' -> '$var = VALUE' change,\nwhich is not necessary and artifically inflates the size of patch, this\nchunk wouldn't even be present.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"142904","messageId":"AANLkTinUrK3zdFCWashnGquEtVovmihT1qYQAM5gHk5X@mail.gmail.com","threadId":"23991","inReplyTo":"1275573356-21466-1-git-send-email-pavan.sss1991@gmail.com","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-03T16:00:09Z","receivedAt":"2010-06-03T16:00:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jun 3, 2010 at 13:55, Pavan Kumar Sunkara\n<pavan.sss1991@gmail.com> wrote:\n> +sub evaluate_gitweb_config {\n> +       # die if there are errors parsing config file\n> +       if (-e $GITWEB_CONFIG) {\n> +               do $GITWEB_CONFIG;\n> +               die $@ if $@;\n> +       } elsif (-e $GITWEB_CONFIG_SYSTEM) {\n> +               do $GITWEB_CONFIG_SYSTEM;\n> +               die $@ if $@;\n> +       }\n> +}\n\nI think I mentioned this before, but why not *optionally* use\nConfig::Any (or something similar) and if it doesn't exists fall back\non do(), and document this, along with a way to disable Perl\nexecution.\n\nIt'd be completely compatible, but admins could then allow someone to\nedit a gitweb config file without opening themselves up to that\nsomeone having permission to execute code as the webserver.\n\nCheck out Gitalist (the Catalyst rewrite of Gitweb) for some prior\nart.\n"},{"id":"142905","messageId":"20100603160605.GF20775@machine.or.cz","threadId":"23991","inReplyTo":"201006031755.29814.jnareb@gmail.com","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-06-03T16:06:05Z","receivedAt":"2010-06-03T16:06:05Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Thu, Jun 03, 2010 at 05:55:28PM +0200, Jakub Narebski wrote:\n> But from what I've heard FindBin is not recommended anymore, although\n> perhaps the disadvantages of FindBin doesn't matter in our situation.\n\nAh, never mind then. It's probably more trouble than it's worth, even if\nit's not problematic for us right at the moment.\n\n> > > +\n> > > +use Gitweb::Config;\n> > >  \n> > >  BEGIN {\n> > >  \tCGI->compile() if $ENV{'MOD_PERL'};\n> > >  }\n> > >  \n> > > -our $version = \"++GIT_VERSION++\";\n> > > +$version = \"++GIT_VERSION++\";\n> \n> This change is not necessary.\n> \n>   our $version = \"++GIT_VERSION++\";\n> \n> would keep working even if '$version' is declared in other module and\n> exported by this module (is imported into current scope).\n\nHmm, that's right, but it feels dirty since it strongly suggests that\n$version is then a variable local to this package. It would reduce the\ndiff size, but I perosnally don't think it's worth it. I'll leave this\nup to Pavan personally, though.\n\n> Perhaps we should provide some sane default fallback values, like for\n> example\n> \n>   \tour $GIT = \"git\";\n\nI considered that, but I'm not too fond of this within this patch,\nI'd rather keep the pieces simple and stupid.\n\n> > >  # name of your site or organization to appear in page titles\n> > >  # replace this with something more descriptive for clearer bookmarks\n> > > -our $site_name = \"++GITWEB_SITENAME++\"\n> > > +$site_name = \"++GITWEB_SITENAME++\"\n> > >                   || ($ENV{'SERVER_NAME'} || \"Untitled\") . \" Git\";\n> > \n> > This looks like some new feature; please do that in a separate patch.\n> > (BTW, I assume that there are no other changes like this in the rest of\n> > the moved code blocks!)\n> \n> No, it isn't.  And without 'our $var = VALUE' -> '$var = VALUE' change,\n> which is not necessary and artifically inflates the size of patch, this\n> chunk wouldn't even be present.\n\nI'm sorry, I was just blind.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe true meaning of life is to plant a tree under whose shade\nyou will never sit.\n"},{"id":"142906","messageId":"AANLkTimoA95U0vivTzrc0XZ8i6q-SfCFA6RgMWK67OWl@mail.gmail.com","threadId":"23991","inReplyTo":"201006031755.29814.jnareb@gmail.com","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-03T16:11:00Z","receivedAt":"2010-06-03T16:11:00Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"2010/6/3 Jakub Narebski <jnareb@gmail.com>:\n> On Tue, 3 Jun 2010, Petr Baudis wrote:\n>>\n>>   I have couple of concerns; maybe they were addressed in the previous\n>> discussion which I admit I did not read completely, but in that case\n>> they ought to be addressed in the commit message as well.\n>>\n>> On Thu, Jun 03, 2010 at 07:25:54PM +0530, Pavan Kumar Sunkara wrote:\n>> > -our $t0;\n>> > -if (eval { require Time::HiRes; 1; }) {\n>> > -   $t0 = [Time::HiRes::gettimeofday()];\n>>\n>> Why is this moved to Gitweb::Config? Shouldn't this be rather part of\n>> Gitweb::Request?\n>\n> I also think that this should be either part of Gitweb::Request, oe\n> even be left in gitweb.perl.  I think having it in Gitweb::Request\n> would be a better idea, because it is about time (and number of git\n> commands) it took to process request.\n\nOk. It will be done.\n\n\n>> > +\n>> > +use Gitweb::Config;\n>> >\n>> >  BEGIN {\n>> >     CGI->compile() if $ENV{'MOD_PERL'};\n>> >  }\n>> >\n>> > -our $version = \"++GIT_VERSION++\";\n>> > +$version = \"++GIT_VERSION++\";\n>\n> This change is not necessary.\n>\n>  our $version = \"++GIT_VERSION++\";\n>\n> would keep working even if '$version' is declared in other module and\n> exported by this module (is imported into current scope).\n\nOk. Will change it.\n\n>> >\n>> >  our ($my_url, $my_uri, $base_url, $path_info, $home_link);\n>> >  sub evaluate_uri {\n>> > @@ -68,402 +71,58 @@ sub evaluate_uri {\n>> >\n>> >  # core git executable to use\n>> >  # this can just be \"git\" if your webserver has a sensible PATH\n>> > -our $GIT = \"++GIT_BINDIR++/git\";\n>> > +$GIT = \"++GIT_BINDIR++/git\";\n>>\n>> I dislike the new schema in one aspect - the list of configuration\n>> variables together with their description is not at a single place\n>> anymore: the build-time overridable variables have their descriptions\n>> still in gitweb.pl and only very brief mentions in Gitweb::Config, while\n>> the rest has moved fully to Gitweb::Config. I think it would be best to\n>> move all descriptions to Gitweb::Config and keep only the override\n>> assignments in gitweb.pl. So, Gitweb::Config would have\n>>\n>>       # core git executable to use\n>>       # this can just be \"git\" if your webserver has a sensible PATH\n>>       our $GIT;\n>\n> Good idea.\n>\n> Perhaps we should provide some sane default fallback values, like for\n> example\n>\n>        our $GIT = \"git\";\n>\n>>\n>> and gitweb.pl would have _just_\n>>\n>>       $GIT = \"++GIT_BINDIR++/git\";\n>\n> I would say\n>\n>        our $GIT = \"++GIT_BINDIR++/git\";\n\nBut, I think when we start reading the code, it would seem that 'our\n$GIT' implies that it is a variable created locally rather than an\nexported variable from Gitweb::Config module.\n\nEven though it increases the patch size, I don't think it will be much\nof a concern when it comes to good redability of code.\n\nJakub: Can you reply, what you think about this argument ?\n\nThanks,\nPavan.\n"},{"id":"142910","messageId":"201006031859.39278.jnareb@gmail.com","threadId":"23991","inReplyTo":"AANLkTikUmFA658jzd27cu1NmjJsV8T9Hkrd7z2WNY3R7@mail.gmail.com","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-03T16:59:38Z","receivedAt":"2010-06-03T16:59:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> On Thu, Jun 3, 2010 at 15:20, Petr Baudis <pasky@suse.cz> wrote:\n>> On Thu, Jun 03, 2010 at 07:25:54PM +0530, Pavan Kumar Sunkara wrote:\n>>>\n>>> +our @ISA = qw(Exporter);\n> \n> This is also re-arranging deck chairs on the Titanic, but 'use base\n> qw(Exporter)' is nicer.\n\nOr simply 'use Exporter qw(import);', as Perl 5.8+ supports \n'use Module LIST' form.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"142913","messageId":"AANLkTikNzhIhrjbgF88EKf_FhJ6p_tzXHi0MaD-EMiLl@mail.gmail.com","threadId":"23991","inReplyTo":"201006031859.39278.jnareb@gmail.com","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-03T17:04:14Z","receivedAt":"2010-06-03T17:04:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jun 3, 2010 at 16:59, Jakub Narebski <jnareb@gmail.com> wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>> On Thu, Jun 3, 2010 at 15:20, Petr Baudis <pasky@suse.cz> wrote:\n>>> On Thu, Jun 03, 2010 at 07:25:54PM +0530, Pavan Kumar Sunkara wrote:\n>>>>\n>>>> +our @ISA = qw(Exporter);\n>>\n>> This is also re-arranging deck chairs on the Titanic, but 'use base\n>> qw(Exporter)' is nicer.\n>\n> Or simply 'use Exporter qw(import);', as Perl 5.8+ supports\n> 'use Module LIST' form.\n\nAh yes, if the code is lucky enough to require 5.8 in the first place.\nI didn't know whether gitweb was one of those things.\n"},{"id":"142916","messageId":"201006032043.14071.jnareb@gmail.com","threadId":"23991","inReplyTo":"AANLkTimoA95U0vivTzrc0XZ8i6q-SfCFA6RgMWK67OWl@mail.gmail.com","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-03T18:43:13Z","receivedAt":"2010-06-03T18:43:13Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Pavan Kumar Sunkara wrote:\n> 2010/6/3 Jakub Narebski <jnareb@gmail.com>:\n>> On Tue, 3 Jun 2010, Petr Baudis wrote:\n\n>>>> +\n>>>> +use Gitweb::Config;\n>>>>\n>>>>  BEGIN {\n>>>>     CGI->compile() if $ENV{'MOD_PERL'};\n>>>>  }\n>>>>\n>>>> -our $version = \"++GIT_VERSION++\";\n>>>> +$version = \"++GIT_VERSION++\";\n>>\n>> This change is not necessary.\n>>\n>>  our $version = \"++GIT_VERSION++\";\n>>\n>> would keep working even if '$version' is declared in other module and\n>> exported by this module (is imported into current scope).\n> \n> Ok. Will change it.\n\n[...]\n>> I would say\n>>\n>>        our $GIT = \"++GIT_BINDIR++/git\";\n> \n> But, I think when we start reading the code, it would seem that 'our\n> $GIT' implies that it is a variable created locally rather than an\n> exported variable from Gitweb::Config module.\n\nFrom `perldoc -f our`:\n\n  An \"our\" declares the listed variables to be valid globals within the\n  enclosing block, file, or \"eval\".  That is, it has the same scoping\n  rules as a \"my\" declaration, but does not create a local variable.\n\n  [...] The package in which the variable is entered is determined at\n  the point of the declaration, [...]\n\n\nSo if the variable already exists in given scope, for example if the\nvariable was imported from other package, the 'our' declaration would\nbe a no-op.\n\n> \n> Even though it increases the patch size, I don't think it will be much\n> of a concern when it comes to good redability of code.\n> \n> Jakub: Can you reply, what you think about this argument ?\n\nBut I agree that first, 'our $var' seems to imply that it is _new_\nvariable declared in current scope, and second if we make a typo in\nvariable name it wouldn't be detected as different from exported\nvariable: 'our' will create new variable.\n\nSo I agree that removing 'our' is a good idea, especially together\nwith putting all variables that should be there in Gitweb::Config\ntogether with comments, even if they are configured during build\nprocess.\n\nPerhaps those declarations in Gitweb::Config should have in-line\ncomment that they are defined in gitweb.cgi / gitweb.perl?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"142917","messageId":"AANLkTilnEtoZmi0GDmaUBk6cVLyLx8JhhEmldqFuhySM@mail.gmail.com","threadId":"23991","inReplyTo":"201006032043.14071.jnareb@gmail.com","subject":"Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-03T18:50:01Z","receivedAt":"2010-06-03T18:50:01Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"On Fri, Jun 4, 2010 at 12:13 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> Pavan Kumar Sunkara wrote:\n>>\n>> Even though it increases the patch size, I don't think it will be much\n>> of a concern when it comes to good redability of code.\n>>\n>> Jakub: Can you reply, what you think about this argument ?\n>\n> But I agree that first, 'our $var' seems to imply that it is _new_\n> variable declared in current scope, and second if we make a typo in\n> variable name it wouldn't be detected as different from exported\n> variable: 'our' will create new variable.\n\nAnd it's hard to detect the typo while debugging.\n\n> So I agree that removing 'our' is a good idea, especially together\n> with putting all variables that should be there in Gitweb::Config\n> together with comments, even if they are configured during build\n> process.\n>\n> Perhaps those declarations in Gitweb::Config should have in-line\n> comment that they are defined in gitweb.cgi / gitweb.perl?\n>\n\nYeah, Sure.\n\nThanks,\nPavan.\n"}]}