On 05/04/2012 07:22, Marvin Humphrey wrote:
The patch sequence looks great. +1 to commit, with the caveat that I'd like to
relitigate the minimum version for Module::Build. :)
A few notes:
* The end product, Clownfish::CFC::Perl::Build has no POD describing its
public API. That's the most important part! You're such a strong coder
that we can have a lot of confidence in your implementations -- but we
need to build consensus about the public interface.
I suppose all the features utilized by the extension Build.PL file you
provided that are not in the Module::Build API constitute the
public API, no?
I will write some POD once the implementation is stable. Currently, the
C::C::P::B API looks like this:
C::C::P::B is a subclass of Module::Build.
There's a new constructor argument 'clownfish_params' that takes a
hashref containing the following entries:
* cf_include_dirs: arrayref of Clownfish include dirs containing .cfh
files. Empty by default. Extensions will typically set this to
cf_system_include_dirs (see below).
* extra_c_sources: arrayref of extra C source dirs or files. Empty by
default.
* autogen_header: header for autogenerated files. Defaults to a "do not
edit" text without license.
* core_source_dir: directory containing the core Clownfish sources (.cfh
and .c files). Defaults to 'core' or '../core'. Doesn't have to be
provided normally.
There's also an accessor for 'clownfish_params'.
Then there are two class methods:
my @dirs = C::C::P::B->cf_system_include_dirs;
Returns the list of system-wide Clownfish include dirs in
$Config{installsitearch} and $Config{installvendorarch}.
my $path = C::C::P::B->cf_system_library_file($module_name);
Returns the installed library file (.so or .dll) for a module. This is
needed by extensions which have to link against the library of the
module they're extending.
* We currently require Module::Build version 0.2808_01, which is what
shipped with Perl 5.10.0. I'm reluctant to bump the Module::Build version
number to 0.3404 in order to get c_source accepting multiple directories,
since that would exclude stock installs of Perl 5.10.x and 5.12.x. Can we
remain with 0.2808_01 and hack up something approximating the
clownfish_params property instead of bumping to 0.31 to get
Module::Build#add_property?
Yes, that's possible. I can try to mirror the API we would get with
Module::Build#add_property using an inside-out class attribute.
* See http://markmail.org/message/hx3nmnu2mtt2t3mb for why we have been
avoiding M::B#dist_version up till now. Man, how I hate Perl's TMTOWTDI
version numbers. :(
I'm OK with this change nevertheless, because the worst it will do is
cause XSLoader to error out because of a mismatch between an X.Y.Z version
in the shared object and an X.YYYZZZ version in Lucy.pm. If that happens,
we'll get a bunch of CPAN testers failures.
Doesn't that mean that Lucy can't be loaded which sounds like the worst
case to me? Is it that only that some versions of XSLoader have
problems? It would be easy to switch to the old way of parsing the
version number from Lucy.pm.
* I'd like to do some work on parcels. It looked like you were handling
them as an implementation detail to be generalized, so I don't think we'll
collide.
Yes, I simply use the last part of the module name as parcel without any
configuration mechanism. That seems to work for extensions.
Nick
