* [PATCH 0/2] gitweb misc fixes mkII @ 2010-02-02 21:56 John 'Warthog9' Hawley 2010-02-02 21:56 ` [PATCH 1/2] gitweb: Add an option to force version match John 'Warthog9' Hawley 0 siblings, 1 reply; 9+ messages in thread From: John 'Warthog9' Hawley @ 2010-02-02 21:56 UTC (permalink / raw) To: git This is a new patch series dealing with the slightly controversial option to force version matching, and this adds a new patch that solves a minor issue discovered on kernel.org. John 'Warthog9' Hawley (2): gitweb: Add an option to force version match gitweb: Fix chop_str to allow for & characters in strings gitweb/README | 5 ++++ gitweb/gitweb.perl | 33 ++++++++++++++++++++++++++++- t/gitweb-lib.sh | 1 + t/t9501-gitweb-standalone-http-status.sh | 27 ++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] gitweb: Add an option to force version match 2010-02-02 21:56 [PATCH 0/2] gitweb misc fixes mkII John 'Warthog9' Hawley @ 2010-02-02 21:56 ` John 'Warthog9' Hawley 2010-02-02 21:56 ` [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings John 'Warthog9' Hawley 2010-02-02 22:59 ` [PATCH 1/2] gitweb: Add an option to force version match Junio C Hamano 0 siblings, 2 replies; 9+ messages in thread From: John 'Warthog9' Hawley @ 2010-02-02 21:56 UTC (permalink / raw) To: git From: John 'Warthog9' Hawley <warthog9@kernel.org> This adds $git_versions_must_match variable, which if set to true, checks that we are running on the same version of git that we shipped with, and if not throw '500 Internal Server Error' error. What is checked is the version of gitweb (embedded in building gitweb.cgi), against version of runtime git binary used. Gitweb can usually run with a mismatched git install. This is more here to give an obvious warning as to what's going on vs. silently failing. By default this feature is turned on. Add tests to t9501-gitweb-standalone-http-status.sh that this feature works correctly (as expected) if turned on, both in match and no match case. Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org> Signed-off-by: Jakub Narebski <jnareb@gmail.com> --- gitweb/README | 5 ++++ gitweb/gitweb.perl | 32 +++++++++++++++++++++++++++++- t/gitweb-lib.sh | 1 + t/t9501-gitweb-standalone-http-status.sh | 27 +++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletions(-) diff --git a/gitweb/README b/gitweb/README index 6c2c8e1..ec93da6 100644 --- a/gitweb/README +++ b/gitweb/README @@ -233,6 +233,11 @@ not include variables usually directly set during build): If server load exceed this value then return "503 Service Unavaliable" error. Server load is taken to be 0 if gitweb cannot determine its value. Set it to undefined value to turn it off. The default is 300. + * $git_versions_must_match + If set to true value, gitweb fails with "500 Internal Server Error" error + if the version of the gitweb doesn't match version of the git binary. + Gitweb can usually run with a mismatched git install. The default is 1 + (true). Projects list file format diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index d0c3ff2..57771a0 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -221,6 +221,9 @@ our %avatar_size = ( 'double' => 32 ); +# If it is true, exit if gitweb version and git binary version don't match +our $git_versions_must_match = 1; + # Used to set the maximum load that we will still respond to gitweb queries. # If server load exceed this value then return "503 server busy" error. # If gitweb cannot determined server load, it is taken to be 0. @@ -550,10 +553,10 @@ sub filter_snapshot_fmts { } our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++"; +our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++"; if (-e $GITWEB_CONFIG) { do $GITWEB_CONFIG; } else { - our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++"; do $GITWEB_CONFIG_SYSTEM if -e $GITWEB_CONFIG_SYSTEM; } @@ -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)]} +(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); +} + $projects_list ||= $projectroot; if (defined $maxload && get_loadavg() > $maxload) { diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh index 5a734b1..66a3e2d 100644 --- a/t/gitweb-lib.sh +++ b/t/gitweb-lib.sh @@ -26,6 +26,7 @@ our \$projects_list = ''; our \$export_ok = ''; our \$strict_export = ''; our \$maxload = undef; +our \$git_versions_must_match = 0; EOF diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh index 7590f10..e195f97 100755 --- a/t/t9501-gitweb-standalone-http-status.sh +++ b/t/t9501-gitweb-standalone-http-status.sh @@ -133,5 +133,32 @@ cat >>gitweb_config.perl <<\EOF our $maxload = undef; EOF +# ====================================================================== +# check $git_versions_must_match feature +# should be last section, just in case +cp -f gitweb_config.perl gitweb_config.perl.bak +echo 'our $git_versions_must_match = 1;' >>gitweb_config.perl + +cat <<\EOF >>gitweb_config.perl +our $version = "current"; +EOF +test_expect_success 'force version match: no match' ' + gitweb_run "p=.git" && + grep "Status: 500 Internal Server Error" gitweb.headers && + grep "500 - Internal server error" gitweb.body +' +test_debug 'cat gitweb.headers' + +cat <<\EOF >>gitweb_config.perl +# must be kept in sync with code in gitweb/gitweb.perl +our $version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown"; +EOF +test_expect_success 'force version match: match' ' + gitweb_run "p=.git" && + grep "Status: 200 OK" gitweb.headers +' +test_debug 'cat gitweb.headers' + +mv -f gitweb_config.perl.bak gitweb_config.perl test_done -- 1.6.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings 2010-02-02 21:56 ` [PATCH 1/2] gitweb: Add an option to force version match John 'Warthog9' Hawley @ 2010-02-02 21:56 ` John 'Warthog9' Hawley 2010-02-02 23:43 ` Jakub Narebski 2010-02-02 22:59 ` [PATCH 1/2] gitweb: Add an option to force version match Junio C Hamano 1 sibling, 1 reply; 9+ messages in thread From: John 'Warthog9' Hawley @ 2010-02-02 21:56 UTC (permalink / raw) To: git I'm unsure why this was placed in their originally, and it seems to ultimately stem from code from before gitweb was merged into git core, but there's an instance where git chops a string incorrectly based on this. Specifically: API & protocol: support option to force written data immediately to disk from http://git.kernel.org/?p=daemon/distsrv/chunkd.git;a=commit;h=3b02f749df2cb1288f345a689d85e7061f507e54 The short version of the title gets chopped to API ... where it should be API & protocol: support option to force written data... This reverts that specific problem. --- gitweb/gitweb.perl | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 57771a0..4cc6d19 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1382,7 +1382,6 @@ sub chop_str { my $body = $1; my $tail = $2; if (length($tail) > 4) { - $body =~ s/&[^;]*$//; $tail = "... "; } return "$body$tail"; -- 1.6.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings 2010-02-02 21:56 ` [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings John 'Warthog9' Hawley @ 2010-02-02 23:43 ` Jakub Narebski 2010-02-02 23:54 ` J.H. 0 siblings, 1 reply; 9+ messages in thread From: Jakub Narebski @ 2010-02-02 23:43 UTC (permalink / raw) To: John 'Warthog9' Hawley; +Cc: git "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes: > I'm unsure why this was placed in their originally, and it seems to > ultimately stem from code from before gitweb was merged into git core, > but there's an instance where git chops a string incorrectly based on > this. > > Specifically: > > API & protocol: support option to force written data immediately to disk > > from http://git.kernel.org/?p=daemon/distsrv/chunkd.git;a=commit;h=3b02f749df2cb1288f345a689d85e7061f507e54 > > The short version of the title gets chopped to > > API ... > > where it should be > > API & protocol: support option to force written data... > > This reverts that specific problem. > --- > gitweb/gitweb.perl | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index 57771a0..4cc6d19 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -1382,7 +1382,6 @@ sub chop_str { > my $body = $1; > my $tail = $2; > if (length($tail) > 4) { > - $body =~ s/&[^;]*$//; > $tail = "... "; > } > return "$body$tail"; I think it is a good change. chop_str is meant to be used _before_ HTML escaping (esc_html or equivalent) is to be applied; removed line looks like it was meant (badly) to always remove HTML entities fully... but those entities are only added later. So now what is left is to come up with proper commit message, and add Signed-off-by: John 'Warthog9' Hawley" <warthog9@kernel.org> Acked-by: Jakub Narebski <jnareb@gmail.com> -- Jakub Narebski Poland ShadeHawk on #git ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings 2010-02-02 23:43 ` Jakub Narebski @ 2010-02-02 23:54 ` J.H. 2010-02-03 11:28 ` [PATCH] gitweb: Simplify (and fix) chop_str Jakub Narebski 0 siblings, 1 reply; 9+ messages in thread From: J.H. @ 2010-02-02 23:54 UTC (permalink / raw) To: Jakub Narebski; +Cc: git On 02/02/2010 03:43 PM, Jakub Narebski wrote: > "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes: > >> I'm unsure why this was placed in their originally, and it seems to >> ultimately stem from code from before gitweb was merged into git core, >> but there's an instance where git chops a string incorrectly based on >> this. >> >> Specifically: >> >> API & protocol: support option to force written data immediately to disk >> >> from http://git.kernel.org/?p=daemon/distsrv/chunkd.git;a=commit;h=3b02f749df2cb1288f345a689d85e7061f507e54 >> >> The short version of the title gets chopped to >> >> API ... >> >> where it should be >> >> API & protocol: support option to force written data... >> >> This reverts that specific problem. >> --- >> gitweb/gitweb.perl | 1 - >> 1 files changed, 0 insertions(+), 1 deletions(-) >> >> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl >> index 57771a0..4cc6d19 100755 >> --- a/gitweb/gitweb.perl >> +++ b/gitweb/gitweb.perl >> @@ -1382,7 +1382,6 @@ sub chop_str { >> my $body = $1; >> my $tail = $2; >> if (length($tail) > 4) { >> - $body =~ s/&[^;]*$//; >> $tail = "... "; >> } >> return "$body$tail"; > > I think it is a good change. chop_str is meant to be used _before_ > HTML escaping (esc_html or equivalent) is to be applied; removed line > looks like it was meant (badly) to always remove HTML entities > fully... but those entities are only added later. > > So now what is left is to come up with proper commit message, and add > > Signed-off-by: John 'Warthog9' Hawley" <warthog9@kernel.org> > Acked-by: Jakub Narebski <jnareb@gmail.com> > There's a couple more lines, similar to that, ins chop_str. The bug I needed to fix didn't tickle those, so I made the smallest change possible. But those should probably be looked at as well. - John 'Warthog9' Hawley ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] gitweb: Simplify (and fix) chop_str 2010-02-02 23:54 ` J.H. @ 2010-02-03 11:28 ` Jakub Narebski 2010-02-03 18:25 ` J.H. 0 siblings, 1 reply; 9+ messages in thread From: Jakub Narebski @ 2010-02-03 11:28 UTC (permalink / raw) To: J.H.; +Cc: git, John 'Warthog9' Hawley From: John 'Warthog9' Hawley <warthog9@kernel.org> The chop_str subroutine is meant to be used on strings (such as commit description / title) *before* HTML escaping, which means before applying esc_html or equivalent. Therefore get rid of the failed attempt to always remove full HTML entities (like e.g. & or ). It is not necessary (HTML entities gets added later), and it can cause chop_str to chop a string incorrectly. Specifically: API & protocol: support option to force written data immediately to disk from http://git.kernel.org/?p=daemon/distsrv/chunkd.git;a=commit;h=3b02f749df2cb1288f345a689d85e7061f507e54 The short version of the title gets chopped to API ... where it should be API & protocol: support option to force written data... Noticed-by: John 'Warthog9' Hawley <warthog9@kernel.org> Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org> Signed-off-by: Jakub Narebski <jnareb@gmail.com> --- I have retained J.H. authorship of this patch. I have rewritten commit message, added signoffs, and removed all instances of failed attempt of removing HTML entities whole, even though only one of them is used. gitweb/gitweb.perl | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index d0c3ff2..1f6978a 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1330,7 +1330,6 @@ sub chop_str { $str =~ m/^(.*?)($begre)$/; my ($lead, $body) = ($1, $2); if (length($lead) > 4) { - $body =~ s/^[^;]*;// if ($lead =~ m/&[^;]*$/); $lead = " ..."; } return "$lead$body"; @@ -1341,8 +1340,6 @@ sub chop_str { $str =~ m/^(.*?)($begre)$/; my ($mid, $right) = ($1, $2); if (length($mid) > 5) { - $left =~ s/&[^;]*$//; - $right =~ s/^[^;]*;// if ($mid =~ m/&[^;]*$/); $mid = " ... "; } return "$left$mid$right"; @@ -1352,7 +1349,6 @@ sub chop_str { my $body = $1; my $tail = $2; if (length($tail) > 4) { - $body =~ s/&[^;]*$//; $tail = "... "; } return "$body$tail"; -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] gitweb: Simplify (and fix) chop_str 2010-02-03 11:28 ` [PATCH] gitweb: Simplify (and fix) chop_str Jakub Narebski @ 2010-02-03 18:25 ` J.H. 0 siblings, 0 replies; 9+ messages in thread From: J.H. @ 2010-02-03 18:25 UTC (permalink / raw) To: Jakub Narebski; +Cc: git, John 'Warthog9' Hawley This looks good to me. - John 'Warthog9' Hawley On 02/03/2010 03:28 AM, Jakub Narebski wrote: > From: John 'Warthog9' Hawley <warthog9@kernel.org> > > The chop_str subroutine is meant to be used on strings (such as commit > description / title) *before* HTML escaping, which means before > applying esc_html or equivalent. > > Therefore get rid of the failed attempt to always remove full HTML > entities (like e.g. & or ). It is not necessary (HTML > entities gets added later), and it can cause chop_str to chop a string > incorrectly. > > Specifically: > > API & protocol: support option to force written data immediately to disk > > from http://git.kernel.org/?p=daemon/distsrv/chunkd.git;a=commit;h=3b02f749df2cb1288f345a689d85e7061f507e54 > > The short version of the title gets chopped to > > API ... > > where it should be > > API & protocol: support option to force written data... > > Noticed-by: John 'Warthog9' Hawley <warthog9@kernel.org> > Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org> > Signed-off-by: Jakub Narebski <jnareb@gmail.com> > --- > I have retained J.H. authorship of this patch. I have rewritten > commit message, added signoffs, and removed all instances of failed > attempt of removing HTML entities whole, even though only one of them > is used. > > gitweb/gitweb.perl | 4 ---- > 1 files changed, 0 insertions(+), 4 deletions(-) > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index d0c3ff2..1f6978a 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -1330,7 +1330,6 @@ sub chop_str { > $str =~ m/^(.*?)($begre)$/; > my ($lead, $body) = ($1, $2); > if (length($lead) > 4) { > - $body =~ s/^[^;]*;// if ($lead =~ m/&[^;]*$/); > $lead = " ..."; > } > return "$lead$body"; > @@ -1341,8 +1340,6 @@ sub chop_str { > $str =~ m/^(.*?)($begre)$/; > my ($mid, $right) = ($1, $2); > if (length($mid) > 5) { > - $left =~ s/&[^;]*$//; > - $right =~ s/^[^;]*;// if ($mid =~ m/&[^;]*$/); > $mid = " ... "; > } > return "$left$mid$right"; > @@ -1352,7 +1349,6 @@ sub chop_str { > my $body = $1; > my $tail = $2; > if (length($tail) > 4) { > - $body =~ s/&[^;]*$//; > $tail = "... "; > } > return "$body$tail"; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] gitweb: Add an option to force version match 2010-02-02 21:56 ` [PATCH 1/2] gitweb: Add an option to force version match John 'Warthog9' Hawley 2010-02-02 21:56 ` [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings John 'Warthog9' Hawley @ 2010-02-02 22:59 ` Junio C Hamano 2010-02-02 23:56 ` Jakub Narebski 1 sibling, 1 reply; 9+ messages in thread From: Junio C Hamano @ 2010-02-02 22:59 UTC (permalink / raw) To: John 'Warthog9' Hawley; +Cc: git "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes: John, I'm sorry but I have to say this is somewhat incoherent. > + * $git_versions_must_match > + If set to true value, gitweb fails with "500 Internal Server Error" error > + if the version of the gitweb doesn't match version of the git binary. > + Gitweb can usually run with a mismatched git install. The default is 1 > + (true). I would understand that it if this were "Gitweb seldom runs correctly with unmatched version of git, so this defaults to true". If it can _usually_ run just fine, why should everybody need to flip this off? This doesn't make any sense to me. > @@ -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)]} > +(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); 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. To whom are you giving this _warning_? Please read the message yourself again. The message tells _you_ to consider using matching versions (as if _you_ have the choice and authority to do so), hints _you_ to decide if you are Ok with running an unmatched combination (again, as if _you_ have the authority to make that decision), and then instructs _you_ to contact the server administrator (who presumably can flip the bit). That doesn't make _any_ sense to me. Hopefully anybody who installs or upgrades gitweb/git will hit his gitweb installation at least once before end users start hitting, so I would understand it if you wrote the above message addressed to the server administrator. If somebody updates his git without bothering to update gitweb, on the other hand, the end user may see the message before the administrator does. If git and gitweb might be managed by different people at a particular site (k.org?), I would understand that the administrator of the gitweb side _might_ want to be told about it by the end user, and the above might be an attempt to make that happen. But even in that case, out of the three instructions, only the last one is for the end user, and telling him to be certain the combinations do work before bugging the gitweb administrator doesn't make much sense to me. So I have to ask a basic question I asked (at least I tried to) last night again. Whom are you trying to help? Even if it is to help a gitweb administrator who is not in charge of other people in the administrator group who would install unmatching versions of git without telling him, would this really be the best solution? You'd be the first to suffer from this when HPA or whoever installs a new version of git at k.org. There should be a better way to help communication between the people in the administration group, without involving or inconveniencing the end users like this patch seems to do. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] gitweb: Add an option to force version match 2010-02-02 22:59 ` [PATCH 1/2] gitweb: Add an option to force version match Junio C Hamano @ 2010-02-02 23:56 ` Jakub Narebski 0 siblings, 0 replies; 9+ messages in thread From: Jakub Narebski @ 2010-02-02 23:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: John 'Warthog9' Hawley, git Junio C Hamano <gitster@pobox.com> writes: > "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes: > > @@ -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) [...]' > > +(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). > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-02-03 18:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-02-02 21:56 [PATCH 0/2] gitweb misc fixes mkII John 'Warthog9' Hawley 2010-02-02 21:56 ` [PATCH 1/2] gitweb: Add an option to force version match John 'Warthog9' Hawley 2010-02-02 21:56 ` [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings John 'Warthog9' Hawley 2010-02-02 23:43 ` Jakub Narebski 2010-02-02 23:54 ` J.H. 2010-02-03 11:28 ` [PATCH] gitweb: Simplify (and fix) chop_str Jakub Narebski 2010-02-03 18:25 ` J.H. 2010-02-02 22:59 ` [PATCH 1/2] gitweb: Add an option to force version match Junio C Hamano 2010-02-02 23:56 ` Jakub Narebski
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).