pybind/cephfs: Fix cython compilation error by kotreshhr · Pull Request #62099 · ceph/ceph

kotreshhr · 2025-03-04T08:09:02Z

cephfs.pyx:2324:40: Cannot assign type
'void (int, const void *, size_t, const void *, size_t, void *) except * nogil'

to

'libcephfs_c_completion_t' (alias of 'void (*)(int, const void *, size_t, const void *, size_t, void *) noexcept nogil').

Introduced-by: 72d0a76

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins test classic perf Jenkins Job | Jenkins Job Definition
jenkins test crimson perf Jenkins Job | Jenkins Job Definition
jenkins test signed Jenkins Job | Jenkins Job Definition
jenkins test make check Jenkins Job | Jenkins Job Definition
jenkins test make check arm64 Jenkins Job | Jenkins Job Definition
jenkins test submodules Jenkins Job | Jenkins Job Definition
jenkins test dashboard Jenkins Job | Jenkins Job Definition
jenkins test dashboard cephadm Jenkins Job | Jenkins Job Definition
jenkins test api Jenkins Job | Jenkins Job Definition
jenkins test docs ReadTheDocs | Github Workflow Definition
jenkins test ceph-volume all Jenkins Jobs | Jenkins Jobs Definition
jenkins test windows Jenkins Job | Jenkins Job Definition
jenkins test rook e2e Jenkins Job | Jenkins Job Definition

vshankar

LGTM

vshankar · 2025-03-04T09:30:45Z

