Re: gitweb problem?

Eli Barzilay [off-list ref] writes:

Jakub Narebski [off-list ref] writes:

quoted

Eli Barzilay [off-list ref] writes:

quoted

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.

[...]

quoted

quoted

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