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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 12, 2010, 01:22 UTC
Message-ID: <AANLkTimfzjZ00ua23FDmUJLil-_OEPEkiS73syVCQ52f@mail.gmail.com>
In-Reply-To: <201006120301.37931.jnareb@gmail.com>

On Sat, Jun 12, 2010 at 01:01, Jakub Narebski <jnareb@gmail.com> wrote:

Show 11 quoted lines

> On Tue, 8 June 2010, Ævar Arnfjörð Bjarmason wrote:
>
>> I haven't contributed to Gitweb, nor do I have to deal with it. But
>> I've followed this series and reviewed most of the Perl code in
>> Git. Take these with a grain of salt.
>>
>> It would be very useful for the future of our Perl code if we had a
>> dual-life system in Git. I.e. a cpan/ directory where we could drop
>> CPAN modules that should be shipped with Git.
>
> The standard name for such directory is 'inc/', I think.

Perl itself uses cpan/, but Module::Install started the inc/. What we call it really doesn't matter though.

> There is for example 'inc::latest' module (unfortunately in core only
> since latest Perl, i.e. version 5.12), which uses modules bundled in
> 'inc/' if they are newer than installed ones.  It is in Module-Build
> distribution.

Interesting. I hadn't looked at it.

> BTW. it's a pity that PAR (Perl Archiving Toolkit, par.perl.org)
> is not in core...

It has a bunch of off edge cases unlike its namesake .jar, but in any case using it wouldn't help us.

Show 9 quoted lines

>> We already do this in a less sophisticated way for Error.pm, is there
>> any reason not to expand it to install more CPAN modules if they
>> aren't present on the system? That'd allow us to use them, but still
>> only depend on vanilla Perl.
>
> Sidenote about Error.pm: from what I understand modern consensus is
> that Error.pm was a failed approach, and currently recommended way to
> use exceptions in Perl is via block form of eval, e.g. Try::Tiny or
> TryCatch (this requires Moose and PPI), and not Error.

Yeah, but it was just an example of how we can (and should) bundle modules with Git.

Show 8 quoted lines

>> Then we could just use e.g. Config::General (~3k lines of code)
>> instead of writing our own config system. There are probably lots of
>> wheels that we're inventing (and are going to invent) that have been
>> done better elsewhere, with more testing.
>
> The problem with _optional_ Config::General config is that people
> would have incompatibile gitweb config files, some using Config::General
> syntax, some current configuration in Perl.

Isn't the current patch series the first attempt at config file support? I.e. it's always been editing the source until now.

In any case, proper non-executable config file support could easily be made optional, hopefully with a transition the non-executable one.

Show 10 quoted lines

>> Unlike Python or Java, Perl's policy is for core modules is to only
>> include those required to better bootstrap the CPAN toolchain. So if
>> we continue sticking to core Perl our code is only going to drift
>> further away from Perl best practices.
>
> Right.
>
> Still, we don't want to require having half of CPAN in 'inc/' to
> install gitweb.  Perhaps "no non-core dependencies" is too strict,
> and should be replaced by "minimal non-core depencencies".

Obviously we'd pick modules to include carefully.