git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
       [not found] <20101128081048.13668.67286.reportbug@sb74.startrek>
@ 2010-11-28 16:27 ` Jonathan Nieder
  2010-11-28 17:25   ` Giuseppe Bilotta
  0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Nieder @ 2010-11-28 16:27 UTC (permalink / raw)
  To: git; +Cc: Daniel Reichelt, Jakub Narebski, Giuseppe Bilotta

Hi Jakub et al,

Daniel Reichelt wrote[1]:

> I just noticed that integrating gitweb via reverse proxy is impossible when
> path_info is enabled in gitweb.conf. The base href sent on gitweb.cgi:3427
> contains the "internal" URL called by the reverse proxy mechanism, not the
> original one called by the user agent which makes it impossible for the
> client to display CSS, images, etc...
> 
> I suggest an additional config variable, e.g.
> $feature{'pathinfo'}{'basehrefoverride'} which could override the base href
> tag determinted by the cgi script (or disable sending a base href tag at all
> - at least I was able to achieve my desired setup by adjusting the URLs for
> CSS etc in gitweb.conf to fitting absolute URLs).

Any advice for Daniel?  Is it a good idea?

Regards,
Jonathan

[1] http://bugs.debian.org/605217

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 16:27 ` gitweb: false base href sent when integrated via reverse proxy and path_info is active Jonathan Nieder
@ 2010-11-28 17:25   ` Giuseppe Bilotta
  2010-11-28 17:47     ` Jakub Narebski
  0 siblings, 1 reply; 15+ messages in thread
From: Giuseppe Bilotta @ 2010-11-28 17:25 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Daniel Reichelt, Jakub Narebski

On Sun, Nov 28, 2010 at 5:27 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi Jakub et al,
>
> Daniel Reichelt wrote[1]:
>
>> I just noticed that integrating gitweb via reverse proxy is impossible when
>> path_info is enabled in gitweb.conf. The base href sent on gitweb.cgi:3427
>> contains the "internal" URL called by the reverse proxy mechanism, not the
>> original one called by the user agent which makes it impossible for the
>> client to display CSS, images, etc...
>>
>> I suggest an additional config variable, e.g.
>> $feature{'pathinfo'}{'basehrefoverride'} which could override the base href
>> tag determinted by the cgi script (or disable sending a base href tag at all
>> - at least I was able to achieve my desired setup by adjusting the URLs for
>> CSS etc in gitweb.conf to fitting absolute URLs).
>
> Any advice for Daniel?  Is it a good idea?

I'm not familiar with the way reverse proxies operate. Is there some
information that the script can scrape to understand that its request
is being reverse-proxied? Or are there options that the reverse
proxies can be configured with to pretend that the URL is not being
rewritten? These would be better solutions.

Lacking that, a plain

our $base_url  = 'whatever';

in the gitweb config should probably work, as the gitweb config is
evaluated _after_ the internal URI variables are set. Can the bug
submitter confirm that this does indeed work?

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 17:25   ` Giuseppe Bilotta
@ 2010-11-28 17:47     ` Jakub Narebski
  2010-11-28 18:12       ` Jonathan Nieder
  2010-11-28 20:30       ` Daniel Reichelt
  0 siblings, 2 replies; 15+ messages in thread
From: Jakub Narebski @ 2010-11-28 17:47 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: Jonathan Nieder, git, Daniel Reichelt

On Sun, 28 Nov 2010, Giuseppe Bilotta wrote:
> On Sun, Nov 28, 2010 at 5:27 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Hi Jakub et al,
>>
>> Daniel Reichelt wrote[1]:
>>
>>> I just noticed that integrating gitweb via reverse proxy is impossible when
>>> path_info is enabled in gitweb.conf. The base href sent on gitweb.cgi:3427
>>> contains the "internal" URL called by the reverse proxy mechanism, not the
>>> original one called by the user agent which makes it impossible for the
>>> client to display CSS, images, etc...
>>>
>>> I suggest an additional config variable, e.g.
>>> $feature{'pathinfo'}{'basehrefoverride'} which could override the base href
>>> tag determinted by the cgi script (or disable sending a base href tag at all
>>> - at least I was able to achieve my desired setup by adjusting the URLs for
>>> CSS etc in gitweb.conf to fitting absolute URLs).
>>
>> Any advice for Daniel?  Is it a good idea?
> 
> I'm not familiar with the way reverse proxies operate. Is there some
> information that the script can scrape to understand that its request
> is being reverse-proxied? Or are there options that the reverse
> proxies can be configured with to pretend that the URL is not being
> rewritten? These would be better solutions.
> 
> Lacking that, a plain
> 
> our $base_url  = 'whatever';
> 
> in the gitweb config should probably work, as the gitweb config is
> evaluated _after_ the internal URI variables are set. Can the bug
> submitter confirm that this does indeed work?

See also gitweb/README, the "Gitweb config file variables" section:

 * $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 "/".

The key word here is "usually" ;-)

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 17:47     ` Jakub Narebski
@ 2010-11-28 18:12       ` Jonathan Nieder
  2010-11-30 18:22         ` Jakub Narebski
  2010-11-28 20:30       ` Daniel Reichelt
  1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Nieder @ 2010-11-28 18:12 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Giuseppe Bilotta, git, Daniel Reichelt

Jakub Narebski wrote:

> See also gitweb/README, the "Gitweb config file variables" section:
> 
>  * $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 "/".
> 
> The key word here is "usually" ;-)

Thanks, all.  That helps.

No time for it at the moment, but I'm taking this as evidence that
we need a gitweb.conf(5) manual page. :)  Would you be interested
in such a thing (as POD or asciidoc)?

Jonathan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 17:47     ` Jakub Narebski
  2010-11-28 18:12       ` Jonathan Nieder
@ 2010-11-28 20:30       ` Daniel Reichelt
  2010-11-28 21:07         ` Jakub Narebski
  2010-11-28 21:10         ` Jonathan Nieder
  1 sibling, 2 replies; 15+ messages in thread
