sagemath/sage

Update LinBox to most recent upstream release

pcpa opened this issue · 106 comments

pcpa commented

The version of LinBox used in Sage is 4 years old, we should update.

  1. Observe: Dependencies
  2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.6.0.spkg
  3. Install: http://boxen.math.washington.edu/home/jdemeyer/spkg/linbox-1.3.2.spkg
  4. Apply: attachment: matrix_modn_dense_no_linbox.patch
  5. Apply: attachment: sage-linbox.2.patch
  6. Apply: attachment: trac_12883_linbox_deps.patch and attachment: trac_12883-spkg-install.patch to SAGE_ROOT repository
  7. Apply: attachment: trac_12883_hg_ignore_fflas_config.patch to the scripts (SAGE_LOCAL/bin) repository.

SPKG Repositories:

  1. https://bitbucket.org/malb/fflas-ffpack-spkg
  2. https://bitbucket.org/malb/linbox-spkg

Note for release manager: This ticket must be merged simultaneously with #13164, they depend on each other.

Depends on #13118
Depends on #12840
Depends on #12841
Depends on #13164

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

CC: @jpflori

Component: linbox

Author: Paulo César Pereira de Andrade, Martin Albrecht

Reviewer: Volker Braun, Jeroen Demeyer

Merged: sage-5.4.beta1

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

comment:1

What's the title supposed to tell us (if you get build errors with that version)?

For GCC 4.7.x / C++11 issues see #12762; for (problems with) updating LinBox see #11718. (I think I've read about complications when trying to upgrade to 1.2.2 elsewhere as well.) See #12865 for yet another issue, which is still present in 1.2.2.

pcpa commented
comment:2

Sorry for lack of a better description. I am trying to address and submit reports to involved upstream parties about some patches and version issues of some components, while checking how far I can go building sagemath in Fedora, and using Fedora packages.

For example, the first error:

error: 'linbox_modn_dense_echelonize' was not declared in this scope

in /usr/include/linbox/linbox-sage.h I see only

EXTERN unsigned long int linbox_modn_dense_echelonize_double (double modulus, double*matrix, size_t nrows, size_t ncols);

EXTERN unsigned long int linbox_modn_dense_echelonize_float (float modulus, float * matrix,size_t nrows, size_t ncols);

Same happens for linbox_modn_dense_delete_array that now have prototypes for linbox_modn_dense_delete_array_double and linbox_modn_dense_delete_array_float.

Also linbox_integer_dense_matrix_matrix_multiply changed prototype from

/* ans must be a pre-allocated and pre-initialized array of GMP ints. */
EXTERN int linbox_integer_dense_matrix_matrix_multiply(mpz_t** ans, mpz_t **A, mpz_t **B,
			      size_t A_nr, size_t A_nc, size_t B_nr, size_t B_nc);

in the sagemath linbox-1.1.6 spkg, to

/* ans must be a pre-allocated and pre-initialized array of GMP ints. */
int linbox_integer_dense_matrix_matrix_multiply (mpz_t** ans, mpz_t **A, mpz_t **B, size_t m, size_t n, size_t k);

in linbox-1.2.2.

I am trying to have upstream aware of these issues, and if not possible to get components working, I can try to get an extra linbox 1.1.6 package in Fedora.

pcpa commented
comment:3

Upstream linbox 1.2.2 should need this patch

--- linbox-1.2.2/interfaces/sage/linbox-sage.h.orig     2012-04-26 16:25:37.500132509 -0300
+++ linbox-1.2.2/interfaces/sage/linbox-sage.h  2012-04-26 16:26:00.705133519 -0300
@@ -48,7 +48,9 @@
 EXTERN void linbox_modn_dense_delete_array_double (double *f);
 EXTERN void linbox_modn_dense_delete_array_float (float *f);
 
-
+template <class Element>
+unsigned long int linbox_modn_dense_echelonize (Element modulus, Element* matrix,
+                                               size_t nrows, size_t ncols);
 EXTERN unsigned long int linbox_modn_dense_echelonize_double (double modulus, double*matrix, size_t nrows, size_t ncols);
 EXTERN unsigned long int linbox_modn_dense_echelonize_float (float modulus, float * matrix,size_t nrows, size_t ncols);
 

But it is not yet enough because the sage matrix interface was changed from "jagged pointers" to a single sequential vector.

