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

switch the default mp library to gmp #30752

Closed
dimpase opened this issue Oct 11, 2020 · 42 comments
Closed

switch the default mp library to gmp #30752

dimpase opened this issue Oct 11, 2020 · 42 comments

Comments

@dimpase
Copy link
Member

dimpase commented Oct 11, 2020

currently the default mp library is mpir: the default --with-mp is system, and in
build/pkgs/mpir/spkg-configure.m4 we have

SAGE_SPKG_CONFIGURE([mpir], [
dnl Implement cases for what to do on different options here
    case "$with_mp" in
        system)
            AC_CHECK_HEADER(gmp.h, [], [sage_spkg_install_mpir=yes])
...

dnl Set SAGE_MP_LIBRARY depending on the with_mp option
    case "$with_mp" in
    mpir|system)
        SAGE_MP_LIBRARY=mpir
        ;;
    gmp)
        SAGE_MP_LIBRARY=gmp

as mpir is getting more and more bitrotten, this should be switched to gmp as the default.

Also,

  --with-mp=system        use the system GMP as multiprecision library, if
                          possible (default)

is a lie, as can be seen from above, the default is mpir, not gmp.

CC: @jhpalmieri @mkoeppe @orlitzky

Component: build: configure

Author: Dima Pasechnik, Matthias Koeppe

Branch/Commit: d2500f1

Reviewer: Matthias Koeppe, Dima Pasechnik

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

@dimpase dimpase added this to the sage-9.3 milestone Oct 11, 2020
@dimpase
Copy link
Member Author

dimpase commented Oct 11, 2020

@dimpase
Copy link
Member Author

dimpase commented Oct 11, 2020

comment:1

marking it critical, as it corrects the ./configure -h misleading info.


New commits:

3af6738make gmp (and not mpir) the default mp lib

@dimpase
Copy link
Member Author

dimpase commented Oct 11, 2020

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Oct 11, 2020

Commit: 3af6738

@dimpase
Copy link
Member Author

dimpase commented Oct 11, 2020

comment:2

the reporting is still a bit misleading:

...
gmp-6.1.2:                                   using system package; SPKG will not be installed
...
mpir-3.0.0-644faf502c56f97d9accd301965fc57d6ec70868.p0:no suitable system package; will be installed as an SPKG
...

but this not a regression.

@jhpalmieri
Copy link
Member

comment:3

In principle this is okay, and I'm testing it on OS X, but I think it should be merged early in a release, not late in this one, since it needs testing on a wide variety of platforms.

@dimpase
Copy link
Member Author

dimpase commented Oct 11, 2020

comment:4

perhaps you'd like to try this together with #30625 - upgrade of Sage's GMP.

I've moved this to 9.3.

@dimpase dimpase modified the milestones: sage-9.2, sage-9.3 Oct 11, 2020
@jhpalmieri
Copy link
Member

comment:5

With Xcode 12, OS X Catalina: works on its own and in combination with #30625.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 10, 2020

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 10, 2020

comment:6

This will affect all -minimal builds - so I am testing it with GH Actions

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 11, 2020

comment:7