From: Daniel Reichelt @ 2010-11-28 20:30 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Giuseppe Bilotta, Jonathan Nieder, git

Hi all

>> I'm not familiar with the way reverse proxies operate. Is there some
>> information that the script can scrape to understand that its request
>> is being reverse-proxied?

Nope. Reverse proxy works like this:
- a "public" apache VirtualHost receives an http(s) request for a
certain URL /a/b/c... and has the location /a/.* configured as a
revProxy location
- the public vhost repeats that request to the revProxy target
- the revProxy target receives a request from the "public" vhost and
serves it as usual (this is were the gitweb.cgi script actually is run,
only seeing the request made by the public vhost, not the initially
requesting client, thus all information about the publicly visible git
URI is lost and never gets passed on to gitweb.cgi)
- the public vhost receives the response and forwards it to the client,
pretending he himself had answered the request


>> Lacking that, a plain
>>
>> our $base_url  = 'whatever';
>>
>> in the gitweb config should probably work

Nope again, I'm afraid it doesn't (see further down)


> See also gitweb/README, the "Gitweb config file variables" section:
> 
>  * $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 "/".
> 
> The key word here is "usually" ;-)
> 

*oops* thank you all for the hint! I totally missed that.

However, I just tried that and it failed. $base_url gets ignored in
gitweb.conf and even setting $my_url and $my_uri in gitweb.conf seems to
have no effect at all. For testing purposes I printed the relevant
variables to the html header:


gitweb.conf:
************
our $feature{'pathinfo'}{'default'} = [1];
our $base_url = "https://foobar";
our $my_url = "https://foo";
our $my_uri = "https://bar";
************

gitweb.cgi:3424
************
# the stylesheet, favicon etc urls won't work correctly with path_info
# unless we set the appropriate base URL
if ($ENV{'PATH_INFO'}) {
        print "<base href=\"".esc_url($base_url)."\" />\n";
}
print "<!--
$base_url
$my_url
$my_uri
-->";
************


