Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spkg-configure.m4 for gmp and mpir #27212

Closed
dimpase opened this issue Feb 4, 2019 · 145 comments
Closed

spkg-configure.m4 for gmp and mpir #27212

dimpase opened this issue Feb 4, 2019 · 145 comments

Comments

@dimpase
Copy link
Member

dimpase commented Feb 4, 2019

This should be easy, and it's a potential source of library conflicts, as gmp is often installed...

note that GMP is pulled in via ./configure --with-mp=gmp
(as well as by specifying SAGE_MP_LIBRARY?)

Configure:
http://users.ox.ac.uk/~coml0531/sage/configure-316.tar.gz

CC: @embray

Component: packages: optional

Author: Erik Bray, Dima Pasechnik

Branch: e735b4b

Reviewer: Dima Pasechnik

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

@dimpase dimpase added this to the sage-8.7 milestone Feb 4, 2019
@dimpase
Copy link
Member Author

dimpase commented Feb 4, 2019

Commit: 1ed38e1

@dimpase
Copy link
Member Author

dimpase commented Feb 4, 2019

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Feb 4, 2019

Branch: u/dimpase/packages/gmp_m4

@dimpase
Copy link
Member Author

dimpase commented Feb 4, 2019

comment:1

here we check that GMP is new enough, by verifying it has a new function.
The rest is just copypaste.

To actually build with GMP, one needs

./configure --with-mp=gmp

(configure does the test regardless)


New commits:

1ed38e1spkg-configure for GMP

@dimpase
Copy link
Member Author

dimpase commented Feb 5, 2019

comment:2

I suppose due to a special treatment of GMP/MPIR, despite configure reporting GMP as not being installed, it still gets picked up for installation from source.
(I tested this on a clean build...)

@embray
Copy link
Contributor

embray commented Feb 6, 2019

comment:3

Yes, I've already put some thought into how to handle this case, though I don't have a solid answer. The spkg-configure.m4 seems fine, but we can't just leave it at that.

The tricky thing is that with MPIR it can be compiled with or without gmp-compatibility mode. On Sage we use --with-gmpcompat so if we link to a gmp we're actually getting the one installed by MPIR. But some systems might have MPIR built without gmp-compatiblity, and yet no separate libgmp.so. So in that case we'd actually want to use libmpir (if such a system exists I'm not sure).

So if we run ./configure with no --with-mp= flag the question is what to do? Do we just try to detect any "libgmp" and use that if found? Do look for a libmpir, ever? If the user passes --with-mp=mpir or --with-mp=gmp should that imply that it should just build the Sage SPKG, or do we explicitly go searching for, say, a libmpir in the former case?

@embray
Copy link
Contributor

embray commented Feb 6, 2019

comment:4

Deciding which BLAS to use from the system raises similar questions.

@embray
Copy link
Contributor

embray commented Feb 6, 2019

comment:5

The more I think about it, the more I'm inclined toward --with-mp=mpir should mean that the MP library must by MPIR by explicit request of the user, and if not found on the system then we should build the SPKG.

If --with-mp= is not otherwise specified then we can take whatever MP lib was found on the system.

We should also add an spkg-configure.m4 for MPIR. We can impose an ordering (e.g. first check for MPIR, and then for plain GMP) by putting AC_REQURE(SAGE_SPKG_CONFIGURE_MPIR) at the beginning of the spkg-configure.m4 for gmp. I did something similar for gfortran/gcc and seems to work.

@embray
Copy link
Contributor

embray commented Feb 6, 2019

comment:6

It should also be noted that we have also in the past patched other SPKGS (see e.g. #21188) to always link with -lgmp and #include <gmp.h> (and never use -lmpir and #include <mpir.h> respectively).

I'm not 100% convinced that that's necessary, though certainly one or the other should be used. So it's probably best to always ensure that there is at least a working libgmp and a gmp.h. The question then is whether or not it's possible to determine if that libgmp happened to be provided by MPIR.

Update: It seems that a libgmp.so provided by MPIR does contain a __mpir_version symbol, so that's something we can look for.

@embray
Copy link
Contributor

embray commented Feb 6, 2019

comment:7

It seems MPIR is not in fact packaged for Debian (not sure about other distros): https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=708391 I had just assumed it would be a question of using either MPIR or GMP (in other words, they conflict with each other, and installing either one or the other is possible, as is the case in Sage).

@dimpase
Copy link
Member Author

dimpase commented Feb 6, 2019

comment:8

Replying to @embray:

Yes, I've already put some thought into how to handle this case, though I don't have a solid answer. The spkg-configure.m4 seems fine, but we can't just leave it at that.

The tricky thing is that with MPIR it can be compiled with or without gmp-compatibility mode. On Sage we use --with-gmpcompat so if we link to a gmp we're actually getting the one installed by MPIR. But some systems might have MPIR built without gmp-compatiblity, and yet no separate libgmp.so. So in that case we'd actually want to use libmpir (if such a system exists I'm not sure).

FreeBSD provides GMP and MPIR, not in GMP-compatibility mode (I looked at the corresponding makefile)
https://www.freshports.org/math/mpir
https://www.freshports.org/math/gmp

both are in /usr/local/ there.

I would not be worried about a hypothetical situation of many libgmp's on the system
(in fact, MPFR has a very long configure script dealing with detecting all sorts of info about GMP (or a GMP-compatible thing)

@embray
Copy link
Contributor

embray commented Feb 7, 2019

comment:9

I guess the main question is what to do with the --with-mp= flag in Sage's ./configure script. I don't think it necessarily needs to continue working exactly the same way, but whatever it does it should be sensible. The way I see there are a couple possible ways to interpret it:

One way is that there are really three possible values:

  • detect
  • gmp
  • mpir

where the default value is "detect". In the "detect" case we just try to find the best working libgmp on the system and use it if found. If not found the next fallback would be to install the mpir SPKG (the current default). If passed "mpir" or "gmp" explicitly we don't bother detecting and just install the specifically requested SPKG.

Another possible interpretation is something more like:

  • gmp
  • mpir

where now default value "gmp" is similar to the first case: just try to detect any libgmp on the system to use, and failing that install an SPKG for some libgmp (where we might still use MPIR, but that fact is an implementation detail), whereas "mpir" means "detect MPIR specifically", which is to say find a libgmp that also, at a minimum, contains the __mpir_version flag. Perhaps, for some reason, a user might specifically want to use MPIR e.g. for performance reasons. If not found then we build the MPIR SPKG and use that.

Perhaps there are some other ways of interpreting/re-interpreting this flag, but those are a couple I have in mind.

@embray
Copy link
Contributor

embray commented Feb 7, 2019

comment:10

I think I lean more toward the latter case. Most people who want to build Sage do not care what GMP implementation they use, so I think it's good enough to use the one they have, if any, and if not pick one for them. Whereas if someone is explicitly passing --with-mp=mpir then the really want MPIR specifically, and we should not give them something that isn't it.

OTOH I'd also be curious in what cases MPIR gives better performance, and if we're doing users a disservice by not at least strongly recommending that they use it. I think this is more a question for the mailing list though. The details of what to do at configure-time can be malleable.

@dimpase
Copy link
Member Author

dimpase commented Feb 7, 2019

comment:11

Perhaps we could leave the mpir-based toolchain aside for a moment, after all mpir is an optional package, so it is a bug if it is pulled in once GMP is chosen,
and concentrate on the rest of the surrounding number theory libs: mpc, mpfr, gf2x, ntl, pari.

So first we have to sort out why GMP is pulled in despite being recognised by configure as not to be installed, no?


Note that this spurious GMP installation isn't even triggered before some of spkgs dependent on GMP are installed, e.g. MPFR happily installs long before it's started on GMP.
I get

$ ldd local/lib/libmpfr.so
	linux-vdso.so.1 (0x00007ffcc02e9000)
	libgmp.so.10 => /usr/lib64/libgmp.so.10 (0x00007f9345368000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f9345199000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f9345696000)

but the beginning of MPFR install log says, wrongly,

  --with-gmp="/mnt/opt/Sage/sage-dev/local"

This is of course as expected, as it's hardwired in its spkg-install, as
sdh_configure --with-gmp="$SAGE_LOCAL"...

It appears to me that instead the main ./configure should set up something like SAGE_GMP to be either SAGE_LOCAL or a location computed by GMP's spkg-configure.m4, and then this SAGE_GMP must be used by MPFR...

@dimpase
Copy link
Member Author

dimpase commented Feb 7, 2019

comment:12

And MPFR's only part of trouble:

$ grep "gmp=..SAGE_LOCAL." */spkg-install  
arb/spkg-install:    --with-gmp="$SAGE_LOCAL" --with-mpfr="$SAGE_LOCAL"
deformation/spkg-install:            --with-gmp="$SAGE_LOCAL" --with-mpfr="$SAGE_LOCAL" \
ecm/spkg-install:sdh_configure --with-gmp="$SAGE_LOCAL" $ECM_CONFIGURE
flint/spkg-install:    --with-gmp="$SAGE_LOCAL" \
gap/spkg-install:sdh_configure --with-gmp="$SAGE_LOCAL"
gcc/spkg-install:    --with-gmp="$SAGE_LOCAL" --with-mpfr="$SAGE_LOCAL" --with-mpc="$SAGE_LOCAL" \
gdb/spkg-install:    --with-gmp="$SAGE_LOCAL"
gfortran/spkg-install:    --with-gmp="$SAGE_LOCAL" --with-mpfr="$SAGE_LOCAL" --with-mpc="$SAGE_LOCAL" \
givaro/spkg-install:sdh_configure --with-gmp="$SAGE_LOCAL" --enable-shared $GIVARO_CONFIGURE
glpk/spkg-install:#       GMP/MPIR and zlib, so we should just use `--with-gmp="$SAGE_LOCAL"` and
mpc/spkg-install:sdh_configure --with-gmp="$SAGE_LOCAL" --with-mpfr="$SAGE_LOCAL" $EXTRA
mpfi/spkg-install:sdh_configure --with-mpfr="$SAGE_LOCAL" --with-gmp="$SAGE_LOCAL"
mpfrcx/spkg-install:sdh_configure --with-gmp="$SAGE_LOCAL" --with-mpfr="$SAGE_LOCAL" \
mpfr/spkg-install:        ./configure --with-gmp="$SAGE_LOCAL" $SAGE_CONF_OPTS $MPFR_CONFIGURE) &>/dev/null;
mpfr/spkg-install:    sdh_configure --with-gmp="$SAGE_LOCAL" \
pari/spkg-install:    --with-readline="$SAGE_LOCAL" --with-gmp="$SAGE_LOCAL" \
polylib/spkg-install:sdh_configure --with-libgmp=${SAGE_LOCAL}/lib
singular/spkg-install:                  --with-gmp="$SAGE_LOCAL" \
topcom/spkg-install:    --with-gmp="$SAGE_LOCAL" \

So passing SAGE_LOCAL left and right for everything is something to work on.

@dimpase
Copy link
Member Author

dimpase commented Feb 7, 2019

comment:13

I don't think I understand how the .dummy target for skipping "dummy" packages
in build/make/Makefile works, in particular, why it's just a single $(INST)/.dummy file for all of them.

Or perhaps it's because gmp spkg is known as well as $(MP_LIBRARY), and it's not found on the list of "dummy" packages, which duly includes gmp...

@dimpase
Copy link
Member Author

dimpase commented Feb 8, 2019

comment:14

I've added spkg-configures to the whole "toolchain" bunch (see attachment) - the one for MPIR is basically just a plug, and will now try running the build to see if GMP install still surfaces...

@dimpase
Copy link
Member Author

dimpase commented Feb 8, 2019

comment:15

Attachment: mp_star.patch.gz

no, this does not help - and while I didn't have patience for the build to come to a halt, I still saw GMP getting built. (and none of MP*'s)

@embray
Copy link
Contributor

embray commented Feb 8, 2019

comment:16

The thing to do with all of these is that configure needs to output some config file based on its results which contains the correct prefixes for all these libraries (and likely others to come).

For this it might be useful to use the existing sage-env-config file, since I believe it gets sourced when building each package. It would probably be useful to have there since having the correct paths to libraries may also be useful at runtime (e.g. when running cython(), I think). To this end it would also be nice to have a sort of sage.env_config Python module that is also generated, so these values can easily be loaded by the Python library without sage-env being sourced, but this is not necessary for the SPKG build process.

Then every --with-gmp=$SAGE_LOCAL" should be replaced with --with-gmp=$SAGE_GMP_PREFIX", or something like that (the value of which is still ofc $SAGE_LOCAL when using a GMP installed from an SPKG).

What I'm less sure about is how exactly to determine the value for $SAGE_GMP_PREFIX. Macros like AC_SEARCH_LIBS only tests that linking with -l<libname> works; it has no clue about how the system searches library paths in that case. I think the best way to handle this is not unlike the code I wrote in #27230, unfortunately.

@dimpase
Copy link
Member Author

dimpase commented Feb 8, 2019

comment:17

I think the .dummy magic breaks for $(MP_LIBRARY). To prove this, I did

diff --git a/build/make/deps b/build/make/deps
index e9008d20e6..c37b6062ee 100644
--- a/build/make/deps
+++ b/build/make/deps
@@ -77,7 +77,7 @@ toolchain: $(foreach pkgname,$(TOOLCHAIN),$(inst_$(pkgname)))
 # See #14168 and #14232.
 #
 # Note: This list consists of only the *runtime* dependencies of the toolchain.
-TOOLCHAIN_DEPS = zlib $(MP_LIBRARY) mpfr mpc
+TOOLCHAIN_DEPS = zlib gmp mpfr mpc
 TOOLCHAIN_DEP_INSTS = \
        $(foreach pkgname,$(TOOLCHAIN_DEPS),$(inst_$(pkgname)))
 
@@ -156,7 +156,6 @@ sagelib: \
                $(inst_mpc) \
                $(inst_mpfi) \
                $(inst_mpfr) \
-               $(MP_LIBRARY) \
                $(inst_ntl) \
                $(inst_numpy) \
                $(inst_pari) \

(probably only the 2nd change, removal from sagelib: deps matters, not the 1st change.)

and, viola, no GMP building pops up. Needless to say, "dummies" of the form $(inst_...) do not get built.

@embray
Copy link
Contributor

embray commented Feb 8, 2019

comment:18

Replying to @dimpase:

I think the .dummy magic breaks for $(MP_LIBRARY). To prove this, I did

Sorry, I didn't realize you were being stymied by this. Yes, to me there was no mystery here, which is in part why I was focused on how best to handle the existing --with-mp flag. I should have been more clear about that.

@embray
Copy link
Contributor

embray commented Feb 8, 2019

comment:19

Replying to @embray:

What I'm less sure about is how exactly to determine the value for $SAGE_GMP_PREFIX. Macros like AC_SEARCH_LIBS only tests that linking with -l<libname> works; it has no clue about how the system searches library paths in that case. I think the best way to handle this is not unlike the code I wrote in #27230, unfortunately.

Another possibility of course is to just leave it empty and not pass --with-gmp flags at all, in this case they will be forced to just pick up whatever GMP they find on the default paths as well, which usually will be the same one.

@dimpase
Copy link
Member Author

dimpase commented Feb 8, 2019

comment:20

Pragmatically, I'd say that--with-mp=foo ought to always install foo from Sage's source, unless foo=detect. This is your first possibility from comment 9.
(And leaving it out would result in either picking up system's "GMP", be it real GMP or MPIR in compatibility mode.)

This is reasonably flexible. Experts who need special versions of GMP/MPIR may just update Sage's packages and use them, or install them system-wide.

Most of all, it would not need fiddling with recongising "hidden" MPIR in a system's libgmp, something that would otherwise cater for non-existing systems, i.e. pure over-design for nothing and nobody...

@dimpase
Copy link
Member Author

dimpase commented Apr 18, 2019

comment:112

and now this fails on OSX 10.14 with external gmp, specifically, flint does not build:

          make build/interfaces/NTL-interface.lo; \
          clang++ -std=gnu++11 -fno-common -pedantic -Wall -O2 -funroll-loops -g -mpopcnt   -shared -install_name /Users/administrator/
py/sage/local/lib/libflint-13.5.2.dylib -compatibility_version 13.5 -current_version 13.5.2 -Wl,-rpath,/usr/local/lib -Wl,-rpath,/Users
/administrator/py/sage/local/lib build/interfaces/NTL-interface.lo  build/printf.lo  build/fprintf.lo  build/sprintf.lo  build/scanf.lo
  build/fscanf.lo  build/sscanf.lo  build/clz_tab.lo  build/memory_manager.lo  build/version.lo  build/profiler.lo  build/thread_support.lo  build/ulong_extras.lo  build/long_extras.lo  build/perm.lo  build/fmpz.lo  build/fmpz_vec.lo  build/fmpz_poly.lo  build/fmpq_poly.lo  build/fmpz_mat.lo  build/fmpz_lll.lo  build/mpfr_vec.lo  build/mpfr_mat.lo  build/mpf_vec.lo  build/mpf_mat.lo  build/nmod_vec.lo  build/nmod_poly.lo  build/nmod_poly_factor.lo  build/arith.lo  build/mpn_extras.lo  build/nmod_mat.lo  build/fmpq.lo  build/fmpq_vec.lo  build/fmpq_mat.lo  build/padic.lo  build/fmpz_poly_q.lo  build/fmpz_poly_mat.lo  build/nmod_poly_mat.lo  build/fmpz_mod_poly.lo  build/fmpz_mod_poly_factor.lo  build/fmpz_factor.lo  build/fmpz_poly_factor.lo  build/fft.lo  build/qsieve.lo  build/double_extras.lo  build/d_vec.lo  build/d_mat.lo  build/padic_poly.lo  build/padic_mat.lo  build/qadic.lo  build/fq.lo  build/fq_vec.lo  build/fq_mat.lo  build/fq_poly.lo  build/fq_poly_factor.lo  build/fq_nmod.lo  build/fq_nmod_vec.lo  build/fq_nmod_mat.lo  build/fq_nmod_poly.lo  build/fq_nmod_poly_factor.lo  build/fq_zech.lo  build/fq_zech_vec.lo  build/fq_zech_mat.lo  build/fq_zech_poly.lo  build/fq_zech_poly_factor.lo  -o libflint-13.5.2.dylib -L/Users/administrator/py/sage/local/lib -Wl,-rpath,/Users/administrator/py/sage/local/lib  -L/Users/administrator/py/sage/local/var/tmp/sage/build/flint-2.5.2.p4/src -L/usr/local/lib -L/Users/administrator/py/sage/local/lib -L/Users/administrator/py/sage/local/lib -lmpfr -lgmp -lm -lntl -lpthread ; \
        fi
clang++ -std=gnu++11 -fPIC -fno-common -pedantic -Wall -O2 -funroll-loops -g -mpopcnt  -I/Users/administrator/py/sage/local/var/tmp/sage/build/flint-2.5.2.p4/src -I/usr/local/include -I/Users/administrator/py/sage/local/include -I/Users/administrator/py/sage/local/include -c interfaces/NTL-interface.cpp -o build/interfaces/NTL-interface.lo
ld: illegal thread local variable reference to regular symbol __ZN3NTL8ZZ_pInfoE for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[4]: *** [libflint-13.5.2.dylib] Error 1

note that mpfr and ntl, its dependencies build just fine with SAGE_CHECK=yes, so that's something in flint C++ interface that is broken...

@dimpase
Copy link
Member Author

dimpase commented Apr 18, 2019

comment:113

I suppose this is caused by the very recent (Apr 6) NTL update.

@dimpase
Copy link
Member Author

dimpase commented Apr 18, 2019

comment:114

No, it must be either lack of testing on OSX 10.14 (I tested on 10.13 before), or new clang in Xcode:

$ clang++ -v
Apple LLVM version 10.0.1 (clang-1001.0.46.3)
Target: x86_64-apple-darwin18.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

@dimpase
Copy link
Member Author

dimpase commented Apr 18, 2019

comment:115

Replying to @vbraun:

The one and only OSX buildbot worker is for testing that Sage installs on a as-vanilla-as-possible OSX install...

Well, if you like I can install autotools on your OSX bot just from sources. (I recall doing that on OSX 10.12, it should still be possible on 10.14).
This would be vanilla enough then, still...

@dimpase
Copy link
Member Author

dimpase commented Apr 18, 2019

comment:116

Replying to @dimpase:

No, it must be either lack of testing on OSX 10.14 (I tested on 10.13 before), or new clang in Xcode:

$ clang++ -v
Apple LLVM version 10.0.1 (clang-1001.0.46.3)
Target: x86_64-apple-darwin18.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

The problem seems to be caused by -std=gnu++11 option, see #25532 comment:88

@dimpase
Copy link
Member Author

dimpase commented Apr 18, 2019

comment:117

facepalm.... That was just a library clash - I did have libntl installed in /usr/local, and this seems to be deadly on OSX...

@vbraun
Copy link
Member

vbraun commented Apr 18, 2019

comment:118

Replying to @dimpase:

Well, if you like I can install autotools on your OSX bot just from sources. (I recall doing that on OSX 10.12, it should still be possible on 10.14).

Again, unless we make autotools a prerequisite we need a way to verify that Sage builds on OSX without autotools installed...

@dimpase
Copy link
Member Author

dimpase commented Apr 18, 2019

comment:119

Replying to @vbraun:

Replying to @dimpase:

Well, if you like I can install autotools on your OSX bot just from sources. (I recall doing that on OSX 10.12, it should still be possible on 10.14).

Again, unless we make autotools a prerequisite we need a way to verify that Sage builds on OSX without autotools installed...

Do we have Linux buildbots without autotools installed?

Anyhow, one can have autotools installed in a non-standard location only buildbot knows, so that it can run ./bootstrap, and then it's as if there are not present.

It's hard for me to understand the problem without knowing how exactly a release is made.
(if be any chance you are in Berlin, you can show me - I'm here until 21st Apr).

@vbraun
Copy link
Member

vbraun commented Apr 18, 2019

Changed branch from public/packages/gmp_m4 to e735b4b

@vbraun
Copy link
Member

vbraun commented Apr 18, 2019

comment:121

No matter how you slice it, either the buildbot has access to autotools OR it tests that you can build without autotools. The only safe way to avoid generating a confball by hand is to have some automatic system, e.g. whenever I submit a new buildbot job. That requires some work to avoid version clashes when I back out commits. Truth be told, changes to the configure in Sage are pretty rare so this issue does not come up very often.

I'm in Berlin, hit me up when you have some time. I'm located in the East (Elsenstrasse).

@vbraun
Copy link
Member

vbraun commented Apr 18, 2019

Changed commit from e735b4b to none

@embray
Copy link
Contributor

embray commented Apr 19, 2019

comment:122

Replying to @dimpase:

facepalm.... That was just a library clash - I did have libntl installed in /usr/local, and this seems to be deadly on OSX...

I have had similar problems--not necessarily with NTL--but with other packages where having a copy of the development package already installed on my system created difficult to avoid clashes when trying to build some Sage SPKGs. See e.g. #22768. In that case it's more due to bugginess with Python's module build system, but I can envision similar problems in other build systems that make too many assumptions about your paths in some context.

@dimpase
Copy link
Member Author

dimpase commented Apr 19, 2019

comment:123

Replying to @vbraun:

Truth be told, changes to the configure in Sage are pretty rare

I have a dozen tickets open that do change configure, see #27330.
For the purpose of testing the configure spkg versioning, today I attached
configure tarballs to 3 of them:
#27258, #27265, #27186. No matter in which order they would be merged, I'd need to re-do these tarballs for all but one, you know. This is not exactly entertaining...

@jdemeyer
Copy link

comment:124

Breakage: #27751

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants