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

system pari is not used when optional pkg pari_seadata is not installed #32425

Open
tornaria opened this issue Aug 26, 2021 · 9 comments
Open

Comments

@tornaria
Copy link
Contributor

Context: system pari is installed with databases elldata, galdata, galpol and seadata-small.

Behaviour: system pari is discarded because pari-seadata is not isntalled; in fact, sage will not install pari-seadata either because it is optional.

Expected behaviour: system pari is used.

Trivial fix:

diff --git a/build/pkgs/pari/spkg-configure.m4 b/build/pkgs/pari/spkg-configure.m4
index 02d472c0c8..162792d414 100644
--- a/build/pkgs/pari/spkg-configure.m4
+++ b/build/pkgs/pari/spkg-configure.m4
@@ -56,13 +56,13 @@ SAGE_SPKG_CONFIGURE([pari], [
             AC_MSG_NOTICE([Otherwise Sage will build its own pari/GP.])
             sage_spkg_install_pari=yes
         fi
-        AC_MSG_CHECKING([is pari_seadata installed? ])
-        gp_seadat_check=`echo "poldegree(ellmodulareqn(211)[[1]])" | $GP -qf 2>> config.log`
-        if test x$gp_seadat_check = x212; then
+        AC_MSG_CHECKING([is pari_seadata_small installed? ])
+        gp_seadat_check=`echo "poldegree(ellmodulareqn(11)[[1]])" | $GP -qf 2>> config.log`
+        if test x$gp_seadat_check = x12; then
             AC_MSG_RESULT([yes])
         else
-            AC_MSG_RESULT([no; cannot use system pari/GP without seadata package])
-            AC_MSG_NOTICE([Install seadata package and reconfigure.])
+            AC_MSG_RESULT([no; cannot use system pari/GP without seadata-small package])
+            AC_MSG_NOTICE([Install seadata-small package and reconfigure.])
             AC_MSG_NOTICE([Otherwise Sage will build its own pari/GP.])
             sage_spkg_install_pari=yes
         fi

CC: @dkwo @dimpase @orlitzky

Component: packages: standard

Keywords: pari

Author: Gonzalo Tornaría

Branch/Commit: public/32425 @ 6b8e901

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

@tornaria
Copy link
Contributor Author

Commit: 6b8e901

@tornaria
Copy link
Contributor Author

Branch: public/32425

@tornaria
Copy link
Contributor Author

New commits:

6b8e901Trac #32425: accept pari-seadata-small system package

@dimpase
Copy link
Member

dimpase commented Sep 21, 2021

comment:5

If system-wide pari-seadata is not installed, by system-wide pari is used, Sage has no way to install its pari_seadata spkg.

That's why we check for pari-seadata, not for its smaller brother.

Thus, this patch will break an optional package for no good reason.

@orlitzky
Copy link
Contributor

comment:6

PARI and its data packages are a bit of a mess because its "datadir" can't be overridden:

https://pari.math.u-bordeaux.fr/dochtml/html-stable/GP_defaults.html#datadir

As a result, the optional packages all have to be installed the same way that pari itself was installed, and is why the pari spkg-configure does checks that you would think belong in the data packages themselves.

For a real fix, someone needs to ask upstream about making the datadir use a search path override with an environment variable like PARI_DATADIR. Then we could install the optional data anywhere we want, and add that location to the search path, even if pari was installed by someone else.

@orlitzky
Copy link
Contributor

comment:7

I sent a feature request upstream but it hasn't shown up on the bug tracker yet.

Edit: https://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=2316

@orlitzky
Copy link
Contributor

comment:8

I think we are on our own with the datadir. So long as the data packages cannot be installed independently, they don't really make sense as optional SPKGs, and our "pari" package is really "pari with all of the additional data." They must all be installed the same way, either as system packages, or as SPKGs.

Given that, I think it makes the most sense to keep requiring the full seadata package, just to keep the (already complicated) mess as simple as possible. But I don't feel strongly about it.

@dimpase dimpase removed this from the sage-9.5 milestone Sep 21, 2021
@tornaria
Copy link
Contributor Author

comment:10

I understand the issue. However, if one has pari-seadata-small installed from distro, then one can install pari-seadata from distro as well. As an added advantage one can also install pari-seadata-big if necessary.

The situation where this change would be a problem is a user without root privileges compiling sage in a system where pari is installed with pari-seadata-small, but in fact needing pari-seadata. I think this situation has --with-system-pari=no as a workaround.

On the other hand, if I want to compile sage with distro pari and just pari-seadata-small I can't, and --with-system-pari=force won't work either.

Maybe it's possible that installation of seadata as a sage package (or any pari database, for that matter) will check that pari itself is installed as a sage package. In case it's not, it would refuse to install explaining to the user to either (a) install the package from distro; or (b) use configure to disable system-pari and recompile sage (hopefully this will just install pari, gp2c, giac, and relink sage).

Also, a warning could be printed the first time configure is run to indicate that pari-seadata is not installed and suggesting to disable system pari if one might need extra packages and can't install them.

I guess all of this applies as well to pari_elldata, pari_galpol and pari_nftables, which are also optional.

@dimpase
Copy link
Member

dimpase commented Sep 25, 2021

comment:11

Why do people insist on using pari-seadata-small, etc?

Should we send you $20 for a bigger disk? :-)

But, seriously, without upstream cooperation, involving designing a better way to handle datadir, these is no way out here.

What one still can do is to provide a switch to turn these checks into different checks, using -small versions of packages, instead. But this is a feature bloat.

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