#13031 closed enhancement (fixed)
Use cythonize() from cython for Sage module building.
Reported by:  robertwb  Owned by:  GeorgSWeber 

Priority:  major  Milestone:  sage5.9 
Component:  build  Keywords:  sd40.5 
Cc:  jdemeyer, roed, ohanar, ppurka, kini, mhansen  Merged in:  sage5.9.beta4 
Authors:  Robert Bradshaw, R. Andrew Ohana  Reviewers:  Jeroen Demeyer, R. Andrew Ohana 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  #13029, #13432  Stopgaps: 
Description (last modified by )
Cython's cythonize is now robust enough to replace our custom dependency logic, with a few extra features such as * syntax for cython extensions and cycache (still needs work).
Apply to the Sage library:
Attachments (12)
Change History (86)
Changed 7 years ago by
comment:1 Changed 7 years ago by
 Cc jdemeyer roed ohanar ppurka added
 Status changed from new to needs_review
comment:2 Changed 7 years ago by
comment:3 Changed 7 years ago by
 Cc kini added
comment:4 Changed 7 years ago by
 Cc mhansen added
 Keywords sd40.5 added
comment:5 Changed 7 years ago by
 Status changed from needs_review to needs_work
Changing to needs work since  as mentioned above  this doesn't apply to sage5.1.beta0:
adding 13031cythonize.patch to series file applying 13031cythonize.patch patching file module_list.py Hunk #1 FAILED at 144 Hunk #3 FAILED at 223 Hunk #13 FAILED at 834 Hunk #16 FAILED at 1475 Hunk #20 succeeded at 1698 with fuzz 2 (offset 75 lines). Hunk #21 succeeded at 1712 with fuzz 2 (offset 79 lines). 4 out of 22 hunks FAILED  saving rejects to file module_list.py.rej patching file setup.py Hunk #1 succeeded at 17 with fuzz 2 (offset 0 lines). Hunk #3 FAILED at 513 1 out of 3 hunks FAILED  saving rejects to file setup.py.rej patch failed, unable to continue (try v) patch failed, rejects left in working dir errors during apply, please fix and refresh 13031cythonize.patch wstein@sage:/tmp/wstein/sage5.1.beta0boxenx86_64Linux/devel/sage/sage$
comment:6 Changed 7 years ago by
Would you please make the following change:

(a) a vs. (b) b
a b 1 ext_modules = cythonize(ext_modules, exclude=exclude_modules, nthreads=nthreads, cache=os.path.join(DOT_SAGE, 'cythoncache'))) 1 if 'CYCACHE_DIR' in os.environ: 2 CYCACHE_DIR = os.environ['CYCACHE_DIR'] 3 else: 4 CYCACHE_DIR = os.path.join(DOT_SAGE,'cycache') 5 ext_modules = cythonize(ext_modules, exclude=exclude_modules, nthreads=nthreads, cache=CYCACHE_DIR))
This way we can easily specify the cache directory separately from DOT_SAGE
.
comment:7 Changed 7 years ago by
 Status changed from needs_work to needs_review
Changed 7 years ago by
Changed 7 years ago by
comment:8 Changed 7 years ago by
 Status changed from needs_review to needs_work
I attached an updated patch that
 is rebased on the final 5.1.beta2
 fixes a
ZeroDivisionError
that was preventing documentation from building  more throughly cleans up
module_list.py
with the use of *s  does other clean up with
module_list.py
since this patch will constantly need to be rebased with each development release (may as well clean up everything while we are at it)
That said, there is still some issue with Cython's cythonize, even using the old module_list.py
you encounter the issue:
sage: cython('a = 5') sage: a Traceback (most recent call last): ... NameError: name 'a' is not defined
You'll find that tons of doctests fail, and frequently they are having NameError
s like this (although they aren't necessarily calling Sage's cython function).
comment:9 Changed 7 years ago by
Thanks for looking at this. I fixed the patch, Sage relies on a deprecated "feature" of globals(). Running all tests...
comment:10 Changed 7 years ago by
FYI, all tests passed for me.
comment:11 Changed 7 years ago by
 Status changed from needs_work to needs_review
