On Thu, Apr 5, 2012 at 11:26 AM, Nick Wellnhofer <[email protected]> wrote:
> 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.
OK, that makes sense.
Can we still consolidate "core_source_dir" and "extra_c_sources" into
a single "source" param?
>> 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.
So your approach is to do what it takes to make those .h files available
because otherwise the autogenerated files won't compile -- but the
installation of those .h files is an implementation detail and they are not
part of the interface exposed to extension writers.
Sounds ideal. :)
>> 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.
We might benefit from taking a step back and considering how various compiler
interfaces allow the user to communicate what files should be compiled.
* cc, gcc, clang etc.
* Various IDEs, e.g XCode.
* javac
* perl/python/ruby interpreters
* etc.
Right now, CFC is closer to an IDE, javac, or an interpreter than to
command-line cc, in that CFC wants to compile the whole project in one session
rather than a compile a single source file to a single object file.
CFC is also closer to an IDE than any of the others in that it starts from a
multi-file specification.
>>> 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.
IMO, it's desirable to avoid burdening extension writers with having to write
their $VERSION assignment code in a format we can parse.
>> 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?
+1 to keep dist_version(). Both you and David Wheeler have suggested it
independently. I'm persuaded.
In addition, we should apply the patch below my sig, which changes
XSLoader::load to use the same X.Y.Z version format that we supply as the
dist_version argument to the Lucy::Build constructor.
For the record, I'd misremembered the call to XSLoader::load(). I had thought
it looked like this:
XSLoader::load( 'Lucy', $Lucy::VERSION );
Turns out we were already passing a literal:
XSLoader::load( 'Lucy', '0.003000' );
The patch below changes it to be a different literal:
XSLoader::load( 'Lucy', '0.3.0' );
That variant tested successfully with both stock perl 5.10.0 and stock perl
5.15.4.
Marvin Humphrey
diff --git a/devel/bin/update_version b/devel/bin/update_version
index 72e5ad4..1e27126 100755
--- a/devel/bin/update_version
+++ b/devel/bin/update_version
@@ -50,7 +50,7 @@ my $buf;
$buf = read_file('perl/lib/Lucy.pm');
$buf =~ s/(our \$VERSION\ +=\ +)'.+?'/$1'$x_yyyzzz_version'/
or die "no match";
-$buf =~ s/XSLoader::load\( 'Lucy', '(.+?)'/XSLoader::load\( 'Lucy',
'$x_yyyzzz_version'/
+$buf =~ s/XSLoader::load\( 'Lucy', '(.+?)'/XSLoader::load\( 'Lucy',
'$x_y_z_version'/
or die "no match";
write_file( 'perl/lib/Lucy.pm', $buf );
diff --git a/perl/lib/Lucy.pm b/perl/lib/Lucy.pm
index df942ce..97c3c47 100644
--- a/perl/lib/Lucy.pm
+++ b/perl/lib/Lucy.pm
@@ -27,7 +27,7 @@ $VERSION = eval $VERSION;
use XSLoader;
# This loads a large number of disparate subs.
BEGIN {
- XSLoader::load( 'Lucy', '0.003000' );
+ XSLoader::load( 'Lucy', '0.3.0' );
_init_autobindings();
}
