gentoo: System gcc with multilib support generates linker (ld) warnings when running doctests, ntl-related

[strogdon]

With 9.3.rc0 typical results with multilib support in gcc:

./sage -t --random-seed=0 src/sage/misc/cython.py
**********************************************************************
File "src/sage/misc/cython.py", line 136, in sage.misc.cython.?
Failed example:
    cython(os.linesep.join(code))
Expected nothing
Got:
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libdl.so when searching for -ldl
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libdl.a when searching for -ldl
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libdl.so when searching for -ldl
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libdl.a when searching for -ldl
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libm.so when searching for -lm
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libm.a when searching for -lm
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libpthread.so when searching for -lpthread
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libpthread.a when searching for -lpthread
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libc.so when searching for -lc
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible ///usr/lib/libc.a when searching for -lc

There is no such problem with 9.3.beta7.

Upstream NTL pull request to add a .pc file

• https://github.com/libntl/ntl/pull/7

CC: @kiwifb @mkoeppe @orlitzky @dimpase @isuruf

Component: doctest framework

Author: Steven Trogdon

Branch/Commit: a2ab555

Reviewer: Matthias Koeppe

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

[strogdon]

comment:1

Using git bisect to locate the commit that is causing the issue I have from git bisect log:

git bisect start
# bad: [2c25f07cfd0cbb5cb4a9b57b7bc62ec997039987] Updated SageMath version to 9.3.rc0
git bisect bad 2c25f07cfd0cbb5cb4a9b57b7bc62ec997039987
# good: [8453ffb849b047893b6c61dd09176a84c9133342] Updated SageMath version to 9.3.beta7
git bisect good 8453ffb849b047893b6c61dd09176a84c9133342
# bad: [d32f41ee20a421ca91f36abbf889b4b4792f4822] Trac #20784: not all symbolic equations stay unevaluated
git bisect bad d32f41ee20a421ca91f36abbf889b4b4792f4822
# good: [8ff8b0b90c78d09fb2e71ee35205e96371e60f05] Trac #30537: Upgrade giac to 1.6.0-47
git bisect good 8ff8b0b90c78d09fb2e71ee35205e96371e60f05
# good: [2bcac009dd878272858ece0d853a67571fcd1c0d] Trac #31317: eclib interface uses bad default value for elliptic curve modular symbols
git bisect good 2bcac009dd878272858ece0d853a67571fcd1c0d
# bad: [64c1336fc356f17406c579e2471e0fa92f0e9123] Trac #31373: Upgrade ipython to 7.20.0 and jedi to 0.18.0
git bisect bad 64c1336fc356f17406c579e2471e0fa92f0e9123
# good: [74cfd6dbc612f8aad20cbfb67d672a4157f60b89] Trac #31358: python3 spkg-configure.m4: Do not reject python based on sysconfig LDFLAGS containing "-L."
git bisect good 74cfd6dbc612f8aad20cbfb67d672a4157f60b89
# good: [70cbb47cb79de70c21d00ed2aad8eb90f035f8b5] Trac #31364: Don't use deprecated numpy type aliases
git bisect good 70cbb47cb79de70c21d00ed2aad8eb90f035f8b5

The next commit that is to be tested is

commit 7b1e27b96e143d6d44b448692ba266050e667399 (HEAD)
Author: Matthias Koeppe <mkoeppe@math.ucdavis.edu>
Date:   Wed Feb 10 19:36:48 2021 -0800

    Merge distutils directives for Cython files using ntl

which fails to build with

Traceback (most recent call last):
  File "/local/sage-git/sage/build/pkgs/sagelib/src/setup.py", line 21, in <module>
    import sage.env
  File "/local/sage-git/sage/build/pkgs/sagelib/src/sage/env.py", line 160, in <module>
    HOSTNAME = var("HOSTNAME", socket.gethostname())
  File "/local/sage-git/sage/build/pkgs/sagelib/src/sage/env.py", line 144, in var
    import sage_conf
  File "/local/sage-git/sage/local/lib/python3.9/site-packages/sage_conf.py", line 9
    NTL_INCDIR = ".
                   ^
SyntaxError: EOL while scanning string literal

Somehow sage_conf.py has gotten corrupted?

# build/pkgs/sage_conf/src/sage_conf.py.  Generated from sage_conf.py.in by configure.

VERSION = "9.3.beta7"

MAXIMA = "/local/sage-git/sage/local/bin/maxima"

ARB_LIBRARY = "arb"

NTL_INCDIR = ".
.
///usr/include/NTL"
NTL_LIBDIR = ".
.
///usr/include"

[strogdon]

comment:2

@mkoeppe perhaps you have some insight into this. It's not clear to me why sage_conf.py is corrupted which prevents further bisecting. I have implemented the bisecting twice with the same result.

[mkoeppe]

comment:3

You should be able to see these corrupted values already in config.status. They are determined by build/pkgs/ntl/spkg-configure.m4 using a (horrible) macro called AX_ABSOLUTE_HEADER.

[strogdon]

comment:4

From config.status there is

S["SAGE_FLINT_PREFIX"]=""
S["NTL_LIBDIR"]=".\n"\
".\n"\
"///usr/include"
S["NTL_INCDIR"]=".\n"\
".\n"\
"///usr/include/NTL"
S["SAGE_NTL_PREFIX"]=""

[mkoeppe]

comment:5

Yes. So, as I thought, the error is made in the configure script.

You can try to debug this using CONFIG_SHELL="bash -x" ./configure.

[mkoeppe]

comment:6

What platform is this, by the way?

[strogdon]

comment:7

Replying to @mkoeppe:

What platform is this, by the way?

Gentoo

Linux hp-probook 5.4.97-gentoo-x86_64 #16 SMP Mon Mar 15 21:41:09 MDT 2021 x86_64 AMD Ryzen 7 4700U with Radeon Graphics AuthenticAMD GNU/Linux

Same issue on

Linux hp-envy 5.4.38-gentoo #7 SMP Sat Jan 16 21:46:51 MST 2021 x86_64 Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz GenuineIntel GNU/Linux

[mkoeppe]

comment:8

The NTL variables in sage_conf were introduced in 6e30e3ac2d891a16fdd1ad89506db767d4edb9e5 in #31365.

Help with fixing this issue is welcome. Note that 9.3.rc0 built fine on gentoo in https://github.com/sagemath/sage/runs/2180104858?check_suite_focus=true

[strogdon]

comment:9

Replying to @mkoeppe:

The NTL variables in sage_conf were introduced in 6e30e3ac2d891a16fdd1ad89506db767d4edb9e5 in #31365.

Help with fixing this issue is welcome. Note that 9.3.rc0 built fine on gentoo in https://github.com/sagemath/sage/runs/2180104858?check_suite_focus=true

For 9.3.rc0, sage_conf.py contains

# build/pkgs/sage_conf/src/sage_conf.py.  Generated from sage_conf.py.in by configure.

VERSION = "9.3.rc0"

MAXIMA = "/local/sage-git/sage/local/bin/maxima"

ARB_LIBRARY = "arb"

NTL_INCDIR = "///usr/include"
NTL_LIBDIR = "///usr/lib" 

So the question is whether the /// are causing the linker warnings when doctesting?

[mkoeppe]

comment:10

You can edit this file and run make sage_conf to have it installed

[mkoeppe]

comment:11

Replying to @strogdon:

I have implemented the bisecting twice with the same result.

You may want to use git bisect --first-parent, so that you don't trip over intermediate commits within a ticket's branch.

[strogdon]

comment:12

FWIW, Sage-on-Gentoo does this to avoid linker warnings. But this change does not resolve things on vanilla Sage.

[strogdon]

comment:13

Replying to @mkoeppe:

You can edit this file and run make sage_conf to have it installed

Correct me if I misunderstand. For 9.3.rc0 I changed

NTL_INCDIR = "///usr/include"
NTL_LIBDIR = "///usr/lib" 

to

NTL_INCDIR = "/usr/include"
NTL_LIBDIR = "/usr/lib"

did make sage_conf and ./sage -t --random-seed=0 src/sage/misc/cython.py still displays linker warnings. If correct I could make this change when bisecting to see if bisecting can proceed.

[strogdon]

comment:14

Ahh, maybe the issue is

NTL_LIBDIR = "/usr/lib"

Shouldn't this be

NTL_LIBDIR = "/usr/lib64"

[strogdon]

comment:15

Replying to @strogdon:

Ahh, maybe the issue is

NTL_LIBDIR = "/usr/lib"

Shouldn't this be

NTL_LIBDIR = "/usr/lib64"

Yes, this is the issue.

[mkoeppe]

comment:16

OK, so ntl/spkg-configure.m4 needs changing so that it detects the correct library dir.

[kiwifb]

comment:18

Yes, a number of packages don't do proper detection and try to do location instead. Which can cause troubles on multilib install. I'll have a look at the macro when I can.

[strogdon]

comment:19

The title is not really descriptive of the issue. It was a poor attempt to describe what what was happening. It should probably be changed.

[kiwifb]

comment:20

From the end ntl/spkg-configure.m4

    if test x$sage_spkg_install_ntl = xyes; then
        AC_SUBST(SAGE_NTL_PREFIX, ['$SAGE_LOCAL'])
    else
        AC_SUBST(SAGE_NTL_PREFIX, [''])
        AX_ABSOLUTE_HEADER([NTL/ZZ.h])
        ntl_inc_ntl_dir=`AS_DIRNAME(["$gl_cv_absolute_NTL_ZZ_h"])`
        ntl_inc_dir=`AS_DIRNAME(["$ntl_inc_ntl_dir"])`
        ntl_prefix=`AS_DIRNAME(["$ntl_inc_dir"])`
        AC_SUBST(NTL_INCDIR, [$ntl_inc_dir])
        AC_SUBST(NTL_LIBDIR, [$ntl_prefix/lib])
    fi

First of, the assumption on the value of NTL_LIBDIR is completely unwarranted. What is more extra-ordinary to me is that if the previous detection test before you get to this branch are successful nothing after AC_SUBST(SAGE_NTL_PREFIX, ['']) is useful.

Were any of these value provided for detection? No. So, why would they be needed later on?

If someone wants to provide some values for NTL_INCDIR and NTL_LIBDIR that can be arranged but then they should be tested - separately.

As far as I am concerned, those two variables should just be eliminated which would quite the patch during a rc.

[kiwifb]

comment:21

Also the macro AX_ABSOLUTE_HEADER is generating those strange /// at the beginning on purpose. So that some compiler (Sun originally) wouldn't complain with the result if it happens to be a system location. Not making it up, it is in its own documentation https://www.gnu.org/software/autoconf-archive/ax_absolute_header.html. I was really looking for a AX_ABSOLUTE_LIBRARY equivalent (there isn't).

[mkoeppe]

comment:22

Replying to @kiwifb:

From the end ntl/spkg-configure.m4

    if test x$sage_spkg_install_ntl = xyes; then
        AC_SUBST(SAGE_NTL_PREFIX, ['$SAGE_LOCAL'])
    else
        AC_SUBST(SAGE_NTL_PREFIX, [''])
        AX_ABSOLUTE_HEADER([NTL/ZZ.h])
        ntl_inc_ntl_dir=`AS_DIRNAME(["$gl_cv_absolute_NTL_ZZ_h"])`
        ntl_inc_dir=`AS_DIRNAME(["$ntl_inc_ntl_dir"])`
        ntl_prefix=`AS_DIRNAME(["$ntl_inc_dir"])`
        AC_SUBST(NTL_INCDIR, [$ntl_inc_dir])
        AC_SUBST(NTL_LIBDIR, [$ntl_prefix/lib])
    fi

First of, the assumption on the value of NTL_LIBDIR is completely unwarranted.

I agree, this was sloppy.

What is more extra-ordinary to me is that if the previous detection test before you get to this branch are successful nothing after AC_SUBST(SAGE_NTL_PREFIX, ['']) is useful.

Were any of these value provided for detection? No. So, why would they be needed later on?

They are needed because of the unfortunate need for a compile time environment at runtime of Sage when using the cython function. In this situation we cannot rely on the proper compile time environment being set (such as on macOS using the .homebrew-build-env.)

It would be fine with me to back out the setting of these two variables for Sage 9.3 if you think this helps.

[kiwifb]

comment:23

Replying to @mkoeppe:

They are needed because of the unfortunate need for a compile time environment at runtime of Sage when using the cython function. In this situation we cannot rely on the proper compile time environment being set (such as on macOS using the .homebrew-build-env.)

That's unfortunately a valid use case. We may have had that discussion already. Total removal would break that use case while we are currently only dealing with extra warnings. Noisy, but not breaking any functionality.

[kiwifb]

comment:24

I think we should try AC_LIB_LINKFLAGS and see if that return something that we can use sensibly https://www.gnu.org/software/gnulib/manual/html_node/Searching-for-Libraries.html. This is gnulib stuff however.

[mkoeppe]

comment:25

For the general problem I have a ticket (#31041 - Set environment for sage.misc.cython) but not a solution yet

[kiwifb]

comment:26

Replying to @mkoeppe:

For the general problem I have a ticket (#31041 - Set environment for sage.misc.cython) but not a solution yet

Yes, we should work on that there, that may turn out to be a bit of a patch bomb. At best I say we deal with accepting the potential warnings in the present ticket.

[mkoeppe]

comment:27

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review.

[dimpase]

comment:29

Replying to @kiwifb:

Also the macro AX_ABSOLUTE_HEADER is generating those strange /// at the beginning on purpose. So that some compiler (Sun originally) wouldn't complain with the result if it happens to be a system location. Not making it up, it is in its own documentation https://www.gnu.org/software/autoconf-archive/ax_absolute_header.html. I was really looking for a AX_ABSOLUTE_LIBRARY equivalent (there isn't).

well, on #28906 I wrote something akin to AX_ABSOLUTE_LIBRARY, see https://github.com/sagemath/sagetrac-mirror/blob/develop/m4/sage_absolute_lib.m4?id=2d4a1cdac919203e68f0d56174cd6257f600fe00

[dimpase]

comment:30

Replying to @mkoeppe:

Replying to @kiwifb:

From the end ntl/spkg-configure.m4

    if test x$sage_spkg_install_ntl = xyes; then
        AC_SUBST(SAGE_NTL_PREFIX, ['$SAGE_LOCAL'])
    else
        AC_SUBST(SAGE_NTL_PREFIX, [''])
        AX_ABSOLUTE_HEADER([NTL/ZZ.h])
        ntl_inc_ntl_dir=`AS_DIRNAME(["$gl_cv_absolute_NTL_ZZ_h"])`
        ntl_inc_dir=`AS_DIRNAME(["$ntl_inc_ntl_dir"])`
        ntl_prefix=`AS_DIRNAME(["$ntl_inc_dir"])`
        AC_SUBST(NTL_INCDIR, [$ntl_inc_dir])
        AC_SUBST(NTL_LIBDIR, [$ntl_prefix/lib])
    fi

First of, the assumption on the value of NTL_LIBDIR is completely unwarranted.

I agree, this was sloppy.

short of using a kind of macro mentioned in comment:29, how about checking for presence of $ntl_prefix/lib/libntl.* - and if there is none, try $ntl_prefix/lib64/libntl.*

Should we error out if none of these works?

[kiwifb]

comment:31

Replying to @dimpase:

how about checking for presence of $ntl_prefix/lib/libntl.* - and if there is none, try $ntl_prefix/lib64/libntl.*

What if they are both present? Does that cover Debian's way of doing multilib? Your macro is interesting.

[dimpase]

comment:32

even though such a macro might not work on weird platforms, it would be a solution for Linux multilib. I don't recall if I tested on MacOS though. (I guess Arm vs non-arm cases might be interesting)

[mkoeppe]

comment:33

Replying to @dimpase:

well, on #28906 I wrote something akin to AX_ABSOLUTE_LIBRARY, see https://github.com/sagemath/sagetrac-mirror/blob/develop/m4/sage_absolute_lib.m4?id=2d4a1cdac919203e68f0d56174cd6257f600fe00

I don't think this is the right approach. Mixing in runtime library paths can only create more confusion.

[mkoeppe]

comment:34

Wouldn't gentoo be able to provide a .pc file for NTL that we can check for first?

[dimpase]

comment:35

Replying to @mkoeppe:

Wouldn't gentoo be able to provide a .pc file for NTL that we can check for first?

it doesn't; even if it did, it won't help, as normally speaking -L is only specified for a library in its *.pc file only if the location is not known to the linker. Here, the location is known.

Amazingly, there is no standard way to find out this path...

[dimpase]

comment:36

Replying to @mkoeppe:

Replying to @dimpase:

well, on #28906 I wrote something akin to AX_ABSOLUTE_LIBRARY, see https://github.com/sagemath/sagetrac-mirror/blob/develop/m4/sage_absolute_lib.m4?id=2d4a1cdac919203e68f0d56174cd6257f600fe00

I don't think this is the right approach. Mixing in runtime library paths can only create more confusion.

it already does create confusion.

Should NTL_LIBDIR be just empty list if the location of libntl is one of standard locations?

[mkoeppe]

comment:37

Most pkgconfig files do provide variables such as libdir.

[mkoeppe]

comment:38

Besides, we do not really need this absolute path. We only use these variables in cython_aliases because there is no .pc file to use.

[orlitzky]

comment:39

What exactly is the problem with cython? Is it just that people start sage without setting up their build environment (e.g. source .homebrew-build-env), and then expect to be able to compile things via cython()?

[mkoeppe]

comment:40

So, how about an upstream pull request that adds a pkgconfig file to NTL.

[mkoeppe]

comment:41

Replying to @orlitzky:

What exactly is the problem with cython? Is it just that people start sage without setting up their build environment (e.g. source .homebrew-build-env), and then expect to be able to compile things via cython()?

Yes, precisely. (I know...)

[kiwifb]

comment:42

Replying to @mkoeppe:

So, how about an upstream pull request that adds a pkgconfig file to NTL.

And we'll have to do it again and again with any packages we need? Not that it is a bad thing in and of itself.

[mkoeppe]

comment:43

Yes, one package at a time.

[orlitzky]

comment:44

Oh, well, that doesn't work =)

Seriously though, this problem is going to manifest elsewhere as well. Trying to guess at the exact environment that was present during ./configure so that it can be replicated during cython() will be a losing battle.

Adding a .pc file upstream might help or it might make things worse. There are two separate libdirs, and I'm not sure off the top of my head how pkg-config decides which *.pc file to use (and thus which libdir to return) on a multilib system.

Even if it works, you're basically forbidding people from installing things manually and passing -L and -I during the build, instead forcing everyone to both provide *.pc files and then override PKG_CONFIG_DIR. It seems like a lot of antipattern to bury ourselves under to avoid telling people to set up their build environment before they want to build.

[mkoeppe]

comment:45

The problem really just applies to the libraries listed in sage.misc.cython._standard_libs_libdirs_incdirs_aliases: the small set of libraries that were anointed as "standard" sometime in the ancient past.

[mkoeppe]

comment:46

https://github.com/libntl/ntl is taking pull requests.

[dimpase]

comment:47

I already *.pc-fied mpfr, m4ri, eclib, perhaps something else as well, anyone wants to take the token?

[dimpase]

comment:48

Replying to @dimpase:

I already *.pc-fied mpfr, m4ri, eclib, perhaps something else as well, anyone wants to take the token?

also, I don't speak Perl, and for NTL config this is needed: https://github.com/libntl/ntl/blob/main/src/DoConfig

[kiwifb]

comment:49

Replying to @dimpase:

Replying to @dimpase:

I already *.pc-fied mpfr, m4ri, eclib, perhaps something else as well, anyone wants to take the token?

also, I don't speak Perl, and for NTL config this is needed: https://github.com/libntl/ntl/blob/main/src/DoConfig

Darn! I do, I am a bit rusty but the script is not really complicated enough to be a big issue.

[kiwifb]

comment:50

OK, I have started work at https://github.com/kiwifb/ntl/tree/pcfile so you can send your criticism already. There is one more little piece I think should dealt with before sending the PR is setting the variable PACKAGE_VERSION. Unfortunately, that bit will be a bit more complicated. It is contained in the file DIRNAME but preceded by the string ntl-. Some extra reading and parsing needs to be added to DoConfig for it.

[kiwifb]

comment:51

OK people can now experiment with the attached patch to see if it behaves as they expect.

[strogdon]

comment:52

Replying to @kiwifb:

OK people can now experiment with the attached patch to see if it behaves as they expect.

Perhaps cosmetic for purposes here but in the makefile there is INCLUDEDIR=$(PREFIX)/include which on a gentoo install gives

$ pkg-config --cflags ntl
-I$(PREFIX)/include/NTL

i.e. from the installed ntl.pc

includedir=$(PREFIX)/include/NTL

Somehow (PREFIX) should be {PREFIX}.