Opened 12 years ago
Closed 12 years ago
#10751 closed defect (fixed)
Upgrading 4.5.3 -> 4.6.2.alpha4 fails
Reported by: | jdemeyer | Owned by: | GeorgSWeber |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7 |
Component: | build | Keywords: | cython upgrade dependencies |
Cc: | vbraun | Merged in: | sage-4.7.alpha1 |
Authors: | Volker Braun | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This is a problem I discovered when upgrading from sage-4.5.3 to a candidate 4.6.2.alpha4. Probably, the problem has existed for a while, it just "surfaced" or got noticed now.
The problem is that the file sage/algebras/quatalg/quaternion_algebra_element.cpp
is not recreated from the corresponding .pyx file when upgrading. This then causes a compile error.
The file sage/algebras/quatalg/quaternion_algebra_element.pyx
includes (using Cython's include
statement) sage/ext/gmp.pxi
and the latter file was changed some Sage versions ago.
The following happens:
$ ./sage -b ---------------------------------------------------------- sage: Building and installing modified Sage library files. Installing c_lib scons: `install' is up to date. =================== setup.py ======================== Updating Cython code.... Time to execute 0 commands: 1.31130218506e-05 seconds Finished compiling Cython code (time = 0.0120220184326 seconds) running install running build running build_py running build_ext building 'sage.algebras.quatalg.quaternion_algebra_element' extension building 'sage.calculus.riemann' extension [...] building 'sage.symbolic.pynac' extension building 'sage.symbolic.ring' extension gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -fPIC -I/mnt/usb1/scratch/jdemeyer/sage-4.5.3-4.6.2.alpha4/local/include/FLINT/ -I/mnt/usb1/scratch/jdemeyer/sage-4.5.3-4.6.2.alpha4/local/include -I/mnt/usb1/scratch/jdemeyer/sage-4.5.3-4.6.2.alpha4/local/include/csage -I/mnt/usb1/scratch/jdemeyer/sage-4.5.3-4.6.2.alpha4/devel/sage/sage/ext -I/mnt/usb1/scratch/jdemeyer/sage-4.5.3-4.6.2.alpha4/local/include/python2.6 -c sage/algebras/quatalg/quaternion_algebra_element.cpp -o build/temp.linux-x86_64-2.6/sage/algebras/quatalg/quaternion_algebra_element.o -w [trouble...]
Depends on #10233
Apply trac_10751_fix.patch
That is, first apply trac_10233_fix_cython_include_path.patch
from the dependency and then trac_10751_fix.patch
.
Attachments (2)
Change History (18)
comment:1 Changed 12 years ago by
Cc: | vbraun added |
---|
Changed 12 years ago by
Attachment: | trac_10751_setup_misses_cython_dependencies.patch added |
---|
comment:2 Changed 12 years ago by
Authors: | → Volker Braun |
---|---|
Keywords: | dependencies added |
Status: | new → needs_work |
Here is the initial version of my rewrite. The basic idea is to split the dependency tracking off into the sage library (sage/misc/dependencies.py
) so it can be reused, documented, and doctested. And instead of stuffing filenames as strings into dictionaries, classes encapsulating source file names are introduced. Let me know if you have any comments on these design decisions.
I still want to polish the code a bit more, but the current patch fixes Jeroen's bug:
[vbraun@volker-two hg]$ touch sage/ext/gmp.pxi [vbraun@volker-two hg]$ sage -b ---------------------------------------------------------- sage: Building and installing modified Sage library files. Installing c_lib scons: `install' is up to date. Updating Cython code.... Building sage/algebras/quatalg/quaternion_algebra_element.pyx because it depends on sage/ext/gmp.pxi. Building sage/combinat/expnums.pyx because it depends on sage/ext/gmp.pxi. Building sage/ext/multi_modular.pyx because it depends on sage/ext/gmp.pxi. Building sage/matrix/matrix_cyclo_dense.pyx because it depends on sage/ext/gmp.pxi. Building sage/matrix/matrix_integer_dense.pyx because it depends on sage/ext/gmp.pxi. Building sage/matrix/matrix_rational_dense.pyx because it depends on sage/ext/gmp.pxi. Building sage/matrix/matrix_rational_sparse.pyx because it depends on sage/ext/gmp.pxi. Building sage/matrix/misc.pyx because it depends on sage/ext/gmp.pxi. Building sage/modular/modform/eis_series_cython.pyx because it depends on sage/ext/gmp.pxi. Building sage/quadratic_forms/count_local_2.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/integer.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/integer_ring.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/fast_arith.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/fraction_field_FpT.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/rational.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/padics/padic_base_coercion.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/padics/padic_capped_absolute_element.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/padics/padic_capped_relative_element.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/padics/padic_fixed_mod_element.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/padics/padic_generic_element.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/padics/padic_printing.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/padics/pow_computer.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/padics/pow_computer_ext.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/polynomial/polynomial_integer_dense_flint.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/polynomial/polynomial_rational_flint.pyx because it depends on sage/ext/gmp.pxi. Building sage/rings/polynomial/real_roots.pyx because it depends on sage/ext/gmp.pxi. Execute 26 commands (using 4 threads)
comment:3 Changed 12 years ago by
It's big. I guess the split in depency.py may make thing more manageable in the long term. We how to discuss the potential removal of debian stuff on the the list, #5903 doesn't say anything about setup.py and module_list.py (which I was thinking of cleaning).
The only mistake I can see from a first look is on line 529 of dependency.py
sage: gmp = SourceFile('/home/vbraun/opt/sage-4.6.2.alpha4/local/include/gmp.h')
I don't think we want to hardcode your setup :)
comment:4 Changed 12 years ago by
We should switch Sage to use http://wiki.cython.org/enhancements/distutils_preprocessing . This would let us get rid of most of the manual module_list.py declarations as well.
comment:5 Changed 12 years ago by
Priority: | blocker → major |
---|
comment:6 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_review |
Robert's distutils-preprocessing seems to be the best way to do things, and we should investigate that once the requisite Cython-14.1 is included.
In the meantime, I've made a small patch that fixes the dependency discovery bug (tested by touching sage/ext/gmp.pxi as above). This leaves the setup.py code in its current somewhat messy state, but I think we should go with the minimal bugfix until we can give distutils-preprocessing a try.
Also, enables parallel compile by default. I didn't even know we supported that. You can still disable it via setting the MAKE='make -j1'
environment variable. Rebuilding the Sage library dropped from half an hour to 5 minutes ;-)
comment:7 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:8 Changed 12 years ago by
Description: | modified (diff) |
---|
Hmm patch bot still lists both patches erroneously.
Apply trac_10751_fix.patch
comment:9 Changed 12 years ago by
Status: | needs_review → needs_work |
---|
make -j
without argument to -j
means: use unlimited number of threads. Therefore, the code
pos = MAKE.rfind('-j') if pos>=0: nthreads = int(MAKE[pos+2:].split()[0])
should be changed to
pos = MAKE.rfind(' -j') if pos >= 0: try: nthreads = int(MAKE[pos+3:].split()[0]) except ValueError: # If there is no argument to -j, use maximum number of threads nthreads = 2*cpu_count
In any case, the ValueError
must be handled somehow.
Note also that I have added a space before -j
.
comment:12 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
comment:13 Changed 12 years ago by
Status: | needs_review → needs_work |
---|
I think you need to add a try
/except
as follows:
try: if MAKE[pos+3] == '=': # make -j=N is the same as make -jN pos += 1 except IndexError: pass
comment:14 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
Yep, that if clause needs to be moved into the existing try/except block. Updated patch follows...
comment:15 Changed 12 years ago by
Reviewers: | → Jeroen Demeyer |
---|---|
Status: | needs_review → positive_review |
Looks good to me.
comment:16 Changed 12 years ago by
Merged in: | → sage-4.7.alpha1 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Initial patch