capnproto / pycapnp

Cap'n Proto serialization/RPC system - Python bindings
BSD 2-Clause "Simplified" License
458 stars 125 forks source link

Make `reraise_kj_exception` available to downstream #344

Closed LasseBlaauwbroek closed 8 months ago

LasseBlaauwbroek commented 8 months ago

I'm using Pycapnp in a project, where we compile .capnp files directly to Cython instead of using the dynamic interface (for speed). For this, we need access to the reraise_kj_exception C function defined by Pycapnp. This is not possible, because Cython does not automatically make this function available to downstream users.

My previous solution, in #301, was rather flawed. The file capabilityHelper.cpp, where reraise_kj_exception is defined, was bundled into the distribution, so that this file could be included in downstream libraries. This turns out to be a terrible idea, because it redefines a bunch of other things like ReadPromiseAdapter. For reasons not entirely clear to me, this leads to segmentation faults. This PR revers #301.

Instead, in this PR I've made reraise_kj_exception a Cython-level function, that can be used by downstream libraries. The C-level variant has been renamed to c_reraise_kj_exception.

LasseBlaauwbroek commented 8 months ago

The CI failures are unrelated. Master is also failing: https://github.com/LasseBlaauwbroek/pycapnp/actions/runs/6704200082/job/18216165343 Seems like something has changed in the ecosystem.

LasseBlaauwbroek commented 8 months ago

I have no idea what is going on here. I can reproduce this locally:

This suggests to me that there is a loadpath issue. Python seems to be looking up the .py files in the local directory. This is normal as far as I know. But then, it cannot find the binary capnp.cpython-*.so file locally. But I'd think it should be looking that up in the lib location of the virtualenv. I checked, and sys.path indeed includes the lib directory of the virualenv.

I'm at a loss here. @haata @tobiasah can you reproduce this? Any idea what is going on?

haata commented 8 months ago

I'll try to take a look later this weekend.

LasseBlaauwbroek commented 8 months ago

Looks like the build issue has resolved itself. Probably some dependency that was temporarily faulty or something like that.

haata commented 8 months ago

(Re-read your PR after commenting, makes sense)