#26025 closed enhancement (fixed)

Sage should not use GP_DATA_DIR

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.4
Component: interfaces Keywords:
Cc: gh-timokau, fbissey Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 63f8844 (Commits) Commit: 63f8844f96acb90228a64b179a9b1fcf08e78dae
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

It's an environment variable specific to PARI/GP and the location which is set in interfaces/gp.py is the default anyway.

Change History (20)

comment:1 Changed 16 months ago by jdemeyer

  • Branch set to u/jdemeyer/sage_should_not_use_gp_data_dir

comment:2 Changed 16 months ago by fbissey

  • Commit set to 63f8844f96acb90228a64b179a9b1fcf08e78dae

Yes, my experience suggests that's not useful.


New commits:

63f8844No need to set GP datadir to GP_DATA_DIR

comment:3 Changed 16 months ago by jdemeyer

  • Status changed from new to needs_review

It's not useful because PARI already uses the GP_DATA_DIR environment variable for that.

comment:4 Changed 16 months ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

Looks good to me.

comment:5 Changed 16 months ago by jdemeyer

  • Description modified (diff)

comment:6 in reply to: ↑ description ; follow-up: Changed 16 months ago by gh-timokau

  • Status changed from positive_review to needs_info

Replying to jdemeyer:

is the default anyway.

Well that's not the point of configurability. Also how can the default be in SAGE_SHARE? Is it determined relative to the pari binary? Or how else is it determined? Is GP_DATA_DIR still respected after this change?

comment:7 Changed 16 months ago by fbissey

From pari's code

src/language/init.c:  pari_datadir = os_getenv("GP_DATA_DIR");

So it is enough for the variable to be in the environment.

comment:8 in reply to: ↑ 6 Changed 16 months ago by jdemeyer

  • Status changed from needs_info to positive_review

Replying to gh-timokau:

Is GP_DATA_DIR still respected after this change?

Yes.

comment:9 Changed 16 months ago by jdemeyer

The point of this ticket is that there is no point for Sage to deal with GP_DATA_DIR since PARI already deals with it.

comment:10 Changed 16 months ago by gh-timokau

Okay, thanks for the clarification.

comment:11 Changed 16 months ago by gh-timokau

Does one of you know if there is a reliable way of setting GP_DATA_DIR as a distribution? I thought about a wrapper for the pari binaries, but that won't work when pari is used as a library. A better option seems to be /etc/gprc, but that is apparently overwritten (instead of extended) by ~/.gprc.

comment:12 Changed 16 months ago by gh-timokau

I just noticed that its possible to set --datadir at configure time. Thats not perfect because it needs re-compilation when adding data, but its something. If somebody knows a better option, that would be appreciated.

