gitweb obtains configuration from the following sources:
1. per-instance configuration file (default: gitweb_conf.perl)
2. system-wide configuration file (default: /etc/gitweb.conf)
If per-instance configuration file exists, then system-wide
configuration was _not used at all_. This is quite untypical and
suprising behavior.
This commit changes gitweb behavior so that configuration in
per-instance configuration file can _override_ settings from
system-wide configuration file.
Suggested-by: Drew Northup <redacted>
Signed-off-by: Jakub Narebski <redacted>
---
This is the response to discussion in the
[PATCH/WIP] Starting work on a man page for /etc/gitweb.conf
http://thread.gmane.org/gmane.comp.version-control.git/173422
The patch to gitweb.perl itself (without the commit message) was send
as part of
http://thread.gmane.org/gmane.comp.version-control.git/173422/focus=173489
(embedded in body of email).
Note that changes to gitweb/README and gitweb/INSTALL were
minimalized, so the final result might not be the best... but I think
it is good enough.
gitweb/INSTALL | 8 +++++---
gitweb/README | 2 +-
gitweb/gitweb.perl | 7 ++++---
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index 4964a67..0584919 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -98,15 +98,17 @@ Gitweb config file
See also "Runtime gitweb configuration" section in README file
for gitweb (in gitweb/README).
-- You can configure gitweb further using the gitweb configuration file;
+- You can configure gitweb further using the per-instance gitweb configuration file;
by default this is a file named gitweb_config.perl in the same place as
gitweb.cgi script. You can control the default place for the config file
using the GITWEB_CONFIG build configuration variable, and you can set it
- using the GITWEB_CONFIG environment variable. If this file does not
- exist, gitweb looks for a system-wide configuration file, normally
+ using the GITWEB_CONFIG environment variable.
+ gitweb also looks for a system-wide configuration file, normally
/etc/gitweb.conf. You can change the default using the
GITWEB_CONFIG_SYSTEM build configuration variable, and override it
through the GITWEB_CONFIG_SYSTEM environment variable.
+ Settings from per-instance configuration file override those from
+ system-wide configuration file.
- The gitweb config file is a fragment of perl code. You can set variables
using "our $variable = value"; text from "#" character until the end
diff --git a/gitweb/README b/gitweb/README
index a92bde7..334f13e 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -126,7 +126,7 @@ Runtime gitweb configuration
You can adjust gitweb behaviour using the file specified in `GITWEB_CONFIG`
(defaults to 'gitweb_config.perl' in the same directory as the CGI), and
-as a fallback `GITWEB_CONFIG_SYSTEM` (defaults to /etc/gitweb.conf).
+`GITWEB_CONFIG_SYSTEM` (defaults to /etc/gitweb.conf), in that order.
The most notable thing that is not configurable at compile time are the
optional features, stored in the '%features' variable.
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index acdc5b8..9527cd2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -637,12 +637,13 @@ sub evaluate_gitweb_config {
our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++";
our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++";
# die if there are errors parsing config file
+ if (-e $GITWEB_CONFIG_SYSTEM) {
+ do $GITWEB_CONFIG_SYSTEM;
+ die $@ if $@;
+ }
if (-e $GITWEB_CONFIG) {
do $GITWEB_CONFIG;
die $@ if $@;
- } elsif (-e $GITWEB_CONFIG_SYSTEM) {
- do $GITWEB_CONFIG_SYSTEM;
- die $@ if $@;
}
}
--
1.7.5
Jakub Narebski wrote:
If per-instance configuration file exists, then system-wide
configuration was _not used at all_. This is quite untypical and
suprising behavior.
I agree. How to avoid breaking existing installations, though? (I'm
especially worried because distro packages tend to ship their own
/etc/gitweb.conf, so the admin might not even know about what's
there.) For example, depending on the content of /etc/gitweb.conf,
this has the potential to break "git instaweb".
It could be simpler to document that users should put
do $GITWEB_CONFIG_SYSTEM;
at the start of gitweb_config.conf to reuse options from the system
configuration file and override them. But that's not very satisfying,
since I don't see a nice way to move to a better behavior after that
without breaking some existing installations. (It would be possible
to check for a new ./gitweb-config-in-addition-to-what-was-in-etc.conf
file but that doesn't seem so nice.)
If this were a command-line tool, I would be happy as long as there is
some way to prevent reading /etc/gitweb.conf through the environment.
For a webapp I don't know how easy it is to set environment variables
typically. So thoughts from people in that corner of the world would
be interesting.
On Sat, 14 May 2011, Jonathan Nieder wrote:
Jakub Narebski wrote:
quoted
If per-instance configuration file exists, then system-wide
configuration was _not used at all_. This is quite untypical and
suprising behavior.
I agree. How to avoid breaking existing installations, though? (I'm
especially worried because distro packages tend to ship their own
/etc/gitweb.conf, so the admin might not even know about what's
there.) For example, depending on the content of /etc/gitweb.conf,
this has the potential to break "git instaweb".
I don't think that this change has potential to break "git instaweb",
because "git instaweb" creates its own gitweb_conf.perl - settings
there would override distro's /etc/gitweb.conf. But I have not checked
if it doesn't rely on some values being default; it shouldn't though.
It is a PITA to have to retain backward compatibility with our bugs
and mistakes. Perhaps this change is for 1.8.0 version boundary, then?
It could be simpler to document that users should put
do $GITWEB_CONFIG_SYSTEM;
Well, you need to check if file exists, and die if there were any bugs
parsing this file (otherwise you can get strange errors which are really
hard to debug, to notice that they stem from broken configuration file;
I am speaking here from my bitter experience ;-)).
at the start of gitweb_config.conf to reuse options from the system
configuration file and override them. But that's not very satisfying,
since I don't see a nice way to move to a better behavior after that
without breaking some existing installations. (It would be possible
to check for a new ./gitweb-config-in-addition-to-what-was-in-etc.conf
file but that doesn't seem so nice.)
Right.
If this were a command-line tool, I would be happy as long as there is
some way to prevent reading /etc/gitweb.conf through the environment.
For a webapp I don't know how easy it is to set environment variables
typically. So thoughts from people in that corner of the world would
be interesting.
For Apache it is as simple as using
SetEnv GITWEB_CONFIG_SYSTEM /dev/null
(Sidenote: I just noticed that if $GITWEB_CONFIG_SYSTEM eq $GITWEB_CONFIG,
then we don't need and should not re-read config file).
For Lighttpd it would be
setenv.add-environment = ( "GITWEB_CONFIG_SYSTEM" => "/dev/null" )
For Mongoose it would be
cgi_env GITWEB_CONFIG_SYSTEM=/dev/null
For nginx it would be
env GITWEB_CONFIG_SYSTEM=/dev/null;
For IIS... is anyone running gitweb under IIS?
All examples taken from either gitweb/README, gitweb/INSTALL, or
git-instaweb.sh. Instead of /dev/null it can be anything that
does not exist, but is not a false value (so "" won't work).
Not tested!
--
Jakub Narebski
Poland
gitweb obtains configuration from the following sources:
1. per-instance configuration file (default: gitweb_conf.perl)
2. system-wide configuration file (default: /etc/gitweb.conf)
If per-instance configuration file exists, then system-wide
configuration is _not used at all_. This is quite untypical and
suprising behavior.
This commit changes gitweb behavior so that configuration in
per-instance configuration file can _override_ settings from
system-wide configuration file.
Note that strictly speaking it is backwards incompatible change.
Check your resulting gitweb configuration, please.
Suggested-by: Drew Northup <redacted>
Signed-off-by: Jakub Narebski <redacted>
---
Jakub Narebski wrote:
(Sidenote: I just noticed that if $GITWEB_CONFIG_SYSTEM eq $GITWEB_CONFIG,
then we don't need and should not re-read config file).
This version addresses this issue.
gitweb/INSTALL | 8 +++++---
gitweb/README | 2 +-
gitweb/gitweb.perl | 10 ++++++----
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index 4964a67..0584919 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -98,15 +98,17 @@ Gitweb config file
See also "Runtime gitweb configuration" section in README file
for gitweb (in gitweb/README).
-- You can configure gitweb further using the gitweb configuration file;
+- You can configure gitweb further using the per-instance gitweb configuration file;
by default this is a file named gitweb_config.perl in the same place as
gitweb.cgi script. You can control the default place for the config file
using the GITWEB_CONFIG build configuration variable, and you can set it
- using the GITWEB_CONFIG environment variable. If this file does not
- exist, gitweb looks for a system-wide configuration file, normally
+ using the GITWEB_CONFIG environment variable.
+ gitweb also looks for a system-wide configuration file, normally
/etc/gitweb.conf. You can change the default using the
GITWEB_CONFIG_SYSTEM build configuration variable, and override it
through the GITWEB_CONFIG_SYSTEM environment variable.
+ Settings from per-instance configuration file override those from
+ system-wide configuration file.
- The gitweb config file is a fragment of perl code. You can set variables
using "our $variable = value"; text from "#" character until the end
diff --git a/gitweb/README b/gitweb/README
index a92bde7..334f13e 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -126,7 +126,7 @@ Runtime gitweb configuration
You can adjust gitweb behaviour using the file specified in `GITWEB_CONFIG`
(defaults to 'gitweb_config.perl' in the same directory as the CGI), and
-as a fallback `GITWEB_CONFIG_SYSTEM` (defaults to /etc/gitweb.conf).
+`GITWEB_CONFIG_SYSTEM` (defaults to /etc/gitweb.conf), in that order.
The most notable thing that is not configurable at compile time are the
optional features, stored in the '%features' variable.
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e4ede13..0a23cb6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -640,13 +640,15 @@ sub evaluate_gitweb_config {
our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++";
our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++";
# die if there are errors parsing config file
- if (-e $GITWEB_CONFIG) {
- do $GITWEB_CONFIG;
- die $@ if $@;
- } elsif (-e $GITWEB_CONFIG_SYSTEM) {
+ if (-e $GITWEB_CONFIG_SYSTEM) {
do $GITWEB_CONFIG_SYSTEM;
die $@ if $@;
}
+ if ($GITWEB_CONFIG ne $GITWEB_CONFIG_SYSTEM &&
+ -e $GITWEB_CONFIG) {
+ do $GITWEB_CONFIG;
+ die $@ if $@;
+ }
}
# Get loadavg of system, to compare against $maxload.--
1.7.5
On Sun, 2011-05-15 at 11:53 +0200, Jakub Narebski wrote:
On Sat, 14 May 2011, Jonathan Nieder wrote:
quoted
Jakub Narebski wrote:
quoted
If per-instance configuration file exists, then system-wide
configuration was _not used at all_. This is quite untypical and
suprising behavior.
I agree. How to avoid breaking existing installations, though? (I'm
especially worried because distro packages tend to ship their own
/etc/gitweb.conf, so the admin might not even know about what's
there.) For example, depending on the content of /etc/gitweb.conf,
this has the potential to break "git instaweb".
I don't think that this change has potential to break "git instaweb",
because "git instaweb" creates its own gitweb_conf.perl - settings
there would override distro's /etc/gitweb.conf. But I have not checked
if it doesn't rely on some values being default; it shouldn't though.
It is a PITA to have to retain backward compatibility with our bugs
and mistakes. Perhaps this change is for 1.8.0 version boundary, then?
I'm comfortable putting this at the 1.8.0 boundary. Frankly, I think
that the default /etc/gitweb.conf should be a file full of commented-out
examples anyway. I do believe that at least one distro is doing that
already. Doing so decreases the potential pain for those still expecting
the old behavior.
--
-Drew Northup
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59