PR#29: Use Cython 3, specify noexcept for cdef functions - rpms/grpc

rpms / grpc

#29 Use Cython 3, specify noexcept for cdef functions

Merged 7 months ago by music. Opened 7 months ago by churchyard.

rpms/ churchyard/grpc cython3 into rawhide

Use Cython 3, specify noexcept for cdef functions

Miro Hrončok • 7 months ago

8bf11b8

0001-Specify-noexcept-for-cdef-functions.patch

file added

+145

		`@@ -0,0 +1,145 @@`
		`+ From 45d31dba83999638808891ee7bf93638106bdb71 Mon Sep 17 00:00:00 2001`
		`+ From: Atri Bhattacharya <badshah400@gmail.com>`
		`+ Date: Thu, 7 Sep 2023 07:06:56 +0200`
		`+ Subject: [PATCH] Specify noexcept for cdef functions.`
		`+ MIME-Version: 1.0`
		`+ Content-Type: text/plain; charset=UTF-8`
		`+ Content-Transfer-Encoding: 8bit`
		`+`
		`+ To build against cython 3.0, cdef functions that do not raise exceptions`
		`+ need to be explicitly declared as noexcept. Fixes issue #33918.`
		`+`
		`+ Co-Authored-By: Miro Hrončok <miro@hroncok.cz>`
		`+ ---`
		`+ .../grpc/_cython/_cygrpc/aio/callback_common.pxd.pxi \| 2 +-`
		`+ .../grpc/_cython/_cygrpc/aio/callback_common.pyx.pxi \| 2 +-`
		`+ .../grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi \| 2 +-`
		`+ .../grpcio/grpc/_cython/_cygrpc/fork_posix.pxd.pxi \| 12 ++++++------`
		`+ .../grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi \| 6 +++---`
		`+ .../grpcio/grpc/_cython/_cygrpc/vtable.pyx.pxi \| 6 +++---`
		`+ 6 files changed, 15 insertions(+), 15 deletions(-)`
		`+`
		`+ diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pxd.pxi`
		`+ index e54e510..26edbdb 100644`
		`+ --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pxd.pxi`
		`+ +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pxd.pxi`
		`+ @@ -48,7 +48,7 @@ cdef class CallbackWrapper:`
		`+ @staticmethod`
		`+ cdef void functor_run(`
		`+ grpc_completion_queue_functor* functor,`
		`+ - int succeed)`
		`+ + int succeed) noexcept`
		`+`
		`+ cdef grpc_completion_queue_functor *c_functor(self)`
		`+`
		`+ diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pyx.pxi`
		`+ index f2d94a9..5dda90a 100644`
		`+ --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pyx.pxi`
		`+ +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pyx.pxi`
		`+ @@ -50,7 +50,7 @@ cdef class CallbackWrapper:`
		`+ @staticmethod`
		`+ cdef void functor_run(`
		`+ grpc_completion_queue_functor* functor,`
		`+ - int success):`
		`+ + int success) noexcept:`
		`+ cdef CallbackContext context = <CallbackContext >functor`
		`+ cdef object waiter = <object>context.waiter`
		`+ if not waiter.cancelled():`
		`+ diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi`
		`+ index 23de3a0..52071f5 100644`
		`+ --- a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi`
		`+ +++ b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi`
		`+ @@ -314,7 +314,7 @@ def server_credentials_ssl_dynamic_cert_config(initial_cert_config,`
		`+ return credentials`
		`+`
		`+ cdef grpc_ssl_certificate_config_reload_status _server_cert_config_fetcher_wrapper(`
		`+ - void* user_data, grpc_ssl_server_certificate_config **config) with gil:`
		`+ + void* user_data, grpc_ssl_server_certificate_config **config) noexcept with gil:`
		`+ # This is a credentials.ServerCertificateConfig`
		`+ cdef ServerCertificateConfig cert_config = None`
		`+ if not user_data:`
		`+ diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pxd.pxi`
		`+ index a925bdd..5e97a6d 100644`
		`+ --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pxd.pxi`
		`+ +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pxd.pxi`
		`+ @@ -15,15 +15,15 @@`
		`+`
		`+ cdef extern from "pthread.h" nogil:`
		`+ int pthread_atfork(`
		`+ - void (*prepare)() nogil,`
		`+ - void (*parent)() nogil,`
		`+ - void (*child)() nogil)`
		`+ + void (*prepare)() noexcept nogil,`
		`+ + void (*parent)() noexcept nogil,`
		`+ + void (*child)() noexcept nogil) noexcept`
		`+`
		`+`
		`+ -cdef void __prefork() nogil`
		`+ +cdef void __prefork() noexcept nogil`
		`+`
		`+`
		`+ -cdef void __postfork_parent() nogil`
		`+ +cdef void __postfork_parent() noexcept nogil`
		`+`
		`+`
		`+ -cdef void __postfork_child() nogil`
		`+ \ No newline at end of file`
		`+ +cdef void __postfork_child() noexcept nogil`
		`+ \ No newline at end of file`
		`+ diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi`
		`+ index 53657e8..d4d1cff 100644`
		`+ --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi`
		`+ +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi`
		`+ @@ -34,7 +34,7 @@ _GRPC_ENABLE_FORK_SUPPORT = (`
		`+`
		`+ _fork_handler_failed = False`
		`+`
		`+ -cdef void __prefork() nogil:`
		`+ +cdef void __prefork() noexcept nogil:`
		`+ with gil:`
		`+ global _fork_handler_failed`
		`+ _fork_handler_failed = False`
		`+ @@ -48,14 +48,14 @@ cdef void __prefork() nogil:`
		`+ _fork_handler_failed = True`
		`+`
		`+`
		`+ -cdef void __postfork_parent() nogil:`
		`+ +cdef void __postfork_parent() noexcept nogil:`
		`+ with gil:`
		`+ with _fork_state.fork_in_progress_condition:`
		`+ _fork_state.fork_in_progress = False`
		`+ _fork_state.fork_in_progress_condition.notify_all()`
		`+`
		`+`
		`+ -cdef void __postfork_child() nogil:`
		`+ +cdef void __postfork_child() noexcept nogil:`
		`+ with gil:`
		`+ try:`
		`+ if _fork_handler_failed:`
		`+ diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/vtable.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/vtable.pyx.pxi`
		`+ index da4b81b..f594100 100644`
		`+ --- a/src/python/grpcio/grpc/_cython/_cygrpc/vtable.pyx.pxi`
		`+ +++ b/src/python/grpcio/grpc/_cython/_cygrpc/vtable.pyx.pxi`
		`+ @@ -13,16 +13,16 @@`
		`+ # limitations under the License.`
		`+`
		`+ # TODO(https://github.com/grpc/grpc/issues/15662): Reform this.`
		`+ -cdef void* _copy_pointer(void* pointer):`
		`+ +cdef void* _copy_pointer(void* pointer) noexcept:`
		`+ return pointer`
		`+`
		`+`
		`+ # TODO(https://github.com/grpc/grpc/issues/15662): Reform this.`
		`+ -cdef void _destroy_pointer(void* pointer):`
		`+ +cdef void _destroy_pointer(void* pointer) noexcept:`
		`+ pass`
		`+`
		`+`
		`+ -cdef int _compare_pointer(void* first_pointer, void* second_pointer):`
		`+ +cdef int _compare_pointer(void* first_pointer, void* second_pointer) noexcept:`
		`+ if first_pointer < second_pointer:`
		`+ return -1`
		`+ elif first_pointer > second_pointer:`
		`+ --`
		`+ 2.41.0`
		`+`

