incubator-lucy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Wellnhofer <wellnho...@aevum.de>
Subject Re: [lucy-dev] Rethinking header file installation
Date Thu, 15 Mar 2012 20:51:16 GMT
On 14/03/2012 23:17, Marvin Humphrey wrote:
> On Wed, Mar 14, 2012 at 1:02 PM, Nick Wellnhofer<wellnhofer@aevum.de>  wrote:
>> On 12/03/2012 00:41, Marvin Humphrey wrote:
>>>
>>> Personally, I believe that these are all positive developments for the
>>> project, so +1 to install the .cfh files instead of the autogenerated .h
>>> files.
>>
>>
>> OK, so I would propose to install them in:
>>
>>     catdir ( $ARCH_DIR, 'Clownfish', '_cfh' );
>
> I have a mild preference for naming that directory "_include" as opposed to
> "_cfh" for a couple reasons:
>
>    1. That dir might wind up containing things other than .cfh files, such as
>       .h files that have been pound-included by .cfh files (in a literal C
>       section).
>    2. The string "cfh" is adequate as a file extension (which has to be short),
>       but in my opinion "_include" is more self-documenting as a directory name.

OK.

>> Then Clownfish has to be extended to read files from multiple source
>> directories. I'd propose to add an array of include directories to the
>> arguments of CFCHierarchy_init and use them to build the hierarchy.
>
> +1 in principle.
>
> I'd advocate for a slightly different implementation: strip the "source"
> argument out of CFCHierarchy_new() and CFCHierarchy_init(), and add two new
> functions:
>
>      /** Add a directory containing Clownfish source files to compile.
>       */
>      CFCHierarchy_add_source_dir(CFCHierarchy *self, const char *dir);
>
>      /** Add an include directory containing interface-defining "header" files.
>       */
>      CFCHierarchy_add_include_dir(CFCHierarchy *self, const char *dir);

This would mean to handle multiple source directories, too. I can't see 
an immediate use case, but it shouldn't be too hard to implement once 
multiple include dirs are supported.

> If you want, feel free to add "include" and "source" labeled parameters to the
> Perl binding for new(), since constructors with lots of labeled params are a
> common Perl idiom:
>
>      my $hierarchy = Clownfish::CFC::Model::Hierarchy->new(
>          dest    =>  $output_dir,   # maybe rename to "output"?
>          source  =>  \@source_dirs,
>          include =>  \@include_dirs,
>      );
>
> Behind the scenes, though, have new() invoke add_source_dir() and
> add_include_dir() before returning.
>
> I have two reasons for making this suggestion:
>
>    * C functions with numerous arguments are not self-documenting and are hard
>      to use.
>    * It's not possible to map a Perl array to "const char**" automatically.
>      You typically have to allocate memory and build up the array manually in
>      the binding, which is messy, annoying, error-prone, and leaks memory when
>      an exception is thrown.

Yes, that makes sense.

>> Parts of the code in Lucy::Build can then be reused to build the extension.
>> This code could be factored out into a module like Clowfish::Build that is
>> installed together with Clownfish.
>
> I think we probably want to stay out of the Clownfish:: first-level namespace
> as much as possible, because that's where all our basic object types are going
> to go.  I realize I'm being a bit hypocritical here because I dumped CFC into
> Clownfish::, but I figured that there would never be a collision.

Ah, good to know. I wondered about that already.

> Would "Clownfish::CFC::Build" be appropriate?  (We're using that privately
> right now, but no big deal to vacate it.)

That's OK with me.

Nick

Mime
View raw message