Re: [PATCH] gitweb: use Git.pm, and use its parse_rev method for git_get_head_hash

From: Jakub Narebski <jnareb@gmail.com>
Date: May 31, 2008, 09:40 UTC
Message-ID: <m3lk1q24nb.fsf@localhost.localdomain>
In-Reply-To: <484087A7.2030107@gmail.com>

Lea Wiemann <lewiemann@gmail.com> writes:

Show 6 quoted lines

> Lea Wiemann wrote:
> > Subject: [PATCH] gitweb: use Git.pm, and use its parse_rev method
> >          for git_get_head_hash
> 
> For clarification, this is my first patch for refactoring Gitweb to
> use Git.pm's API.

Good.

I'm not sure however how much of gitweb engine should be moved to Git.pm. I think calling git commands could be moved from git_cmd() to command_output_pipe() or command_oneline(), replacing global variable $git_dir by global variable $repo. Perhaps gitweb's config file parsing should be added as an option to Git.pm. But I'm not sure for example about parsing subroutines.

> In the end, I'm hoping that all (or at least most) of Gitweb's
> accesses to the repositories will go through this API, which allows us
> to add caching to the Git.pm API (rather than Gitweb) pretty easily
> and cleanly.

IMVHO caching command output is bad idea. I'd rather have gitweb cache _parsed_ data (i.e. Perl structures).

For example projects list (but also summary page for a project) is composed as result of parsing output of _many_ git commands, in the case of projects list coming from many different repositories. Caching git command output (something like IPC::Cmd::Cached?) would only repeat bad I/O patterns of git commands.

Also I don't think it is a good idea to pollute Git.pm by caching command output. Perhaps in Git::Cached, but as a last resort...

P.S. Here it is what could go to Git.pm:
 * Eager config parsing, using e.g. $repo->parse_config() to prepare
   %config hash, and have $repo->config(VAR) etc. use %config hash.
   This means one git command and not one git command per variable,
   but it also means that we have to convert to bool or int, or color
   in Perl.
 * unquote() (which is not repository instance merhod, but utility
   subroutine) to unquote and unescape filenames.
 * generating rfc2822 and iso-8601 dates from timestamp plus timezone,
   i.e. from author/committer/tagger date; it would be useful for
   example in git-send-email.

I'm not sure about parse_* subroutines (some of which are not generic enough, I guess).

-- 
Jakub Narebski
Poland
ShadeHawk on #git