mesonbuild / meson-python

Meson PEP 517 Python build backend
https://mesonbuild.com/meson-python/
MIT License
118 stars 59 forks source link

Document how to use generated Cython `.pxi` headers #589

Open WillAyd opened 4 months ago

WillAyd commented 4 months ago

Sometimes when compiling pandas you end up having to compile twice to get the library to completely build. The files that get compiled the second time tend to be the outputs of a Tempita process.

Here are some small excerpts from pandas (some parts intentionally omitted to try and keep minimal):

_hashtable_class_helper = custom_target('hashtable_class_helper_pxi',
    output: 'hashtable_class_helper.pxi',
    input: 'hashtable_class_helper.pxi.in',
    command: [
        py, tempita, '@INPUT@', '-o', '@OUTDIR@'
    ]
)
_hashtable_func_helper = custom_target('hashtable_func_helper_pxi',
    output: 'hashtable_func_helper.pxi',
    input: 'hashtable_func_helper.pxi.in',
    command: [
        py, tempita, '@INPUT@', '-o', '@OUTDIR@'
    ]
)

...
cython_args = [
    '--include-dir',
    meson.current_build_dir(),
    '-X always_allow_keywords=true'
]

py.extension_module(
    hashtable,
    sources: ['hashtable.pyx', _hashtable_class_helper, _hashtable_func_helper],
    cython_args: cython_args,    
    ...
)

When I look at the dotgraph that ninja generates, I noticed that the tempita outputs are declared as dependencies of both the cython_COMPILER and c_COMPILER steps, which is where I think a possible race condition leading up to the c_COMPILER step could be what requires a recompile.

Shouldn't the generated cython files only be a dependency for the cython_COMPILER? Is there a way to explicitly declare this?

WillAyd commented 4 months ago

Meant to include the dotgraph - here it is for reference image

dnicolodi commented 3 months ago

At a quick look this looks like a bug. However, the bug is in Meson, not in meson-python. It should be reported there. If you can synthesize a minimal example that shows the problem (not the compilation error, but the bogus dependencies) it would make isolating and fixing the problem much easier. I suspect that there are not that many projects using generated .pxi files, thus this has not being noticed before.

dnicolodi commented 3 months ago

I had a quick look and actually Meson includes a test case that does precisely this: a Cython extension module with a generated .pxi import. However, the test case does not add the .pxi files to the extension module sources: it adds them as dependencies.

In your example, this would be something like:

_hashtable_pxi_dep = declare_dependency(sources: _hashtable_class_helper, _hashtable_func_helper)

py.extension_module(
    hashtable,
    'hashtable.pyx',
    dependencies: _hashtable_pxi_dep,
    cython_args: cython_args,    
    ...
)

Can you check if this solves your issue?

WillAyd commented 3 months ago

Ah OK - that makes sense. As far as the dotgraph is concerned I still the same output but will test and see

WillAyd commented 3 months ago

Looks like that did it - thanks @dnicolodi !

dnicolodi commented 3 months ago

There are a few subtleties in compiling Cython. It would be nice to have a section in the documentation dedicated to Cython. Let's turn this issue into a documentation issue.