- git repo listing
public url: https://sb74/projects/gitweb
revProxy url: https://localhost:446/projects/gitweb
************
<head>
<meta http-equiv="content-type" content="application/xhtml+xml;
charset=utf-8"/>
<meta name="generator" content="gitweb/1.7.2.3 git/1.7.2.3"/>
<meta name="robots" content="index, nofollow"/>
<title>localhost Git</title>
<!--
		https://sb74:446/projects/gitweb
		https://sb74:446/projects/gitweb
		/projects/gitweb
		--><link rel="stylesheet" type="text/css"
href="/projects/gitweb/gitweb/gitweb.css"/>
<link rel="alternate" title="localhost Git projects list"
href="/projects/gitweb?a=project_index" type="text/plain; charset=utf-8" />
<link rel="alternate" title="localhost Git projects feeds"
href="/projects/gitweb?a=opml" type="text/x-opml" />
<link rel="shortcut icon" href="/projects/gitweb/gitweb/git-favicon.png"
type="image/png" />
</head>
************


- git summary of repo "test1"
public url: https://sb74/projects/gitweb/test1/summary
revProxy url: https://localhost:446/projects/gitweb/test1/summary
************
<head>
<meta http-equiv="content-type" content="application/xhtml+xml;
charset=utf-8"/>
<meta name="generator" content="gitweb/1.7.2.3 git/1.7.2.3"/>
<meta name="robots" content="index, nofollow"/>
<title>localhost Git - test1/summary</title>
<base href="https://sb74:446/projects/gitweb" />
<!--
		https://sb74:446/projects/gitweb
		https://sb74:446/projects/gitweb
		/projects/gitweb
		--><link rel="stylesheet" type="text/css"
href="/projects/gitweb/gitweb/gitweb.css"/>
<link rel="alternate" title="test1 - log - RSS feed"
href="/projects/gitweb/test1/rss" type="application/rss+xml" />
<link rel="alternate" title="test1 - log - RSS feed (no merges)"
href="/projects/gitweb/test1/rss?opt=--no-merges"
type="application/rss+xml" />
<link rel="alternate" title="test1 - log - Atom feed"
href="/projects/gitweb/test1/atom?opt=--no-merges"
type="application/atom+xml" />
<link rel="alternate" title="test1 - log - Atom feed (no merges)"
href="/projects/gitweb/test1/atom?opt=--no-merges"
type="application/atom+xml" />

<link rel="shortcut icon" href="/projects/gitweb/gitweb/git-favicon.png"
type="image/png" />
</head>
************

Any ideas?

Regards
Daniel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 20:30       ` Daniel Reichelt
@ 2010-11-28 21:07         ` Jakub Narebski
  2010-11-28 21:25           ` Daniel Reichelt
  2010-11-28 21:10         ` Jonathan Nieder
  1 sibling, 1 reply; 15+ messages in thread
From: Jakub Narebski @ 2010-11-28 21:07 UTC (permalink / raw)
  To: Daniel Reichelt; +Cc: Giuseppe Bilotta, Jonathan Nieder, git

On Sun, 28 Nov 2010, Daniel Reichelt wrote:

>>> Lacking that, a plain
>>>
>>> our $base_url  = 'whatever';
>>>
>>> in the gitweb config should probably work
> 
> Nope again, I'm afraid it doesn't (see further down)

Strange, it works for me (see below). 
 
>> See also gitweb/README, the "Gitweb config file variables" section:
>> 
>>  * $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 "/".
>> 
>> The key word here is "usually" ;-)
>> 
> 
> *oops* thank you all for the hint! I totally missed that.
> 
> However, I just tried that and it failed. $base_url gets ignored in
> gitweb.conf and even setting $my_url and $my_uri in gitweb.conf seems to
> have no effect at all. For testing purposes I printed the relevant
> variables to the html header:
> 
> 
> gitweb.conf:
> ************

The default name of gitweb config file is gitweb_config.perl, not 
gitweb.conf.  Are you sure you are picking correct config file?


