sagemath/sage

Upgrade Cython to 0.17

Closed this issue · 29 comments

It's not released yet, but the following spkg should be close and the patch needs reviewing.

Apply:

and install http://sage.math.washington.edu/home/robertwb/cython/cython-0.17pre.spkg

CC: @jdemeyer @roed314 @ohanar @ppurka @kini

Component: packages: standard

Author: Robert Bradshaw

Reviewer: R. Andrew Ohana

Merged: sage-5.2.beta1

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

comment:3

Please describe why the hack in sage.rings.integer is needed.

kini commented

Description changed:

--- 
+++ 
@@ -1 +1 @@
-It's not release yet, but the following spkg should be close and the patch needs reviewing. 
+It's not released yet, but the following spkg should be close and the patch needs reviewing. 

Description changed:

--- 
+++ 
@@ -1 +1,7 @@
 It's not released yet, but the following spkg should be close and the patch needs reviewing. 
+
+Apply:
+* 13029_cython-0.17_5.1.beta2.patch
+* 13029_cython-0.17-bug.patch
+
+and install http://sage.math.washington.edu/home/robertwb/cython/cython-0.17pre.spkg
comment:7

on sage.math everything compiles fine, but upon startup

ImportError
...
/home/ohanar/sage-dev/sage-5.1.beta2/local/lib/python2.7/site-packages/sage/modular/arithgroup/all.py in <module>()
     15                             degeneracy_coset_representatives_gamma1)
     16 
---> 17 from farey_symbol import Farey as FareySymbol
     18 
     19 

ImportError: /home/ohanar/sage-dev/sage-5.1.beta2/local/lib/python2.7/site-packages/sage/modular/arithgroup/farey_symbol.so: undefined symbol: convert_to_long
Error importing ipy_profile_sage - perhaps you should run %upgrade?
WARNING: Loading of ipy_profile_sage failed.

This was from a clean sage source tarball built with the new cython.

Reviewer: R. Andrew Ohana

comment:9

Replying to @ohanar:

on sage.math everything compiles fine, but upon startup ...

wow, I can't read :p, testing now

comment:10

Please update the SPKG.txt in the spkg, otherwise everything looks good. I would hold out on making a new spkg until you can fix #13031, since that is one of the primary reasons for upgrading.

comment:11

Applied the patch + spkg to Sage 5.1beta2, and it all works good. make ptestlong passed all tests.

Work Issues: Update SPKG.txt

comment:12

FYI, it still works in beta5. Once you update the SPKG.txt, you may mark this with a positive review.

comment:13

Updated SPKG.txt and re-uploaded spkg.

Changed work issues from Update SPKG.txt to none

comment:15

Why the change from

cdef void _pari_trap "_pari_trap" (long errno, long retries) except *: 

to

cdef public void _pari_trap "_pari_trap" (long errno, long retries) except *: 

This causes a file sage/libs/pari/gen.h to be generated. Either this file should not be distributed, or it should be added to .hgignore.

comment:16

It's public to prevent it from being declared as static which conflicts with the declaration in pari_err.h. I've added an entry to .hgignore.

Description changed:

--- 
+++ 
@@ -3,5 +3,6 @@
 Apply:
 * 13029_cython-0.17_5.1.beta2.patch
 * 13029_cython-0.17-bug.patch
+* 13029-ignore-gen.h.patch
 
 and install http://sage.math.washington.edu/home/robertwb/cython/cython-0.17pre.spkg
comment:18

Makes sense.

comment:19

Attachment: 13029_cython-0.17.patch.gz

Patch rebased to sage-5.2.beta0.

Description changed:

--- 
+++ 
@@ -1,8 +1,8 @@
 It's not released yet, but the following spkg should be close and the patch needs reviewing. 
 
 Apply:
-* 13029_cython-0.17_5.1.beta2.patch
-* 13029_cython-0.17-bug.patch
-* 13029-ignore-gen.h.patch
+* [attachment: 13029_cython-0.17.patch](https://github.com/sagemath/sage-prod/files/10655642/13029_cython-0.17.patch.gz)
+* [attachment: 13029_cython-0.17-bug.patch](https://github.com/sagemath/sage-prod/files/10655640/13029_cython-0.17-bug.patch.gz)
+* [attachment: 13029-ignore-gen.h.patch](https://github.com/sagemath/sage-prod/files/10655641/13029-ignore-gen.h.patch.gz)
 
 and install http://sage.math.washington.edu/home/robertwb/cython/cython-0.17pre.spkg
comment:20

Just a quick question Robert, where can I find the sources matching the spkg? Is a real cython 0.17 going to be released soon?

Merged: sage-5.2.beta1

comment:22

For the record, the spkg is based on the commit at cython/cython@a7d6ec0

We don't have a timeline for the 0.17 release (it's being worked on, intermittently), so we shouldn't wait on that.

kini commented
comment:23

Then maybe this SPKG should be versioned cython-0.16-a7d6ec0?

comment:24

Fine by me, I'm just happy to see this finally get in :). When Cython 0.17 is finally released, it should be a trivial upgrade.

comment:25

It is just a problem sage-on-gentoo side because I don't have an upstream package. I will have to distribute an ebuild fetching the spkg until 0.17 is released. If there was a snapshot on the cython website I would prefer to use that.

comment:27

Replying to @robertwb:

http://cython.org/release/Cython-0.17pre.tar.gz

I am grateful that you put that up Robert. Thanks a lot.