Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module

[Jakub Narebski <jnareb@gmail.com>]

On Tue, 8 Jun 2010, Petr Baudis wrote:
> On Tue, Jun 08, 2010 at 02:46:20PM +0200, Jakub Narebski wrote:
> >
> > Third, and I think most important, is that the whole splitting gitweb into
> > modules series seems to alck direction, some underlying architecture
> > design.  For example Gitweb::HTML, Gitweb::HTML::Link, Gitweb::HTML::String
> > seems to me too detailed, too fine-grained modules.
> 
>   I agree!
> 
> > It was not visible at first, because Gitweb::Config, Gitweb::Request and to
> > a bit lesser extent Gitweb::Git fell out naturally.  But should there be
> > for example Gitweb::Escape module, or should its functionality be a part of
> > Gitweb::Util?  Those issues needs to be addressed.  Perhaps they were
> > discussed with this GSoC project mentors (via IRC, private email, IM), but
> > we don't know what is the intended architecture design of gitweb.
> 
>   I would expect Gitweb::Escape functionality to live in Gitweb::HTML
> (HTML escaping) and/or Gitweb::Request (URL escaping).

I agree... well, partially, depending if we allow Gitweb::Request to be
only about request i.e. inbound links, or also with generating outbound
links.  Thos two are tied together, e.g. with %cgi_param_mapping.

>From what Pavan says, it seems that the problem with splitting gitweb is
interdependency of various commands, and overdepenency on global variables.
That was caused by the fact that gitweb was (well, is currently) big large
monolithic script, and there were no mechanism protecting against adding
(inter)dependencies.

Perhaps instead of fine-splitting gitweb into tiny modules to avoid
circular dependencies (module A needs module B because of variable a,
module B needs module A because of variable b) we should fix overdependence
on global variables by passing more as parameters - at least where it makes
sense.

> > Should we try for Model-Viewer-Controller pattern without backing MVC
> > (micro)framework?  (One of design decisions for gitweb was have it working
> > out of the box if Perl and git are installed, without requiring to install
> > extra modules; but now we can install extra Perl modules e.g. from CPAN
> > under lib/...).  How should we organize gitweb code into packages
> > (modules)?
> 
>   I thought we already discussed MVC and sort of agreed that it's an
> overkill at this point. At least that is still my opinion on it; I'm not
> opposed to MVC per se, but to me, this modularization is a good
> intermediate step even if we go the MVC way later, and doing MVC properly
> would mean much huger large-scale refactoring than just naming a module
> Gitweb::View instead of Gitweb::HTML. Let's do it not at all, or
> properly sometime later. I think it's well out-of-scope for GSoC.

Well, let's not forget that the whole reason behind including splitting
gitweb into project that is about adding write capabilities to gitweb,
making a web interface equivalent to git-gui.  Splitting gitweb was meant
to make it easier to add new functionality without having gitweb become too
large, and maintenance nightmare.

So it would be enough to have Gitweb with core of gitweb, gitweb.perl
top-level script, Gitweb::Write or something like that for new write
functionality and Gitweb::Util containing things that are needed by Gitweb
and by Gitweb::Write(r).

> 
> > Perhaps having gitweb.perl, Gitweb::Git, Gitweb::Config, Gitweb::Request,
> > Gitweb::Util and Gitweb would be enough?
> 
>   I'm not sure what would fall into Gitweb::Util. I think Gitweb::HTML
> makes a lot of sense to have, but I don't see the advantage of finer
> graining than that - I dislike the Gitweb::HTML::* submodules as well.

Gitweb does its work in the following way: first it parses path_info,
query parameters etc. and saves them in global variables.  This is in
Gitweb::Request.  But to parse-out project name from pathinfo it needs
$projectsroot from Gitweb::Config.  Then it runs appropriate git commands
using subroutines from Gitweb::Cmd / Gitweb::Git, setting $git_dir first.
Then it parses output of git commands (probably Gitweb::Git too).
Finally it composes a response, usually HTML, but also feed (OPML, RSS,
Atom), plain text, and other output (blob_plain, snapshot); probably
Gitweb::Response, or Gitweb::Output, or Gitweb::View.

There are of course some problems.

To properly extract project from path_info in Gitweb::Request one needs
$projectroot from Gitweb::Config.  Also generated gitweb links,
i.e. href() and esc_param() etc., are tied with parsing request URL, at
least via %cgi_param_mapping.

Gitweb::Config isn't that simple either.  There is git configuration, e.g
$GIT, there is gitweb configuration, e.g. $projectroot, and there is
per-project configuration or configuration override.

Gitweb::Git / Gitweb::Cmd / Gitweb::Git::Cmd needs $GIT, but also
$git_dir, which is set using $projectroot and $project from
Gitweb::Request.  Should it include parsing output of git commands, and
auxiliary commands dealing directly with files in git repository?

Then there is dispatch and generating response.  It uses many utility
functions, like chop_str, or die_error.  Then there aresubroutines
responsible for actions / views.


Fnally there is a question which parts of those the futture write support
would need...

-- 
Jakub Narebski
Poland