Re: [PATCH/RFC] gitweb/README: About $base_url etc. and $per_request_config

From: Jakub Narebski <jnareb@gmail.com>
Date: Nov 29, 2010, 00:51 UTC
Message-ID: <201011290151.14341.jnareb@gmail.com>
In-Reply-To: <20101129001908.GA26358@burratino>

Subject fixed.

On Mon, 29 Nov 2010, Jonathan Nieder wrote:

Show 10 quoted lines

> Jakub Narebski wrote:
> 
> > If $base_url was defined, then do not redefine it in evaluate_uri().
> 
> How about $my_uri and $my_url?
> 
> What happens if $ENV{PATH_INFO} or $cgi->url(...) changes between
> requests?  This is partly my ignorance: perhaps FastCGI et al
> guarantee that such configuration changes can't happen within a single
> process?

I think that at least $ENV{PATH_INFO} can change with request, though I guess that $base_url should not change from request to request... though there might be some corner cases.

$my_uri and $my_url usually do change with each request...

Show 20 quoted lines

>
> Maintaining backward compatibility while avoiding this last concern
> seems hard.  Since gitweb_config can contain something like
> 
> 	$my_uri =~ s/foo/bar/;
> 
> one would want to populate $my_uri in advance.  Meanwhile, if the
> default for $my_uri could change between requests, we would want to be
> able to detect the case when $my_uri is not set.  But what if
> gitweb_config contains
> 
> 	$my_uri = "something";
> 
> where "something" happens to match the default value for $my_uri in
> the first request?
> 
> It is tempting to change the documentation now and worry about code
> changes later.  Something like this?
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

..therefore I think it is a better solution.

Acked-by: Jakub Narebski <jnareb@gmail.com>

Show 12 quoted lines

> ---
> diff --git a/gitweb/README b/gitweb/README
> index 6646fda..a9421e0 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -177,13 +177,15 @@ not include variables usually directly set during build):
>   * $my_url, $my_uri
>     Full URL and absolute URL of gitweb script;
>     in earlier versions of gitweb you might have need to set those
> -   variables, now there should be no need to do it.
> +   variables, now there should be no need to do it.  See
> +   $per_request_config if you need to set them still.

Very minor nitpick: perhaps it would be better to use

      variables, now there should be no need to do it.
  +   See $per_request_config if you need to set them still.

Show 19 quoted lines

>   * $base_url
>     Base URL for relative URLs in pages generated by gitweb,
>     (e.g. $logo, $favicon, @stylesheets if they are relative URLs),
>     needed and used only for URLs with nonempty PATH_INFO via
>     <base href="$base_url">.  Usually gitweb sets its value correctly,
>     and there is no need to set this variable, e.g. to $my_uri or "/".
> +   See $per_request_config if you need to set it anyway.
>   * $home_link
>     Target of the home link on top of all pages (the first part of view
>     "breadcrumbs").  By default set to absolute URI of a page ($my_uri).
> @@ -252,7 +254,10 @@ not include variables usually directly set during build):
>       sub { $ENV{GL_USER} = $cgi->remote_user || "gitweb"; }
>     Otherwise it is treated as boolean value: if true gitweb would process
>     config file once per request, if false it would process config file only
> -   once.  The default is true.
> +   once.  Note: $my_url, $my_uri, and $base_url are overwritten with
> +   their default values before every request, so if you want to change
> +   them, be sure to set this variable to true or a code reference effecting
> +   the desired changes.  The default is true.

Perhaps:

      once.  The default is true.
  +   Note: $my_url, $my_uri, and $base_url are overwritten with
  +   their default values before every request, so if you want to change
  +   them, be sure to set this variable to true or a code reference effecting
  +   the desired changes.

-- 
Jakub Narebski
Poland