On Wed, 24 Nov 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@xxxxxxxxx> writes:
>
> > The default value for $per_request_config is 1 (true), which means that
> > old configuration that require to change per session (like gitolite's)
> > will keep working.
> >
> > Because there is a call to evaluate_gitweb_config() in the run() subroutine
> > (the call in run_config() is not invoked at first request to avoid double
> > running it), therefore evaluate_git_version() could be moved back there, and
> > is now evaluated only once.
> >
> > Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx>
>
> I like the "we can keep the old behaviour by default, but you can go
> fancier if you wanted to" attitude of this patch.
Thanks.
> I however am not sure if this particular implementation is regression
> free. People who wanted the per-request configuration used to be able to
> access what evaluate_uri() did while running evaluate_gitweb_config(),
> which in turn means that their custom configuration could take things like
> my-url and path-info into account, but now for the first invocation they
> cannot. It probably is just the matter of moving some code around from
> their old custom gitweb-config to a per_request_config sub they need to
> write and point $per_request_config variable at, but that makes "setting 1
> will keep the old behaviour" a false promise, no?
Thanks for catching this; I forgot to take into account the fact that
gitweb config changing with request would need request-related variables.
So instead of code flow looking like this
evaluate_gitweb_config();
$first_request = 1;
while (...) {
if ($per_request_config) {
if (ref($per_request_config) eq 'CODE') {
$per_request_config->();
} elsif (!$first_request) {
evaluate_gitweb_config();
}
}
$first_request = 0;
}
we would move to
$first_request = 1;
while (...) {
if ($first_request) {
evaluate_gitweb_config();
} elsif ($per_request_config) {
if (ref($per_request_config) eq 'CODE') {
$per_request_config->();
} else {
evaluate_gitweb_config();
}
}
$first_request = 0;
}
I'll send new version soon.
P.S. if we could assume that nobody ever edits gitweb.cgi directly
to set $per_request_config to non-default value, we could probably
get rid of $first_request variable...
> > @@ -1068,12 +1076,18 @@ sub reset_timer {
> > our $number_of_git_cmds = 0;
> > }
> >
> > +our $first_request = 1;
> > sub run_request {
> > reset_timer();
> >
> > evaluate_uri();
> > - evaluate_gitweb_config();
> > - evaluate_git_version();
> > + if ($per_request_config) {
> > + if (ref($per_request_config) eq 'CODE') {
> > + $per_request_config->();
> > + } elsif (!$first_request) {
> > + evaluate_gitweb_config();
> > + }
> > + }
> > check_loadavg();
> >
> > # $projectroot and $projects_list might be set in gitweb config file
> > @@ -1126,6 +1140,10 @@ sub evaluate_argv {
> >
> > sub run {
> > evaluate_argv();
> > + evaluate_gitweb_config();
> > + evaluate_git_version();
> > +
> > + $first_request = 1;
> >
> > $pre_listen_hook->()
> > if $pre_listen_hook;
> > @@ -1139,6 +1157,7 @@ sub run {
> >
> > $post_dispatch_hook->()
> > if $post_dispatch_hook;
> > + $first_request = 0;
> >
> > last REQUEST if ($is_last_request->());
> > }
>
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html