Remove package mpir
Closed this issue · 66 comments
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 #29620 comment:10
Depends on #30350
Depends on #31163
CC: @jhpalmieri @orlitzky @dimpase @wbhart brg@gladman.plus.com riemannic@gmail.com
Component: packages: standard
Author: Matthias Koeppe, John Palmieri, Dima Pasechnik
Branch: 286b39c
Reviewer: Dima Pasechnik, Matthias Koeppe
Issue created by migration from https://trac.sagemath.org/ticket/32549
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 ?
+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.
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.
Branch: u/mkoeppe/remove_package_mpir
Author: Matthias Koeppe
Branch pushed to git repo; I updated commit sha1. New commits:
fabdbb3 | build/pkgs/*/SPKG.rst: Clear out redundant 'Dependencies' sections |
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.
Branch pushed to git repo; I updated commit sha1. New commits:
724c3ff | src/doc/en/developer/portability_testing.rst: Remove references to package yasm |
Branch pushed to git repo; I updated commit sha1. New commits:
519dc83 | m4/sage_spkg_configure.m4: In documentation, do not use yasm as an example |
Branch pushed to git repo; I updated commit sha1. New commits:
01406bf | build/pkgs/*/SPKG.rst: Clear out more redundant 'Dependencies' sections |
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'):
Branch pushed to git repo; I updated commit sha1. New commits:
1eb83e3 | build/pkgs/ratpoints/SPKG.rst: Remove redundant 'Dependencies' section |
Replying to @mkoeppe:
In two places of the Sage library, also
'mpir'appears as a possible value of analgorithmparameter: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.
Branch pushed to git repo; I updated commit sha1. New commits:
f883ea4 | Integer.binomial: Add algorithm='gmp', keep 'mpir' as an alias |
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
there is still an #optional : mpir tag in integer.pyx, line 6427.
And analogous gmp tag few lines down.
Branch pushed to git repo; I updated commit sha1. New commits:
1647e2f | src/sage/rings/integer.pyx: Rewrite a doctest so it works with gmp, mpir, 64bit, 32bit |
Replying to @dimpase:
there is still an
#optional : mpirtag in integer.pyx, line 6427.
And analogousgmptag 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
Branch pushed to git repo; I updated commit sha1. New commits:
34526ca | build/pkgs/*/SPKG.rst: Remove some more redundant 'Dependencies' sections |
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...)
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
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?
Here are some proposed changes:
diff --git a/COPYING.txt b/COPYING.txt
index 2da3a8e492..7ce750a1aa 100644
--- a/COPYING.txt
+++ b/COPYING.txt
@@ -86,7 +86,6 @@ mistune Modified BSD
mpc LGPLv3+
mpfi COPYING is GPLv2, source files state LGPLv2.1+
mpfr LGPLv3+
-mpir LGPLv3+
mpmath Modified BSD
networkx Modified BSD
ntl GPLv2+
diff --git a/build/pkgs/ecm/spkg-configure.m4 b/build/pkgs/ecm/spkg-configure.m4
index e62c21cb32..d5302a89fb 100644
--- a/build/pkgs/ecm/spkg-configure.m4
+++ b/build/pkgs/ecm/spkg-configure.m4
@@ -1,7 +1,7 @@
SAGE_SPKG_CONFIGURE([ecm], [
m4_pushdef([SAGE_ECM_MINVER],[7.0.4])
AC_REQUIRE([SAGE_SPKG_CONFIGURE_GMP])
- AC_MSG_CHECKING([installing gmp/mpir? ])
+ AC_MSG_CHECKING([installing gmp? ])
if test x$sage_spkg_install_gmp = xyes; then
AC_MSG_RESULT([yes; install ecm as well])
sage_spkg_install_ecm=yes
diff --git a/build/pkgs/gdb/SPKG.rst b/build/pkgs/gdb/SPKG.rst
index b89773fe67..0716ac53fa 100644
--- a/build/pkgs/gdb/SPKG.rst
+++ b/build/pkgs/gdb/SPKG.rst
@@ -19,17 +19,6 @@ Upstream Contact
http://www.gnu.org/software/gdb/
-Dependencies
-------------
-
-- python
-- mpc
-- mpfr
-- ppl
-- gmp/mpir
-- makeinfo (external)
-
-
Special Update/Build Instructions
---------------------------------
diff --git a/build/pkgs/isl/spkg-configure.m4 b/build/pkgs/isl/spkg-configure.m4
index 1b16f70870..d7bbef80c6 100644
--- a/build/pkgs/isl/spkg-configure.m4
+++ b/build/pkgs/isl/spkg-configure.m4
@@ -1,6 +1,6 @@
SAGE_SPKG_CONFIGURE([isl], [
AC_REQUIRE([SAGE_SPKG_CONFIGURE_GMP])
- AC_MSG_CHECKING([installing gmp/mpir? ])
+ AC_MSG_CHECKING([installing gmp? ])
if test x$sage_spkg_install_gmp = xyes; then
AC_MSG_RESULT([yes; install isl as well])
sage_spkg_install_isl=yes
diff --git a/build/pkgs/mpfr/spkg-configure.m4 b/build/pkgs/mpfr/spkg-configure.m4
index 2300be6707..f64ede4b75 100644
--- a/build/pkgs/mpfr/spkg-configure.m4
+++ b/build/pkgs/mpfr/spkg-configure.m4
@@ -1,6 +1,6 @@
SAGE_SPKG_CONFIGURE([mpfr], [
AC_REQUIRE([SAGE_SPKG_CONFIGURE_GMP])
- AC_MSG_CHECKING([installing gmp/mpir? ])
+ AC_MSG_CHECKING([installing gmp? ])
if test x$sage_spkg_install_gmp = xyes; then
AC_MSG_RESULT([yes; install mpfr as well])
sage_spkg_install_mpfr=yes
diff --git a/m4/sage_spkg_collect.m4 b/m4/sage_spkg_collect.m4
index 1ab62d6fa4..c364ab62b7 100644
--- a/m4/sage_spkg_collect.m4
+++ b/m4/sage_spkg_collect.m4
@@ -259,14 +259,6 @@ for DIR in $SAGE_ROOT/build/pkgs/*; do
AS_VAR_POPDEF([sage_require])dnl
AS_VAR_POPDEF([sage_spkg_install])dnl
- # 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
-
# Determine package source
#
if test -f "$DIR/requirements.txt"; thenAll looking fine, please push them to the ticket
Changed branch from u/mkoeppe/remove_package_mpir to u/jhpalmieri/remove_package_mpir
Changed author from Matthias Koeppe to Matthias Koeppe, John Palmieri
Let's use this ticket to update to consistently use SAGE_SPKG_DEPCHECK instead of that if test ... in all the packages affected.
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:
ccd0ff5 | use SAGE_SPKG_DEPCHECK |
Changed branch from u/jhpalmieri/remove_package_mpir to u/dimpase/remove_package_mpir
Reviewer: Dima Pasechnik
Changed author from Matthias Koeppe, John Palmieri to Matthias Koeppe, John Palmieri, Dima Pasechnik
Looking good. Let's merge #30350 (ATLAS removal) to resolve merge conflicts in the documentation and other places
Changed reviewer from Dima Pasechnik to Dima Pasechnik, Matthias Koeppe
Changed branch from u/dimpase/remove_package_mpir to u/mkoeppe/remove_package_mpir
Branch pushed to git repo; I updated commit sha1. New commits:
9ed04f7 | Merge tag '9.5.beta2' into t/32549/remove_package_mpir |
there were a couple of docbuild errors in FAQs in my branch. Should merge the latest commit from u/dimpase/remove_package_mpir
OK, this works.
Merge conflict
Branch pushed to git repo; I updated commit sha1. New commits:
2576f86 | build/make/Makefile.in: If a script package has no spkg-install, run "sage -info" and exit with error |
feb8de7 | build/pkgs/: Remove spkg-install scripts for dummy script packages |
8f782c0 | .github/workflows/tox-{optional,experimental}.yml: Do not try to test dummy script packages |
4b292be | build/pkgs/perl_mongodb/spkg-install: Remove |
a7b6352 | build/bin/sage-spkg-info: Fix display of system packages |
e65b309 | bootstrap: Do not provide ./configure --enable-SPKG options for dummy optional packages |
b485d46 | m4/sage_spkg_collect.m4: Do not advertise dummy optional packages as installable |
286b39c | Merge #31163 |
Changed branch from u/mkoeppe/remove_package_mpir to 286b39c
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.
Changed commit from 286b39c to none