Move module_list aliases to env.py

[jdemeyer]

Move the aliases from src/module_list.py to src/sage/env.py such that they can be used at runtime by cython() too. This is needed for #22461.

[git]

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

​7a04c1c

Import pkgconfig only when needed

[git]

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

​f06d3b3

Merge tag '8.1.beta5' into t/23799/ticket/23799

[vdelecroix]

Given that the procedure is the same each time

X_pc = pkgconfig.parse('X')
X_libs = X_pc['libraries']
X_library_dirs = X_pc['library_dirs']
X_cflags = pkgconfig.cflags('X').split()

could that be factored? (slight lie: no cflags involved for gsl, no include dirs for linbox, ... why?)

[jdemeyer]

You mean messing with globals()? I don't like that.

[vdelecroix]

Replying to jdemeyer:

You mean messing with globals()? I don't like that.

I meant

packages = ['fflas-ffpack', 'givaro', 'linbox', 'gsl', 'singular']
ans = {}
for pkg in packages:
    pc = pkgconfig.parse(pkg)
    name = pkg.replace('-', '').upper()
    ans['%s_LIBRARIES' %name] = pc['libraries']
    ans['%s_LIBDIR' %name] = pc['library_dirs']
    ans['%s_INCDIR' %name] = pc['include_dirs']
    ans['%s_CFLAGS' %name] = pkgconfig.cflags(pkg).split()
return ans

[git]

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

​07ff908

Generate aliases automatically

[jdemeyer]

Good suggestion, done.

[fbissey]

I like the ticket but I have one more question before setting it to positive review

@@ -881,14 +829,14 @@ ext_modules = [
     Extension('sage.matrix.matrix_modn_dense_float',
               sources = ['sage/matrix/matrix_modn_dense_float.pyx'],
               language="c++",
-              libraries = linbox_libs + cblas_libs,
+              libraries = cblas_libs,
               library_dirs = cblas_library_dirs,
               include_dirs = cblas_include_dirs),
 
     Extension('sage.matrix.matrix_modn_dense_double',
               sources = ['sage/matrix/matrix_modn_dense_double.pyx'],
               language="c++",
-              libraries = linbox_libs + cblas_libs,
+              libraries = cblas_libs,
               library_dirs = cblas_library_dirs,
               include_dirs = cblas_include_dirs,
               extra_compile_args = ["-D_XPG6"]),

It is correct that those don't link to linbox, but the linbox flags provide linking flags to fflas, ffpack and givaro that are needed. Are those brought by a dependency that can escape a cursory look? I could not find any distutils instructions in those particular files to account for the dependencies on fflas, ffpack and givaro.

[jdemeyer]

Replying to fbissey:

I like the ticket but I have one more question before setting it to positive review

@@ -881,14 +829,14 @@ ext_modules = [
     Extension('sage.matrix.matrix_modn_dense_float',
               sources = ['sage/matrix/matrix_modn_dense_float.pyx'],
               language="c++",
-              libraries = linbox_libs + cblas_libs,
+              libraries = cblas_libs,
               library_dirs = cblas_library_dirs,
               include_dirs = cblas_include_dirs),
 
     Extension('sage.matrix.matrix_modn_dense_double',
               sources = ['sage/matrix/matrix_modn_dense_double.pyx'],
               language="c++",
-              libraries = linbox_libs + cblas_libs,
+              libraries = cblas_libs,
               library_dirs = cblas_library_dirs,
               include_dirs = cblas_include_dirs,
               extra_compile_args = ["-D_XPG6"]),

As far as I know, those files do not actually depend on linbox, fflas-ffpack or givaro.

[fbissey]

In sage-on-gentoo which is compiled with --as-needed I get

fbissey@moonloop ~ $ readelf -d /usr/lib64/python2.7/site-packages/sage/matrix/matrix_modn_dense_float.so

Dynamic section at offset 0x5ea98 contains 32 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libfflas.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libffpack.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libgivaro.so.9]
 0x0000000000000001 (NEEDED)             Shared library: [libgmp.so.10]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libpython2.7.so.1.0]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

I wouldn't expect those to be present unless some symbols are directly needed. That warrant a closer look.

[jdemeyer]

Sorry, I was wrong indeed. I missed the modn part, I thought that this was about floating-point matrices.

[jdemeyer]

Replying to fbissey:

I like the ticket but I have one more question before setting it to positive review

@@ -881,14 +829,14 @@ ext_modules = [
     Extension('sage.matrix.matrix_modn_dense_float',
               sources = ['sage/matrix/matrix_modn_dense_float.pyx'],
               language="c++",
-              libraries = linbox_libs + cblas_libs,
+              libraries = cblas_libs,
               library_dirs = cblas_library_dirs,
               include_dirs = cblas_include_dirs),
 
     Extension('sage.matrix.matrix_modn_dense_double',
               sources = ['sage/matrix/matrix_modn_dense_double.pyx'],
               language="c++",
-              libraries = linbox_libs + cblas_libs,
+              libraries = cblas_libs,
               library_dirs = cblas_library_dirs,
               include_dirs = cblas_include_dirs,
               extra_compile_args = ["-D_XPG6"]),

It is correct that those don't link to linbox

They do actually link against linbox, but the linbox, fflas-ffpack and givaro flags are brought in by cimports of sage.libs.linbox.... at the beginning of src/sage/matrix/matrix_modn_dense_float.pyx and src/sage/matrix/matrix_modn_dense_double.pyx

[fbissey]

Yet the final product doesn't have any symbols from linbox, otherwise it would be in the list of libraries. I am guessing the content of sage.libs.linbox still reflects the time when fflas-ffpack wasn't split yet from linbox.

[jdemeyer]

Replying to fbissey:

Yet the final product doesn't have any symbols from linbox, otherwise it would be in the list of libraries.

Which list of libraries do you mean?

At least when compiling those extensions, -llinbox -llinboxsage is specified. This is true before and after this ticket.

[jdemeyer]

If your comment is that -llinbox is specified while it is not needed: that is outside the scope of this ticket. There are probably many such cases.

[fbissey]

Replying to jdemeyer:

If your comment is that -llinbox is specified while it is not needed: that is outside the scope of this ticket. There are probably many such cases.

That is the case. But now I am more worried about the lack of -lfflas -lffpack -lgivaro but given the bot is green, I am guessing it gets in the same way as -llinbox. Let's move things along.