Skip to content

Comments

pybind/cephfs: use legacy noexcept for cdefs for cython 3.Y.Z#62107

Merged
batrick merged 2 commits intoceph:mainfrom
batrick:cython-fix
Mar 6, 2025
Merged

pybind/cephfs: use legacy noexcept for cdefs for cython 3.Y.Z#62107
batrick merged 2 commits intoceph:mainfrom
batrick:cython-fix

Conversation

@batrick
Copy link
Member

@batrick batrick commented Mar 4, 2025 •
edited
Loading

For some newer versions of cython, it appears it tries to choose noexcept by default.

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)
Show available Jenkins commands

@batrick batrick mentioned this pull request Mar 4, 2025
Closed
14 tasks
@github-actions github-actions bot added cephfs Ceph File System pybind labels Mar 4, 2025
@batrick batrick force-pushed the cython-fix branch 5 times, most recently from cb9f5a2 to b2c2d95 Compare March 5, 2025 16:38
@batrick
pybind/cephfs: increment ref before calling out to c++
395263c 
At the time this construction seemed safe since the caller should have a
reference but it could conveivably be the only ref. We don't want the ref count
to reach 0.

Additionally, catch errors so this callback is genuinely noexcept.

Signed-off-by: Patrick Donnelly <[email protected]>
@batrick batrick force-pushed the cython-fix branch 3 times, most recently from 15179b5 to 8a8f1cf Compare March 5, 2025 16:56
@batrick
Copy link
Member Author

batrick commented Mar 5, 2025

This PR is under test in https://tracker.ceph.com/issues/70321.

@batrick batrick mentioned this pull request Mar 5, 2025
Merged
14 tasks
@cbodley
Copy link
Contributor

cbodley commented Mar 5, 2025

building commit 8a8f1cf in a python 3.13 virtualenv with Cython 3.0.12 installed:

(venv) cbodley@fedora ~/ceph/build $ cmake --build . --target cython_cephfs
[1/1] Generating ../../../lib/cython_modules/lib.3/cephfs.cpython-313-x86_64-linux-gnu.so
FAILED: lib/cython_modules/lib.3/cephfs.cpython-313-x86_64-linux-gnu.so /home/cbodley/ceph/build/lib/cython_modules/lib.3/cephfs.cpython-313-x86_64-linux-gnu.so
cd /home/cbodley/ceph/src/pybind/cephfs && env CC="/usr/bin/cc" CFLAGS="" CPPFLAGS="-iquote/home/cbodley/ceph/src/include -w -D'void0=dead_function(void)' -D'__Pyx_check_single_interpreter(ARG)=ARG##0'" CXX="/usr/bin/c++" LDSHARED="/usr/bin/cc -shared" OPT="-DNDEBUG -g -fwrapv -O2 -w" LDFLAGS="-L/home/cbodley/ceph/build/lib" CYTHON_BUILD_DIR=/home/cbodley/ceph/build/src/pybind/cephfs CEPH_LIBDIR=/home/cbodley/ceph/build/lib /home/cbodley/ceph/build/venv/bin/python3.13 /home/cbodley/ceph/src/pybind/cephfs/setup.py build --build-base /home/cbodley/ceph/build/lib/cython_modules --build-platlib /home/cbodley/ceph/build/lib/cython_modules/lib.3
/home/cbodley/ceph/src/pybind/cephfs/setup.py:8: DeprecationWarning: 'pkgutil.find_loader' is deprecated and slated for removal in Python 3.14; use importlib.
util.find_spec() instead
  if not pkgutil.find_loader('setuptools'):

Error compiling Cython file:
------------------------------------------------------------
...
# cython: language_level=3
# cython: legacy_implicit_noexcept=true
^
------------------------------------------------------------

cephfs.pyx:2:0: legacy_implicit_noexcept directive must be set to True or False, got 'true'
warning: cephfs.pyx:13:0: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
warning: cephfs.pyx:291:8: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
warning: cephfs.pyx:367:8: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310

Compiling cephfs.pyx because it changed.
[1/1] Cythonizing cephfs.pyx
Traceback (most recent call last):
  File "/home/cbodley/ceph/src/pybind/cephfs/setup.py", line 200, in <module>
    ext_modules=cythonize(
                ~~~~~~~~~^
        [
        ^
    ...<8 lines>...
        **cythonize_args
        ^^^^^^^^^^^^^^^^
    ),
    ^
  File "/home/cbodley/ceph/build/venv/lib64/python3.13/site-packages/Cython/Build/Dependencies.py", line 1154, in cythonize
    cythonize_one(*args)
    ~~~~~~~~~~~~~^^^^^^^
  File "/home/cbodley/ceph/build/venv/lib64/python3.13/site-packages/Cython/Build/Dependencies.py", line 1321, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: cephfs.pyx
ninja: build stopped: subcommand failed.

@batrick
pybind/cephfs: use legacy noexcept for cdefs for cython 3.Y.Z
90ac740 
For some newer versions of cython, it appears it requires explicitly specifying
noexcept but old versions of Cython 0.29.Z do not understand that attribute.

See: https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html#exception-values-and-noexcept

Signed-off-by: Patrick Donnelly <[email protected]>
@cbodley
Copy link
Contributor

cbodley commented Mar 5, 2025

updated commit 90ac740 built successfully 👍

@batrick
Copy link
Member Author

batrick commented Mar 5, 2025

updated commit 90ac740 built successfully 👍

Thanks, I was going to test it myself before asking again :) You don't deserve to be a compiler!

@batrick batrick changed the title pybind/cephfs: note except type of completion cdef pybind/cephfs: use legacy noexcept for cdefs for cython 3.Y.Z Mar 5, 2025
@batrick
Copy link
Member Author

batrick commented Mar 5, 2025

This PR is under test in https://tracker.ceph.com/issues/70328.

@batrick batrick mentioned this pull request Mar 5, 2025
Merged
@batrick
Copy link
Member Author

batrick commented Mar 6, 2025

jenkins test windows

kotreshhr
kotreshhr approved these changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@kotreshhr kotreshhr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR works for me

@batrick batrick merged commit b0890e3 into ceph:main Mar 6, 2025
12 checks passed
@batrick batrick deleted the cython-fix branch March 6, 2025 13:38
@batrick
Copy link
Member Author

batrick commented Mar 6, 2025

backported via #62095

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cbodley cbodley cbodley approved these changes

@kotreshhr kotreshhr kotreshhr approved these changes

Assignees

No one assigned

Labels

cephfs Ceph File System pybind wip-pdonnell-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@batrick @cbodley @kotreshhr