> our $feature{'pathinfo'}{'default'} = [1];
> our $base_url = "https://foobar";
> our $my_url = "https://foo";
> our $my_uri = "https://bar";

Try adding

  our $site_name = "foo";

to check if you are picking correct config file.

> - git summary of repo "test1"
> public url: https://sb74/projects/gitweb/test1/summary
> revProxy url: https://localhost:446/projects/gitweb/test1/summary
> ************
> <head>
> <meta http-equiv="content-type" content="application/xhtml+xml; charset=utf-8"/>
> <meta name="generator" content="gitweb/1.7.2.3 git/1.7.2.3"/>
> <meta name="robots" content="index, nofollow"/>
> <title>localhost Git - test1/summary</title>
> <base href="https://sb74:446/projects/gitweb" />
[...]
> </head>

I get the following when running with config file that contains:

  our $version = "current";
  [...]
  our $site_name = "[localhost]";
  [...]
  our $base_url = "https://localhost/gitweb/";

$ gitweb-run.sh "" "/git.git"
  [...]
  <head>
  <meta http-equiv="content-type" content="text/html; charset=utf-8"/>
  <meta name="generator" content="gitweb/current git/1.7.3.2.171.g8ccd7"/>
  <meta name="robots" content="index, nofollow"/>
  <title>[localhost] - git.git/summary</title>
  <base href="https://localhost/gitweb/" />
  [...]

So it works for me.
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 20:30       ` Daniel Reichelt
  2010-11-28 21:07         ` Jakub Narebski
@ 2010-11-28 21:10         ` Jonathan Nieder
  2010-11-28 21:28           ` Daniel Reichelt
  2010-11-28 22:05           ` Jakub Narebski
  1 sibling, 2 replies; 15+ messages in thread
From: Jonathan Nieder @ 2010-11-28 21:10 UTC (permalink / raw)
  To: Daniel Reichelt; +Cc: Jakub Narebski, Giuseppe Bilotta, git

Daniel Reichelt wrote:

> However, I just tried that and it failed. $base_url gets ignored in
> gitweb.conf and even setting $my_url and $my_uri in gitweb.conf seems to
> have no effect at all.

Aha.  The bug fixed by v1.7.3-rc0~85^2 (gitweb: allow configurations
that change with each request, 2010-07-30) strikes again.

Daniel, could you try again with version 1:1.7.2.3-2.1 (from Debian sid)?

Jakub, we should probably tweak evaluate_uri to do something special if
$base_url is already set, or advertise that $base_url needs to be set
in per_request_config when that feature is enabled.

Thanks again.
Jonathan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 21:07         ` Jakub Narebski
@ 2010-11-28 21:25           ` Daniel Reichelt
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Reichelt @ 2010-11-28 21:25 UTC (permalink / raw)
  Cc: Giuseppe Bilotta, Jonathan Nieder, git

> The default name of gitweb config file is gitweb_config.perl, not
> gitweb.conf.  Are you sure you are picking correct config file?

Suppose this is Debian-specific:
************
our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM);
sub evaluate_gitweb_config {
        our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "gitweb_config.perl";
        our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} ||
"/etc/gitweb.conf";
        # 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 $@;
        }
}
************


> Try adding
>
>   our $site_name = "foo";
>
> to check if you are picking correct config file.

Yap, $site_name shows up in my browser's title.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 21:10         ` Jonathan Nieder
@ 2010-11-28 21:28           ` Daniel Reichelt
  2010-11-28 22:05           ` Jakub Narebski
  1 sibling, 0 replies; 15+ messages in thread
From: Daniel Reichelt @ 2010-11-28 21:28 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Jakub Narebski, Giuseppe Bilotta, git

> Aha.  The bug fixed by v1.7.3-rc0~85^2 (gitweb: allow configurations
> that change with each request, 2010-07-30) strikes again.
>
> Daniel, could you try again with version 1:1.7.2.3-2.1 (from Debian sid)?