(needs to be included in squid backport #62095)

kotreshhr · 2025-03-04T11:06:33Z

Weird!! It's now failing https://jenkins.ceph.com/job/ceph-pull-requests/152959/consoleFull#-183961699c212b007-e891-4176-9ee7-2f60eca393b7

Error compiling Cython file:
------------------------------------------------------------
...
    PyObject *PyBytes_FromStringAndSize(char *v, Py_ssize_t len) except NULL
    char* PyBytes_AsString(PyObject *string) except NULL
    int _PyBytes_Resize(PyObject **string, Py_ssize_t newsize) except -1
    void PyEval_InitThreads()

cdef void completion_callback(int rc, const void* out, size_t outlen, const void* outs, size_t outslen, void* ud) nogil noexcept:
                                                                                                                       ^
------------------------------------------------------------

cephfs.pyx:114:120: Syntax error in C variable declaration

Something is different in my local setup!!! I think we don't need this fix ?

If this matters

kotresh:build$ cython --version
Cython version 3.0.9
kotresh:build$ python --version
Python 3.12.8
kotresh:build$ g++ --version
g++ (GCC) 14.2.1 20240912 (Red Hat 14.2.1-3)

vshankar · 2025-03-04T11:34:42Z

Something is different in my local setup!!! I think we don't need this fix ?

Not too much familiar with Cython, but were builds failing in shaman without this change?

EDIT: s/shaman/make check/

dparmar18 · 2025-03-04T11:54:46Z

Weird!! It's now failing https://jenkins.ceph.com/job/ceph-pull-requests/152959/consoleFull#-183961699c212b007-e891-4176-9ee7-2f60eca393b7

Error compiling Cython file:
------------------------------------------------------------
...
    PyObject *PyBytes_FromStringAndSize(char *v, Py_ssize_t len) except NULL
    char* PyBytes_AsString(PyObject *string) except NULL
    int _PyBytes_Resize(PyObject **string, Py_ssize_t newsize) except -1
    void PyEval_InitThreads()

cdef void completion_callback(int rc, const void* out, size_t outlen, const void* outs, size_t outslen, void* ud) nogil noexcept:
                                                                                                                       ^
------------------------------------------------------------

cephfs.pyx:114:120: Syntax error in C variable declaration

Something is different in my local setup!!! I think we don't need this fix ?

If this matters

kotresh:build$ cython --version
Cython version 3.0.9
kotresh:build$ python --version
Python 3.12.8
kotresh:build$ g++ --version
g++ (GCC) 14.2.1 20240912 (Red Hat 14.2.1-3)

Can you try interchanging the position of noexcept and nogill? im coming from this article https://youtrack.jetbrains.com/issue/PY-59249/Latest-Cython-3.0.0b1-except-noexcept-legal-syntax-red-lined, i guess it's just a syntactical change in cython 3.X.X

batrick · 2025-03-04T14:47:01Z

Does this patch work for you?

diff --git a/src/pybind/cephfs/c_cephfs.pxd b/src/pybind/cephfs/c_cephfs.pxd
index ddb2659a4dfd..882d6ac6757e 100644
--- a/src/pybind/cephfs/c_cephfs.pxd
+++ b/src/pybind/cephfs/c_cephfs.pxd
@@ -72,7 +72,7 @@ cdef extern from "cephfs/libcephfs.h" nogil:
     int ceph_setattrx(ceph_mount_info *cmount, const char *relpath, statx *stx, int mask, int flags)
     int ceph_fsetattrx(ceph_mount_info *cmount, int fd, statx *stx, int mask)
 
-    ctypedef void (*libcephfs_c_completion_t)(int rc, const void* out, size_t outlen, const void* outs, size_t outslen, void* ud) nogil
+    ctypedef void (*libcephfs_c_completion_t)(int rc, const void* out, size_t outlen, const void* outs, size_t outslen, void* ud) except * nogil
     int ceph_mds_command2(ceph_mount_info* cmount, const char* mds_spec, const char** cmd, size_t cmdlen, const char* inbuf, size_t inbuflen, int one_shot, libcephfs_c_completion_t c, void* ud)
 
     int ceph_mds_command(ceph_mount_info *cmount, const char *mds_spec, const char **cmd, size_t cmdlen,
diff --git a/src/pybind/cephfs/cephfs.pyx b/src/pybind/cephfs/cephfs.pyx
index b6b810c7ca61..6b71b458d3d8 100644
--- a/src/pybind/cephfs/cephfs.pyx
+++ b/src/pybind/cephfs/cephfs.pyx
@@ -111,7 +111,7 @@ cdef extern from "Python.h":
     int _PyBytes_Resize(PyObject **string, Py_ssize_t newsize) except -1
     void PyEval_InitThreads()
 
-cdef void completion_callback(int rc, const void* out, size_t outlen, const void* outs, size_t outslen, void* ud) nogil:
+cdef void completion_callback(int rc, const void* out, size_t outlen, const void* outs, size_t outslen, void* ud) except * nogil:
     # This GIL awkwardness is due to incompatible types with function pointers defined with mds_command2:
     with gil:
         pyout = (<unsigned char*>out)[:outlen]

batrick · 2025-03-04T16:26:36Z

Testing the above patch in make check here: #62107

kotreshhr · 2025-03-04T16:28:00Z

src/pybind/cephfs/c_cephfs.pxd

Yes, it works but I see a peformance hint. I think that should be fine ?

performance hint: cephfs.pyx:114:5: Exception check on 'completion_callback' will always require the GIL to be acquired.
Possible solutions:
	1. Declare 'completion_callback' as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
	2. Use an 'int' return type on 'completion_callback' to allow an error code to be returned.
Compiling cephfs.pyx because it changed.
[1/1] Cythonizing cephfs.pyx
running build
running build_ext
building 'cephfs' extension
/usr/lib64/ccache/gcc -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -fexceptions -fcf-protection -fexceptions -fcf-protection -fexceptions -fcf-protection -iquote/home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/src/include -w -Dvoid0=dead_function(void) -D__Pyx_check_single_interpreter(ARG)=ARG##0 -fPIC -I/usr/include/python3.12 -I/usr/include/python3.12 -c /home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/build/src/pybind/cephfs/cephfs.c -o /home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/build/lib/cython_modules/temp.linux-x86_64-cpython-312/home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/build/src/pybind/cephfs/cephfs.o -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -fexceptions -fcf-protection -fexceptions -fcf-protection -fexceptions -fcf-protection -iquote/home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/src/include -w -Dvoid0=dead_function(void) -D__Pyx_check_single_interpreter(ARG)=ARG##0 -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -fexceptions -fcf-protection -fexceptions -fcf-protection -fexceptions -fcf-protection
/usr/lib64/ccache/gcc -shared -L/home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/build/lib -iquote/home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/src/include -w -Dvoid0=dead_function(void) -D__Pyx_check_single_interpreter(ARG)=ARG##0 /home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/build/lib/cython_modules/temp.linux-x86_64-cpython-312/home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/build/src/pybind/cephfs/cephfs.o -L/usr/lib64 -L/usr/lib64/python3.12/config-3.12-x86_64-linux-gnu -L/usr/lib64 -lcephfs -ldl -lm -o /home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/build/lib/cython_modules/lib.3/cephfs.cpython-312-x86_64-linux-gnu.so

batrick · 2025-03-04T16:44:06Z

I think that's acceptable. The method needs to acquire the GIL anyway. I don't know a good way to shut that warning up.

batrick · 2025-03-04T16:44:50Z

------------------------------------------------------------
...
    PyObject *PyBytes_FromStringAndSize(char *v, Py_ssize_t len) except NULL
    char* PyBytes_AsString(PyObject *string) except NULL
    int _PyBytes_Resize(PyObject **string, Py_ssize_t newsize) except -1
    void PyEval_InitThreads()

cdef void completion_callback(int rc, const void* out, size_t outlen, const void* outs, size_t outslen, void* ud) except * nogil:
                                                                                                                          ^
------------------------------------------------------------

cephfs.pyx:114:123: Syntax error in C variable declaration
Compiling cephfs.pyx because it changed.
[1/1] Cythonizing cephfs.pyx
Traceback (most recent call last):
  File "/home/jenkins-build/build/workspace/ceph-pull-requests-arm64/src/pybind/cephfs/setup.py", line 200, in <module>
    ext_modules=cythonize(
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1127, in cythonize
    cythonize_one(*args)
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1250, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: cephfs.pyx

ugh

cephfs.pyx:2324:40: Cannot assign type 'void (int, const void *, size_t, const void *, size_t, void *) except * nogil' to 'libcephfs_c_completion_t' (alias of 'void (*)(int, const void *, size_t, const void *, size_t, void *) noexcept nogil'). Introduced-by: 72d0a76 Signed-off-by: Kotresh HR <[email protected]>

kotreshhr · 2025-03-05T13:37:34Z

Weird!! It's now failing https://jenkins.ceph.com/job/ceph-pull-requests/152959/consoleFull#-183961699c212b007-e891-4176-9ee7-2f60eca393b7
Error compiling Cython file:
------------------------------------------------------------
...
    PyObject *PyBytes_FromStringAndSize(char *v, Py_ssize_t len) except NULL
    char* PyBytes_AsString(PyObject *string) except NULL
    int _PyBytes_Resize(PyObject **string, Py_ssize_t newsize) except -1
    void PyEval_InitThreads()

cdef void completion_callback(int rc, const void* out, size_t outlen, const void* outs, size_t outslen, void* ud) nogil noexcept:
                                                                                                                       ^
------------------------------------------------------------

cephfs.pyx:114:120: Syntax error in C variable declaration
Something is different in my local setup!!! I think we don't need this fix ?
If this matters
kotresh:build$ cython --version
Cython version 3.0.9
kotresh:build$ python --version
Python 3.12.8
kotresh:build$ g++ --version
g++ (GCC) 14.2.1 20240912 (Red Hat 14.2.1-3)
Can you try interchanging the position of noexcept and nogill? im coming from this article https://youtrack.jetbrains.com/issue/PY-59249/Latest-Cython-3.0.0b1-except-noexcept-legal-syntax-red-lined, i guess it's just a syntactical change in cython 3.X.X

Pushed with this change. Let's see.

batrick · 2025-03-05T17:10:31Z

@kotreshhr please try latest update to #62107

batrick · 2025-03-05T17:47:08Z

Superseded by #62107

kotreshhr · 2025-03-06T08:27:13Z

@kotreshhr please try latest update to #62107

Thank you! It worked.

kotreshhr requested review from a team, batrick and vshankar March 4, 2025 08:09

github-actions bot added cephfs Ceph File System pybind labels Mar 4, 2025

ronen-fr approved these changes Mar 4, 2025

View reviewed changes

vshankar approved these changes Mar 4, 2025

View reviewed changes

vshankar mentioned this pull request Mar 4, 2025

client,mds: case-insensitive directory trees #60746

Merged

14 tasks

kotreshhr force-pushed the wip-cython-error branch from abb117d to 5949b3f Compare March 5, 2025 13:36

batrick closed this Mar 5, 2025

Comments

Conversation

kotreshhr commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar commented Mar 4, 2025

Uh oh!

kotreshhr commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vshankar commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dparmar18 commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

batrick commented Mar 4, 2025

Uh oh!

batrick commented Mar 4, 2025

Uh oh!

kotreshhr commented Mar 4, 2025

Uh oh!

batrick commented Mar 4, 2025

Uh oh!

batrick commented Mar 4, 2025

Uh oh!

kotreshhr commented Mar 5, 2025

Uh oh!

batrick commented Mar 5, 2025

Uh oh!

batrick commented Mar 5, 2025

Uh oh!

kotreshhr commented Mar 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kotreshhr commented Mar 4, 2025 •

edited

Loading

kotreshhr commented Mar 4, 2025 •

edited

Loading

vshankar commented Mar 4, 2025 •

edited

Loading

dparmar18 commented Mar 4, 2025 •

edited

Loading