I believe I could make a patch for sage/libs/linbox/linbox.{pxd,pyx} but I do not feel confident about other places where sage interfaces to linbox matrices.

pcpa commented

Experimental linbox 1.2.2 patch

pcpa commented
comment:4

Attachment: sage-4.8-linbox.patch.gz

The attached patches allow continuing experimenting with a new sagemath rpm.
The patches are only tested to know they make sagemath and linbox agree on api/abi, but no testing done.
There were missing prototypes/templates in linbox-sage.h as well as wrong ones.
I am not fully confident that some place may do wrong dereferencing of matrix fields due to upstream change from a vector of rows where every row is an allocated pointer to a single memory chunk.
Another part I am not confident is that I did the correct logic in the change from
A_nr, A_nc, B_nr, B_nc to test in sage code if A_nr != B_nc: ..., and convert the values of m, n, k as
A_nr, A_nc, B_nr.

This is not a proposal of a final patch, but a extra call for more feedback. As this was also the first time I actually did look at linbox code, so, just patched the interfaces to make both sides agree.

pcpa commented

fflas-ffpack-64bit.patch

pcpa commented

Attachment: fflas-ffpack-64bit.patch.gz

Attachment: linbox-sagemath.patch.gz

linbox-sagemath.patch

pcpa commented
comment:5

Is it reasonably to define mod_int to C long?

If not, safest approach should be to use uint32_t I believe, otherwise, should require coding 64 bit templates in linbox 1.2.2.

The new attached fflas-ffpack-64bit.patch is required to generate proper 64 bit code, and the updated linbox-sagemath.patch should be very close to the final format, but note that in 64 bit arch, linbox 1.2.2 with the attached patches needs also to have -D!__LINBOX_HAVE_INT64=1 in CFLAGS and CXXFLAGS.

I should update the sage-4.8-linbox.patch at some point, as the initial patch I attached was only enough to get to another build failure in Fedora. But the next update should be for sage-5.0.

malb commented

Description changed:

--- 
+++ 
@@ -1,38 +1,5 @@
-  Reported build problems with linbox 1.2.2 to upstream a few minutes ago.
+The version of LinBox used in Sage is **4 years** old, we should update.
 
