Opened 3 months ago

Closed 8 weeks ago

Last modified 7 weeks ago

#32549 closed enhancement (fixed)

Remove package mpir

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.5
Component: packages: standard Keywords:
Cc: jhpalmieri, mjo, dimpase, ​wbhart, brg@…, riemannic@… Merged in:
Authors: Matthias Koeppe, John Palmieri, Dima Pasechnik Reviewers: Dima Pasechnik, Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 286b39c (Commits, GitHub, GitLab) Commit:
Dependencies: #30350, #31163 Stopgaps:

Status badges

Description

MPIR, a fork of GMP, has not been updated upstream. The latest release, 3.0.0, is from 2017-03-01 (https://mpir.org/downloads.html).

Sage currently provides an SPKG for MPIR as a configurable alternative to GMP. We should remove this option and package, as previously suggested in https://trac.sagemath.org/ticket/29620#comment:10

Change History (44)

comment:1 Changed 3 months ago by tmonteil

  • Cc ​wbhart brg@… riemannic@… added

It seems there are some commits since then (the last is from December 2020 for Linux version and August 2021 for Windows version), perhaps should we ask to the people involved in MPIR how they see the future of MPIR ?

comment:2 follow-up: Changed 3 months ago by mjo

+1

Regardless of the state of MPIR upstream, sage never really developed a coherent plan for packages that are substitutes for one another. The ./configure options that are available now don't really make sense, and the best thing to do for our end users is to eliminate the confusion (if also the choice). If someone later wants to overhaul the build system to handle these choices consistently, then OK; but I don't think anyone does right now.

comment:3 Changed 3 months ago by dimpase

The upstream (Bill) told me some time ago that due to Spectre and other CPU vulns discovered few years ago, parts of MPIR became obsolete, as speedups there relied on CPUs being unpatched.

IMHO it's semi-abandonware now.

comment:4 in reply to: ↑ 2 Changed 3 months ago by mkoeppe

Replying to mjo:

sage never really developed a coherent plan for packages that are substitutes for one another. The ./configure options that are available now don't really make sense

#29620 tracks this

comment:5 Changed 2 months ago by mkoeppe

  • Branch set to u/mkoeppe/remove_package_mpir

comment:6 Changed 2 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to 81331d7af671345c3159662bc113b094b8660423
  • Status changed from new to needs_review

New commits:

9fa072ebuild/pkgs/mpir: Remove
81331d7build/pkgs/yasm: Remove (was mpir dependency)

comment:7 Changed 2 months ago by git

  • Commit changed from 81331d7af671345c3159662bc113b094b8660423 to fabdbb3144335ee7fb33957f78d4591b2c176e42

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

fabdbb3build/pkgs/*/SPKG.rst: Clear out redundant 'Dependencies' sections

comment:8 Changed 2 months ago by jhpalmieri

There are a few references to yasm: a comment in m4/sage_spkg_configure.m4 and many mentions in doc/en/developer/portability_testing.rst.

comment:9 Changed 2 months ago by git

  • Commit changed from fabdbb3144335ee7fb33957f78d4591b2c176e42 to 724c3ffd97682ced5521000df878a426a4901bd3

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

724c3ffsrc/doc/en/developer/portability_testing.rst: Remove references to package yasm

comment:10 Changed 2 months ago by git

  • Commit changed from 724c3ffd97682ced5521000df878a426a4901bd3 to 519dc83bc151b00a07d7e534d392f1139c615696

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

519dc83m4/sage_spkg_configure.m4: In documentation, do not use yasm as an example

comment:11 Changed 2 months ago by git

  • Commit changed from 519dc83bc151b00a07d7e534d392f1139c615696 to 01406bfea4ce7469c1d6520713f5952b6f133d2a

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

01406bfbuild/pkgs/*/SPKG.rst: Clear out more redundant 'Dependencies' sections

comment:12 Changed 2 months ago by git

  • Commit changed from 01406bfea4ce7469c1d6520713f5952b6f133d2a to e7fdb3578039895ff83e372ec511363b67b3e584

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

205931csrc/doc/en/installation/source.rst: Remove doc of envvar SAGE_MP_LIBRARY
e7fdb35src/doc/en/installation/source.rst: Remove use of mpir as an example package

comment:13 follow-up: Changed 2 months ago by mkoeppe

In two places of the Sage library, also 'mpir' appears as a possible value of an algorithm parameter:

src/sage/arith/misc.py:        sage: a = binomial(100, 45, algorithm='mpir')
src/sage/rings/integer.pyx:    def binomial(self, m, algorithm='mpir'):

comment:14 Changed 2 months ago by git

  • Commit changed from e7fdb3578039895ff83e372ec511363b67b3e584 to 1eb83e30878c74b245547c22b6e165d4acf0595e

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

1eb83e3build/pkgs/ratpoints/SPKG.rst: Remove redundant 'Dependencies' section

comment:15 in reply to: ↑ 13 ; follow-up: Changed 2 months ago by dimpase

Replying to mkoeppe:

In two places of the Sage library, also 'mpir' appears as a possible value of an algorithm parameter:

src/sage/arith/misc.py:        sage: a = binomial(100, 45, algorithm='mpir')
src/sage/rings/integer.pyx:    def binomial(self, m, algorithm='mpir'):

this option should be renamed gmp, and respective tests should drop # optional tag. Indeed, it just calls a gmp/mpir function (which are more or less the same, and mpir provided a replacement for gmp there).

sage: 100.binomial(45, algorithm='mpir')
61448471214136179596720592960

see, it still works, without any mpir installed.

comment:16 Changed 2 months ago by git

  • Commit changed from 1eb83e30878c74b245547c22b6e165d4acf0595e to f883ea4c76e96fdbf0cbf8028fd65ce4513a786d

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

f883ea4Integer.binomial: Add algorithm='gmp', keep 'mpir' as an alias

comment:17 in reply to: ↑ 15 Changed 2 months ago by mkoeppe

Replying to dimpase:

this option should be renamed gmp

I have kept mpir as an alias, without deprecation warnings; after all, if we use system GMP, that could as well be MPIR

comment:18 follow-up: Changed 2 months ago by dimpase

there is still an #optional : mpir tag in integer.pyx, line 6427. And analogous gmp tag few lines down.

comment:19 Changed 2 months ago by git

  • Commit changed from f883ea4c76e96fdbf0cbf8028fd65ce4513a786d to 1647e2f9ce387e40273b2cb42b46b7f6de378562

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

1647e2fsrc/sage/rings/integer.pyx: Rewrite a doctest so it works with gmp, mpir, 64bit, 32bit

comment:20 in reply to: ↑ 18 Changed 2 months ago by mkoeppe

Replying to dimpase:

there is still an #optional : mpir tag in integer.pyx, line 6427. And analogous gmp tag few lines down.

These doctests weren't actually being run because neither mpir nor gmp were optional packages. I've rewritten it in a less specific way, it now runs

comment:21 Changed 2 months ago by git

  • Commit changed from 1647e2f9ce387e40273b2cb42b46b7f6de378562 to 34526ca3ce293701c88607ec385a35b9d8a33e27

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

34526cabuild/pkgs/*/SPKG.rst: Remove some more redundant 'Dependencies' sections

comment:22 Changed 2 months ago by jhpalmieri

There is a similar test in src/sage/ext/memory.pyx. We can also remove mpir from COPYING.txt. (Was there ever talk of autogenerating that file? Each package could have a file license.txt...)

comment:23 Changed 2 months ago by jhpalmieri

Also in m4/sage_spkg_collect.m4:

    # Packages that should be included in the source distribution
    # This includes all standard packages and two special cases
    case "$SPKG_NAME" in
    mpir)
        in_sdist=yes
        ;;
    esac

comment:24 Changed 2 months ago by jhpalmieri

I don't understand this comment, line 286 in src/sage/all.py:

    # Free globally allocated mpir integers.

Just delete the comment and leave the code? Replace the comment with something else? Remove the code?

comment:25 Changed 2 months ago by jhpalmieri

Here are some proposed changes:

  • COPYING.txt

    diff --git a/COPYING.txt b/COPYING.txt
    index 2da3a8e492..7ce750a1aa 100644
    a b mistune Modified BSD 
    8686mpc                         LGPLv3+
    8787mpfi                        COPYING is GPLv2, source files state LGPLv2.1+
    8888mpfr                        LGPLv3+
    89 mpir                        LGPLv3+
    9089mpmath                      Modified BSD
    9190networkx                    Modified BSD
    9291ntl                         GPLv2+
  • build/pkgs/ecm/spkg-configure.m4

    diff --git a/build/pkgs/ecm/spkg-configure.m4 b/build/pkgs/ecm/spkg-configure.m4
    index e62c21cb32..d5302a89fb 100644
    a b  
    11SAGE_SPKG_CONFIGURE([ecm], [
    22    m4_pushdef([SAGE_ECM_MINVER],[7.0.4])
    33    AC_REQUIRE([SAGE_SPKG_CONFIGURE_GMP])
    4     AC_MSG_CHECKING([installing gmp/mpir? ])
     4    AC_MSG_CHECKING([installing gmp? ])
    55    if test x$sage_spkg_install_gmp = xyes; then
    66        AC_MSG_RESULT([yes; install ecm as well])
    77        sage_spkg_install_ecm=yes
  • build/pkgs/gdb/SPKG.rst

    diff --git a/build/pkgs/gdb/SPKG.rst b/build/pkgs/gdb/SPKG.rst
    index b89773fe67..0716ac53fa 100644
    a b Upstream Contact 
    1919
    2020http://www.gnu.org/software/gdb/
    2121
    22 Dependencies
    23 ------------
    24 
    25 -  python
    26 -  mpc
    27 -  mpfr
    28 -  ppl
    29 -  gmp/mpir
    30 -  makeinfo (external)
    31 
    32 
    3322Special Update/Build Instructions
    3423---------------------------------
    3524
  • build/pkgs/isl/spkg-configure.m4

    diff --git a/build/pkgs/isl/spkg-configure.m4 b/build/pkgs/isl/spkg-configure.m4
    index 1b16f70870..d7bbef80c6 100644
    a b  
    11SAGE_SPKG_CONFIGURE([isl], [
    22    AC_REQUIRE([SAGE_SPKG_CONFIGURE_GMP])
    3     AC_MSG_CHECKING([installing gmp/mpir? ])
     3    AC_MSG_CHECKING([installing gmp? ])
    44    if test x$sage_spkg_install_gmp = xyes; then
    55        AC_MSG_RESULT([yes; install isl as well])
    66        sage_spkg_install_isl=yes
  • build/pkgs/mpfr/spkg-configure.m4

    diff --git a/build/pkgs/mpfr/spkg-configure.m4 b/build/pkgs/mpfr/spkg-configure.m4
    index 2300be6707..f64ede4b75 100644
    a b  
    11SAGE_SPKG_CONFIGURE([mpfr], [
    22    AC_REQUIRE([SAGE_SPKG_CONFIGURE_GMP])
    3     AC_MSG_CHECKING([installing gmp/mpir? ])
     3    AC_MSG_CHECKING([installing gmp? ])
    44    if test x$sage_spkg_install_gmp = xyes; then
    55        AC_MSG_RESULT([yes; install mpfr as well])
    66        sage_spkg_install_mpfr=yes
  • m4/sage_spkg_collect.m4

    diff --git a/m4/sage_spkg_collect.m4 b/m4/sage_spkg_collect.m4
    index 1ab62d6fa4..c364ab62b7 100644
    a b for DIR in $SAGE_ROOT/build/pkgs/*; do 
    259259        AS_VAR_POPDEF([sage_require])dnl
    260260        AS_VAR_POPDEF([sage_spkg_install])dnl
    261261
    262     # Packages that should be included in the source distribution
    263     # This includes all standard packages and two special cases
    264     case "$SPKG_NAME" in
    265     mpir)
    266         in_sdist=yes
    267         ;;
    268     esac
    269 
    270262    # Determine package source
    271263    #
    272264    if test -f "$DIR/requirements.txt"; then

comment:26 follow-up: Changed 2 months ago by mkoeppe

All looking fine, please push them to the ticket

comment:27 Changed 2 months ago by jhpalmieri

  • Branch changed from u/mkoeppe/remove_package_mpir to u/jhpalmieri/remove_package_mpir

comment:28 in reply to: ↑ 26 Changed 2 months ago by jhpalmieri

  • Commit changed from 34526ca3ce293701c88607ec385a35b9d8a33e27 to e3ff6aa5aacc6cde5ea466322878a69437b2b376

Replying to mkoeppe:

All looking fine, please push them to the ticket

Done


New commits:

e3ff6aatrac 32549: misc small changes, plus don't include mpir in sdist

comment:29 Changed 2 months ago by mkoeppe

  • Authors changed from Matthias Koeppe to Matthias Koeppe, John Palmieri

comment:30 Changed 2 months ago by dimpase

Let's use this ticket to update to consistently use SAGE_SPKG_DEPCHECK instead of that if test ... in all the packages affected.

comment:31 Changed 2 months ago by dimpase

  • Branch changed from u/jhpalmieri/remove_package_mpir to u/dimpase/remove_package_mpir
  • Commit changed from e3ff6aa5aacc6cde5ea466322878a69437b2b376 to ccd0ff5770e18037d638e31c2cf67fdbd3f21b22

the monster isn't completely gone yet:

build//pkgs/ppl/spkg-install.in:# Make sure that we prefer Sage's mpir library over system-wide gmp/mpir installs
src/doc/it/faq/faq-usage.rst:    rm spkg/installed/mpir* spkg/installed/atlas*
src/doc/it/faq/faq-general.rst:`MPIR <http://www.mpir.org>`_,
src/doc/en/faq/faq-usage.rst:    $ rm spkg/installed/mpir* spkg/installed/atlas*
src/doc/en/faq/faq-general.rst:`MPIR <http://www.mpir.org>`_,
src/sage/ext_data/valgrind/sage-additional.supp:   fun:sage_mpir_malloc
src/sage/ext_data/valgrind/sage-additional.supp:   mpir_realloc
src/sage/ext_data/valgrind/sage-additional.supp:   fun:sage_mpir_realloc
src/sage/ext/memory.pyx:    sage: 2^(2^63-2)      # optional - mpir
src/sage/all.py:    # Free globally allocated mpir integers.

taking care of this now.


New commits:

ccd0ff5use SAGE_SPKG_DEPCHECK
Last edited 2 months ago by dimpase (previous) (diff)

comment:32 Changed 2 months ago by git

  • Commit changed from ccd0ff5770e18037d638e31c2cf67fdbd3f21b22 to 07da6c3872534e01975d3d90affc102a19a8169d

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

e400f0emakes gmp test unconditional, remove mpir test
07da6c3fix mpir mentions in FAQs and source comments

comment:33 Changed 2 months ago by dimpase

  • Authors changed from Matthias Koeppe, John Palmieri to Matthias Koeppe, John Palmieri, Dima Pasechnik
  • Reviewers set to Dima Pasechnik

comment:34 Changed 2 months ago by mkoeppe

  • Dependencies set to #30350
  • Reviewers changed from Dima Pasechnik to Dima Pasechnik, Matthias Koeppe

Looking good. Let's merge #30350 (ATLAS removal) to resolve merge conflicts in the documentation and other places

Last edited 2 months ago by mkoeppe (previous) (diff)

comment:35 Changed 2 months ago by mkoeppe

  • Branch changed from u/dimpase/remove_package_mpir to u/mkoeppe/remove_package_mpir

comment:36 Changed 2 months ago by git

  • Commit changed from 07da6c3872534e01975d3d90affc102a19a8169d to 9ed04f76161ee49e89e961e764fb41414286fa98

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

9ed04f7Merge tag '9.5.beta2' into t/32549/remove_package_mpir

comment:37 Changed 2 months ago by dimpase

there were a couple of docbuild errors in FAQs in my branch. Should merge the latest commit from u/dimpase/remove_package_mpir

comment:38 Changed 2 months ago by git

  • Commit changed from 9ed04f76161ee49e89e961e764fb41414286fa98 to 7c84f31668b4de402673fe8431ef77d7d9e0fcfc

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

8c071a8https-fy consistently
7c84f31Merge branch 'u/dimpase/remove_package_mpir' of git://trac.sagemath.org/sage into t/32549/remove_package_mpir

comment:39 Changed 2 months ago by dimpase

  • Status changed from needs_review to positive_review

OK, this works.

comment:40 Changed 2 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:41 Changed 2 months ago by git

  • Commit changed from 7c84f31668b4de402673fe8431ef77d7d9e0fcfc to 286b39c22f710795775506cfbdcb0cb3be51ced3

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

2576f86build/make/Makefile.in: If a script package has no spkg-install, run "sage -info" and exit with error
feb8de7build/pkgs/: Remove spkg-install scripts for dummy script packages
8f782c0.github/workflows/tox-{optional,experimental}.yml: Do not try to test dummy script packages
4b292bebuild/pkgs/perl_mongodb/spkg-install: Remove
a7b6352build/bin/sage-spkg-info: Fix display of system packages
e65b309bootstrap: Do not provide ./configure --enable-SPKG options for dummy optional packages
b485d46m4/sage_spkg_collect.m4: Do not advertise dummy optional packages as installable
286b39cMerge #31163

comment:42 Changed 2 months ago by mkoeppe

  • Dependencies changed from #30350 to #30350, #31163
  • Status changed from needs_work to positive_review

comment:43 Changed 8 weeks ago by vbraun

  • Branch changed from u/mkoeppe/remove_package_mpir to 286b39c22f710795775506cfbdcb0cb3be51ced3
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:44 Changed 7 weeks ago by was

  • Commit 286b39c22f710795775506cfbdcb0cb3be51ced3 deleted

perhaps should we ask to the people involved in MPIR how they see the future of MPIR ?

Bill also told me recently that MPIR is not going forward at this point for a combination of reasons. +1 to this ticket.

Note: See TracTickets for help on using tickets.