Upgrade cypari2 to 2.1.3

Question

Upgrade cypari2 to 2.1.3

Closed this issue 3 years ago · 27 comments

kliem commented 3 years ago

This is needed for python 3.11 support (#33842) and to make cypari compatible with current pari version

https://groups.google.com/g/sage-devel/c/jqmr6bDjDrk/m/XE2GlB_GBgAJ

Necessary follow ups:

make cypari thread safe: see cypari2 #116
remove optional build time dependency of cysignals on pari: see cypari2 #130

Depends on #33864

CC: @videlec @dimpase

Component: build

Author: Matthias Koeppe

Branch/Commit: 0ac9905

Reviewer: Dima Pasechnik

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

Answer 1 · 2022-05-21T08:20:19.000Z

Dependencies: #33864

Answer 2 · 2022-05-21T08:48:55.000Z

Branch: public/33878

Answer 3 · 2022-05-21T08:48:55.000Z

Commit: f3f1b5d

Answer 4 · 2022-05-21T08:48:55.000Z

New commits:

`92e9cff`	`build/pkgs/cython: Update to 0.29.30`
`f3f1b5d`	`test new changes to pari`

Answer 5 · 2022-05-21T21:18:32.000Z

comment:3

Is there any reason you are using snapshots from your fork of cypari2? Or is it just for testing and development?

Answer 6 · 2022-05-22T06:38:46.000Z

comment:4

Just for testing and development. My own GitHub account is lots faster with the GitHub actions than sagemath. Also I use this trac ticket for cysignal's CI. Once cypari and possibly cysignal are ready, we switch back to the official sources.

Answer 7 · 2022-05-27T20:15:41.000Z

comment:5

Still getting

[cypari-2.1.3b1]   [7/7] Cythonizing cypari2/string_utils.pyx
[cypari-2.1.3b1]   cypari2/convert.c:3347:21: error: cannot take the address of an rvalue of type 'Py_ssize_t' (aka 'long')
[cypari-2.1.3b1]     __pyx_v_sizeptr = &Py_SIZE(((PyObject *)__pyx_v_x));
[cypari-2.1.3b1]                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

with this ticket

Answer 8 · 2022-05-28T07:09:25.000Z

comment:6

Sorry about that. I messed up with the tags. I'm rather confident the fix is working, it's just that the fix is not contained in the tag I created.

Answer 9 · 2022-05-28T07:18:22.000Z

comment:7

The new commit contains the recent changes.

The test release contains basically the following:

sagemath/cypari2#103 (requires cython 0.29.20, which videlec does not like: sagemath/cypari2#103 (comment);

an alternative approach without raised cython requirement could be sagemath/cypari2#115)
sagemath/cypari2#109 (videlec doesn't like this approach much and rather wants cypari2 to push to some signal handling stack of cysignals)
sagemath/cypari2#116 (doesn't work for python 3.6 yet).

Answer 10 · 2022-06-02T12:47:20.000Z

Changed commit from f3f1b5d to 481d782

Answer 11 · 2022-06-02T12:47:20.000Z

Branch pushed to git repo; I updated commit sha1. New commits:

`481d782`	`test cypari update`

Answer 12 · 2022-06-02T12:48:52.000Z

Changed commit from 481d782 to b494326

Answer 13 · 2022-06-02T12:48:52.000Z

Branch pushed to git repo; I updated commit sha1. New commits:

`11a0e57`	`build/bin/write-dockerfile.sh: Add bootstrap-conda`
`b494326`	`Merge branch 'u/mkoeppe/fix_tox_docker_builds' of git://trac.sagemath.org/sage into public/33878`

Answer 14 · 2022-11-11T18:47:03.000Z

Description changed:

--- 
+++ 
@@ -1,4 +1,6 @@
-This is needed for #33842, but the new release of cypari (and possibly cysignals) might also make
-- cypari thread safe
-- cypari compatible with current pari version
+This is needed for python 3.11 support (#33842) and to make cypari compatible with current pari version
+
+Necessary follow ups:
+- make cypari thread safe
 - remove optional build time dependency of cysignals on pari
+

Answer 15 · 2022-11-11T18:52:09.000Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`0ac9905`	`build/pkgs/cypari: Update to 2.1.3`

Answer 16 · 2022-11-11T18:52:09.000Z

Changed commit from b494326 to 0ac9905

Answer 17 · 2022-11-11T18:54:15.000Z

Description changed:

--- 
+++ 
@@ -1,4 +1,6 @@
 This is needed for python 3.11 support (#33842) and to make cypari compatible with current pari version
+
+https://groups.google.com/g/sage-devel/c/jqmr6bDjDrk/m/XE2GlB_GBgAJ
 
 Necessary follow ups:
 - make cypari thread safe

Answer 18 · 2022-11-11T18:58:29.000Z

comment:16

Regarding the ticket description: to make pari threadsafe, the only way I explored so far was to use a "thread pool executor", see sagemath/cypari2#116. Because of its stack management, pari needs some care when used within threads.

Answer 19 · 2022-11-11T19:00:44.000Z

Description changed:

--- 
+++ 
@@ -3,6 +3,6 @@
 https://groups.google.com/g/sage-devel/c/jqmr6bDjDrk/m/XE2GlB_GBgAJ
 
 Necessary follow ups:
-- make cypari thread safe
-- remove optional build time dependency of cysignals on pari
+- make cypari thread safe: see [cypari2 #116](https://github.com/sagemath/cypari2/pull/116)
+- remove optional build time dependency of cysignals on pari: see [cypari2 #130](https://github.com/sagemath/cypari2/pull/130)

Answer 20 · 2022-11-11T19:01:22.000Z

comment:18

I don't remember the details about this, but there is also a separate related ticket: #28800

Answer 21 · 2022-11-11T19:02:54.000Z

Author: Matthias Koeppe

Answer 22 · 2022-11-11T22:03:40.000Z

Reviewer: https://github.com/mkoeppe/sage/actions/runs/3448209182

Answer 23 · 2022-11-12T21:57:06.000Z

comment:21

Tests look OK, let's get this in

Answer 24 · 2022-11-13T19:18:39.000Z

comment:22

lgtm - tested with Pari 2.,15.1

Answer 25 · 2022-11-13T19:18:39.000Z

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/3448209182 to Dima Pasechnik

Answer 26 · 2022-11-13T19:19:51.000Z

comment:23

Thanks!

Answer 27 · 2022-11-15T23:43:10.000Z

Changed branch from public/33878 to 0ac9905