Yay - now setting $base_url in gitweb.conf works (and on a side note
$my_url and $my_uri as well). Thanks to all of you guys for helping in
tracking this down.

Cheers
Daniel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 21:10         ` Jonathan Nieder
  2010-11-28 21:28           ` Daniel Reichelt
@ 2010-11-28 22:05           ` Jakub Narebski
  2010-11-29  0:19             ` [PATCH/RFC] gitweb: Preserve $base_url if it was set Jonathan Nieder
  1 sibling, 1 reply; 15+ messages in thread
From: Jakub Narebski @ 2010-11-28 22:05 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Daniel Reichelt, Giuseppe Bilotta, git

On Sun, 28 Nov 2010, Jonathan Nieder wrote:
> Daniel Reichelt wrote:
> 
> > However, I just tried that and it failed. $base_url gets ignored in
> > gitweb.conf and even setting $my_url and $my_uri in gitweb.conf seems to
> > have no effect at all.
> 
> Aha.  The bug fixed by v1.7.3-rc0~85^2 (gitweb: allow configurations
> that change with each request, 2010-07-30) strikes again.
> 
> Daniel, could you try again with version 1:1.7.2.3-2.1 (from Debian sid)?
> 
> Jakub, we should probably tweak evaluate_uri to do something special if
> $base_url is already set, or advertise that $base_url needs to be set
> in per_request_config when that feature is enabled.

Jonathan, something like this, perhaps?

-- >8 ---- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH/RFC] gitweb: Preserve $base_url if it was set

If $base_url was defined, then do not redefine it in evaluate_uri().
This matters only in the case when $base_url was set in gitweb config
file, and $per_request_config is false or is coderef; without this
change $base_url would get overwritten on subsequent requests from the
value set in config.

If you need $base_url that can change with request, but the default
value set by gitweb doesn't work for you, and you want most of gitweb
config file to be evaluated only once, you need to use $per_request_config
coderef to set it.

Possible-issue-reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
On top of (gitweb: selectable configurations that change with each
request, 2010-11-28).

 gitweb/gitweb.perl |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1d94718..5efee0d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -36,10 +36,9 @@ sub evaluate_uri {
 	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;
-
+	# $base_url contains base URL for relative URLs in gitweb ($logo, $favicon, ...);
+	# it is needed and it is used only for URLs with nonempty PATH_INFO
+	#
 	# 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
@@ -57,7 +56,11 @@ sub evaluate_uri {
 		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'};
+			# preserve existing $base_url
+			$base_url ||= $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};
+		} else {
+			# in case e.g. $ENV{'SCRIPT_NAME'} is not defined
+			$base_url ||= $my_url;
 		}
 	}
 
-- 
1.7.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH/RFC] gitweb: Preserve $base_url if it was set
  2010-11-28 22:05           ` Jakub Narebski
@ 2010-11-29  0:19             ` Jonathan Nieder
  2010-11-29  0:51               ` [PATCH/RFC] gitweb/README: About $base_url etc. and $per_request_config Jakub Narebski
  2010-11-29 17:57               ` [PATCH/RFC] gitweb: Preserve $base_url if it was set Junio C Hamano
  0 siblings, 2 replies; 15+ messages in thread
