Eli Barzilay <eli@xxxxxxxxxxxx> writes:
> Jakub Narebski <jnareb@xxxxxxxxx> writes:
>> Eli Barzilay <eli@xxxxxxxxxxxx> writes:
>>
>>> Whenever I view the toplevel gitweb page (running as a cgi script
>>> under apache), but not when in a specific repo, I get this in my error
>>> log:
>>>
>>> gitweb.cgi: Use of uninitialized value $git_dir in concatenation (.) or string at /home/git/gitweb/gitweb.cgi line 2065.
>>> fatal: error processing config file(s)
>>> gitweb.cgi: Use of uninitialized value $git_dir in concatenation (.) or string at /home/git/gitweb/gitweb.cgi line 2221.
>>> gitweb.cgi: Use of uninitialized value $git_dir in concatenation (.) or string at /home/git/gitweb/gitweb.cgi line 2218.
[...]
>>> Looking at the source, the last two line numbers are in
>>> `git_get_project_config' -- so my guess is that the code is trying to
>>> get the options from the repository config file even when showing the
>>> toplevel page. Based on this, and also guessing that $git_dir is
>>> unset when viewing the toplevel page, I added
>>>
>>> return unless (defined $git_dir);
>>>
>>> to the top (of the `git_get_project_config' function), and I get no
>>> warnings and everything works as it should.
BTW. the first line number is in git_cmd invoked (called) from
git_get_project_config.
[...]
> No, that's not it. I've tracked all calls to
> `git_get_project_config', and the two offending calls are the ones in
> `feature_snapshot' and `feature_avatar'. I then verified that
> commenting out these two lines in my config
>
> $feature{'snapshot'}{'override'} = 1;
> $feature{'avatar'}{'override'} = 1;
>
> avoids the calls -- and the comments indicate that it should be fine
> to do that.
Ah, now I have found it. This is a bug in gitweb.
The problem is in the following fragment:
# 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 $git_dir is neither. gitweb_get_feature()
subroutine calls git_get_project_config() if project specific override
is turned on... but we don't have project here.
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.
# get config
if (!defined $config_file ||
$config_file ne "$git_dir/config") {
%config = git_parse_project_config('gitweb');
$config_file = "$git_dir/config";
}
The last solution, while requiring most work, has the advantage of being
able to do override via user (~apache/.gitconfig) or system (/etc/gitconfig)
gitweb config file. But I am not sure if it is worth it...
As to why t9500 test didn't detect that: there is no test for project list
page when features are configured to be overridable...
--
Jakub Narebski
Poland
ShadeHawk on #git
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html