Ok, looks good to me and works well with 5.1.beta2, so I'm going to mark this back as needing review.
As expected, it needs to be rebased again on more recent betas, but since the changes to module_list.py
is so massive, I'm not going to bother with rebasing until Jeroen decides which beta he would like to merge it in.
For whoever reviews this, please use 5.1.beta2 as a basis.
comment:12 Changed 7 years ago by
The new patch fixes an issue with DOT_SAGE
not being defined in setup.py
.
comment:13 Changed 7 years ago by
Rather than code like this:
if not os.path.exists(cache_dir): os.mkdir(cache_dir)
you should use sage.misc.misc.sage_makedirs
(or copypaste the code from there). A tryexcept block is safer than testing whether than the directory exists first.
comment:14 Changed 7 years ago by
 Description modified (diff)
I just rebased on 5.2.beta1 since the dependency was merged into that release. However, in the process I discovered that cycache is not ready for primetime yet:
$ grep 'cdef class MonoDict:' sage R sage/sets/disjoint_set.c: * cdef class MonoDict: # <<<<<<<<<<<<<< sage/matrix/matrix_modn_dense_float.cpp: * cdef class MonoDict: # <<<<<<<<<<<<<<
Neither of these modules changed between 5.1.beta2 and 5.2.beta1, but their dependencies changed, so there is some issue with the hashing that needs to be resolved.
For now I've disabled cycache, since it will be trivial to readd once this ticket gets merged.
comment:15 followup: ↓ 16 Changed 7 years ago by
Yes, I think it's perfectly fine to disable cycache for now to get this in (as it will then be much easier to work on). Could you expound on what the issue was though? I'm not quite following your grep is trying to show...
comment:16 in reply to: ↑ 15 Changed 7 years ago by
Replying to robertwb:
Yes, I think it's perfectly fine to disable cycache for now to get this in (as it will then be much easier to work on). Could you expound on what the issue was though? I'm not quite following your grep is trying to show...
The grep is of some cython code that existed in 5.1.beta2 in sage/structure/coerce_dict.pxd
, that is no longer present in 5.2.beta1. However, cython detected that I had a cached version of the disjoint_set
module (which didn't change between versions, but imports the coerce_dict
module) and incorrectly pulls the the cached version of disjoint_set.c
when it actually needs to rebuild it since coerce_dict.pxd
changed (same situation for the other module in the grep).
comment:17 Changed 7 years ago by
OK, rebased. Be nice if someone reviewed this soon.
comment:18 followup: ↓ 19 Changed 7 years ago by
comment:19 in reply to: ↑ 18 Changed 7 years ago by
Replying to jdemeyer:
I'll do that once they are merged. This ticket so heavily affects module_list.py
that I would rather get this reviewed on some clean development build, and then try to figure out with you what tickets I'll need to rebase against to merge this into whatever beta you feel is appropriate. Since this has been waiting review for a bit, I rather not jump the gun and start rebasing against things that haven't been merged yet.
comment:20 Changed 7 years ago by
 Status changed from needs_review to needs_work
 Work issues set to Sphinx
