Skip to content

Commit

Permalink
gitweb: Fix warnings with override permitted but no repo override
Browse files Browse the repository at this point in the history
When a feature like "blame" is permitted to be overridden in the
repository configuration but it is not actually set in the repository,
a warning is emitted due to the undefined value of the repository
configuration, even though it's a perfectly normal condition.
Emitting warning is grounds for test failure in the gitweb test
script.

This error was caused by rewrite of git_get_project_config from using
"git config [<type>] <name>" for each individual configuration
variable checked to parsing "git config --list --null" output in
commit b201927 (gitweb: Read repo config using 'git config -z -l').
Earlier version of git_get_project_config was returning empty string
if variable do not exist in config; newer version is meant to return
undef in this case, therefore change in feature_bool was needed.

Additionally config_to_* subroutines were meant to be invoked only if
configuration variable exists; therefore we added early return to
git_get_project_config: it now returns no value if variable does not
exists in config.  Otherwise config_to_* subroutines (config_to_bool
in paryicular) wouldn't be able to distinguish between the case where
variable does not exist and the case where variable doesn't have value
(the "[section] noval" case, which evaluates to true for boolean).

While at it fix bug in config_to_bool, where checking if $val is
defined (if config variable has value) was done _after_ stripping
leading and trailing whitespace, which lead to 'Use of uninitialized
value' warning.

Add test case for features overridable but not overriden in repo
config, and case for no value boolean configuration variable.

Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Marcel M. Cary authored and Junio C Hamano committed Feb 18, 2009
1 parent bf3c20f commit df5d10a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
16 changes: 10 additions & 6 deletions gitweb/gitweb.perl
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,13 @@ sub feature_bool {
my $key = shift;
my ($val) = git_get_project_config($key, '--bool');

if ($val eq 'true') {
if (!defined $val) {
return ($_[0]);
} elsif ($val eq 'true') {
return (1);
} elsif ($val eq 'false') {
return (0);
}

return ($_[0]);
}

sub feature_snapshot {
Expand Down Expand Up @@ -1914,18 +1914,19 @@ sub git_parse_project_config {
return %config;
}

# convert config value to boolean, 'true' or 'false'
# convert config value to boolean: 'true' or 'false'
# no value, number > 0, 'true' and 'yes' values are true
# rest of values are treated as false (never as error)
sub config_to_bool {
my $val = shift;

return 1 if !defined $val; # section.key

# strip leading and trailing whitespace
$val =~ s/^\s+//;
$val =~ s/\s+$//;

return (!defined $val || # section.key
($val =~ /^\d+$/ && $val) || # section.key = 1
return (($val =~ /^\d+$/ && $val) || # section.key = 1
($val =~ /^(?:true|yes)$/i)); # section.key = true
}

Expand Down Expand Up @@ -1978,6 +1979,9 @@ sub git_get_project_config {
$config_file = "$git_dir/config";
}

# check if config variable (key) exists
return unless exists $config{"gitweb.$key"};

# ensure given type
if (!defined $type) {
return $config{"gitweb.$key"};
Expand Down
18 changes: 17 additions & 1 deletion t/t9500-gitweb-standalone-no-errors.sh
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,11 @@ cat >>gitweb_config.perl <<EOF
\$feature{'snapshot'}{'override'} = 1;
EOF

test_expect_success \
'config override: tree view, features not overridden in repo config' \
'gitweb_run "p=.git;a=tree"'
test_debug 'cat gitweb.log'

test_expect_success \
'config override: tree view, features disabled in repo config' \
'git config gitweb.blame no &&
Expand All @@ -669,12 +674,23 @@ test_expect_success \
test_debug 'cat gitweb.log'

test_expect_success \
'config override: tree view, features enabled in repo config' \
'config override: tree view, features enabled in repo config (1)' \
'git config gitweb.blame yes &&
git config gitweb.snapshot "zip,tgz, tbz2" &&
gitweb_run "p=.git;a=tree"'
test_debug 'cat gitweb.log'

cat >.git/config <<\EOF
# testing noval and alternate separator
[gitweb]
blame
snapshot = zip tgz
EOF
test_expect_success \
'config override: tree view, features enabled in repo config (2)' \
'gitweb_run "p=.git;a=tree"'
test_debug 'cat gitweb.log'

# ----------------------------------------------------------------------
# non-ASCII in README.html

Expand Down

0 comments on commit df5d10a

Please sign in to comment.