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