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

On Tue, 8 Jun 2010, Petr Baudis wrote:

On Tue, Jun 08, 2010 at 02:46:20PM +0200, Jakub Narebski wrote:

quoted

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!

quoted

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.

quoted

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).

quoted

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