Re: [PATCH 1/2] gitweb: Add an option to force version match
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Feb 2, 2010, 23:56 UTC
- Message-ID
- <m3iqaf9oqu.fsf@localhost.localdomain>
- In-Reply-To
- <7vvdef1by2.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
Show 24 quoted lines
> > @@ -583,6 +586,33 @@ sub get_loadavg {
> > our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
> > $number_of_git_cmds++;
> >
> > +# Throw an error if git versions does not match, if $git_versions_must_match is true.
> > +if ($git_versions_must_match &&
> > + $git_version ne $version) {
> > + my $admin_contact =
> > + defined $ENV{'SERVER_ADMIN'} ? ", $ENV{'SERVER_ADMIN'}," : '';
> > + my $err_msg = <<EOT;
> > +<h1 align="center">*** Warning ***</h1>
> > +<p>
> > +This version of gitweb was compiled for <b>@{[esc_html($version)]}</b>,
> > +however git version <b>@{[esc_html($git_version)]}</b> was found on server.
> > +Running an instance of gitweb that is not matched to the git binaries may
> > +result in unexpected behavior of gitweb, and loss of functionality or
> > +incorrect data on displayed pages.
> > +</p>
> > +<p>
> > +Please update the git or gitweb installation so that their versions match, or
> > +if you feel you are sure that you wish to proceed with running gitweb
> > +with unmatched versions please contact the server administrator${admin_contact}
> > +to configure gitweb to allow mismatched versions. This can be done by
> > +setting \$git_versions_must_match to @{[esc_html($git_versions_must_match)]}Errr... the above line does not makes sense, as $git_versions_must_match is true (true value), and not false (false value). so you would get something like:
'setting $git_versions_must_match to 1 (false value) [...]'
Show 5 quoted lines
> > +(false value) in gitweb configuration file,
> > +'@{[esc_path(-e $GITWEB_CONFIG ? $GITWEB_CONFIG : $GITWEB_CONFIG_SYSTEM)]}'.
> > +</p>
> > +EOT
> > + die_error(500, 'Internal server error', $err_msg);As I wrote in my earlier response, this is not enough. If this error message (description of this situation) is meant to describe how to turn this feature off, it has to deal with situation where config file does not exist. With this feature off by default we knew that it had to be changed in some gitweb config file, so one of $GITWEB_CONFIG and $GITWEB_CONFIG_SYSTEM must exists. This is not true if this feature is turned on by default.
You need to describe how to deal with the following situations:
* Both $GITWEB_CONFIG and $GITWEB_CONFIG_SYSTEM are empty or undefined. You might skip this situation, as by default both are set by gitweb/Makefile, $GITWEB_CONFIG to gitweb_config.perl and $GITWEB_CONFIG_SYSTEM to /etc/gitweb.conf * Both $GITWEB_CONFIG and $GITWEB_CONFIG_SYSTEM are set and non-empty, but neither file exists (this means that $projects_list is set during build stage).
Current code deals correctly only with situation where either $GITWEB_CONFIG or $GITWEB_CONFIG_SYSTEM exists, and it further assumes that neither is undefined (you would get perl errors in case if either is undefined).
Show 5 quoted lines
> Why, why, why? > > This is not even a "*** Warning ***". You are refusing to let them do > anything useful until they either flip the bit off or reinstall git and/or > gitweb. It is a _fatal error_ message.
Ooops.
> > To whom are you giving this _warning_? Please read the message yourself > again.
The original message (from "Gitweb caching v2" thread) was ment purely for server administrator. Current version tries to address both ordinary user (which has to contact gitweb administrator) and gitweb administrator (who needs to know how to remote error condition, either by bringing git and gitweb versions in sync, or by changing configuration). And does it badly...
[...]
> So I have to ask a basic question I asked (at least I tried to) last night > again. Whom are you trying to help?
[...]
-- Jakub Narebski Poland ShadeHawk on #git