grpc.spec

file modified

+9 -22

		`@@ -253,28 +253,7 @@`
		`# grpcio (setup.py) setup_requires (with GRPC_PYTHON_BUILD_WITH_CYTHON, or`
		`# absent generated sources); also needed for grpcio_tools`
		`# (tools/distrib/python/grpcio_tools/setup.py)`
		`- #`
		`- # Not yet compatible with Cython 3, due to errors like:`
		`- #`
		`- # Error compiling Cython file:`
		`- # ------------------------------------------------------------`
		`- # ...`
		`- # return 1`
		`- # else:`
		`- # return 0`
		`- #`
		`- # cdef grpc_arg_pointer_vtable default_vtable`
		`- # default_vtable.copy = &_copy_pointer`
		`- # ^`
		`- # ------------------------------------------------------------`
		`- #`
		`- # src/python/grpcio/grpc/_cython/_cygrpc/vtable.pyx.pxi:34:22: Cannot assign`
		`- # type 'void ()(void ) except ' to 'void ()(void *) noexcept'`
		`- #`
		`- # See:`
		`- # Cython 3.0.0b1 and its impact on packages in Fedora`
		`- # https://github.com/cython/cython/issues/5305`
		`- BuildRequires: ((python3dist(cython) > 0.23) with (python3dist(cython) < 3~~))`
		`+ BuildRequires: python3dist(cython) > 0.23`

		`# grpcio (setup.py) install_requires:`
		`# grpcio_tests (src/python/grpcio_tests/setup.py) install_requires:`
		`@@ -429,6 +408,14 @@`
		`#`
		`# Backported to 1.48.4.`
		`Patch: 0001-http2-Dont-drop-connections-on-metadata-limit-exceed.patch`
		`+ # [Python] Specify noexcept for cdef functions (#34242)`
		`+ #`
		`+ # This is needed to build grpc with Cython 3.`
		`+ #`
		`+ # https://github.com/grpc/grpc/issues/33918#issuecomment-1703386656`
		`+ # https://github.com/grpc/grpc/issues/33918#issuecomment-1788823585`
		`+ # https://github.com/grpc/grpc/pull/34242`
		`+ Patch: 0001-Specify-noexcept-for-cdef-functions.patch`

		`Requires: grpc-data = %{version}-%{release}`

