spkg-configure for GSL

[git]

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

​e5c91db	update pkgconfig to v1.5.1. See trac #28883
​1720b38	a bare minumim openblas config
​925266b	use openblas.pc for blas and lapack, don't require atlas is to be used
​b89df80	move blas handling to build/pkgs/
​6341576	create cblas.pc link, too, not only blas and lapack
​3a95431	spkg-configure for gsl

[isuruf]

Did you check this with an underlinked GSL (A blas library is not linked in) or a fully linked one like Sage's GSL?

[git]

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

​137c972

typo in variable name

[dimpase]

Replying to isuruf:

Did you check this with an underlinked GSL (A blas library is not linked in) or a fully linked one like Sage's GSL?

Only on debian and gentoo, where it's linked to its own gslcblas

[fbissey]

Replying to dimpase:

Replying to isuruf:

Did you check this with an underlinked GSL (A blas library is not linked in) or a fully linked one like Sage's GSL?

Only on debian and gentoo, where it's linked to its own gslcblas

Not on my gentoo box, but that would be the default. In any case neither debian or gentoo will ship it underlinked. I don't think any major distro ship underlinked libraries. OK as much as they can help it. Stuff that people build themselves is another story

[isuruf]

What happens when gslcblas and openblas are both linked in?

[fbissey]

A bit pathological, but basically nothing visible until you test something with a high precision where there can be divergences between the two. If you run GSL test suite with openblas rather than gslcblas you'll get failures of the numerical noise kind.

Honestly, I am not sure which one would be used. My guess would be the first one on the list of libraries linked.

[isuruf]

Okay. Another concern is when system's gsl.pc is used by downstream packages, they'll be linking to gslcblas instead of openblas, but sage's gsl.pc would link the downstream package to openblas.

[fbissey]

fbissey@moonloop ~ $  cat /usr/lib64/pkgconfig/gsl.pc 
prefix=/usr
exec_prefix=/usr
libdir=/usr/lib64
includedir=/usr/include
GSL_CBLAS_LIB=-lcblas

Name: GSL
Description: GNU Scientific Library
Version: 2.5
Libs: -L/usr/lib64 -lgsl ${GSL_CBLAS_LIB} -lm -lm 
Cflags: -I/usr/include

distro will usually patch gsl.pc to match the linking they are doing. Supposing that libgsl is linked with cblas1 and sage tries to link extensions gslext.pyx using libgsl to cblas2. If there are direct call to cblas in glsext.pyx, cblas2 will be used for these, otherwise -lcblas2 will be ignored. If there is no direct calls to cblas in gslext.pyx, libgsl will use cblas1, since cblas2 will be discarded. If cblas2 is not discarded because there are calls to cblas in gslext.pyx what you get in libgsl is not clearly defined and may depend on what is loaded first.

[isuruf]

On Ubuntu,

(base) isuru@isuru:~$ cat /usr/lib/x86_64-linux-gnu/pkgconfig/gsl.pc 
prefix=/usr
exec_prefix=/usr
libdir=/usr/lib/x86_64-linux-gnu
includedir=/usr/include
GSL_CBLAS_LIB=-lgslcblas

Name: GSL
Description: GNU Scientific Library
Version: 2.4
Libs: -L/usr/lib/x86_64-linux-gnu -lgsl ${GSL_CBLAS_LIB} -lm -lm 
Cflags: -I/usr/include
(base) isuru@isuru:~$ ls /usr/lib/x86_64-linux-gnu/libgslcblas.so
/usr/lib/x86_64-linux-gnu/libgslcblas.so
(base) isuru@isuru:~$ ls -al /usr/lib/x86_64-linux-gnu/libgslcblas.so
lrwxrwxrwx 1 root root 20 Aug  9  2017 /usr/lib/x86_64-linux-gnu/libgslcblas.so -> libgslcblas.so.0.0.0
(base) isuru@isuru:~$ ls -al /usr/lib/x86_64-linux-gnu/libgslcblas.so.0.0.0 
-rw-r--r-- 1 root root 255968 Aug  9  2017 /usr/lib/x86_64-linux-gnu/libgslcblas.so.0.0.0

[isuruf]

It is underlinked as far as I can tell.

(base) isuru@isuru:~$ ldd /usr/lib/x86_64-linux-gnu/libgsl.so.23
        linux-vdso.so.1 (0x00007fffb1ba6000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f8a703f4000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f8a70003000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f8a70bf4000)

[fbissey]

OK, I am surprised. Then cblas as to be provided with anything linking libgsl. And if we are taking it from gsl.pc that may be gslcblas. At worst we take performance hit.

An option would be to substitute gslcblas by the the default cblas we are using in the linking flags.

[isuruf]

In the openblas spkg-configure script, openblas.pc is copied into $SAGE_LOCAL/lib/pkgconfig/cblas.pc. How about copying the system gsl.pc into $SAGE_LOCAL/lib/pkgconfig/gsl.pc and replacing the gslcblas line with openblas? (or whatever BLAS is selected)

