compilers: add .pxd as a cython header extensions

[dcbaker]

@rgommers, I think this is correct, pxd files are used by cython, but in much the same way that a .h file is used by C/C++?

[xclaesse]

What bug does this fix?

[xclaesse]

Not that it is wrong, but I would like to understand the reason for this change, because I'm working on a huge refactor branch that affects a lot transpilers so I would like to make sure my branch gets this correct too.

[dcbaker]

I pulled this out of my branch for structured_sources with cython, but basically if you try to pass a .pxd file as an order dep (since we don't have depfile support yet), then we'll error with "no compiler can handle .pxd files"

[eli-schwartz]

It's used for generated sources for passing as order-only dependencies (foo.o: ../foo.c || foo.h)

[codecov]

Codecov Report

Merging #10146 (f48c3dd) into master (7c20890) will decrease coverage by 7.61%.
The diff coverage is n/a.

❗ Current head f48c3dd differs from pull request most recent head 1b0d088. Consider uploading reports for the commit 1b0d088 to get more accurate results

@@            Coverage Diff             @@
##           master   #10146      +/-   ##
==========================================
- Coverage   69.10%   61.49%   -7.62%     
==========================================
  Files         203      203              
  Lines       43417    43066     -351     
  Branches     9660     9457     -203     
==========================================
- Hits        30005    26485    -3520     
- Misses      11121    14466    +3345     
+ Partials     2291     2115     -176     

Impacted Files	Coverage Δ
mesonbuild/compilers/compilers.py
mesonbuild/compilers/cpp.py
mesonbuild/compilers/fortran.py
mesonbuild/coredata.py
mesonbuild/modules/gnome.py
mesonbuild/modules/python.py
mesonbuild/modules/unstable_rust.py
mesonbuild/cmake/common.py
mesonbuild/compilers/mixins/arm.py
mesonbuild/optinterpreter.py
... and 396 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c20890...1b0d088. Read the comment docs.

[tristan957]

https://cython.readthedocs.io/en/latest/src/tutorial/pxd_files.html?highlight=pxd#pxd-files

[xclaesse]

I haven't tested this branch, but I think it's a little bit more complicated than this, we don't pass header_deps to generate_cython_transpile() at all in ninjabackend. I have a branch that fix this already.

[xclaesse]

The header dependency issue should be fixed by #10149.

[dcbaker]

Right, I'm aware that it's more complicated, but you currently can't even do:

py.extension_module('foo', 'foo.pyx', 'foo.pxd')

(where 'foo.pxd' is just a static file, but you want to track it as a header) and this will at least fix that part. But you're right, the whole transpile mess is a mess right now.

[rgommers]

Thanks @dcbaker, this looks clearly correct. pxd files are indeed like C headers.

Cython actually has a third file type, pxi: https://cython.readthedocs.io/en/latest/src/userguide/language_basics.html#cython-file-types. They're not quite header files, but sort of (because you do include somefile.pxi in .pyx files). The difference with pxd files is in (a) what Cython content they can contain, whether they're used when you do cimport or include, and (c) pxd files can be shipped in a package for external users while pxi files are normally for source code reuse within a package only.

But long story short: I think you should add pxi next to pxd as a header file type here.

The header dependency issue should be fixed by #10149.

Interesting (and complex!). Please let me know if/when you want me to test your branch with the SciPy build @xclaesse.

[xclaesse]

Right, I'm aware that it's more complicated, but you currently can't even do:

py.extension_module('foo', 'foo.pyx', 'foo.pxd

Oh ok, I thought it was accepted but didn't produce the correct dependency. LGTM then!

[dcbaker]

I’ve added pxi as well, but I’m on my phone so we’ll need to do a squash merge.

[dcbaker]

Squashed locally and re-pushed

[rgommers]

Testing this, the configure stage ends with:

scipy/special/meson.build:357:4: ERROR: No host machine compiler for "linalg.pxd"

A full log can be found at /home/rgommers/code/scipy/build/meson-logs/meson-log.txt
Meson build setup failed! ({0} elapsed)

The log file has nothing more informative than that one error. Comes from:
https://github.com/scipy/scipy/blob/8b804ab637a480a81a2b22fc95e3e5cb9dfaab65/scipy/special/meson.build#L346-L361

I'm not sure if that's an actual issue with this PR, or if it simply disagrees with the workaround that we had to use until now.

[eli-schwartz]

On the general topic of this PR:

So, this enables .pxd as a "header dep", also known in the ninja manual as an "order-only" dep.

foo-generated.o: ../foo.c || ../header.h

Updates to foo.c force a rebuild of foo.o
foo.c is the input to foo.o

header.h does NOT force a rebuild of foo.o, and it is not used as input either. What does it do? It must exist, with any timestamp whatsoever.

Additionally, if there is a separate build rule for header.h and header.h is considered out of date, it will be built first, before foo.o is built.

How does this translate to cython?

# pxd is a source checked into git
foo-generated.c: ../foo.pyx || ../foo.pxd

# pxd is a generated source
foo-generated.c: ../foo.pyx || foo-generated.pxd

In the first case:

• foo.pxd does nothing. It must exist, because it is checked into git and that would be bad if it were missing. If it were missing, ninja would say it cannot even try to run cython, though.
• Updates to foo.pxd do not trigger re-cythoning. Updates to foo.pyx do trigger re-cythoning.

In the second case:

• foo.pxd must be generated before the first cythoning, and ninja will ensure that this happens
• later updates to foo.pxd do not trigger re-cythoning. Updates to foo.pyx do trigger re-cythoning.

It was observed above that this does not actually handle generated pxd files to add them as order-only ("header") deps, so that would mean this PR only unlocks the first case and not the second case.

The first case does NOT need header deps to do a clean ("scratch") build.

The second case DOES need header deps to do a clean ("scratch") build.

Both cases need depfiles, which is an unrelated topic, in order to do an incremental rebuild.

[eli-schwartz]

Given my analysis, I think this is not an urgent 0.62.0 bugfix.

Given @rgommers' test above, I'm afraid to merge this without looking a lot closer into the relevant issues, and with the safety net of a development cycle to avoid stable regressions.

[ev-br]

(since we don't have depfile support yet)

Given that Cython supports depfiles for versions >=0.29.27, I sort of wonder how to add their support to meson and whether this changes anything for this PR

[rgommers]

To connect the dots: this fixes gh-9049.

Given @rgommers' test above, I'm afraid to merge this without looking a lot closer into the relevant issues, and with the safety net of a development cycle to avoid stable regressions.

I think it'll be helpful to derive a new test case from the generator pattern we're using in SciPy now to compile SciPy code and deal with generated .pyx files + .pxd files. I'll put this on my TODO list.

[eli-schwartz]

I did add depfile support to cython's non-setuptools frontend, and it's backported into the 0.29.x series -- it just still hasn't been tagged in a new cython release.