This breaks lots of -minimal builds - see for example fedora-31-minimal (https://github.com/mkoeppe/sage/runs/1381716520)

  gcc -pthread -shared -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -L. -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib build/temp.linux-x86_64-3.8/cypari2/closure.o -L/sage/local/lib -L/sage/local/lib64 -L/sage/local/lib -lgmp -lpari -o build/lib.linux-x86_64-3.8/cypari2/closure.cpython-38-x86_64-linux-gnu.so -lpari
  /usr/bin/ld: cannot find -lgmp
  collect2: error: ld returned 1 exit status
  error: command 'gcc' failed with exit status 1
  Building wheel for cypari2 (setup.py): finished with status 'error'

@dimpase
Copy link
Member Author

dimpase commented Nov 12, 2020

comment:8

a logic error in spkg-configure (of mpir or gmp)

2020-11-10T22:56:48.4470802Z Checking whether SageMath should install SPKG mpir...
2020-11-10T22:56:48.4471312Z checking gmp.h usability... no
2020-11-10T22:56:48.4471747Z checking gmp.h presence... no
2020-11-10T22:56:48.4472119Z checking for gmp.h... no
2020-11-10T22:56:48.4472533Z checking gmpxx.h usability... no
2020-11-10T22:56:48.4472984Z checking gmpxx.h presence... no
2020-11-10T22:56:48.4473541Z checking for gmpxx.h... no
2020-11-10T22:56:48.4473965Z checking for library containing __gmpq_cmp_z... no
2020-11-10T22:56:48.4474813Z configure: will use system package and not install SPKG mpir
2020-11-10T22:56:48.4475426Z using gmp SPKG (via --with-mp=gmp)
2020-11-10T22:56:48.4475906Z -----------------------------------------------------------------------------
2020-11-10T22:56:48.4476313Z Checking whether SageMath should install SPKG gmp...
2020-11-10T22:56:48.4476849Z configure: will use system package and not install SPKG gmp

somehow it thinks that system GMP is available.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 12, 2020

Changed commit from 3af6738 to a1b552c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 12, 2020

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

25de198make gmp (and not mpir) the default mp lib
a1b552ccorrect logic for spkg-configures of gmp and mpir

@dimpase
Copy link
Member Author

dimpase commented Nov 12, 2020

comment:10

fixed this (and rebased over latest beta).
Tested with/without gmp present, with various options for --with-mp=, all seems to work.

@vbraun
Copy link
Member

vbraun commented Nov 22, 2020

Changed branch from u/dimpase/packages/make_gmp_default_mp to a1b552c

@vbraun
Copy link
Member

vbraun commented Nov 22, 2020

Changed commit from a1b552c to none

@vbraun
Copy link
Member

vbraun commented Nov 22, 2020

comment:20

On OSX:

vbraun@osx Sage % ./sage -p gmp                       
Found local metadata for gmp-6.2.0
Using cached file /Users/vbraun/Sage/upstream/gmp-6.2.0.tar.xz
gmp-6.2.0
====================================================
Setting up build directory for gmp-6.2.0
Error: Unknown file type: /Users/vbraun/Sage/upstream/gmp-6.2.0.tar.xz
Finished extraction

The inability to extract xz is apparently ignored, but obviously install fails sooner or later.

@vbraun vbraun reopened this Nov 22, 2020
@vbraun
Copy link
Member

vbraun commented Nov 22, 2020

Commit: a1b552c

@vbraun
Copy link
Member

vbraun commented Nov 22, 2020

New commits:

25de198make gmp (and not mpir) the default mp lib
a1b552ccorrect logic for spkg-configures of gmp and mpir

@vbraun
Copy link
Member

vbraun commented Nov 22, 2020

Changed branch from a1b552c to u/dimpase/packages/make_gmp_default_mp

@jhpalmieri
Copy link
Member

comment:22

What does this have to do with this ticket? If you can't extract xz files, then you can't extract tarballs for Python, symmetrica, gcc, and gmp.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 22, 2020

comment:23

These other packages declare a dependency on xz (see #30559)

@jhpalmieri
Copy link
Member

comment:24

But on OS X, the included version of Python 3, at least as of 10.15.7, can extract .tar.xz files using its tarfile module. It looks like this feature was added in Python 3.3. We shouldn't need the xz executable for this, unless the system only has an old version of Python. Rather than using #30948, or maybe in addition, maybe we should modify the code in sage_bootstrap.uncompress.tar_file that handles xz files.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 23, 2020

comment:25

That's true, but as of #29890, we prefer system python 2 because with python3 we have run into SSL problems. (The correct fix is discussed in the ticket description of #29418.)

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 23, 2020

comment:26

Replying to @mkoeppe:

The correct fix is discussed in the ticket description of #29418.

I've opened #30950 for this -- but for the present ticket, let's just add xz to the dependencies.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

Changed commit from a1b552c to d2500f1

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

New commits:

deb738cMerge tag '9.3.beta2' into t/30752/packages/make_gmp_default_mp
d2500f1build/pkgs/gmp/dependencies: Add xz

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

Changed reviewer from Matthias Koeppe to Matthias Koeppe, ...

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

Changed author from Dima Pasechnik to Dima Pasechnik, Matthias Koeppe

@dimpase
Copy link
Member Author

dimpase commented Nov 25, 2020

Changed reviewer from Matthias Koeppe, ... to Matthias Koeppe, Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Nov 25, 2020

comment:30

lgtm

@vbraun
Copy link
Member

vbraun commented Dec 5, 2020

Changed branch from u/mkoeppe/packages/make_gmp_default_mp to d2500f1

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

4 participants