From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: Eli Barzilay <eli@barzilay.org>
Subject: [PATCH] gitweb: Fix project-specific feature override behavior
Date: Mon, 01 Mar 2010 22:51:34 +0100 [thread overview]
Message-ID: <20100301215040.32574.8149.stgit@localhost.localdomain> (raw)
In-Reply-To: <m37hpwma9u.fsf@localhost.localdomain>
This commit fixes a bug in processing project-specific override in
a situation when there is no project, e.g. for the projects list page.
When 'snapshot' feature had project specific config override enabled
by putting
$feature{'snapshot'}{'override'} = 1;
(or equivalent) in $GITWEB_CONFIG, and when viewing toplevel gitweb
page, which means the projects list page (to be more exact this
happens for any project-less action), gitweb would put the following
Perl warnings in error log:
gitweb.cgi: Use of uninitialized value $git_dir in concatenation (.) or string at gitweb.cgi line 2065.
fatal: error processing config file(s)
gitweb.cgi: Use of uninitialized value $git_dir in concatenation (.) or string at gitweb.cgi line 2221.
gitweb.cgi: Use of uninitialized value $git_dir in concatenation (.) or string at gitweb.cgi line 2218.
The problem is in the following fragment of code:
# path to the current git repository
our $git_dir;
$git_dir = "$projectroot/$project" if $project;
# list of supported snapshot formats
our @snapshot_fmts = gitweb_get_feature('snapshot');
@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
For the toplevel gitweb page, which is the list of projects, $project is not
defined, therefore neither is $git_dir. gitweb_get_feature() subroutine
calls git_get_project_config() if project specific override is turned
on... but we don't have project here.
Those errors mentioned above occur in the following fragment of code in
git_get_project_config():
# get config
if (!defined $config_file ||
$config_file ne "$git_dir/config") {
%config = git_parse_project_config('gitweb');
$config_file = "$git_dir/config";
}
git_parse_project_config() calls git_cmd() which has '--git-dir='.$git_dir
There are (at least) three possible solutions:
1. Harden gitweb_get_feature() so that it doesn't call
git_get_project_config() if $project (and therefore $git_dir) is not
defined; there is no project for project specific config.
2. Harden git_get_project_config() like you did in your fix, returning early
if $git_dir is not defined.
3. Harden git_cmd() so that it doesn't add "--git-dir=$git_dir" if $git_dir
is not defined, and change git_get_project_config() so that it doesn't
even try to access $git_dir if it is not defined.
This commit implements both 1.) and 2.), i.e. gitweb_get_feature() doesn't
call project-specific override if $git_dir is not defined (if there is no
project), and git_get_project_config() returns early if $git_dir is not
defined.
Add a test for this bug to t/t9500-gitweb-standalone-no-errors.sh test.
Reported-by: Eli Barzilay <eli@barzilay.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is a fix, with much too long commit message ;-)
gitweb/gitweb.perl | 9 ++++++++-
t/t9500-gitweb-standalone-no-errors.sh | 16 ++++++++++++----
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1ce4de1..d02734e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -467,7 +467,11 @@ sub gitweb_get_feature {
$feature{$name}{'sub'},
$feature{$name}{'override'},
@{$feature{$name}{'default'}});
- if (!$override) { return @defaults; }
+ # project specific override is possible only if we have project
+ our $git_dir; # global variable, declared later
+ if (!$override || !defined $git_dir) {
+ return @defaults;
+ }
if (!defined $sub) {
warn "feature $name is not overridable";
return @defaults;
@@ -2225,6 +2229,9 @@ sub config_to_multi {
sub git_get_project_config {
my ($key, $type) = @_;
+ # do we have project
+ return unless (defined $project && defined $git_dir);
+
# key sanity check
return unless ($key);
$key =~ s/^gitweb\.//;
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 0fe9beb..4f2b9b0 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -591,14 +591,22 @@ test_debug 'cat gitweb.log'
# ----------------------------------------------------------------------
# gitweb config and repo config
-cat >>gitweb_config.perl <<EOF
+cat >>gitweb_config.perl <<\EOF
-\$feature{'blame'}{'override'} = 1;
-\$feature{'snapshot'}{'override'} = 1;
-\$feature{'avatar'}{'override'} = 1;
+# turn on override for each overridable feature
+foreach my $key (keys %feature) {
+ if ($feature{$key}{'sub'}) {
+ $feature{$key}{'override'} = 1;
+ }
+}
EOF
test_expect_success \
+ 'config override: projects list (implicit)' \
+ 'gitweb_run'
+test_debug 'cat gitweb.log'
+
+test_expect_success \
'config override: tree view, features not overridden in repo config' \
'gitweb_run "p=.git;a=tree"'
test_debug 'cat gitweb.log'
next prev parent reply other threads:[~2010-03-01 21:52 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-01 6:54 gitweb problem? Eli Barzilay
2010-03-01 10:24 ` Jakub Narebski
2010-03-01 11:51 ` Eli Barzilay
2010-03-01 13:40 ` Jakub Narebski
2010-03-01 21:51 ` Jakub Narebski [this message]
2010-03-02 4:40 ` Eli Barzilay
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100301215040.32574.8149.stgit@localhost.localdomain \
--to=jnareb@gmail.com \
--cc=eli@barzilay.org \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.