Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyRFC versions < 3.1 fail to build from source with Cython 3 #332

Closed
bsrdjan opened this issue Jul 25, 2023 · 5 comments
Closed

PyRFC versions < 3.1 fail to build from source with Cython 3 #332

bsrdjan opened this issue Jul 25, 2023 · 5 comments
Assignees

Comments

@bsrdjan
Copy link
Contributor

bsrdjan commented Jul 25, 2023

After Cython 3.0 released last week, older PyRFC versions fail to pip install on Linux systems and build from source.

To fix the issue upgrade PyRFC to latest version.

PyRFC versions prior to 3.1 require Cython 0.29 for build from source, for example

pip install 'Cython<3'
pip install pyrfc==2.4.1

Recommended solution is using the latest PyRFC version, or Cython < 3, if PyRFC version < 3.1 required.

@da-woods
Copy link

da-woods commented Aug 9, 2023

The issue is mainly that Cython now allows cdef functions to raise exceptions by default, but you're trying to assign to function pointers that don't accept Python exceptions.

The solution is to add noexcept to any cdef functions that need assigning to an exception-free function pointers.

Please do ping me if you need further help!

@bsrdjan
Copy link
Contributor Author

bsrdjan commented Aug 9, 2023

Thanks @da-woods! It should work with PyRFC version 3.1, did you try it?

@da-woods
Copy link

da-woods commented Aug 9, 2023

Ah sorry I didn't read this carefully enough and didn't realise you already had a version that worked with Cython 3.

(I was writing as a Cython maintainer who thought you might need help rather than as someone trying to use PyRFC.)

@bsrdjan
Copy link
Contributor Author

bsrdjan commented Aug 17, 2023

Thanks @da-woods, I just made the pinned issue comment more clear. I managed to build but I did not manage yet to split the single big Cython source file I currently have. There are few classes in there, like Client and Server, both using common functions. Recommendation how to better structure this Cython monolith would be really helpful.

@da-woods
Copy link

Recommendation how to better structure this Cython monolith would be really helpful.

  • You might well be able to change the enums in https://github.com/SAP/PyRFC/blob/main/src/pyrfc/csapnwrfc.pxd to cpdef enum - that creates the Python wrapper manually, so you don't need to do https://github.com/SAP/PyRFC/blob/2c0432baf2b3c8d7bd86979030661bc806fa94dc/src/pyrfc/_cyrfc.pyx#L42-L46. You might need create a csapnwrfc.pyx file for this to work - I'm not sure off the top of my head. It probably doesn't need much in it.

  • Anything that's a standalone def function or a regular (i.e. non-cdef) class gets compiled by Cython, but other Cython functions have no-special visible visibility into them. This means you could move them out of the monolithic module into their own extension modules, and just import them (nothing to cimport).

  • It looks like you have 4 main cdef classes. You could split ConnectionParameters and Connection parameters in one module. (declare them in a .pxd file so that other classes can us them). Server looks like a thing on its own. Not sure about Throughput

  • Use pairs of files something.pxd and something.pyx. something.pyx creates a shared module. something.pxd allows other Cython stuff to cimport from that module. Some you'd have a connection_stuff.pxd/pyx pair, and server.pyx can do cimport connection_stuff.

Hopefully that's some use. It doesn't look like there's hugely complicated interdependencies there, so it's mostly just a case of splitting them up into natural Python modules, and using .pxd and cimport to access static typing between the modules.

@bsrdjan bsrdjan closed this as completed Oct 9, 2023
@bsrdjan bsrdjan unpinned this issue Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants