sagemath/sage

spkg-configure.m4 for iml

dimpase opened this issue · 52 comments

iml only works with MP_LIBRARY provided in SAGE_LOCAL, at least this is the case on Debian 9.

they have a very old autoconf macro to detect GMP, which it particular won't work for libgmp in some arch-specfic location such as /usr/lib64/.

This breaks #27212. It works with GMP provided by the system if I just remove the call to their macro, re-run their bootstrap, repackage tarball.

--- a/configure.ac
+++ b/configure.ac
@@ -68,13 +68,6 @@ AC_CHECK_FUNCS(calloc floor)
 
 IML_VERBOSE_MODE
 IML_DEFAULT_PATH
-IML_CHECK_GMP(,,[AC_MSG_ERROR(
-GMP not found! 
-GMP version 3.1.1 or greater is required for this library to compile. 
-Please make sure GMP is installed and specify the header and libraries 
-location with the options --with-gmp-include=<path> and --with-gmp-lib=<path>
-respectively when running configure.
-)])
 
 IML_CHECK_CBLAS(,,[AC_MSG_WARN(
 CBLAS not found!

While we are at it, it makes sense to update iml to the latest version 1.0.5 (we are at 1.0.4).

It should also get spkg-configure.m4

tarball: http://www.cs.uwaterloo.ca/~astorjoh/iml-1.0.5.tar.bz2

CC: @embray @orlitzky @mkoeppe @isuruf

Component: build: configure

Keywords: freebsd

Author: Dima Pasechnik

Branch/Commit: 5a34c90

Reviewer: Isuru Fernando

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

Description changed:

--- 
+++ 
@@ -25,3 +25,5 @@
 
 While we are at it, it makes sense to update iml to the latest version 1.0.5 (we are at 1.0.4).
 
+It should also get spkg-configure.m4
+

Author: Dima Pasechnik

comment:3

The easiest fix would be to skip the iml's testing for MP_LIBRARY if we use the one from the system.

But how does one test for this? Is there a variable (provided by #27212) in the environment that can be tested?

Description changed:

--- 
+++ 
@@ -27,3 +27,5 @@
 
 It should also get spkg-configure.m4
 
+tarball: http://www.cs.uwaterloo.ca/~astorjoh/iml-1.0.5.tar.bz2
+

Commit: 12cd746

comment:5

the patch I generate to conditionally disable checks for GMP is kind of big, as it's the result of autoreconf running...


New commits:

356621acorrectly setup sagelib dependencies-trac #27254
da291d8spkg-configure for GMP
f7698adMove all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
27b4692Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
566bd21Reworked this a bit more
a4ab468fix typo
b979d7badded a bit more explanation
1e48b30correct logic for SAGE_GMP_PREFIX etc
12cd746iml 1.0.5 + autoconf/gmp fixes
comment:6

spkg-configure.m4 has to wait until BLAS/LAPACK is sorted out

comment:7

errors in linbox - one package that uses iml

sage -t --long --warn-long 84.1 src/sage/matrix/matrix_integer_dense.pyx  # 1 doctest failed
sage -t --long --warn-long 84.1 src/sage/matrix/matrix_modn_sparse.pyx  # 1 doctest failed
sage -t --long --warn-long 84.1 src/sage/matrix/matrix_integer_sparse.pyx  # 1 doctest failed
comment:8

this is the patch to be applied to the original imp directory, before
./bootstrap && autoreconf -ivf:

diff --git a/configure.ac b/configure.ac
index 2a1d666..d362f31 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,6 +68,7 @@ AC_CHECK_FUNCS(calloc floor)
 
 IML_VERBOSE_MODE
 IML_DEFAULT_PATH
+AS_IF([test "x$SAGE_GMP_PREFIX" = "x$SAGE_LOCAL"], [
 IML_CHECK_GMP(,,[AC_MSG_ERROR(
 GMP not found! 
 GMP version 3.1.1 or greater is required for this library to compile. 
@@ -75,6 +76,7 @@ Please make sure GMP is installed and specify the header and libraries
 location with the options --with-gmp-include=<path> and --with-gmp-lib=<path>
 respectively when running configure.
 )])
+])
 
 IML_CHECK_CBLAS(,,[AC_MSG_WARN(
 CBLAS not found!

1.0.4 log - configure is run twice (sic!)

comment:9

Attachment: iml-1.0.4p1.p2.log

see attachment for failed attempt to go back to the current Sage IML and doing the same patch there - configure is run twice, and the 2nd time it's invoked by make, with wrong parameters, leading to a failure in case of system GMP.

comment:10

Pruned output of make -d:

Updating makefiles....
 Considering target file 'Makefile'.
   Considering target file 'Makefile.in'.
     Considering target file 'aclocal.m4'.
      Prerequisite 'config/libtool.m4' is newer than target 'aclocal.m4'.
      Prerequisite 'config/ltoptions.m4' is newer than target 'aclocal.m4'.
      Prerequisite 'config/ltsugar.m4' is newer than target 'aclocal.m4'.
      Prerequisite 'config/ltversion.m4' is newer than target 'aclocal.m4'.
      Prerequisite 'config/lt~obsolete.m4' is newer than target 'aclocal.m4'.
     Must remake target 'aclocal.m4'.
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /home/marc/tmp/iml-1.0.4p1/missing aclocal-1.16 -I config
     Successfully remade target file 'aclocal.m4'.
    Prerequisite 'aclocal.m4' is newer than target 'Makefile.in'.
   Must remake target 'Makefile.in'.
 cd . && /bin/bash /home/marc/tmp/iml-1.0.4p1/missing automake-1.16 --gnu
   Successfully remade target file 'Makefile.in'.
   Considering target file 'config.status'.
     Considering target file 'configure'.
      Prerequisite 'aclocal.m4' is newer than target 'configure'.
     Must remake target 'configure'.
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /home/marc/tmp/iml-1.0.4p1/missing autoconf
    Prerequisite 'configure' is newer than target 'config.status'.
   Must remake target 'config.status'.
/bin/bash ./config.status --recheck
running CONFIG_SHELL=/bin/bash /bin/bash ./configure --no-create --no-recursion
comment:11

So this has to do with timestamps being wrong. I have success with applying the patch in comment 8, followed by autoreconf -ivf, and packaging the result. Such a tarball does not trigger the 2nd run of configure.

comment:12

I'm sort of confused by this patch. Why are we patching even the INSTALL instructions? Many of the files being patched here are also generated files that probably don't need to be patched so much, as they have nothing to do with the change you made to configure.ac. Perhaps I can take a look at this more later tomorrow or something.

comment:13

So the problem I'm having with iml, at least on Debian, is simply the fact that for blas and for GMP it has some kind of overwrought non-standard macros for detecting them, that run into the same kinds of problems that led me to declare in the first place that I did not want to provide some mechanism to search for the exact path to the GMP shared object (as opposed to just trying -lgmp and seeing if it works as AC_CHECK_LIB and the like do).

Instead, iml's autoconf macros (see e.g. config/gmp-check.m4 in the iml source) really wants to check the full path for the header files and libraries, and check that those files exist at some specific path. To do this it has a list of default prefixes it checks in (/usr and /usr/local), which in the Sage spkg-install we also prepend $SAGE_LOCAL to with --with-default=$SAGE_LOCAL.

However, on Debian and Ubuntu which use a multiarch layout this fails: The default settings will not find the GMP headers and shared object under /usr/include/x86_64-linux-gnu and /usr/lib/x86_64-linux-gnu (for example) to use the system lib.

I was able to get iml to build in Sage with the system GMP by configuring it like:

./configure --with-gmp-include=/usr/include/$(gcc -print-multiarch) \
            --with-gmp-lib=/usr/lib/$(gcc -print-multiarch) \
            --with-cblas="$(pkg-config --libs cblas)" \
            --with-cblas-includes="$(pkg-config --cflags cblas)"

(for BLAS just using pkg-config works fine).

So I think the best thing to do, while annoying, is to simply pass the correct --with-gmp-include and --with-gmp-lib flags for the platform we're building on. Which kind of stinks because in principle those could take any possible value. In practice, however, for most systems it will be in one of just a few places, and I have some more generic code elsewhere I can integrate here that will work for most platforms.

In other words, we need some custom code here that works similarly to what iml's configure.ac is doing, but checks in a wider range of locations, including multi-arch directories where they exist, then pass the correct values for these flags.

comment:14

Replying to @embray:

I'm sort of confused by this patch. Why are we patching even the INSTALL instructions? Many of the files being patched here are also generated files that probably don't need to be patched so much, as they have nothing to do with the change you made to configure.ac. Perhaps I can take a look at this more later tomorrow or something.

this is what happens if you run autoreconf after editing configure.ac
(perhaps using the matching autoconf version would lead to a smaller patch...)

comment:15

Even the install instructions though?

In any case, I don't think anything needs to be patched here. IML already has a mechanism for pointing to the correct GMP. It's just a bit needlessly cumbersome, since there's no reason it needs to have the exact paths to the headers in order to just find that there is a gmp.h on the standard include paths (as something like AC_CHECK_HEADERS does).

So now we have the additional burden, at least when building iml against a system GMP, of grubbing around for the right system include path in the correct order, make sure gmp.h is in one of them, and return that path. I think we can do this easily enough. It's just an extra hassle.

If I'm reading the implementation of IML_CHECK_GMP correctly we only need to do this for the header too, since it literally reads test -r "${GMP_HEADER}/gmp.h" (where GMP_HEADER is something like /usr/include).

comment:16

Compiling a one-liner with #include <gmp.h> using gcc -M ought to produce the right include path. Not sure about clang.

comment:17

I found a macro that does essentially this. It should work for clang as well: http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_absolute_header.m4

I just experimented with it and it looks like it gets the job done. I'm going to try it with iml now.

comment:18

Replying to @embray:

Even the install instructions though?

yep, it's one from autotools, and apparently it evolved...

In any case, I don't think anything needs to be patched here. IML already has a mechanism for pointing to the correct GMP. It's just a bit needlessly cumbersome, since there's no reason it needs to have the exact paths to the headers in order to just find that there is a gmp.h on the standard include paths (as something like AC_CHECK_HEADERS does).

So now we have the additional burden, at least when building iml against a system GMP, of grubbing around for the right system include path in the correct order, make sure gmp.h is in one of them, and return that path. I think we can do this easily enough. It's just an extra hassle.

If I'm reading the implementation of IML_CHECK_GMP correctly we only need to do this for the header too, since it literally reads test -r "${GMP_HEADER}/gmp.h" (where GMP_HEADER is something like /usr/include).

I don't think that IML_CHECK_GMP will work if the compiler does not have /usr/local/include in its list of default paths, or the linker does not know about libs in /usr/local/lib without being supplied with the -L pointing there---something that might be the case on *BSD (and our favourite, OSX :-))

It might be easier just to replace it by something that works in all cases.

comment:19

I made this work (I think) as part of #27212.

Dependencies: #27212

comment:20

To be clear what I meant, on FreeBSD, where GMP lives in /usr/local, to make IML's configure to pass GMP checks, I need to do

CC="cc -I/usr/local/include" ./configure 

as without setting CC this way it fails with

Default checking path = /usr /usr/local
checking for GMP >= 3.1.1... not found
configure: error: GMP not found!
GMP version 3.1.1 or greater is required for this library to compile.
Please make sure GMP is installed and specify the header and libraries
location with the options --with-gmp-include=<path> and --with-gmp-lib=<path>
respectively when running configure.

Note that specifying --with-gmp-include=/usr/local/include --with-gmp-lib=/usr/local/lib without setting CC as above fails just the same.

The reason is as I said: /usr/local/include is not in the default cc's path:

$ cat t.c
#include <gmp.h>
$ cc -c t.c                      # fails
t.c:1:10: fatal error: 'gmp.h' file not found
#include <gmp.h>
         ^~~~~~~
1 error generated.
$ cc -I/usr/local/include -c t.c # OK
comment:21

Up to this point you never said anything about FreeBSD on this ticket, only Debian 9. That seems like a totally unrelated issue.

There does seem to be an annoyance in IML's configure that it just assumes /usr/local/include is on the compiler's default include path. ISTM that could be resolved by passing it in CFLAGS and not overriding CC but I'm not sure.

In any case, that does not seem to be the issue you're describing in the ticket description.

comment:22

It is related to how exactly IML_CHECK_GMP is broken, but I am fine with dealing with this on another ticket.

comment:23

I agree that, as implemented, it's not good.

Why does GMP live in /usr/local though?

comment:24

that's a typical *BSD (and Solaris) thing, they keep extra packages in various places out of the main tree, see e.g. https://unix.stackexchange.com/questions/332764/role-of-the-usr-local-directory-in-freebsd

comment:25

OK, let's keep this on the wishlist. Ideally it's got to be fixed upstream.

Changed commit from 12cd746 to none

Changed branch from u/dimpase/packages/iml_105 to none

Changed keywords from none to freebsd

comment:26

In this case I'm still fine with applying a patch, but all it needs is a simple and generic patch to not just assume that /usr/include and /usr/local/include must be on the compiler's default include path.

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
 iml only works with MP_LIBRARY provided in SAGE_LOCAL, at least this is the case on Debian 9.
 
-they have a very only autoconf macro to detect GMP, which it particular won't work for libgmp in some arch-specfic location such as /usr/lib64/.
+they have a very old autoconf macro to detect GMP, which it particular won't work for libgmp in some arch-specfic location such as /usr/lib64/.
  
 This breaks #27212. It works with GMP provided by the system if I just remove the call to their macro, re-run their bootstrap, repackage tarball. 
 
comment:28

I talked to upstream, and they would be happy to get patches for that GMP business.

comment:29

here is a simple spkg-configure, working on Debian buster (which has iml 1.0.4).


New commits:

4928cb9spkg-configure for iml

Changed dependencies from #27212 to none

Commit: 4928cb9

comment:31

Works on Gentoo with sci-libs/iml. Is that SAGE_IML_MINVER doing anything?

comment:32

Needs distros/

Changed commit from 4928cb9 to 8177463

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

8177463commented out MINVER, added install info for gentoo

Changed commit from 8177463 to 5a34c90

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

5a34c90more distro info added
comment:35

Addded the distro info I could find for iml. Note that both versions 1.0.4 and 1.0.5 appear to work with Sage just fine.

comment:36

By the way, are you getting email notifications from trac?
I don't get any since a day or two.

comment:37

Replying to @dimpase:

By the way, are you getting email notifications from trac?
I don't get any since a day or two.

This has been an ongoing problem with SendGrid for as long as I can remember. They claim to retry 4xx errors for 72 hours, but they don't -- at least not in the cases where I get a phone call because some email from them went missing, which is often. I've attempted to whitelist their IP space on our servers, but identifying new IPs requires us to first miss a message from that IP. Anyone whose mail administrator enjoys whack-a-mole less than I do is likely to have even more problems.

comment:38

I now no longer use an gmail address for trac notifications, and it started to work.
Perhaps gmail started dropping SendGrid emails for me, as a special service, arrrgh...

Reviewer: Isuru Fernando

comment:40

Some of the SendGrid servers that we do have whitelisted are on a bunch of blacklists, which may be why those messages went missing. If I hadn't given them an explicit pass, we would have rejected them too. Normally sending to gmail is pretty reliable, because if you can't send to gmail, you go out of business. But the downside is that when things go wrong at gmail you'll never find out why.

Changed branch from u/dimpase/packages/imlconfig to 5a34c90