On Sat, May 21, 2016 at 8:18 AM, Nick Wellnhofer <wellnho...@aevum.de> wrote:
> On 20/05/2016 05:37, Marvin Humphrey wrote:
>>
>> It would also be great if we set our own flags for compiling the core code
>> and kept them the same for all hosts -- instead of compiling the core code
>> using per-host flags as we do right now.
>
> IIRC, we had this discussion already.

You're probably right but I tried to find such discussion in the dev@lucy
archives and came up empty.

> The problem is that there are certain
> C compiler flags that change the generated code in an incompatible way.

This seems similar to the HAS_BOOL issues with Perl XS extensions
(<http://markmail.org/message/5etl3pgzjkjzsxdx>).

> These compiler flags must be reused when compiling code for host language
> bindings. A good example is `-m32` to create 32-bit binaries on 64-bit
> systems. Other examples are '-mms-bitfields` (used by some Windows Perls)
> and flags concerning data alignment.
>
> So we'd have scan the host language C flags and make sure to reuse all flags
> that cause incompatible code to be generated. Since there is no conclusive
> list of all these flags, this approach is error-prone and will probably need
> occasional updates. I'm not saying it can't be done, but for now, I'd prefer
> to simply use all the compiler flags proposed by the host language. Maybe it
> would make more sense to have a blacklist of flags that are safe to remove?

My guess, which admittedly doesn't have a lot to back it up, is that although
there is an open set of potentially incompatible flags, there aren't that many
that we wouldn't find out about right away and be able to fix via
whitelisting.

It has been inconvenient to try to specify our own flags for core when there
may be conflicts with some host-specified flags.  I've run into that when
trying to specify architecture targets, disable optimizations, specify the
desired dialect of C, etc.

>> Combining them is great.  FWIW in the Python bindings I dumped the boot,
>> callbacks and binding code into a single autogenerated file, named after
>> the parcel (e.g. `python/_clownfish.c`, `python/_lucy.c`).  In the Go
>> bindings, all that stuff is in `go/PACKAGE/cfbind.go`, e.g.
>> `go/lucy/cfbind.go`.
>
>
> So these files are not under `autogen`, but somewhere else under the
> `python` or `go` directories?

Right.  Because host build systems often expect files containing extension
code to live in specific locations and they aren't amenable to custom layouts.

We actually ran into this with Module::Build, which expected only a single
source directory for C files -- necessitating a bunch of manual workaround
code to compile all our C source files.

Thus the more general strategy, which I hope will work across all hosts is:

1. Build a static archive from the `core` code.
2. Generate host-specific extension code wherever the host wants it to live.
3. Build the host extension telling the build system about various include
   directories and telling it to link in the static archive.

>> The tricky bit here is that these files have per-host code, as opposed to
>> the core code which is host-agnostic.  So I actually think we should rely
>> on the host to build them rather than the Charmonizer-generated `Makefile`.
>
> Makes sense. I'll change the Perl bindings to follow this approach.

+1

>> I would add these as separate categories:
>>
>> - static test  // ideally in `$parceldir/test` next to `$parceldir/core`
>> - static host-specific test  // maybe `$host_lang/cftest`
>>
>> (I have a slight preference for `test` for the dir where the core tests
>> file
>> live, as opposed to `tests`.)
>
>
> Another idea is to use `$parceldir/core/test` for the tests and have CFC
> ignore directories named `test` (or even any directory starting with a
> lowercase letter). This doesn't clutter up the root directory.

There's a tradeoff of shorter filepaths for test files vs. one more entry in
the package dir; I'd go for shorter filepaths.

> `$host_lang/cftest` is something we don't actually need for now.

There has actually been a small amount of test-only XS code historically, but
it's been baked into the XS binding code under
`perl/buildlib/Lucy/Build/Binding/`.

>> If we accept that there must be a 1:1 relationship between Clownfish
>> parcels and host extensions, this problem gets a lot easier. :)
>
>> Not an issue if there can only be one parcel. :) Then the only problem is
>> to build with or without including the object code for the compiled tests.
>
>
> This is a reasonable approach. Let's play it through:
>
> - We split the generated code into test and non-test files, just like
>   we would for separate binaries.
> - Then we can compile the C code and build two binaries, one with
>   the test and non-test objects, one with only non-test objects.
> - We use the test binary only to run the tests and make sure it won't
>   be installed.

Sounds good.  The devil is in the details though for host bindings, since this
isn't a configuration option they would ordinarily be expected to support.

> I think adding support for multiple parcels with separate binaries per
> parcel isn't much harder. Instead of building a binary with test and
> non-test code, we'd build a binary that contains only the test code. This
> should have other uses besides testing. OTOH, having separate binaries
> places some restrictions on the test code.

That may be, but as discussed before,
(<http://markmail.org/message/tmn4jb3yveftumu5>) Clownfish parcels were
intended to represent atomic units of installation in order to facilitate
versioning of dependencies.  Having multiple parcels installed together
complicates the mechanisms to handle resolving compatible versions and advise
what and when to upgrade.

> I have a local branch that implements separate binaries per parcel for C and
> Perl. A couple of things still have to be sorted out, mainly passing the
> correct -DCFP_* flags (this is something that's easier when having the test
> code in the same binary). I'll post it for review, then we can decide how to
> proceed. Most of the changes will be useful anyway.

OK, looking forward to it!

Marvin Humphrey

Reply via email to