Re: [PATCH/RFC] gitweb: allow configurations that change with each request
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:49:14
On Sat, 31 Jul 2010, Jonathan Nieder wrote:
gitolite's contrib/gitweb/gitweb.conf includes:
$ENV{GL_USER} = $cgi->remote_user || "gitweb";
which is useful for setups where a user has to be authenticated
to access certain repos. Perhaps other typical configurations
change per session in other ways, too.
v1.7.2-rc2~6 (gitweb: Move evaluate_gitweb_config out of run_request,
2010-07-05) broke such configurations for a speedup, by loading
the configuration once per FastCGI process.
Probably in the end there should be a way to specify in the
configuration whether a particular installation wants the speedup or
the flexibility. But for now it is easier to just undo the relevant
change.
This partially reverts commit 869d58813b24c74e84c9388041eafcef40cb51e4.Why only *partially* reverts... ...ah, I see, I did "while at it" change fixing timer and number of git commands info for persistent environments (i.e. gitweb run as FastCGI script).
Reported-by: Julio Lajara <redacted> Analysis-by: Jakub Narebski [off-list ref] Signed-off-by: Jonathan Nieder <redacted>
Reluctantly-acked-by: Jakub Narebski [off-list ref]
--- Jakub Narebski wrote:quoted
I wonder if it would be possible to re-enable this feature (which I think is needed to be able to use $cgi->remote_user) but without having all pay the [slight] performance penalty of including (and I think parsing) config file once per each request.I dunno. Maybe this would be a good place to start.
Currently this is non-issue; the eventual performance penalty is I guess
very small.
The problem would be with adding caching support to gitweb. While
default GitwebCache::* caching engine should have low startup cost,
the CHI generic caching interface that one might want to use instead
uses Moose (Mouse with Any::Moose?) for OOP, which imposes some startup
cost. One enables and configures output caching in gitweb config, so
if gitweb config would be read once per run then cache interface could
be started once per run, not once per request.
Nevertheless this change is backwards incompatibile change, and should
probably wait for 1.7.3 (pity that 1.7.2 was so recently released).
One solution I can think of (still backwards incompatibile) would be to
provide $per_request_config variable, which would hold anonymous sub
with parts of config that need to be done per request (this should work
with global variables (our), but I think it wouldn't work with lexical
variables (my)). For example gitolite's contrib/gitweb/gitweb.conf would
then include:
$per_request_config = sub {
$ENV{GL_USER} = $cgi->remote_user || "gitweb";
}
What do you think about it?
quoted hunk ↗ jump to hunk
gitweb/gitweb.perl | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index e0e9532..300c4b1 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl@@ -1060,8 +1060,12 @@ sub run_request { reset_timer(); evaluate_uri(); + evaluate_gitweb_config(); check_loadavg(); + # $projectroot and $projects_list might be set in gitweb config file + $projects_list ||= $projectroot; + evaluate_query_params(); evaluate_path_info(); evaluate_and_validate_params();@@ -1109,12 +1113,8 @@ sub evaluate_argv { sub run { evaluate_argv(); - evaluate_gitweb_config(); evaluate_git_version(); - # $projectroot and $projects_list might be set in gitweb config file - $projects_list ||= $projectroot; - $pre_listen_hook->() if $pre_listen_hook;
That reminds me: '$projects_list ||= $projectroot;' line should be put, I think, inside evaluate_gitweb_config(). But that of course should not be done in this patch. -- Jakub Narebski Poland