[fbissey]

Remove GSL_CBLAS_LIB and add a line

requires: cblas

which will pull cblas.pc all by itself.

[dimpase]

Just to make it clear, are you proposing to change gsl.pc in the underlinked case only, right?

Will the underlinked GSL work with "cblas", which is in the configuration of #27870 the same as openblas? (I am ignorant about the purpose of gslcblas, sorry).

[fbissey]

Yes it will work. It would work if we did it in all cases. gslcblas is just another complete cblas implementation just as atlas and openblas are.

[dimpase]

Replying to fbissey:

OK, I am surprised. Then cblas as to be provided with anything linking libgsl. And if we are taking it from gsl.pc that may be gslcblas. At worst we take performance hit.

An option would be to substitute gslcblas by the the default cblas we are using in the linking flags.

GSL is also underlinked on Fedora

[dimpase]

Replying to isuruf:

What happens when gslcblas and openblas are both linked in?

this is what I get with this branch:

$ ldd local/lib/python3.7/site-packages/cvxopt/gsl.cpython-37m-x86_64-linux-gnu.so
...
	libgsl.so.23 => /usr/lib/x86_64-linux-gnu/libgsl.so.23 (0x00007d76b701a000)
	libopenblas.so.0 => /usr/lib/x86_64-linux-gnu/libopenblas.so.0 (0x00007d76b4e36000)
...
	libgslcblas.so.0 => /usr/lib/x86_64-linux-gnu/libgslcblas.so.0 (0x00007d76b48db000)
...
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007d76b462e000)
...