From: Jonathan Nieder @ 2010-11-29  0:19 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Daniel Reichelt, Giuseppe Bilotta, git

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?

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>
---
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.
  * $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.
 
 Projects list file format
 ~~~~~~~~~~~~~~~~~~~~~~~~~

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH/RFC] gitweb/README: About $base_url etc. and $per_request_config
  2010-11-29  0:19             ` [PATCH/RFC] gitweb: Preserve $base_url if it was set Jonathan Nieder
@ 2010-11-29  0:51               ` Jakub Narebski
  2010-11-29 17:57               ` [PATCH/RFC] gitweb: Preserve $base_url if it was set Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Jakub Narebski @ 2010-11-29  0:51 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Daniel Reichelt, Giuseppe Bilotta, git

Subject fixed.

On Mon, 29 Nov 2010, Jonathan Nieder wrote:
> 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...

>
> 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>

> ---
> 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.


>   * $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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH/RFC] gitweb: Preserve $base_url if it was set
  2010-11-29  0:19             ` [PATCH/RFC] gitweb: Preserve $base_url if it was set Jonathan Nieder
  2010-11-29  0:51               ` [PATCH/RFC] gitweb/README: About $base_url etc. and $per_request_config Jakub Narebski
@ 2010-11-29 17:57               ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2010-11-29 17:57 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Jakub Narebski, Daniel Reichelt, Giuseppe Bilotta, git

Jonathan Nieder <jrnieder@gmail.com> writes:

> 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?
>
> Maintaining backward compatibility while avoiding this last concern
> seems hard....
> 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>

Sounds like a sensible thing to do; narrows an interface with two
variables into a more flexible new interface with a single variable.

> ---
> 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.
>   * $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.
>  
>  Projects list file format
>  ~~~~~~~~~~~~~~~~~~~~~~~~~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-28 18:12       ` Jonathan Nieder
@ 2010-11-30 18:22         ` Jakub Narebski
  2010-12-01  3:25           ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Narebski @ 2010-11-30 18:22 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Giuseppe Bilotta, git, Daniel Reichelt

On Sun, 28 Nov 2010, Jonathan Nieder wrote:
> Jakub Narebski wrote:
> 
> > See also gitweb/README, the "Gitweb config file variables" section:
> > 
> >  * $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 "/".
> > 
> > The key word here is "usually" ;-)
> 
> Thanks, all.  That helps.
> 
> No time for it at the moment, but I'm taking this as evidence that
> we need a gitweb.conf(5) manual page. :)  Would you be interested
> in such a thing (as POD or asciidoc)?

Well, both git-gui and gitk have their documentation in Documentation/
(in AsciiDoc format), so perhaps it is time to add Documentation/gitweb.txt
and possibly also Documentation/gitweb_config.txt (or gitweb.conf.txt).

With exception of t/README, the gitweb/README file is largest, with around
570 lines, while next largest is (discarding also git-gui/po/README) main
README file with around 50 lines.
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gitweb: false base href sent when integrated via reverse proxy and path_info is active
  2010-11-30 18:22         ` Jakub Narebski
@ 2010-12-01  3:25           ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2010-12-01  3:25 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Jonathan Nieder, Giuseppe Bilotta, git, Daniel Reichelt

Jakub Narebski <jnareb@gmail.com> writes:

> Well, both git-gui and gitk have their documentation in Documentation/
> (in AsciiDoc format), so perhaps it is time to add Documentation/gitweb.txt
> and possibly also Documentation/gitweb_config.txt (or gitweb.conf.txt).

That indeed sounds like a good idea to me.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2010-12-01  3:25 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20101128081048.13668.67286.reportbug@sb74.startrek>
2010-11-28 16:27 ` gitweb: false base href sent when integrated via reverse proxy and path_info is active Jonathan Nieder
2010-11-28 17:25   ` Giuseppe Bilotta
2010-11-28 17:47     ` Jakub Narebski
2010-11-28 18:12       ` Jonathan Nieder
2010-11-30 18:22         ` Jakub Narebski
2010-12-01  3:25           ` Junio C Hamano
2010-11-28 20:30       ` Daniel Reichelt
2010-11-28 21:07         ` Jakub Narebski
2010-11-28 21:25           ` Daniel Reichelt
2010-11-28 21:10         ` Jonathan Nieder
2010-11-28 21:28           ` Daniel Reichelt
2010-11-28 22:05           ` Jakub Narebski
2010-11-29  0:19             ` [PATCH/RFC] gitweb: Preserve $base_url if it was set Jonathan Nieder
2010-11-29  0:51               ` [PATCH/RFC] gitweb/README: About $base_url etc. and $per_request_config Jakub Narebski
2010-11-29 17:57               ` [PATCH/RFC] gitweb: Preserve $base_url if it was set Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).