Re: [PATCH v3] Gitweb: Use a default for a bad env config file variable

From: Jakub Narebski <jnareb@gmail.com>
Date: Aug 10, 2010, 22:42 UTC
Message-ID: <m3tyn22iqd.fsf@localhost.localdomain>
In-Reply-To: <1281458849.28858.18.camel@ping.CS.McGill.CA>

James Shubin <purpleidea@gmail.com> writes:

> From: James Shubin <purpleidea@gmail.com>

Small nitpick: you need line like the above only if the From: header in your email is diferent from the authorship you want to have in commit, i.e. when you are sending email from other email account, or when you are (re)sending someone's else patches.

In this patch situation it is not, I think, necessary.

Show 12 quoted lines

> Signed-off-by: James Shubin <purpleidea@gmail.com>
> ---
> It's quite trivial really, all that changes is that if someone sets
> the environment variable: $GITWEB_CONFIG_SYSTEM, and this points to
> something like: /srv/gitosis/gitweb.conf, which doesn't actually
> exist, then gitweb will default to trying out the built in default of
> /etc/gitweb.conf (if it was built with that value).
> 
> This patch should make it easier for people who are configuring
> gitweb+gitosis, so that a separate gitweb.conf config file can be used
> to call the main config, but which doesn't null out the system
> defaults if it is missing.

First, why it is needed? Why can't you just have GITWEB_CONFIG_SYSTEM (or GITWEB_CONFIG) environment variable visible to gitweb.cgi that points to existing file?

Second, is there any history behind providing this fallback only for $GITWEB_CONFIG_SYSTEM variable, and not for $GITWEB_CONFIG? Currently gitweb use environment variable if it exists, falling back to build-time value (might be default), and using first of $GITWEB_CONFIG and $GITWEB_CONFIG_SYSTEM that exists.

Show 16 quoted lines

> PS: thanks to everyone for their patience with my first patch.
> This applied cleanly with git am, let me know if I should do anything
> else differently.
> 
>  gitweb/gitweb.perl |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4efeebc..43294e1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
>  	} elsif (-e $GITWEB_CONFIG_SYSTEM) {
>  		do $GITWEB_CONFIG_SYSTEM;
>  		die $@ if $@;
> +	# if config file from env is missing, then try the default anyways

Minor nitpick: Actually that is not default, but build-time value, which has a default.

Show 8 quoted lines

> +	} elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
> +		do "++GITWEB_CONFIG_SYSTEM++";
> +		die $@ if $@;
>  	}
>  }
>  
> -- 
> 1.7.0.4

-- 
Jakub Narebski
Poland
ShadeHawk on #git