so both libopenblas.so and libgslblas.so are there. (I suppose it's the same with Sage-supplied GSL and OpenBLAS)

Are you saying that modifyng gsl.pc as suggested here would lead to skipping libgslblas.so ?

IMHO this would not lead to re-linking libgsl.so with libopenblas.so, if it was already linked with libgslblas.so. Woulndn't is need some LD_PRELOAD platform-dependent ld hacks --- I guess MacOS would need something quite different to achieve this.

[isuruf]

(I suppose it's the same with Sage-supplied GSL and OpenBLAS)

No, see ​https://github.com/sagemath/sage/blob/f0ae571d17e685258c1de89c2a29953f4d8fb302/build/pkgs/gsl/patches/gsl-2.1-gslcblas.patch . gslcblas is removed sage-suplied GSL.

Are you saying that modifyng gsl.pc as suggested here would lead to skipping libgslblas.so ?

Yes

IMHO this would not lead to re-linking libgsl.so with libopenblas.so, if it was already linked with libgslblas.so

libgsl.so is not linked to any blas in distros like Ubuntu. It is underlinked. In sage-the-distribution, libgsl.so is linked to libopenblas and the -lgslcblas line is removed from gsl.pc.

[dimpase]

No, why, on Debian 10 I see

$ ldd /usr/lib/x86_64-linux-gnu/libgsl.so
	linux-vdso.so.1 (0x00007ffcf2bb5000)
	libgslcblas.so.0 => /usr/lib/x86_64-linux-gnu/libgslcblas.so.0 (0x00007a0da11b5000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007a0da1032000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007a0da0e71000)
	/lib64/ld-linux-x86-64.so.2 (0x00007a0da1499000)

So libgsl.so is linked to libopenblas.so

[isuruf]

I guess it depends on the distro. I checked Ubuntu 18.04, and it's underlinked there. See ​https://trac.sagemath.org/ticket/28879#comment:13

[dimpase]

Replying to isuruf:

I guess it depends on the distro. I checked Ubuntu 18.04, and it's underlinked there. See ​https://trac.sagemath.org/ticket/28879#comment:13

Yes, it's distro-dependent. So how is this gsl.pc hack supposed to help in such a case (of already linked gslcblas? Are we left with linking to both openblas and gslcblas then?

[isuruf]

In the case that it is already linked to libgslcblas, then the library or the executable using gsl has the option of selecting the blas.

See ​https://wiki.debian.org/DebianScience/LinearAlgebraLibraries section "Using the GSL with an optimized BLAS"

[dimpase]

Replying to isuruf:

In the case that it is already linked to libgslcblas, then the library or the executable using gsl has the option of selecting the blas.

See ​https://wiki.debian.org/DebianScience/LinearAlgebraLibraries section "Using the GSL with an optimized BLAS"

well, this does not quite wotk. After these changes to gsl.pc I see the correct linking command

    gcc -pthread -shared -L/home/dimpase/sage/local/lib -Wl,-rpath,/home/dimpase/sage/local/lib -L. -L/home/dimpase/sage/local/lib -Wl,-rpath,/home/dimpase/sage/local/lib -L/home/dimpase/sage/local/lib -Wl,-rpath,/home/dimpase/sage/local/lib build/temp.linux-x86_64-3.7/src/C/gsl.o -L/home/dimpase/sage/local -L/home/dimpase/sage/local -L/home/dimpase/sage/local/lib -lm -lgsl -lopenblas -lpython3.7m -o build/lib.linux-x86_64-3.7/cvxopt/gsl.cpython-37m-x86_64-linux-gnu.so

but stil the same ldd output as in comment:21.

[isuruf]

ldd gives transitive deps. Try readelf -d local/lib/python3.7/site-packages/cvxopt/gsl.cpython-37m-x86_64-linux-gnu.so

[dimpase]

OK, so readelf -d indeed shows that the only linking to gslcblas comes from one of the linked libs

0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgsl.so.23]
 0x0000000000000001 (NEEDED)             Shared library: [libopenblas.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libpython3.7m.so.1.0]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

and it must be libgsl, for obvious reasons. So that's as good as it gets, we won't try to relink libgsl, right?

[isuruf]

So that's as good as it gets, we won't try to relink libgsl, right?

Yes. Looks good. Can you push your changes to modify gsl.pc?

[git]

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

​2bc86cd

install and modify gsl.pc to use cblas

[dimpase]

This is not ideal, as one should use $SED, not sed. However, calling sed happens in config.status, and I have no idea how to make $SED known there. Probably some magic with "init commands" I don't know.

[isuruf]

sed -i without an extension is GNU sed specific and doesn't work with BSD sed

[git]

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

​c13be38

add extension '' to -i in sed calls

[fbissey]

Replying to isuruf:

sed -i without an extension is GNU sed specific and doesn't work with BSD sed

It is not posix full stop. I am surprised it works as suggested on freeBSD. My experience with AIX sed, which is posix, is that -i isn't supported at all. Fortunately we don't have to support such dinosaur platforms.

@Dima one of the purpose of -e is to be able to chain commands.

sed -i'' -e 's/\${GSL_CBLAS_LIB}\ //'  -e 's/GSL_CBLAS_LIB.*/Require: cblas/' "$SAGE_LOCAL"/lib/pkgconfig/gsl.pc

is perfectly legal. Of course some line separation gain in clarity, but that's why I would break things into several line in a pure shell script

sed  \
       -e 's/\${GSL_CBLAS_LIB}\ //'  \
       -e 's/GSL_CBLAS_LIB.*/Require: cblas/' \
       -i'' $SAGE_LOCAL"/lib/pkgconfig/gsl.pc

[git]

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

​310ba06

get SED into config.status

[dimpase]

OK, all done, also managed to get SED value into config.status.

[dimpase]

if you prefer chained -e in one sed call, let me know. I don't see this comment while I fought value of SED thing :-)

[fbissey]

Replying to dimpase:

if you prefer chained -e in one sed call, let me know. I don't see this comment while I fought value of SED thing :-)

That's just nitpicking and absolutely non-esential. My comment should be taken as informative rather than "take this action, please".

[isuruf]

Replying to fbissey:

Replying to isuruf:

sed -i without an extension is GNU sed specific and doesn't work with BSD sed

It is not posix full stop. I am surprised it works as suggested on freeBSD. My experience with AIX sed, which is posix, is that -i isn't supported at all. Fortunately we don't have to support such dinosaur platforms.

I'm pretty sure this doesn't work on OSX. See ​https://stackoverflow.com/a/5694430/4768820

[dimpase]

So it should be -i '', not -i''? I don't have access to an OSX machine in the coming 2 weeks, sorry.

[isuruf]

-i '' works in BSD/OSX sed, but doesn't in GNU sed. -i'' works in GNU sed, but doesn't in BSD/OSX sed.

Solution is to use -i.bak as in the StackOverflow? answer. This creates a backup file with suffix .bak

[git]

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

​f9d6966

get rid of -i option of sed

[dimpase]

OK, done. Now it's POSIX, I think.

[dimpase]

testing this on Ubuntu (i.e. undelinked libgsl.so) gives a linking error during python import:

$ ./sage --python
Python 3.7.3 (default, Dec 21 2019, 11:29:09) 
[GCC 7.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from sage.rings.complex_double import CDF
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: /usr/lib/i386-linux-gnu/libgsl.so.23: undefined symbol: cblas_ctrmv
>>> 

I suppose some sagelib modules don't know that they must also link with openblas.

[git]

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

​9d9e046

fix a crucial typo in edited gsl.pc

[dimpase]

the .pc syntax needs Requires rather than Require...

[dimpase]

now things work on Ubuntu, too.

[isuruf]

Tested that configure creates gsl.pc correctly in OSX.

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. Last 10 new commits:

​c031c9e	use openblas.pc for blas and lapack, don't require atlas is to be used
​b49f79e	move blas handling to build/pkgs/
​81bf522	create cblas.pc link, too, not only blas and lapack
​244f954	spkg-configure for gsl
​efd642f	typo in variable name
​641e3fa	install and modify gsl.pc to use cblas
​51bc622	add extension '' to -i in sed calls
​13a5434	get SED into config.status
​ac32b13	get rid of -i option of sed
​a180599	fix a crucial typo in edited gsl.pc

[dimpase]

trivial rebase over updated #27870