This seems to cause Sphinx problems:
/release/merger/sage5.4.beta1/devel/sage/doc/en/reference/sage/modular/modform/eis_series_cython.rst:11: WARNING: error while formatting signature for sage.modular.modform.eis_series_cython.clear_mpz_globals: Could not parse cython argspec /release/merger/sage5.4.beta1/devel/sage/doc/en/reference/sage/modular/modform/eis_series_cython.rst:11: WARNING: error while formatting signature for sage.modular.modform.eis_series_cython.gmp_randrange: Could not parse cython argspec /release/merger/sage5.4.beta1/devel/sage/doc/en/reference/sage/modular/modform/eis_series_cython.rst:11: WARNING: error while formatting signature for sage.modular.modform.eis_series_cython.init_mpz_globals: Could not parse cython argspec
comment:21 followup: ↓ 22 Changed 7 years ago by
Can you please clarify what command you ran?
comment:22 in reply to: ↑ 21 Changed 7 years ago by
Replying to ohanar:
Can you please clarify what command you ran?
This is build from scratch, so it's just make doc
. This is probably equivalent to removing the doc/output
directory and then doing make doc
.
comment:23 Changed 7 years ago by
 Status changed from needs_work to needs_review
Ok, rebased on 5.4.beta1.
The issue with sphinx seems to be because of the python functions defined in sage/ext/gmp.pxi
. We probably shouldn't have any python function defined in pxi files anyway (since we currently aren't checking them for coverage or anything). There are python functions in other pxi files, but they aren't causing any issues for some reason (Robert, maybe you could explain why?).
Anyway, for right now I removed one of them (that wasn't being used anywhere), and I moved the other two to sage/rings/integer.pyx
since that is where the sage library looks for them.
comment:24 Changed 6 years ago by
 Description modified (diff)
OK, I've changed this to only switch to use cythonize and get rid of the sphinx errors. All the other changes make this fail to apply with each new Sage release, and it's a pain to constantly rebase. Once this is in, we can fix up the module list piecemeal and do other cleanup (though I'm unconvinced writing os.path.join('sage','rings','padics','*.pyx')
is really preferable to "sage/rings/padics/*.pyx"
) as was introduced in the latest rebase.
comment:25 Changed 6 years ago by
 Reviewers set to Jeroen Demeyer, R. Andrew Ohana
 Status changed from needs_review to positive_review
 Work issues Sphinx deleted
Everything looks good and works well.
comment:26 Changed 6 years ago by
 Status changed from positive_review to needs_work
There is trouble with upgrading:
On rosemary (Linux RHEL 5.6 x86_64), after upgrading from sage4.5.2, I get a failure during the installation of the Conway spkg:
Traceback (most recent call last): File "./spkginstall", line 4, in <module> from sage.all import save File "/home/buildbot/build/sage/rosemary1/rosemary_upgrade_4.5.2/build/sage5.7.beta3/local/lib/python2.7/sitepackages/sage/all.py", line 74, in <module> from sage.matrix.all import * File "/home/buildbot/build/sage/rosemary1/rosemary_upgrade_4.5.2/build/sage5.7.beta3/local/lib/python2.7/sitepackages/sage/matrix/all.py", line 1, in <module> from matrix_space import MatrixSpace, is_MatrixSpace File "/home/buildbot/build/sage/rosemary1/rosemary_upgrade_4.5.2/build/sage5.7.beta3/local/lib/python2.7/sitepackages/sage/matrix/matrix_space.py", line 33, in <module> import matrix File "matrix.pyx", line 1, in init sage.matrix.matrix (sage/matrix/matrix.c:2043) File "matrix2.pyx", line 1, in init sage.matrix.matrix2 (sage/matrix/matrix2.c:71233) File "matrix1.pyx", line 1, in init sage.matrix.matrix1 (sage/matrix/matrix1.c:13429) File "mutability.pxd", line 15, in init sage.matrix.matrix0 (sage/matrix/matrix0.c:29523) ValueError: PyCapsule_GetPointer called with invalid PyCapsule object
Running ./sage baforce
fixes this.
comment:27 Changed 6 years ago by
Are oldstyle Cython modules and newstyle Cython modules (made with cythonize()
) compatible? If yes, the problem is probably with dependency checking (some modules should be rebuilt but aren't).
comment:28 Changed 6 years ago by
Thanks to some sed magic, this is a list of modules rebuilt in sage5.7.beta2 but which were not rebuilt with this ticket:
sage/combinat/words/word_datatypes.pyx sage/ext/interactive_constructors_c.pyx sage/games/sudoku_backtrack.pyx sage/media/channels.pyx sage/misc/refcount.pyx sage/misc/search.pyx sage/quadratic_forms/quadratic_form__evaluate.pyx sage/rings/polynomial/symmetric_reduction.pyx sage/structure/mutability.pyx
In the sage5.7.beta2 log file, there is
Building modified file sage/combinat/words/word_datatypes.pyx. Building modified file sage/ext/interactive_constructors_c.pyx. Building modified file sage/games/sudoku_backtrack.pyx. Building modified file sage/media/channels.pyx. Building modified file sage/misc/refcount.pyx. Building modified file sage/misc/search.pyx. Building modified file sage/quadratic_forms/quadratic_form__evaluate.pyx. Building modified file sage/rings/polynomial/symmetric_reduction.pyx. Building modified file sage/structure/mutability.pyx.
In the log file with #13031, there is no mention of any of these files.
comment:29 Changed 6 years ago by
Files related to mutability.pyx
after building Sage with #13031:
rwxrxrx 1 buildbot buildbot 52116 20130204 04:32:19.000000000 0500 ./devel/sagemain/build/lib.linuxx86_642.7/sage/structure/mutability.so rwxrxrx 1 buildbot buildbot 52116 20130204 04:32:19.000000000 0500 ./devel/sagemain/build/sage/structure/mutability.so rwrr 1 buildbot buildbot 68728 20130204 04:32:19.000000000 0500 ./devel/sagemain/build/temp.linuxx86_642.7/sage/structure/mutability.o rwrr 1 buildbot buildbot 207 20120823 12:00:08.000000000 0400 ./devel/sagemain/doc/en/reference/sage/structure/mutability.rst rwrr 1 buildbot buildbot 71416 20120823 11:33:21.000000000 0400 ./devel/sagemain/sage/structure/mutability.c rwrr 1 buildbot buildbot 529 20100628 12:37:01.000000000 0400 ./devel/sagemain/sage/structure/mutability.pxd rwrr 1 buildbot buildbot 2062 20120823 11:22:27.000000000 0400 ./devel/sagemain/sage/structure/mutability.pyx
comment:30 Changed 6 years ago by
The .so
is recompiled from an old Cython C file which was
/* Generated by Cython 0.12.1 on Thu Aug 23 11:33:21 2012 */
while the old code would always run cython
to regenerate the .c
file.
Sounds like #4797...
comment:31 Changed 6 years ago by
Alternatively, we could also disallow upgrades from versions older than sage4.6. On the other hand, that would probably not be sufficient as some .c
files generated by newer versions of Cython would need to be recreated to account for #13896 for example.
comment:32 followup: ↓ 35 Changed 6 years ago by
So from what I can tell cythonize is in the right  these files don't need to be rebuilt (they nor any of their dependencies have been changed). So it looks like it is an issue of using old cython generated code with new cython generated code.
The other potential fix for this particular issue is to remove sage.structure.mutablity
, since nothing actually uses it (despite getting imported in a few places).
comment:33 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:34 Changed 6 years ago by
 Description modified (diff)
comment:35 in reply to: ↑ 32 Changed 6 years ago by
Replying to ohanar:
So from what I can tell cythonize is in the right  these files don't need to be rebuilt (they nor any of their dependencies have been changed).
It depends what you mean. One could argue that all Cython files should implicitly depend on the Cython program. Similar how, in an autotools build system, everything depends on config.status
(the output of ./configure
) or similar how SCons
and ccache
use the compiler executable as extra dependency of all C files.
comment:36 followup: ↓ 37 Changed 6 years ago by
 Status changed from needs_review to needs_work
Did you fix the issue of C files not being regenerated by a Cython upgrade? Because whether it's the "fault" of Cython or not, it needs to be fixed.
comment:37 in reply to: ↑ 36 ; followups: ↓ 40 ↓ 41 Changed 6 years ago by
Replying to jdemeyer:
Did you fix the issue of C files not being regenerated by a Cython upgrade?
No, I removed the single offending orphaned code, all the other code that is not regenerated causes no issues (since they are they are isolated from all of the modified files). The proper way to have this resolved is to have Cython's dependency checking not just check the timestamp of a file but also what version of Cython was used to generate the C file  but I would rather not have this ticket held up on a future version of Cython.
From what I can tell, the current DependancyTree
only works because it is finding false positives, not because it is checking the Cython version of generated files. So until I find the time to look at fixing this in Cython, I think the correct resolution is to implement #4797.
Changed 6 years ago by
Changed 6 years ago by
comment:38 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
OK, I've added a bit of code to cache and check the version.
comment:39 Changed 6 years ago by
 Description modified (diff)
Apply only trac13031cythonizesimple5.8.beta4.patch trac13031mutablity.patch trac13031cythonizeversion.patch
comment:40 in reply to: ↑ 37 Changed 6 years ago by
Replying to ohanar:
all the other code that is not regenerated causes no issues
Really???????????????????????????????? What about the bug that Cython0.17.4 fixed?
comment:41 in reply to: ↑ 37 Changed 6 years ago by
Replying to ohanar:
From what I can tell, the current
DependancyTree
only works because it is finding false positives, not because it is checking the Cython version of generated files.
True, but at least it works.
comment:42 Changed 6 years ago by
 Status changed from needs_review to positive_review
Ok, Robert's patch looks good to me.
comment:43 Changed 6 years ago by
 Milestone changed from sage5.8 to sage5.9
comment:44 Changed 6 years ago by
 Dependencies changed from #13029 to #13029, #13432
 Status changed from positive_review to needs_work
This needs to be rebased to #13432.
comment:45 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to positive_review
I thought I had rebased this, oh well, it should now apply cleanly on top of #13432.
comment:46 Changed 6 years ago by
 Status changed from positive_review to needs_work
Traceback (most recent call last): File "./spkginstall", line 4, in <module> from sage.all import save File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/all.py", line 80, in <module> from sage.misc.all import * # takes a while File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/misc/all.py", line 83, in <module> from functional import (additive_order, File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/misc/functional.py", line 36, in <module> from sage.rings.complex_double import CDF File "complex_double.pyx", line 91, in init sage.rings.complex_double (sage/rings/complex_double.c:16796) File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/rings/complex_field.py", line 110, in ComplexField C = ComplexField_class(prec) File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/rings/complex_field.py", line 204, in __init__ self._populate_coercion_lists_(coerce_list=[complex_number.RRtoCC(self._real_field(), self)]) File "complex_number.pyx", line 2431, in sage.rings.complex_number.RRtoCC.__init__ (sage/rings/complex_number.c:15890) File "map.pyx", line 125, in sage.categories.map.Map.__init__ (sage/categories/map.c:2478) File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/categories/homset.py", line 263, in Hom H = category.hom_category().parent_class(X, Y, category = category) File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/categories/rings.py", line 636, in __new__ from sage.rings.homset import RingHomset File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/rings/homset.py", line 17, in <module> import morphism File "morphism.pyx", line 985, in init sage.rings.morphism (sage/rings/morphism.c:12304) File "/release/merger/sage5.9.beta1/local/lib/python2.7/sitepackages/sage/structure/all.py", line 49, in <module> from mutability import Mutability ImportError: No module named mutability
comment:47 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
Apply only trac13031cythonizesimple.patch and trac13031cythonizeversion.patch
We can delete mutability in a separate CL, now that we have Cython version tracking.
comment:48 Changed 6 years ago by
 Status changed from needs_review to needs_work
sage t devel/sage/sage/misc/dev_tools.py ********************************************************************** File "devel/sage/sage/misc/dev_tools.py", line 145, in sage.misc.dev_tools.import_statements Failed example: import_statements(ZZ) Expected: ** Warning **: several names for that object: ZZ, Z from sage.rings.integer_ring import ZZ Got: ** Warning **: several names for that object: Z, ZZ from sage.rings.integer_ring import Z ********************************************************************** File "devel/sage/sage/misc/dev_tools.py", line 158, in sage.misc.dev_tools.import_statements Failed example: import_statements(ZZ, verbose=False) Expected: from sage.rings.integer_ring import ZZ Got: from sage.rings.integer_ring import Z **********************************************************************
Changed 6 years ago by
comment:49 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:50 Changed 6 years ago by
I think you are missing some sys.stdout.flush()
statements in cythonize()
, as the message
Cythonizing sage/categories/category_singleton.pyx
might come after
gcc fnostrictaliasing std=gnu99 D_XPG6 DNDEBUG g fwrapv O3 Wall Wstrictprototypes fPIC I/export/home/buildbot/build/sage/hawk1/hawk_full/build/sage5.9.beta1/local/include I/export/home/buildbot/build/sage/hawk1/hawk_full/build/sage5.9.beta1/local/include/csage I/export/home/buildbot/build/sage/hawk1/hawk_full/build/sage5.9.beta1/devel/sage/sage/ext I/export/home/buildbot/build/sage/hawk1/hawk_full/build/sage5.9.beta1/local/include/python2.7 c sage/categories/category_singleton.c o build/temp.solaris2.11i86pc.32bit2.7/sage/categories/category_singleton.o w
which is very confusing, especially when errors occur.
(there is nothing special about this file, it's just an example)
comment:51 Changed 6 years ago by
 Status changed from needs_review to needs_work
comment:52 Changed 6 years ago by
Changed 6 years ago by
comment:53 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
Granted, there's no reason (other than luck) that we couldn't have hit this before just as well.
comment:54 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:55 Changed 6 years ago by
The file sage/schemes/generic/notes/divisor_steinjoyner.txt
is removed in #13062, so 13031doctestfix.patch needs to be rebased.
Changed 6 years ago by
comment:56 Changed 6 years ago by
 Description modified (diff)
comment:57 Changed 6 years ago by
 Merged in set to sage5.9.beta3
 Resolution set to fixed
 Status changed from positive_review to closed
comment:58 Changed 6 years ago by
 Merged in changed from sage5.9.beta3 to sage5.9.beta4
comment:59 Changed 6 years ago by
Robert: any plans to add the version checking upstream to cythonize()
?
comment:60 Changed 6 years ago by
I just found some important bug: it seems the dependencies on the files in c_lib/
are no longer correctly handled.
$ touch devel/sage/c_lib/include/interrupt.h && ./sage b
should rebuild all files depending on interrupt.h
but this doesn't work anymore.
In sage5.9.beta3, one gets instead lots of lines like
Building sage/libs/ecl.pyx because it depends on /mazur/release/merger/sage5.9.beta3/local/include/csage/interrupt.h.
comment:61 Changed 6 years ago by
I'm on it.
comment:62 Changed 6 years ago by
Any progress?
comment:63 Changed 6 years ago by
The fix will be in Cython 0.19.
comment:64 Changed 6 years ago by
(We could backport it if that takes too long.)
comment:65 Changed 6 years ago by
Thanks. Anyway, good to know it's a Cython bug, not Sage bug.
comment:66 Changed 6 years ago by
Proposal: unmerge this for sage5.9, merge in again in sage5.10 together with #14452.
comment:67 Changed 6 years ago by
(or possibly merge #14452 in sage5.9.rc0)
comment:68 Changed 6 years ago by
Alternative proposal: cython0.18.p1. I really would not like to see this unmerged after it finally got in.
comment:69 Changed 6 years ago by
Given that cython0.19 is released today and we have a workaround for the docbuilder crash at #14452, why not cython0.19?
comment:70 followup: ↓ 71 Changed 6 years ago by
There are still serious problems with dependency checking: #14544
comment:71 in reply to: ↑ 70 ; followup: ↓ 72 Changed 6 years ago by
comment:72 in reply to: ↑ 71 ; followup: ↓ 73 Changed 6 years ago by
Replying to leif:
There are also reports on sagedevel that
sage clone
was broken in 5.9 (and later), now rebuilding the whole Sage library. Not sure what exactly introduced that though.
Looks like just the fact that sageclone
doesn't copy $SAGE_SRC/.cython_version
causes this...
(where $SAGE_SRC
is [meanwhile] $SAGE_ROOT/devel/sage
)
Also (but this doesn't seem to be the cause), sageclone
does not (or no longer, presumably at least since a while) copy Cythongenerated header files (*.h
), as these do not contain a comment on the first line stating that they were generated by Cython, so the following doesn't work for them:
print "Copying over all Cython autogenerated .c, .cpp and .h files..." def cpdir(src, dest): if not os.path.isdir(dest): return for F in os.listdir(src): if os.path.isdir(src + '/' + F): cpdir(src + '/' + F, dest + '/' + F) else: ext = os.path.splitext(F)[1] if ext in ['.h', '.c', '.cpp']: if 'Cython' in open(src + '/' + F).readline(): os.link(src + '/' + F, dest + '/' +F) os.utime(dest + '/' +F, None) cpdir(os.path.abspath('sage/sage'), os.path.abspath(branch + '/sage'))
comment:73 in reply to: ↑ 72 ; followup: ↓ 74 Changed 6 years ago by
Replying to leif:
Also (but this doesn't seem to be the cause),
sageclone
does not (or no longer, presumably at least since a while) copy Cythongenerated header files (*.h
), as these do not contain a comment on the first line stating that they were generated by Cython, so the following doesn't work for them:print "Copying over all Cython autogenerated .c, .cpp and .h files..." def cpdir(src, dest): if not os.path.isdir(dest): return for F in os.listdir(src): if os.path.isdir(src + '/' + F): cpdir(src + '/' + F, dest + '/' + F) else: ext = os.path.splitext(F)[1] if ext in ['.h', '.c', '.cpp']: if 'Cython' in open(src + '/' + F).readline(): os.link(src + '/' + F, dest + '/' +F) os.utime(dest + '/' +F, None) cpdir(os.path.abspath('sage/sage'), os.path.abspath(branch + '/sage'))
Furthermore, the above code doesn't copy the Cythongenerated files from devel/sage/ext/interpreters/
, so these also get rebuilt, but that goes back to at least Sage 5.8 (unless I'm missing something).
If I add copying .cython_version
to sageclone
, besides the abovementioned re"cythonizing"[sic!], many (if not all?) Python modules get rebytecompiled, too, for whatever reason... (This doesn't seem to be related to Cython, but maybe I'm just missing something here as well.)
comment:74 in reply to: ↑ 73 Changed 6 years ago by
Replying to leif:
If I add copying
.cython_version
tosageclone
...

sageclone
diff git a/sageclone b/sageclone
a b 53 53 54 54 cpdir(os.path.abspath('sage/sage'), os.path.abspath(branch + '/sage')) 55 55 56 if os.path.isfile('sage/.cython_version'): 57 print "Copying over hidden Cython version file..." 58 os.link('sage/.cython_version', branch+'/.cython_version') 59 56 60 def copy_dtree(src_dir, dest_dir): 57 61 src_root = os.path.abspath(src_dir) 58 62 dest_root = os.path.abspath(dest_dir)
Is the patch based against sage5.1beta0? I upgraded a sage installation from sage5.0rc0 to sage5.1beta0 and it fails to apply there. I will try a sage5.1beta0 tarball sometime later.