churchyard commented 7 months ago

This is untested.

I submitted two scratchbuilds, one with Cython 0.29 and one with Cython 3 -- to see if the dependency needs to be "cython >= 3" or not.

churchyard commented 7 months ago

Alright, this does not work yet.

rebased onto 2500849

7 months ago

rebased onto 8bf11b8

7 months ago

churchyard commented 7 months ago

https://koji.fedoraproject.org/koji/taskinfo?taskID=108412958 cython0.29
https://koji.fedoraproject.org/koji/taskinfo?taskID=108412919 cython3

music commented 7 months ago

I really appreciate your work on this.

churchyard commented 7 months ago

Both scratchbuilds successfully reached %check (at least on x86_64).

churchyard commented 7 months ago

And at least one architecture succeeded completely on each :tada:

churchyard commented 7 months ago

It looks like the scratchbuilds will succeed. In the meantime, I've submitted this to https://copr.fedorainfracloud.org/coprs/g/python/python3.13 as well.

zuul commented 7 months ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/87b3cf0a0af94a8cb20c87bf4ede323e

check-for-arches : SUCCESS in 1m 41s
check-for-eln : SUCCESS in 19s
eln-rpm-scratch-build : SUCCESS in 1h 42m 23s
rpm-scratch-build : SUCCESS in 2h 18m 19s
rpm-scratch-build-s390x : SUCCESS in 2h 28m 54s (non-voting)
rpm-scratch-build-ppc64le : SUCCESS in 1h 42m 05s (non-voting)
rpm-scratch-build-i686 : SUCCESS in 1h 21m 29s (non-voting)
rpm-scratch-build-aarch64 : SUCCESS in 1h 24m 14s (non-voting)
rpm-linter : SUCCESS in 8m 06s (non-voting)
rpm-rpminspect : SUCCESS in 22m 45s (non-voting)
check-for-sti-tests : SUCCESS in 18s
check-for-fmf-tests : SUCCESS in 16s
rpm-install-test : SUCCESS in 1m 56s
Skipped 2 jobs

churchyard commented 7 months ago

Python 3.13 will need more patches. That should not block this PR.

music commented 7 months ago

The Python 3.13 COPR shows the following:

src/python/grpcio/grpc/_cython/cygrpc.cpp: At global scope:
src/python/grpcio/grpc/_cython/cygrpc.cpp:1360:31: error: standard attributes in middle of decl-specifiers
 1360 |         #define CYTHON_UNUSED [[maybe_unused]]

src/python/grpcio/grpc/_cython/cygrpc.cpp: In function â€˜PyObject* __pyx_f_4grpc_7_cython_6cygrpc__initialize()â€™:
src/python/grpcio/grpc/_cython/cygrpc.cpp:126486:10: error: â€˜PyEval_InitThreadsâ€™ was not declared in this scope; did you mean â€˜PyEval_SaveThreadâ€™?
126486 |   (void)(PyEval_InitThreads());
       |          ^~~~~~~~~~~~~~~~~~
       |          PyEval_SaveThread

music commented 7 months ago

This looks good to me. Do you consider it ready to merge?

churchyard commented 7 months ago