-  Reason is that I am experimenting with packaging sagemath in Fedora. As you should know, I have it packaged since late sagemath 3.x up to 4.8 in Mandriva, so I am attempting to repeat it in Fedora.
-
-  Fedora has linaro 1.2.2 (built with --enable-sage) packaged, and givaro-3.5.0 btw :-)
-
-  I reported the same build log at[http://groups.google.com/group/linbox-use/browse_thread/thread/713663b790d0dfe9](http://groups.google.com/group/linbox-use/browse_thread/thread/713663b790d0dfe9) but pasting it here also to hopefully give an idea of the build failure.
-
-```
-gcc -pthread -fno-strict-aliasing -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I/home/pcpa/rpmbuild/BUILDROOT/sagemath-4.8-0.1.x86_64/usr/share/sagemath/local/include -Ic_lib/include -I/home/pcpa/rpmbuild/BUILDROOT/sagemath-4.8-0.1.x86_64/usr/share/sagemath/devel/sage/sage/ext -I/usr/include/python2.7 -c sage/libs/linbox/linbox.cpp -o build/temp.linux-x86_64-2.7/sage/libs/linbox/linbox.o -w
-sage/libs/linbox/linbox.cpp: In function 'int __pyx_f_4sage_4libs_6linbox_6linbox_17Linbox_modn_dense_echelonize(__pyx_obj_4sage_4libs_6linbox_6linbox_Linbox_modn_dense*)':
-sage/libs/linbox/linbox.cpp:2308:123: error: 'linbox_modn_dense_echelonize' was not declared in this scope
-sage/libs/linbox/linbox.cpp: In function 'PyObject* __pyx_pf_4sage_4libs_6linbox_6linbox_17Linbox_modn_dense_4_poly(PyObject*, PyObject*)':
-sage/libs/linbox/linbox.cpp:2480:335: error: no matching function for call to 'linbox_modn_dense_minpoly(size_t&, size_t**, size_t*, size_t&, size_t**&, int&)'
-sage/libs/linbox/linbox.cpp:2480:335: note: candidate is:
-/usr/include/linbox/linbox-sage.h:65:11: note: template<class Element> Element* linbox_modn_dense_minpoly(Element, Element**, size_t*, size_t, Element*)
-sage/libs/linbox/linbox.cpp:2527:43: error: 'linbox_modn_dense_delete_array' was not declared in this scope
-sage/libs/linbox/linbox.cpp: In function 'PyObject* __pyx_f_4sage_4libs_6linbox_6linbox_17Linbox_modn_dense_matrix_matrix_multiply(__pyx_obj_4sage_4libs_6linbox_6linbox_Linbox_modn_dense*, size_t**, size_t**, size_t, size_t)':
-sage/libs/linbox/linbox.cpp:2578:187: error: no matching function for call to 'linbox_modn_dense_matrix_matrix_multiply(size_t&, size_t**&, size_t**&, size_t**&, size_t&, size_t&, size_t&, size_t&)'
-sage/libs/linbox/linbox.cpp:2578:187: note: candidate is:
-/usr/include/linbox/linbox-sage.h:77:9: note: template<class Element> Element* linbox_modn_dense_matrix_matrix_multiply(Element, Element*, Element*, Element*, size_t, size_t, size_t)
-sage/libs/linbox/linbox.cpp: In function 'long unsigned int __pyx_f_4sage_4libs_6linbox_6linbox_17Linbox_modn_dense_rank(__pyx_obj_4sage_4libs_6linbox_6linbox_Linbox_modn_dense*)':
-sage/libs/linbox/linbox.cpp:2634:117: error: 'linbox_modn_dense_rank' was not declared in this scope
-sage/libs/linbox/linbox.cpp: In function 'size_t __pyx_f_4sage_4libs_6linbox_6linbox_17Linbox_modn_dense_det(__pyx_obj_4sage_4libs_6linbox_6linbox_Linbox_modn_dense*, int)':
-sage/libs/linbox/linbox.cpp:2697:116: error: no matching function for call to 'linbox_modn_dense_det(size_t&, size_t**&, size_t&, size_t&)'
-sage/libs/linbox/linbox.cpp:2697:116: note: candidate is:
-/usr/include/linbox/linbox-sage.h:59:9: note: template<class Element> Element linbox_modn_dense_det(Element, Element*, size_t, size_t)
-sage/libs/linbox/linbox.cpp: In function 'PyObject* __pyx_pf_4sage_4libs_6linbox_6linbox_20Linbox_integer_dense_2_poly(PyObject*, PyObject*)':
-sage/libs/linbox/linbox.cpp:5127:253: error: invalid initialization of non-const reference of type '__mpz_struct (*&)[1]' from an rvalue of type '__mpz_struct (**)[1]'
-/usr/include/linbox/linbox-sage.h:108:6: error: in passing argument 1 of 'void linbox_integer_dense_minpoly(__mpz_struct (*&)[1], size_t&, size_t, __mpz_struct (**)[1])'
-sage/libs/linbox/linbox.cpp:5139:254: error: invalid initialization of non-const reference of type '__mpz_struct (*&)[1]' from an rvalue of type '__mpz_struct (**)[1]'
-/usr/include/linbox/linbox-sage.h:110:6: error: in passing argument 1 of 'void linbox_integer_dense_charpoly(__mpz_struct (*&)[1], size_t&, size_t, __mpz_struct (**)[1])'
-sage/libs/linbox/linbox.cpp: In function 'PyObject* __pyx_f_4sage_4libs_6linbox_6linbox_20Linbox_integer_dense_matrix_matrix_multiply(__pyx_obj_4sage_4libs_6linbox_6linbox_Linbox_integer_dense*, __mpz_struct (**)[1], __mpz_struct (**)[1], size_t, size_t)':
-sage/libs/linbox/linbox.cpp:5283:173: error: too many arguments to function 'int linbox_integer_dense_matrix_matrix_multiply(__mpz_struct (**)[1], __mpz_struct (**)[1], __mpz_struct (**)[1], size_t, size_t, size_t)'
-/usr/include/linbox/linbox-sage.h:115:5: note: declared here
-error: command 'gcc' failed with exit status 1
-```
-
+**Observe:** Dependencies
+**Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg
+**Apply:** [attachment: sage-linbox](https://github.com/sagemath/sage/files/ticket12883/sage-linbox.gz)
malb commented

Description changed:

--- 
+++ 
@@ -1,5 +1,5 @@
 The version of LinBox used in Sage is **4 years** old, we should update.
 
-**Observe:** Dependencies
-**Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg
-**Apply:** [attachment: sage-linbox](https://github.com/sagemath/sage/files/ticket12883/sage-linbox.gz)
+1. **Observe:** Dependencies 
+2. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg 
+3. **Apply:** [attachment: sage-linbox](https://github.com/sagemath/sage/files/ticket12883/sage-linbox.gz)
malb commented

Dependencies: #12840, 12841, #9511

malb commented

Description changed:

--- 
+++ 
@@ -1,5 +1,6 @@
 The version of LinBox used in Sage is **4 years** old, we should update.
 
-1. **Observe:** Dependencies 
-2. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg 
-3. **Apply:** [attachment: sage-linbox](https://github.com/sagemath/sage/files/ticket12883/sage-linbox.gz)
+1. **Observe:** Dependencies
+2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.5.0.spkg
+3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg
+4. **Apply:** [attachment: sage-linbox](https://github.com/sagemath/sage/files/ticket12883/sage-linbox.gz)
malb commented

Author: Paulo César Pereira de Andrade, Martin Albrecht

malb commented

Description changed:

--- 
+++ 
@@ -1,6 +1,7 @@
 The version of LinBox used in Sage is **4 years** old, we should update.
 
 1. **Observe:** Dependencies
-2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.5.0.spkg
+2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.5.0.spkg
 3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg
-4. **Apply:** [attachment: sage-linbox](https://github.com/sagemath/sage/files/ticket12883/sage-linbox.gz)
+4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
+5. **Apply:** [attachment: sage-linbox](https://github.com/sagemath/sage/files/ticket12883/sage-linbox.gz)
malb commented

Description changed:

--- 
+++ 
@@ -4,4 +4,4 @@
 2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.5.0.spkg
 3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
-5. **Apply:** [attachment: sage-linbox](https://github.com/sagemath/sage/files/ticket12883/sage-linbox.gz)
+5. **Apply:** [attachment: sage-linbox.patch](https://github.com/sagemath/sage-prod/files/10655418/sage-linbox.patch.gz)
malb commented

Changed dependencies from #12840, 12841, #9511 to #12840, #12841, #9511

malb commented

Description changed:

--- 
+++ 
@@ -1,7 +1,7 @@
 The version of LinBox used in Sage is **4 years** old, we should update.
 
 1. **Observe:** Dependencies
-2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.5.0.spkg
-3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg
+2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.6.0.spkg
+3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.1.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.patch](https://github.com/sagemath/sage-prod/files/10655418/sage-linbox.patch.gz)
malb commented

Description changed:

--- 
+++ 
@@ -5,3 +5,8 @@
 3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.1.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.patch](https://github.com/sagemath/sage-prod/files/10655418/sage-linbox.patch.gz)
+
+SPKG Repositories:
+
+1. https://bitbucket.org/malb/fflas-ffpack-spkg
+2. https://bitbucket.org/malb/linbox-spkg
malb commented

Description changed:

--- 
+++ 
@@ -2,7 +2,7 @@
 
 1. **Observe:** Dependencies
 2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.6.0.spkg
-3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.1.spkg
+3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.2.pre.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.patch](https://github.com/sagemath/sage-prod/files/10655418/sage-linbox.patch.gz)
 
malb commented

Description changed:

--- 
+++ 
@@ -2,7 +2,7 @@
 
 1. **Observe:** Dependencies
 2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.6.0.spkg
-3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.2.pre.spkg
+3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.2.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.patch](https://github.com/sagemath/sage-prod/files/10655418/sage-linbox.patch.gz)
 
malb commented
comment:17

All doctests pass on geom.math and my machine with 5.0.1.

malb commented

Work Issues: Julien's SPKG patch, update for deps

malb commented
comment:19

Needs review.

malb commented

Changed work issues from Julien's SPKG patch, update for deps to none

malb commented

Description changed:

--- 
+++ 
@@ -5,6 +5,7 @@
 3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.2.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.patch](https://github.com/sagemath/sage-prod/files/10655418/sage-linbox.patch.gz)
+6. **Apply:** [attachment: trac_12883_linbox_deps.patch](https://github.com/sagemath/sage-prod/files/10655416/trac_12883_linbox_deps.patch.gz) to `SAGE_ROOT` repository
 
 SPKG Repositories:
 
comment:20

matrix_modn_dense_no_linbox.patch is full of unnecessary whitespace changes that only serve to break any other patch on trac.

malb commented
comment:21

Replying to @vbraun:

matrix_modn_dense_no_linbox.patch is full of unnecessary whitespace changes that only serve to break any other patch on trac.

Fixed

malb commented

Attachment: matrix_modn_dense_no_linbox.patch.gz

fixed indentation error introduced in last update

comment:22

What for/where is matrix_modn_dense used now? I see its commented out in matrix_space.py.

Minor nitpick: missing space in coefficients ofminpoly as a Python list.

malb commented
comment:23

Replying to @vbraun:

What for/where is matrix_modn_dense used now? I see its commented out in matrix_space.py.

Currently, none. But William is resurrecting it at #4968. I assume [attachment: matrix_modn_dense_no_linbox.patch] and #4968 will conflict. But I suggest to deal with this when the problem arises and when we know in which order they'll be merged.

Minor nitpick: missing space in coefficients ofminpoly as a Python list.

I'll fix that.

malb commented
comment:24

Fixed.

comment:25

I believe it is not necessary anymore to add -DDISABLE_COMMENTATOR to CPPFLAGS. IT is added automatically in the right places when you configure with the sage interface.

malb commented
comment:26

Replying to @kiwifb:

I believe it is not necessary anymore to add -DDISABLE_COMMENTATOR to CPPFLAGS. IT is added automatically in the right places when you configure with the sage interface.

I am inclined to postpone this change to another ticket, unless you are pretty certain it isn't needed. The reason I am suggesting this is, that if we get SIGSEGVs then on some exotic platform we are not sure whether it's the 1.3.2 update or that we dropped the -DDISABLE_COMMENTATOR. Of course, we could just try to re-add it and see what happens, but given how slow and bureaucratic Sage development is these days, this could easily take a week to accomplish, which would be annoying. What do you think?

comment:27

Keeping it is "mostly harmless". I just noticed when I created the ebuild for sage-on-gentoo that even so I hadn't added it to the CPPFLAGS it was used in the compilation. Adding it to the CPPFLAGS made it appear twice in those places.

I would have to test with the whole set of new packages and patches to be absolutely certain of course. That could take a few days. And of course that would be on a non-exotic platform first. Make me think I need to put a 5.1beta on power7 to cover my exotic corner.

Reviewer: Volker Braun

comment:28

Looks good to me!

Description changed:

--- 
+++ 
@@ -11,3 +11,5 @@
 
 1. https://bitbucket.org/malb/fflas-ffpack-spkg
 2. https://bitbucket.org/malb/linbox-spkg
+
+Note for release manager: This ticket must be merged simultaneously with #9511, they depend on each other.
comment:30

In SPKG.txt of fflas_ffpack-1.6.0.spkg, replace

=== fflas-ffpack-1.6.0 (Martin Albrecht, 7 June 2012) ===

by

=== fflas_ffpack-1.6.0 (Martin Albrecht, 7 June 2012) ===
comment:32

Just curious -- does this fix #12865? (Won't try myself.)

comment:33

Yes, it does fix #12865.

malb commented
comment:34

SPKG.txt issue is fixed.

comment:37

There is a weird file .swp in the linbox spkg.

comment:39

I've reuploaded the package without that mysterious file (which is not in spkg repo as I've checked) at
http://perso.telecom-paristech.fr/~flori/sage/linbox-1.3.2.spkg

No other changes to the spkg.
So someone should put this back to positive review (after checking I did not break everything).

Edit: Swapped 2 and 3...

comment:40

The new fflas spkg installs a new script in SAGE_LOCAL/bin, which needs to be added to .hgignore. I've included a trivial patch to that extent.

Rest looks good.

Description changed:

--- 
+++ 
@@ -6,6 +6,7 @@
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.patch](https://github.com/sagemath/sage-prod/files/10655418/sage-linbox.patch.gz)
 6. **Apply:** [attachment: trac_12883_linbox_deps.patch](https://github.com/sagemath/sage-prod/files/10655416/trac_12883_linbox_deps.patch.gz) to `SAGE_ROOT` repository
+7. **Apply:** [attachment: trac_12883_hg_ignore_fflas_config.patch](https://github.com/sagemath/sage-prod/files/10655419/trac_12883_hg_ignore_fflas_config.patch.gz) to the scripts (`SAGE_LOCAL/bin`) repository.
 
 SPKG Repositories:
 

Rebased patch

comment:41

Attachment: sage-linbox.2.patch.gz

Rebased patch for sage-5.3.beta1. Just a trivial rebase in module_list.py.

Description changed:

--- 
+++ 
@@ -4,7 +4,7 @@
 2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.6.0.spkg
 3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.2.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
-5. **Apply:** [attachment: sage-linbox.patch](https://github.com/sagemath/sage-prod/files/10655418/sage-linbox.patch.gz)
+5. **Apply:** [attachment: sage-linbox.2.patch](https://github.com/sagemath/sage-prod/files/10655420/sage-linbox.2.patch.gz)
 6. **Apply:** [attachment: trac_12883_linbox_deps.patch](https://github.com/sagemath/sage-prod/files/10655416/trac_12883_linbox_deps.patch.gz) to `SAGE_ROOT` repository
 7. **Apply:** [attachment: trac_12883_hg_ignore_fflas_config.patch](https://github.com/sagemath/sage-prod/files/10655419/trac_12883_hg_ignore_fflas_config.patch.gz) to the scripts (`SAGE_LOCAL/bin`) repository.
 
comment:42

Should the root repo patch also touch spkg/install, adding a line for fflas, like this?

diff --git a/spkg/install b/spkg/install
--- a/spkg/install
+++ b/spkg/install
@@ -296,6 +296,7 @@ ECLIB=`newest_version eclib`
 ECM=`newest_version ecm`
 ELLIPTIC_CURVES=`newest_version elliptic_curves`
 EXTCODE=`newest_version extcode`
+FFLASFFPACK=`newest_version fflas_ffpack`
 FLINT=`newest_version flint`
 FLINTQS=`newest_version flintqs`
 FPLLL=`newest_version libfplll`

Or is this not needed for some reason?

comment:43

Yes, I'm pretty sure thats required. Are you going to post a patch?

comment:44

Sure, here's a patch. Do I need to switch it back to "needs review"?

Description changed:

--- 
+++ 
@@ -5,7 +5,7 @@
 3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.2.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.2.patch](https://github.com/sagemath/sage-prod/files/10655420/sage-linbox.2.patch.gz)
-6. **Apply:** [attachment: trac_12883_linbox_deps.patch](https://github.com/sagemath/sage-prod/files/10655416/trac_12883_linbox_deps.patch.gz) to `SAGE_ROOT` repository
+6. **Apply:** [attachment: trac_12883_linbox_deps.patch](https://github.com/sagemath/sage-prod/files/10655416/trac_12883_linbox_deps.patch.gz) and [attachment: trac_12883-spkg-install.patch](https://github.com/sagemath/sage-prod/files/10655421/trac_12883-spkg-install.patch.gz) to `SAGE_ROOT` repository
 7. **Apply:** [attachment: trac_12883_hg_ignore_fflas_config.patch](https://github.com/sagemath/sage-prod/files/10655419/trac_12883_hg_ignore_fflas_config.patch.gz) to the scripts (`SAGE_LOCAL/bin`) repository.
 
 SPKG Repositories:

Changed reviewer from Volker Braun to Volker Braun, Jeroen Demeyer

comment:47

Why the change to the iml dependencies?

comment:48

The linbox spkg still contains the garbage I mentioned before:

? .swp
comment:49

Fixed. I guess vim poops into cwd when segfaulting (.swp was vim swap file), never heard of /tmp?!?

Description changed:

--- 
+++ 
@@ -2,7 +2,7 @@
 
 1. **Observe:** Dependencies
 2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.6.0.spkg
-3. **Install:** http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.2.spkg
+3. **Install:** http://www.stp.dias.ie/~vbraun/Sage/spkg/linbox-1.3.2.spkg
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.2.patch](https://github.com/sagemath/sage-prod/files/10655420/sage-linbox.2.patch.gz)
 6. **Apply:** [attachment: trac_12883_linbox_deps.patch](https://github.com/sagemath/sage-prod/files/10655416/trac_12883_linbox_deps.patch.gz) and [attachment: trac_12883-spkg-install.patch](https://github.com/sagemath/sage-prod/files/10655421/trac_12883-spkg-install.patch.gz) to `SAGE_ROOT` repository
comment:50

PS: The iml testsuite does link against ATLAS, even though libiml doesn't depend on it.

comment:51

For what it's worth, building from scratch with these patches works (all long tests passed) on sage.math and on my OS X Lion box.

comment:52

The LinBox spkg needs to be rebased to linbox-1.1.6.p11 (see #13118).

Changed dependencies from #12840, #12841, #9511 to #13118, #12840, #12841, #9511,

comment:53

Replying to @jdemeyer:

The LinBox spkg needs to be rebased to linbox-1.1.6.p11 (see #13118).

and also see #12762. By the way, the file SPKG.txt also has two duplicate entries in the change log for version 1.1.6.p8.

comment:54

I've added the changelog entries of linbox-1.1.6p9 - 11 to the SPKG.txt and removed the duplicate. None of those changes are needed any more since upstream has long fixed the issues that gcc 4.7 uncovered and Martin cleaned up the spkg-install.

I've also re-added the spkg-check as disabled-spkg-check and deleted the Makefile.am.patch. Right now the testsuite will not compile without commentator. Would be nice to have the testsuite but not on this ticket.

Likewise, it would be nice if the Sage developers could first search trac if there is already a new spkg before opening a ticket.

Since really only the description changed I'll set this ticket back to positive review.

Changed upstream from None of the above - read trac for reasoning. to Reported upstream. No feedback yet.

comment:56

For the record, I've noted the testsuite issues on the linbox mailinglist at https://groups.google.com/d/topic/linbox-use/z0AlEH8Iyug/discussion

comment:57

Testsuite issues are fixed in svn version 4370.

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

comment:58

Is it intentional that the --with-gmp, --with-givaro and --with-ntl and --with-blas configure options were removed?

Secondly, by "rebasing" this way, you've lost the .hgtags (and probably the whole hg history) for some earlier versions. Not sure whether that's important, but it's something to be pointed out.

comment:59

Also, the .p11 spkg has error messages to stderr, please don't revert this.

comment:61

I think this really needs to be rebased properly. You've reverted some good changes from the .p9--p11 versions. Also, try to keep the Mercurial history.

comment:62

Also, there isn't any documentation of the fact and the reason of renaming spkg-check to disabled-spkg-check.

comment:63

I've wasted my coffee break in preserving the mercurial history of the bikeshedding that went into linbox-1.1.6p(N+1) despite the fact that it is all being ripped out. Hopefully future generations of archaeologists will appreciate the effort.

I've also mentioned in the SPKG.txt that spgk-check is disabled in case anybody can't figure it out from the filename.

Now that spkg-install sets --with-default we don't need --with-foo for every foo.

Now errors go to stderr again.

Changed dependencies from #13118, #12840, #12841, #9511, to #13118, #12840, #12841, #9511

comment:66

This change is still a regression:

-if [ "$SAGE64" = yes ]; then
-    echo "Building a 64-bit version of LinBox."
-    if [ -z "$CFLAG64" ]; then
-        CFLAG64=-m64
-    fi
-    CFLAGS="$CFLAGS $CFLAG64"
-    CXXFLAGS="$CXXFLAGS $CFLAG64"
-    CPPFLAGS="$CPPFLAGS $CFLAG64"
-    LDFLAGS="$LDFLAGS $CFLAG64"
+if [ "$SAGE64" = "yes" ]; then
+   echo "64 bit build"
+   CFLAGS="-m64 $CFLAGS "; export CFLAGS
+   CXXFLAGS="-m64 $CXXFLAGS "; export CXXFLAGS
+   CPPFLAGS="-m64 $CPPFLAGS "; export CPPFLAGS
+   LDFLAGS="-m64 $LDFLAGS"; export LDFLAGS
 fi
comment:67

Also this:

-if [ -z "$SAGE_LOCAL" ]; then
-    echo >&2 "Error: SAGE_LOCAL undefined - exiting..."
-    echo >&2 "Maybe run 'sage -sh'?"
-    exit 1
+if [ "$SAGE_LOCAL" = "" ]; then
+   echo "SAGE_LOCAL undefined ... exiting";
+   echo "Maybe run 'sage -sh'?"
+   exit 1
 fi

Attachment: linbox.diff.gz

FYI only

comment:68

I did notice the removal of the undocumented and nonstandard CFLAG64 environment variable and I consider that a definite plus of the new spkg-install.

The new spkg re-adds the stderr redirection in the SAGE_LOCAL check.

comment:69

It would be nice to add an hg tag for the new release.

comment:70

I've added the tag. Of course this'll never be done consistently as long as sage -pkg doesn't check that the spkg name matches the tag.

comment:71

Replying to @jpflori:

It would be nice to add an hg tag for the new release.

This is also done automatically by the merger script when the spkg is merged into Sage.

comment:72

Good to know.
Was it mentionned anywhere in the developper guide or did I miss it or did I not read it again recently enough?

Additionnal question, what about committing changes or putting them in a queue or not committing anything and letting some script do it?
Is there a prefered method to do it?

comment:73

Committing changes is always required.

Note: with the latest Mercurial, you can use

hg commit --amend

to add changes to the latest commit. So you can effectively emulate a queue with 1 patch like this.

comment:74

This is totally off-topic, but versioning systems are most useful if you commit often. Two logically independent changes should be two commits, even if they end up in the same ticket/spkg. Reviewer nitpicks should be another commit. Any other questions on how to use mercurial should go to sage-devel.

comment:75

Replying to @vbraun:

I did notice the removal of the undocumented and nonstandard CFLAG64 environment variable and I consider that a definite plus of the new spkg-install.

Looks documented to me: http://sagemath.org/doc/installation/source.html#environment-variables

(I do agree that we should get rid of CFLAG64 and SAGE64 in place of using CC and CFLAGS properly, but that's not the point of this ticket)

comment:76

Something else I noted: why was the exit code for a failed patch changed from 1 to -1? Normally, programs should use exit codes in the range 0-127.

comment:77

Oh silly me, I only searched the developer guide for CFLAG64! I also like how the installation guide is dancing around the fact that CFLAG64 is currently useless since it is not used in all spgks, so if you were to use a compiler that doesn't understand its default -m64 value breakage will ensue. So I'll concede that it is documented, but it still doesn't sound like a good idea to me.

I've fixed the offending exit code to be +1.

comment:78

Should the references to #9511 be changed to #13164 (in the dependencies and the ticket description?

Changed dependencies from #13118, #12840, #12841, #9511 to #13118, #12840, #12841, #13164

comment:80

Replying to @vbraun:

I also like how the installation guide is dancing around the fact that CFLAG64 is currently useless since it is not used in all spgks, so if you were to use a compiler that doesn't understand its default -m64 value breakage will ensue. So I'll concede that it is documented, but it still doesn't sound like a good idea to me.

This is all very true, but no good excuse for sloppy rebasing...

Description changed:

--- 
+++ 
@@ -2,7 +2,7 @@
 
 1. **Observe:** Dependencies
 2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.6.0.spkg
-3. **Install:** http://www.stp.dias.ie/~vbraun/Sage/spkg/linbox-1.3.2.spkg
+3. **Install:** [http://boxen.math.washington.edu/home/jdemeyer/spkg/linbox-1.3.2.spkg](http://boxen.math.washington.edu/home/jdemeyer/spkg/linbox-1.3.2.spkg)
 4. **Apply:** [attachment: matrix_modn_dense_no_linbox.patch](https://github.com/sagemath/sage-prod/files/10655417/matrix_modn_dense_no_linbox.patch.gz)
 5. **Apply:** [attachment: sage-linbox.2.patch](https://github.com/sagemath/sage-prod/files/10655420/sage-linbox.2.patch.gz)
 6. **Apply:** [attachment: trac_12883_linbox_deps.patch](https://github.com/sagemath/sage-prod/files/10655416/trac_12883_linbox_deps.patch.gz) and [attachment: trac_12883-spkg-install.patch](https://github.com/sagemath/sage-prod/files/10655421/trac_12883-spkg-install.patch.gz) to `SAGE_ROOT` repository
comment:82

Rebased from scratch, needs review.

Description changed:

--- 
+++ 
@@ -13,4 +13,4 @@
 1. https://bitbucket.org/malb/fflas-ffpack-spkg
 2. https://bitbucket.org/malb/linbox-spkg
 
-Note for release manager: This ticket must be merged simultaneously with #9511, they depend on each other.
+Note for release manager: This ticket must be merged simultaneously with #13164, they depend on each other.

Diff for linbox 1.1.6.p11->1.3.2. For reference / review only.

comment:84

Attachment: linbox-1.3.2.diff.gz

Merged: sage-5.4.beta1