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

From: Lea Wiemann <lewiemann@gmail.com>
Date: May 31, 2008, 12:39 UTC
Message-ID: <4841471E.2070302@gmail.com>
In-Reply-To: <m3lk1q24nb.fsf@localhost.localdomain>

Jakub Narebski wrote:

Show 5 quoted lines

> But I'm not sure for example [that] parsing subroutines [that parse the
> output of the git commands should be in Git.pm].
> 
> [...] IMVHO caching command output is bad idea.  I'd rather have gitweb
> cache _parsed_ data (i.e. Perl structures).

Yes, I agree with you that parsed data should be cached, but for that reason I think that the subroutine that parse git's output should be in Git.pm, not in Gitweb:

If you want to cache parsed data, you need the caching layer in between the application (= Gitweb) and the repository access layer (= command output parsing), or you would have to insert cache code at every call to the repository access layer. So you end up with these layers:

(Layer 0: Front-end [HTML] caching.) Layer 1: Application (Gitweb) Layer 2: Back-end caching Layer 3: Repository access (command parsing) Layer 4: Calls to the git binary

Layer 3 and 4 are application-independent (i.e. not Gitweb specific), and since they form a usable API, they might as well be written as a separate API rather than lumped together with Gitweb. Git.pm is a start of such an API (it does layer 4 and a little bit of layer 3), so it seems natural for me to extend it.

Layer 2 is application-independent as well, so it can become an extra class in Git.pm or a separate module. (It should stay independent of layers 3 and 4).

We may need to have an explicitly implemented layer 0 (front-end caching) in Gitweb for at least a subset of pages (like project pages), but we'll see if that's necessary.

> Caching git command output (something like IPC::Cmd::Cached?)
> would only repeat bad I/O patterns of git commands.

I hope you're not assuming that the back-end cache will reside on disk -- the problem is IO throughput, so having a cache on disk can really only work for a front-end cache. For the back-end cache, we *will* have to use a memory cache (or no cache at all).

> P.S. Here it is what could go [from Gitweb] to Git.pm: [snip]

Good points, I agree those should eventually be moved to Git.pm. (They are not my priority at the moment, since I really want to implement caching. So unless they move naturally as part of my refactoring, I'll leave them in Gitweb for now.)

-- Lea