Re: [PATCH] gitweb: Use GITWEB_CONFIG_SYSTEM even if GITWEB_CONFIG does exist

From: Jakub Narebski <jnareb@gmail.com>
Date: May 15, 2011, 09:53 UTC
Message-ID: <201105151153.08826.jnareb@gmail.com>
In-Reply-To: <20110514210603.GA11502@elie>

On Sat, 14 May 2011, Jonathan Nieder wrote:

Show 11 quoted lines

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

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

Show 7 quoted lines

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

Show 6 quoted lines

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