(Got that here: http://pari.math.u-bordeaux.fr/dochtml/gpman.html)

comment:13 follow-up: Changed 16 months ago by fbissey

Well, it depends a bit on the distribution but you can put special files under /etc/env.d/ or /etc/profile.d - whichever is available - and they will be sourced on interactive login. An example under ubuntu

frb15@p7login-rcc:~$ ll /etc/profile.d/
total 36
drwxr-xr-x   2 root root  4096 Jul 31 22:30 ./
drwxr-xr-x 112 root root 12288 Aug  7 12:49 ../
-rw-r--r--   1 root root   825 Jul 19 22:48 apps-bin-path.sh
-rw-r--r--   1 root root   663 May 18  2016 bash_completion.sh
-rw-r--r--   1 root root  1003 Dec 29  2015 cedilla-portuguese.sh
-rw-r--r--   1 root root   168 Mar 17  2017 pwr7-prefix_path.sh
-rw-r--r--   1 root root  1557 Apr 15  2016 Z97-byobu.sh

If you look the first one for example

frb15@p7login-rcc:~$ cat /etc/profile.d/apps-bin-path.sh 
# shellcheck shell=sh

# Expand $PATH to include the directory where snappy applications go.
snap_bin_path="/snap/bin"
if [ -n "${PATH##*${snap_bin_path}}" -a -n "${PATH##*${snap_bin_path}:*}" ]; then
    export PATH=$PATH:${snap_bin_path}
fi

# Ensure base distro defaults xdg path are set if nothing filed up some
# defaults yet.
if [ -z "$XDG_DATA_DIRS" ]; then
    export XDG_DATA_DIRS="/usr/local/share:/usr/share"
fi

# Desktop files (used by desktop environments within both X11 and Wayland) are
# looked for in XDG_DATA_DIRS; make sure it includes the relevant directory for
# snappy applications' desktop files.
snap_xdg_path="/var/lib/snapd/desktop"
if [ -n "${XDG_DATA_DIRS##*${snap_xdg_path}}" -a -n "${XDG_DATA_DIRS##*${snap_xdg_path}:*}" ]; then
    export XDG_DATA_DIRS="${XDG_DATA_DIRS}:${snap_xdg_path}"
fi

So it is totally possible for your pari or pari database package to add such a file with the right value for GP_DATA_DIR.

comment:14 in reply to: ↑ 13 ; follow-up: Changed 16 months ago by gh-timokau

Replying to fbissey:

Well, it depends a bit on the distribution but you can put special files under /etc/env.d/ or /etc/profile.d - whichever is available - and they will be sourced on interactive login. An example under ubuntu

Thanks for the suggestion. However I think it is not the best one for me in this case. It requires setting global environment variables for the user, which is not ideal. It will also break if the user uses env or something to clear their env vars.

When using --datadir to point to somewhere else than <pari-prefix>/share/pari, the cypari build breaks because it depends on files in share/pari (that are not installed datasets). I'm not sure if the same would have happened with GP_DATA_DIR or if --datadir has somewhat different semantics.

The best solution I see is to symlink the installed datasets to <pari-prefix>/share/pari. Just as the --configure option, that will require rebuilds. Maybe using different datasets is uncommon enough in practice that thats not a big deal.

comment:15 Changed 16 months ago by fbissey

The actual code in pari setting the datadir at runtime

  pari_datadir = os_getenv("GP_DATA_DIR");
  if (!pari_datadir)
  {
#if defined(_WIN32) || defined(__CYGWIN32__)
    if (paricfg_datadir[0]=='@' && paricfg_datadir[1]==0)
      pari_datadir = win32_datadir();
    else
#endif
      pari_datadir = pari_strdup(paricfg_datadir);
  }
  else pari_datadir= pari_strdup(pari_datadir);

So GP_DATA_DIR over-rides the default location set by --datadir at runtime.

comment:16 in reply to: ↑ 14 ; follow-up: Changed 16 months ago by jdemeyer

Replying to gh-timokau:

When using --datadir to point to somewhere else than <pari-prefix>/share/pari, the cypari build breaks because it depends on files in share/pari (that are not installed datasets).

That could be considered a bug in cypari2. I might fix it, but it's not high priority for me. I will accept a pull request though :-)

comment:17 in reply to: ↑ 16 Changed 16 months ago by gh-timokau

Replying to jdemeyer:

Replying to gh-timokau:

When using --datadir to point to somewhere else than <pari-prefix>/share/pari, the cypari build breaks because it depends on files in share/pari (that are not installed datasets).

That could be considered a bug in cypari2. I might fix it, but it's not high priority for me. I will accept a pull request though :-)

The --datadir solution doesn't really have any advantages over just symlinking, but it can't hurt to fix it: https://github.com/defeo/cypari2/pull/71

comment:18 Changed 16 months ago by gh-timokau

I applied this patch on the sage nix package, removed the line that was setting GP_DATA_DIR and symlinked the data packages into pari's share folder. However something is wrong:

    TypeError: Error executing code in GP:
    CODE:
        sage[27]=polgalois(sage[26]);
    PARI/GP ERROR:
      ***   at top-level: sage[27]=polgalois(sage[26])
      ***                          ^-------------------
      *** polgalois: error opening galois file %s: `/nix/store/rcfgmlmqx4iffikynvb7d5dc0gzvv3al-sage-with-env-8.3/share/pari/galdata/NAM9'.

And various other errors of that type. For some reason, gp looks in <sage-prefix>/share/pari/galdata instead of <pari-prefix>/share/pari/galdata.

print(default(datadir)) in gp gives <pari-prefix>/share/pari, as expected.

I only get errors referring to galdata. This worked when I was setting GP_DATA_DIR to a non-standard location. Not sure where that error is coming from.

comment:19 Changed 16 months ago by gh-timokau

Never mind, I messed up the symlinking with a missing closing quote. Still a weird error message though.

comment:20 Changed 16 months ago by vbraun

  • Branch changed from u/jdemeyer/sage_should_not_use_gp_data_dir to 63f8844f96acb90228a64b179a9b1fcf08e78dae
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.