On 05/04/2012 18:35, Marvin Humphrey wrote:
On Thu, Apr 5, 2012 at 5:14 AM, Nick Wellnhofer<[email protected]> wrote:
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.
Could "core_source_dir" and "extra_c_sources" be consolidated into a single
"source" param?
And then can we leverage the standard Module::Build "c_source" param to hold
directories which contain C code exclusively (i.e. no .cfh files)?
I think we can just dump all of Lucy's C source directories into
$clownfish_params{source} to work around the limitations of c_source in
versions of M::B prior to 0.3604.
If we start to use the c_source parameter, M::B will try to compile and
link our .xs and .c files. This should work even with M::B prior to
0.3604 if we copy all .c file into a single directory, but it doesn't
solve the problem of how to actually pass the list of C sources with
older M::B's. I think we'll need an extra parameter for that.
At the moment, we have our own code in ACTION_compile_custom_xs which
doesn't care about c_source. I'd prefer to keep it until we decide it's
OK to require M::B 0.3604 at some point in the future.
The only annoying bit is that we might want
to separate out any .h files lest they be vacuumed up and installed into the
clownfish system include dir.
For now, I decided to only copy selected .h files manually. See patch
0015 in LUCY-215.
On a related note, I would suggest that we rename
$clownfish_params{cf_include_dirs} to $clownfish_params{include}.
* The need for the "cf_" prefix is not strong because the param is nested
within "clownfish_params". (Things would be different if we were adding
top-level M::B params).
My first choice was 'include_dirs'. I only added the 'cf_' prefix to
avoid confusion with the 'include_dirs' parameter of M::B. Using only
'include' is OK with me.
* IMO, it would be nice to parallel the "include" and "source" attributes of
CFCHierarchy by using the same names.
Yes, I thought about that. We should ultimately support multiple source
directories in the same way as include directories. It just needs a bit
of work in various parts of the build process.
I can try to mirror the API we would get with
Module::Build#add_property using an inside-out class attribute.
I think we might consider just using an ordinary hash param inside the
Module::Build object. I seem to recall some weirdness with Module::Build
object serializing and deserializing itself in all the process-spawning chaos,
and I'm not sure that an inside-out attribute will survive.
I had a look at the source code of M::B 0.2808_01 (which we already
require), and it should support add_property just fine. Only the
'default' parameter is passed in a different way, and the 'check'
parameter isn't supported, but we could work around that with a version
check.
I'm also OK with a hash param.
I don't mean that having XSLoader crash out wouldn't be a serious problem, but
it is really easy to recover from with a subsequent release. The version
number in question only ends up in the shared object.
Ah, it's only a compile/install time problem.
It would be easy to switch to the old way of parsing the version number from
Lucy.pm.
I don't think that's true any more because that code block has moved into
Clownfish::CFC::Build::Perl, right?
We could derive the path to the main .pm file from module_name like we
do for the rest of the build files, and try to parse the version from there.
Invoking dist_version() is really the
right design choice.
>
Maybe we can adapt Lucy.pm instead, passing e.g. a literal version string
instead of $Lucy::VERSION.
I'm not sure what's the best way to deal with that. Maybe keep
dist_version and switch back if it causes problems?
Nick
