{"thread":{"id":"24026","subject":"[PATCH/RFC] gitweb: Create Gitweb::Git module","startedAt":"2010-06-06T21:24:11Z","lastAt":"2010-06-07T15:31:35Z","messageCount":5,"participants":["Pavan Kumar Sunkara","Jakub Narebski","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"143108","messageId":"1275859451-21787-1-git-send-email-pavan.sss1991@gmail.com","threadId":"24026","inReplyTo":null,"subject":"[PATCH/RFC] gitweb: Create Gitweb::Git module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-06T21:24:11Z","receivedAt":"2010-06-06T21:24:11Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Create a Gitweb::Git module in 'gitweb/lib/Gitweb/Git.pm'\nto store essential git variables and subs regarding the\ngitweb.perl script\n\nSubroutines moved:\n\tevaluate_git_version\n\tgit_cmd\n\tquote_command\n\nSubroutines yet to move: (Contains not yet packaged subs & vars)\n\tNone\n\nUpdate gitweb/Makefile to install gitweb modules alongside gitweb\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n gitweb/Makefile          |    1 +\n gitweb/gitweb.perl       |   35 +++-----------------------------\n gitweb/lib/Gitweb/Git.pm |   48 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 53 insertions(+), 31 deletions(-)\n create mode 100644 gitweb/lib/Gitweb/Git.pm\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 4343396..fcd4042 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -115,6 +115,7 @@ GITWEB_FILES += static/git-logo.png static/git-favicon.png\n GITWEB_LIB_GITWEB += lib/Gitweb/Config.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/Request.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/Escape.pm\n+GITWEB_LIB_GITWEB += lib/Gitweb/Git.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 e95aaf7..59a65a8 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -24,12 +24,11 @@ sub __DIR__ () {\n }\n use lib __DIR__ . \"/lib\";\n \n+use Gitweb::Git;\n use Gitweb::Config;\n use Gitweb::Request;\n use Gitweb::Escape;\n \n-our $number_of_git_cmds = 0;\n-\n BEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\n }\n@@ -39,9 +38,8 @@ BEGIN {\n # with their descriptions is listed in Gitweb::Config.\n $version = \"++GIT_VERSION++\";\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+#only this variable has it's root in Gitweb::Git\n+$GIT = \"++GIT_BINDIR++/git\";\n \n $projectroot = \"++GITWEB_PROJECTROOT++\";\n $project_maxdepth = \"++GITWEB_PROJECT_MAXDEPTH++\";\n@@ -77,7 +75,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@@ -197,13 +194,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@@ -492,10 +482,8 @@ 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+\t$git_dir = \"$projectroot/$project\" if $project;\n }\n \n our (@snapshot_fmts, $git_avatar);\n@@ -1548,21 +1536,6 @@ sub get_feed_info {\n ## ----------------------------------------------------------------------\n ## git utility subroutines, invoking git commands\n \n-# returns path to the core git executable and the --git-dir parameter as list\n-sub git_cmd {\n-\t$number_of_git_cmds++;\n-\treturn $GIT, '--git-dir='.$git_dir;\n-}\n-\n-# quote the given arguments for passing them to the shell\n-# quote_command(\"command\", \"arg 1\", \"arg with ' and ! characters\")\n-# => \"'command' 'arg 1' 'arg with '\\'' and '\\!' characters'\"\n-# Try to avoid using this function wherever possible.\n-sub quote_command {\n-\treturn join(' ',\n-\t\tmap { my $a = $_; $a =~ s/(['!])/'\\\\$1'/g; \"'$a'\" } @_ );\n-}\n-\n # get HEAD ref of given project as hash\n sub git_get_head_hash {\n \treturn git_get_full_hash(shift, 'HEAD');\ndiff --git a/gitweb/lib/Gitweb/Git.pm b/gitweb/lib/Gitweb/Git.pm\nnew file mode 100644\nindex 0000000..9961e6d\n--- /dev/null\n+++ b/gitweb/lib/Gitweb/Git.pm\n@@ -0,0 +1,48 @@\n+#!/usr/bin/perl\n+#\n+# Gitweb::Git -- gitweb git package\n+#\n+# This program is licensed under the GPLv2\n+\n+package Gitweb::Git;\n+\n+use strict;\n+use warnings;\n+use Exporter qw(import);\n+\n+our @EXPORT = qw($GIT $number_of_git_cmds $git_version $git_dir\n+                 git_cmd quote_command evaluate_git_version);\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+our $number_of_git_cmds = 0;\n+\n+# version of the core git binary\n+our $git_version;\n+\n+# path to the current git repository\n+our $git_dir;\n+\n+# returns path to the core git executable and the --git-dir parameter as list\n+sub git_cmd {\n+\t$number_of_git_cmds++;\n+\treturn $GIT, '--git-dir='.$git_dir;\n+}\n+\n+# quote the given arguments for passing them to the shell\n+# quote_command(\"command\", \"arg 1\", \"arg with ' and ! characters\")\n+# => \"'command' 'arg 1' 'arg with '\\'' and '\\!' characters'\"\n+# Try to avoid using this function wherever possible.\n+sub quote_command {\n+\treturn join(' ',\n+\t\tmap { my $a = $_; $a =~ s/(['!])/'\\\\$1'/g; \"'$a'\" } @_ );\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.450.g21d56\n"},{"id":"143148","messageId":"201006071042.42908.jnareb@gmail.com","threadId":"24026","inReplyTo":"1275859451-21787-1-git-send-email-pavan.sss1991@gmail.com","subject":"Re: [PATCH/RFC] gitweb: Create Gitweb::Git module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-07T08:42:42Z","receivedAt":"2010-06-07T08:42:42Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Summary: minor complaints, mainly about _descriptions_.\n\nOn Sun, 6 June 2010, Pavan Kumar Sunkara wrote:\n\n> Subject: [PATCH/RFC] gitweb: Create Gitweb::Git module\n>\n> Create a Gitweb::Git module in 'gitweb/lib/Gitweb/Git.pm'\n> to store essential git variables and subs regarding the\n> gitweb.perl script\n\nThe pararaph above and the commit description (subject of this mail) do\nnot tell us what does this new module Gitweb::Git is for, what does it\ncontain.  The description of module in header comment is also a bit\nlacking (see my comments below).\n\nI know I suggested, among other forms, the above short form of commit\ndescription, but I think that in this case it is too short.\n\nPerhaps (this is only a proposal):\n\n  gitweb: Create Gitweb::Git module, to run git commands\n\n  Create a Gitweb::Git module in  'gitweb/lib/Gitweb/Git.pm'\n  to deal with running git commands (and also processing output\n  of git commands with external programs) from gitweb.\n\nI think you should also write why $GIT variable is moved to Gitweb::Git,\neven though it is variable which is configured during build, and one\nmight think that it belongs to Gitweb::Config.\n\nPerhaps something like this (it is only a proposal):\n\n  This module is intended as standalone module, which does not require\n  (include) other gitweb' modules to avoid circular dependencies.  That\n  is why it includes $GIT variable, even though this variable is\n  configured during building gitweb.  On the other hand $GIT is more\n  about git configuration, than gitweb configuration.\n\nOr something like that.\n\n> \n> Subroutines moved:\n> \tevaluate_git_version\n> \tgit_cmd\n> \tquote_command\n> \n> Subroutines yet to move: (Contains not yet packaged subs & vars)\n> \tNone\n> \n> Update gitweb/Makefile to install gitweb modules alongside gitweb\n\nIt is not 'gitweb modules', but single gitweb module.\n\n  Update gitweb/Makefile to install Gitweb::Git alongside gitweb.\n\n> \n> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n> ---\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index e95aaf7..59a65a8 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\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> +#only this variable has it's root in Gitweb::Git\n> +$GIT = \"++GIT_BINDIR++/git\";\n\nHmmm... is this comment really needed?  It does not matter, at least not\nmuch, where given subroutine comes from.  Only lack of 'our' indication\nthat it is defined in other package.\n\nPerhaps\n\n  +# $GIT is from Gitweb::Git\n\nor something like that?\n\n> @@ -77,7 +75,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\nNice side-effect.\n\n> @@ -197,13 +194,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\nI guess that evaluate_git_version and $number_of_git_cmds are moved to\nGitweb::Git because of technical reasons (for module to be self\ncontained, and to avoid circular dependencies), isn't it?\n\n> @@ -492,10 +482,8 @@ 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> +\t$git_dir = \"$projectroot/$project\" if $project;\n>  }\n\nO.K.\n\n> diff --git a/gitweb/lib/Gitweb/Git.pm b/gitweb/lib/Gitweb/Git.pm\n> new file mode 100644\n> index 0000000..9961e6d\n> --- /dev/null\n> +++ b/gitweb/lib/Gitweb/Git.pm\n> @@ -0,0 +1,48 @@\n> +#!/usr/bin/perl\n> +#\n> +# Gitweb::Git -- gitweb git package\n> +#\n> +# This program is licensed under the GPLv2\n\nThis description doesn't tell us much.  What does \"git package\" mean?\nI would like to have description here what this package is for, and\nwhet it (should) include.\n\nPerhaps (this is only a proposal):\n\n  +# Gitweb::Git -- gitweb's package dealing with running git commands\n\nor something like that.\n\n> +\n> +package Gitweb::Git;\n> +\n> +use strict;\n> +use warnings;\n> +use Exporter qw(import);\n> +\n> +our @EXPORT = qw($GIT $number_of_git_cmds $git_version $git_dir\n> +                 git_cmd quote_command evaluate_git_version);\n> +\n> +# core git executable to use\n> +# this can just be \"git\" if your webserver has a sensible PATH\n> +our $GIT;\n\nOne could think that this should belong to Gitweb::Config, but it is\nmore about _git_ configuration than about _gitweb_ configuration.\nAnd there are technical reasons for having it there.\n\n> +\n> +our $number_of_git_cmds = 0;\n\nI guess that counting git commands belong there...\n\nBy the way, can anyone check if it is correctly reset, and is counting\nnumber of git commands it took to process _a request_, also when running\nin FastCGI mode?\n\n> +\n> +# version of the core git binary\n> +our $git_version;\n\nHmmm... wouldn't it be better to have this close to evaluate_git_version?\n\nAlso, does $git_version and evaluate_git_version belong in Gitweb::Git?\n\n> +\n> +# path to the current git repository\n> +our $git_dir;\n> +\n> +# returns path to the core git executable and the --git-dir parameter as list\n> +sub git_cmd {\n> +\t$number_of_git_cmds++;\n> +\treturn $GIT, '--git-dir='.$git_dir;\n> +}\n\nO.K.\n\n> +\n> +# quote the given arguments for passing them to the shell\n> +# quote_command(\"command\", \"arg 1\", \"arg with ' and ! characters\")\n> +# => \"'command' 'arg 1' 'arg with '\\'' and '\\!' characters'\"\n> +# Try to avoid using this function wherever possible.\n> +sub quote_command {\n> +\treturn join(' ',\n> +\t\tmap { my $a = $_; $a =~ s/(['!])/'\\\\$1'/g; \"'$a'\" } @_ );\n> +}\n\nO.K.\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> -- \n\n-- \nJakub Narębski\nPoland\n"},{"id":"143164","messageId":"20100607144852.GN20775@machine.or.cz","threadId":"24026","inReplyTo":"1275859451-21787-1-git-send-email-pavan.sss1991@gmail.com","subject":"Re: [PATCH/RFC] gitweb: Create Gitweb::Git module","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-06-07T14:48:52Z","receivedAt":"2010-06-07T14:48:52Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Jun 07, 2010 at 02:54:11AM +0530, Pavan Kumar Sunkara wrote:\n> @@ -39,9 +38,8 @@ BEGIN {\n>  # with their descriptions is listed in Gitweb::Config.\n>  $version = \"++GIT_VERSION++\";\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> +#only this variable has it's root in Gitweb::Git\n> +$GIT = \"++GIT_BINDIR++/git\";\n>  \n>  $projectroot = \"++GITWEB_PROJECTROOT++\";\n>  $project_maxdepth = \"++GITWEB_PROJECT_MAXDEPTH++\";\n\nThat comment is super-cryptic, I'd suggest either rewording it or\ndropping it completely.\n\n> @@ -1548,21 +1536,6 @@ sub get_feed_info {\n>  ## ----------------------------------------------------------------------\n>  ## git utility subroutines, invoking git commands\n\nIs there any reason why didn't you move the rest of the commands from\nthis section to Gitweb::Git as well?\n\n\t\t\t\tPetr \"Pasky\" Baudis\n"},{"id":"143169","messageId":"201006071725.51263.jnareb@gmail.com","threadId":"24026","inReplyTo":"20100607144852.GN20775@machine.or.cz","subject":"Re: [PATCH/RFC] gitweb: Create Gitweb::Git module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-07T15:25:46Z","receivedAt":"2010-06-07T15:25:46Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, Jun 07, 2010, Petr Baudis wrote:\n> On Mon, Jun 07, 2010 at 02:54:11AM +0530, Pavan Kumar Sunkara wrote:\n\n> > @@ -1548,21 +1536,6 @@ sub get_feed_info {\n> >  ## ----------------------------------------------------------------------\n> >  ## git utility subroutines, invoking git commands\n> \n> Is there any reason why didn't you move the rest of the commands from\n> this section to Gitweb::Git as well?\n\nProbably because they are less clear about being about running (git)\ncommands, I guess?\n\nLet's examine those subroutines in more detail:\n\n* git_cmd - requires $GIT and $git_dir, also $number_of_git_cmds\n\n* quote_command - helper command, not exactly about running comands,\n  but about shell escaping / shell quoting.  Should it be in Gitweb::Git\n  or in Gitweb::Escape?\n\n* evaluate_git_version - requires $GIT, sets $number_of_git_cmds\n  and $git_version.  Does it belong to Gitweb::Git, or Gitweb::Config,\n  or perhaps Gitweb::Request?\n\n* git_get_hash (and wrappers: git_get_head_hash, git_get_full_hash,\n  git_get_short_hash) - requires $projectroot, something which other\n  commands do not require, and $git_dir.  Uses git_cmd().\n\n* git_get_type - uses git_cmd().\n\n* git_get_hash_by_path - uses git_cmd()\n* git_get_path_by_hash - uses git_cmd()\n\nSubroutines related to parsing per-repository configuration should be\neither in Gitweb::Config, or in a separate module, e.g. Gitweb::Git::Config\n(or something like that, like Gitweb::RepoConfig, etc.).\n\nNext there are 'git utility functions, directly accessing git repository'\n\n-- \nJakub Narebski\nPoland\n"},{"id":"143170","messageId":"AANLkTilYX3EKifncmM0U2PTWhww6LFYqOiK3P5fi0Zk_@mail.gmail.com","threadId":"24026","inReplyTo":"201006071042.42908.jnareb@gmail.com","subject":"Re: [PATCH/RFC] gitweb: Create Gitweb::Git module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-07T15:31:35Z","receivedAt":"2010-06-07T15:31:35Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"2010/6/7 Jakub Narebski <jnareb@gmail.com>:\n> Summary: minor complaints, mainly about _descriptions_.\n>\n> On Sun, 6 June 2010, Pavan Kumar Sunkara wrote:\n>\n>> Subject: [PATCH/RFC] gitweb: Create Gitweb::Git module\n>>\n>> Create a Gitweb::Git module in 'gitweb/lib/Gitweb/Git.pm'\n>> to store essential git variables and subs regarding the\n>> gitweb.perl script\n>\n> The pararaph above and the commit description (subject of this mail) do\n> not tell us what does this new module Gitweb::Git is for, what does it\n> contain.  The description of module in header comment is also a bit\n> lacking (see my comments below).\n>\n> I know I suggested, among other forms, the above short form of commit\n> description, but I think that in this case it is too short.\n>\n> Perhaps (this is only a proposal):\n>\n>  gitweb: Create Gitweb::Git module, to run git commands\n>\n>  Create a Gitweb::Git module in  'gitweb/lib/Gitweb/Git.pm'\n>  to deal with running git commands (and also processing output\n>  of git commands with external programs) from gitweb.\n>\n> I think you should also write why $GIT variable is moved to Gitweb::Git,\n> even though it is variable which is configured during build, and one\n> might think that it belongs to Gitweb::Config.\n>\n> Perhaps something like this (it is only a proposal):\n>\n>  This module is intended as standalone module, which does not require\n>  (include) other gitweb' modules to avoid circular dependencies.  That\n>  is why it includes $GIT variable, even though this variable is\n>  configured during building gitweb.  On the other hand $GIT is more\n>  about git configuration, than gitweb configuration.\n>\n> Or something like that.\n\nOk.\n\n>>\n>> Subroutines moved:\n>>       evaluate_git_version\n>>       git_cmd\n>>       quote_command\n>>\n>> Subroutines yet to move: (Contains not yet packaged subs & vars)\n>>       None\n>>\n>> Update gitweb/Makefile to install gitweb modules alongside gitweb\n>\n> It is not 'gitweb modules', but single gitweb module.\n>\n>  Update gitweb/Makefile to install Gitweb::Git alongside gitweb.\n>\n>>\n>> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n>> ---\n>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index e95aaf7..59a65a8 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\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>> +#only this variable has it's root in Gitweb::Git\n>> +$GIT = \"++GIT_BINDIR++/git\";\n>\n> Hmmm... is this comment really needed?  It does not matter, at least not\n> much, where given subroutine comes from.  Only lack of 'our' indication\n> that it is defined in other package.\n>\n> Perhaps\n>\n>  +# $GIT is from Gitweb::Git\n>\n> or something like that?\n\nOk.\n\n>> @@ -77,7 +75,6 @@ sub gitweb_get_feature {\n>>               $feature{$name}{'override'},\n>>               @{$feature{$name}{'default'}});\n>>       # project specific override is possible only if we have project\n>> -     our $git_dir; # global variable, declared later\n>>       if (!$override || !defined $git_dir) {\n>>               return @defaults;\n>>       }\n>\n> Nice side-effect.\n>\n>> @@ -197,13 +194,6 @@ sub get_loadavg {\n>>       return 0;\n>>  }\n>>\n>> -# version of the core git binary\n>> -our $git_version;\n>> -sub evaluate_git_version {\n>> -     our $git_version = qx(\"$GIT\" --version) =~ m/git version (.*)$/ ? $1 : \"unknown\";\n>> -     $number_of_git_cmds++;\n>> -}\n>\n> I guess that evaluate_git_version and $number_of_git_cmds are moved to\n> Gitweb::Git because of technical reasons (for module to be self\n> contained, and to avoid circular dependencies), isn't it?\n\nYeah. Exactly.\n\n>> @@ -492,10 +482,8 @@ sub evaluate_and_validate_params {\n>>       }\n>>  }\n>>\n>> -# path to the current git repository\n>> -our $git_dir;\n>>  sub evaluate_git_dir {\n>> -     our $git_dir = \"$projectroot/$project\" if $project;\n>> +     $git_dir = \"$projectroot/$project\" if $project;\n>>  }\n>\n> O.K.\n>\n>> diff --git a/gitweb/lib/Gitweb/Git.pm b/gitweb/lib/Gitweb/Git.pm\n>> new file mode 100644\n>> index 0000000..9961e6d\n>> --- /dev/null\n>> +++ b/gitweb/lib/Gitweb/Git.pm\n>> @@ -0,0 +1,48 @@\n>> +#!/usr/bin/perl\n>> +#\n>> +# Gitweb::Git -- gitweb git package\n>> +#\n>> +# This program is licensed under the GPLv2\n>\n> This description doesn't tell us much.  What does \"git package\" mean?\n> I would like to have description here what this package is for, and\n> whet it (should) include.\n>\n> Perhaps (this is only a proposal):\n>\n>  +# Gitweb::Git -- gitweb's package dealing with running git commands\n>\n> or something like that.\n\nOk.\n\n>> +\n>> +package Gitweb::Git;\n>> +\n>> +use strict;\n>> +use warnings;\n>> +use Exporter qw(import);\n>> +\n>> +our @EXPORT = qw($GIT $number_of_git_cmds $git_version $git_dir\n>> +                 git_cmd quote_command evaluate_git_version);\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> One could think that this should belong to Gitweb::Config, but it is\n> more about _git_ configuration than about _gitweb_ configuration.\n> And there are technical reasons for having it there.\n>\n>> +\n>> +our $number_of_git_cmds = 0;\n>\n> I guess that counting git commands belong there...\n>\n> By the way, can anyone check if it is correctly reset, and is counting\n> number of git commands it took to process _a request_, also when running\n> in FastCGI mode?\n>\n>> +\n>> +# version of the core git binary\n>> +our $git_version;\n>\n> Hmmm... wouldn't it be better to have this close to evaluate_git_version?\n>\n> Also, does $git_version and evaluate_git_version belong in Gitweb::Git?\n\nYes because it is regarding git rather than gitweb.\n\nThanks,\nPavan.\n"}]}