cython: Eliminate use of custom patches

[mkoeppe]

As noted in #29665 comment:86, our cython spkg uses custom patches:

• https://github.com/sagemath/sage-prod/tree/develop/build/pkgs/cython/patches

We should investigate whether they are still necessary.

Depends on #32985

Upstream: Fixed upstream, but not in a stable release.

CC: @orlitzky @kiwifb @antonio-rojas @slel

Component: packages: standard

Author: Michael Orlitzky

Issue created by migration from https://trac.sagemath.org/ticket/32832

[orlitzky]

comment:1

Cython and cypari have patches that work in tandem to address #27267. Neither is really critical IMO, but the patches are not obsolete. The cython patch is a backport from the forthcoming cython-3.0, so we're using a feature from a future release.

[kiwifb]

comment:2

I don't include the patch is sage-on-gentoo which lead to the only persistent doctest failure I have these days.

sage -t --long --random-seed=329662680874814688233998931907435318225 /usr/lib/python3.9/site-packages/sage/cpython/dict_del_by_value.pyx
**********************************************************************
File "/usr/lib/python3.9/site-packages/sage/cpython/dict_del_by_value.pyx", line 268, in sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value
Failed example:
    test_del_dictitem_by_exact_value(D, ZZ, 2)
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.9/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value[7]>", line 1, in <module>
        test_del_dictitem_by_exact_value(D, ZZ, Integer(2))
      File "sage/cpython/dict_del_by_value.pyx", line 275, in sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_9/cythonized/sage/cpython/dict_del_by_value.c:2442)
        del_dictitem_by_exact_value(<PyDictObject *>D, <PyObject *>value, h)
    TypeError: an integer is required
**********************************************************************
File "/usr/lib/python3.9/site-packages/sage/cpython/dict_del_by_value.pyx", line 271, in sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value
Failed example:
    test_del_dictitem_by_exact_value(D, QQ, 1)
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.9/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value[9]>", line 1, in <module>
        test_del_dictitem_by_exact_value(D, QQ, Integer(1))
      File "sage/cpython/dict_del_by_value.pyx", line 275, in sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_9/cythonized/sage/cpython/dict_del_by_value.c:2442)
        del_dictitem_by_exact_value(<PyDictObject *>D, <PyObject *>value, h)
    TypeError: an integer is required
**********************************************************************
1 item had failures:

I have been waiting for that elusive cython 3.0 release, which would get rid of this, for several years now. What is happening with cython upstream?

[mkoeppe]

Dependencies: #29863

[mkoeppe]

comment:3

I have marked this dependent on the Cython 3.0 ticket #29863

[orlitzky]

comment:4

Replying to @kiwifb:

I don't include the patch is sage-on-gentoo which lead to the only persistent doctest failure I have these days.

That's the other patch, hash.patch, also a backport from cython-3.0. It was introduced in #26855 to fix a doctest failure with python3. But if that's all that's affected, we might be able to add int() in a few places to avoid the patch until cython-3.0 actually exists.

The trashcan patch shouldn't really be required per #27267 comment:55

[kiwifb]

comment:5

Never seen any other issues. If there are, they'd need to have new doctests associated to them.

[orlitzky]

comment:6

Replying to @kiwifb:

Never seen any other issues. If there are, they'd need to have new doctests associated to them.

That patch/doctest is also blocking an spkg-configure.m4 for cython, so I'll see what I can do about it and post the branch here.

[orlitzky]

Commit: 78fe28c

[orlitzky]

Branch: u/mjo/ticket/32832

[orlitzky]

comment:7

Removing hash.patch was straightforward. Still waiting on ptestlong, but I expect it will pass.

The remaining trashcan.patch causes one main problem: when introducing spkg-configure.m4 for python packages, we will need a patched cython if we're going to install the patched cypari SPKG. That isn't easy to detect, and the logic is backwards from the usual "are my dependencies installed as SPKGs?" check. We can hack something together for sure, but it will be a little ugly until cython-3.0 is released if we decide to keep the patch.

From #27267 I gather that the stack size issue was only on cygwin. I know we try to support cygwin, but do we have even a single cygwin user? And if so, does he want to allocate 10,000 pari gens in a list? If not maybe we could tone that test down a bit for now to let it pass on cygwin, and then put back the len(enumerate_totallyreal_fields_prim(2,2**15)) test when cython-3.0 is released.

[orlitzky]

Author: Michael Orlitzky

[mkoeppe]

comment:8

I don't think we should introduce regressions just to remove a patch.

[orlitzky]

Changed dependencies from #29863 to none

[orlitzky]

comment:9

Replying to @mkoeppe:

I don't think we should introduce regressions just to remove a patch.

Ok, I have enough other things to do that I'm not desperate to add an spkg-configure for cypari. This is ready then. We'll be forced to remove the other patch during the cython-3.0 upgrade, so we don't have to keep this ticket around.

[mkoeppe]

comment:10

I have inquired with upstream whether we can a backport to 0.29.x - cython/cython#2752

[mkoeppe]

comment:11

Looks like is going into 0.29.25

[mkoeppe]

Upstream: Fixed upstream, but not in a stable release.

[slel]

comment:13

Cython 0.29.25 is out.

• https://github.com/cython/cython/releases/tag/0.29.25

[mkoeppe]

comment:14

I've opened #32985 for the cython update

[mkoeppe]

Dependencies: #32985

[mkoeppe]

comment:16

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

[orlitzky]

Changed commit from 78fe28c to none

[orlitzky]

Changed branch from u/mjo/ticket/32832 to none

[orlitzky]

We beat this with cython-3.0.4.