hardwired paths in src/sage

[felixs]

Several run time paths in the python library are hardwired, in most cases to $SAGE_LOCAL. $SAGE_LOCAL has no meaning outside of Sage (the distribution). So it's better to add them to sage-env.py and make them overridable.

For example in libs.singular.singular_init, the singular shared object is looked for in $SAGE_LOCAL/lib.

[felixs]

Removed the ipython-sage-location check, as it involves $SAGE_LOCAL. Thus dependency on #15100.

[git]

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

[changeset:c19a5f3]	unhardwire sandpile
[changeset:5746289]	unhardwire polymake path
[changeset:8f8a246]	get sage-started.txt from SAGE_ETC
[changeset:ee831c1]	unardwire sage-CSI call in csage
[changeset:16bf607]	initialize SAGE_ETC (from environ) in sage-location before use

[git]

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

[changeset:591471d]

python-sage: whitelist python2.7 multiprocessing in stackframes test

[git]

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

[changeset:0b9cb04]	doctests in src/sage/plot/misc.py
[changeset:1ef62a1]	unhardwire cython.py completed
[changeset:390e28b]	qepcard doctest fix: singular might well be in /usr/bin, not .../local/bin
[changeset:0dcd721]	unhardwire ELLCURVES_DATA_DIR

[jdemeyer]

Needs to be rebased. Given that I totally disagree with #15100, preferably this ticket should not depend on that.

[felixs]

Replying to jdemeyer:

Needs to be rebased. Given that I totally disagree with #15100, preferably this ticket should not depend on that.

#15100 is not too important. particularly for standalone sagelib it doesn't matter. anyhow, the doctest checking the fist line of the ipython (or any other) executable needs to be removed.

[mkoeppe]

This patch needs rebasing

[git]

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

​faa99e7	Merge branch 'master' into runtime_paths
​12f6985	refresh more runtime_paths commits

[git]

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

​8149a85

Merge 'develop' into runtime_paths

[felixs]

this is now rebased.

i tried to resolve the conflicts carefully, but i haven't compiled/tested anything. can't do that anytime soon (no time, no hardware).

hth

[fbissey]

You still have stuff from old version of sage that are not there anymore. All the files where the diff starts with /dev/null. Better get rid of them.

[git]

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

​4846a73

merge develop into runtime_paths

[felixs]

Replying to fbissey:

Better get rid of them.

thanks. missed that. i took that opportunity to squash the three merges into one.

[mkoeppe]

Instead of sage.misc.misc, shouldn't it use sage.env?

[fbissey]

--- a/src/bin/sage-location
+++ b/src/bin/sage-location
@@ -6,6 +6,7 @@ import os, sys, re
 
 SAGE_ROOT     = os.path.realpath(os.environ['SAGE_ROOT'])
 SAGE_LOCAL    = os.environ['SAGE_LOCAL']
+SAGE_ETC      = os.environ['SAGE_ETC']
 
 location_file = os.path.join(SAGE_LOCAL, 'lib', 'sage-current-location.txt')
 force_file    = os.path.join(SAGE_LOCAL, 'lib', 'sage-force-relocate.txt')
@@ -215,8 +216,8 @@ def write_config_files():
     path.
     """
     # Currently only one file (for the experimental package qepcad):
-    # $SAGE_ROOT/local/default.qepcadrc
-    f = open(os.path.join(SAGE_LOCAL, 'default.qepcadrc'), 'w')
+    # $SAGE_ROOT/local/etc/default.qepcadrc
+    f = open(os.path.join(SAGE_ETC, 'default.qepcadrc'), 'w')
     text = \
 """# THIS FILE IS AUTOMATICALLY GENERATED by sage-location -- DO NOT EDIT

I raised the issue of qepcadrc before,but it has to be done in conjunction of fixing the spkg in question.

In sage-on-gentoo I just got rid of sage-started.txt it is one of those appendages that don't really belong anywhere sensible. /var/lib/sage?

I am not sure about the catalogue of DATA_DIR and INCLUDE_DIR (and in line with my previous post csage is gone).

Did you know that lib{s,S}ingular is the only file that is dlopen-ed in sage. There should be a better way of getting to it.

Do not ever use SAGE_SRC for something that's needed at runtime. I could let go of that for doctesting as you do, but that makes things harder to test in sage-on-gentoo.

Overall I am not overly impressed by the amount of stuff showed in env.py. Some of that stuff may also conflict with #20382 which I very much would like to see merged.

[felixs]

I raised the issue of qepcadrc before,but it has to be done in conjunction of fixing the spkg in question.

sage_location (whatever it tries to do) should be moved out of sagelib. it's not needed there. i don't remember what qepcadrc was exactly about, but there was headache involved.

In sage-on-gentoo I just got rid of sage-started.txt

maybe sage should touch ~/.sage/started instead? this is often writable... does sage-started.txt implement anything useful at all?

Did you know that lib{s,S}ingular is the only file that is dlopen-ed in sage. There should be a better way of getting to it.

it's hard to get a broken implementation right. iirc theres has never been a technical reason for not just linking. so maybe... just link? (see my ancient comment in env.py).

Do not ever use SAGE_SRC for something that's needed at runtime. I could let go of that for doctesting as you do, but that makes things harder to test in sage-on-gentoo.

there is some sort of interactive source browser. how would that work without SAGE_SRC? i think i did this [1] (also installed the source) to fix it.

stuff showed in env.py

too many *DIR variables? true. there are better ways: each extension should have its own configuration (read: should be a standalone package depending on sagelib). i think, we are simply not there yet. cf. [2] for the intended (workaround) implementation.

#20382

please go ahead and finish that first. this ticket was never meant to interfere with #20382.

[1] ​https://github.com/felix-salfelder/sage/blob/autotools/src/sage/env.py.in#L103 [2] ​https://github.com/felix-salfelder/sage/blob/autotools/src/sage/env.py.in#L134

[mkoeppe]

Replying to felixs:

stuff showed in env.py

too many *DIR variables? true. there are better ways: each extension should have its own configuration (read: should be a standalone package depending on sagelib). i think, we are simply not there yet. cf. [2] for the intended (workaround) implementation.

I agree with Felix on this.

We do need lots of these variables, because we no longer want to assume that everything lives in the same prefix hierarchy ("SAGE_LOCAL").

Whether these kind of configuration variables should go into env or into another centralized module, is another question.

[fbissey]

Replying to felixs:

I raised the issue of qepcadrc before,but it has to be done in conjunction of fixing the spkg in question.

sage_location (whatever it tries to do) should be moved out of sagelib. it's not needed there. i don't remember what qepcadrc was exactly about, but there was headache involved.

In sage-on-gentoo I just got rid of sage-started.txt

maybe sage should touch ~/.sage/started instead? this is often writable... does sage-started.txt implement anything useful at all?

No, it was introduced as a check by Jeroen (when he was the release manager) to check that sage does starts. So it is produced at the end of the build process and checked and regenerated if missing when sage is started.

Did you know that lib{s,S}ingular is the only file that is dlopen-ed in sage. There should be a better way of getting to it.

it's hard to get a broken implementation right. iirc theres has never been a technical reason for not just linking. so maybe... just link? (see my ancient comment in env.py).

That a novel idea :) but even the singular4 ticket isn't going there.

Do not ever use SAGE_SRC for something that's needed at runtime. I could let go of that for doctesting as you do, but that makes things harder to test in sage-on-gentoo.

there is some sort of interactive source browser. how would that work without SAGE_SRC? i think i did this [1] (also installed the source) to fix it.

I don't really know what that does. Never used it. So in sage-on-gentoo SAGE_SRC points to /usr/lib{,64}/python2.7/site-packages/sage and I install the necessary pxd and pyx to run test. So that probably works. But my attitude on that is that the source shouldn't be considered available at runtime and of course you should be able to run tests before installing. The possibility of doing the last bit is shot down by #21221.

stuff showed in env.py

too many *DIR variables? true. there are better ways: each extension should have its own configuration (read: should be a standalone package depending on sagelib). i think, we are simply not there yet. cf. [2] for the intended (workaround) implementation.

#20382

please go ahead and finish that first. this ticket was never meant to interfere with #20382.

[1] ​https://github.com/felix-salfelder/sage/blob/autotools/src/sage/env.py.in#L103 [2] ​https://github.com/felix-salfelder/sage/blob/autotools/src/sage/env.py.in#L134

Yes I guess if you are thinking of a modular approach where components can be installed anywhere on the system, like hashdist or spack, you want to use variable like CPATH and LIBRARY_PATH, set when you load the components in the environment. Stuff under SAGE_LOCAL/share is still an issue in that case.

On a system like sage-on-gentoo where things still live under a single prefix (but is a more conventional one) keeping SAGE_LOCAL is not a big deal. Apart from etc I just use it like it was --prefix=/usr.

[felixs]

Replying to fbissey:

i appreciate your clear reply. but i'm out of time. maybe next week.

[jdemeyer]

About sage-started.txt: it has two purposes:

1. Check that Sage starts.

2. Initialize any files which should be initialized at build time (this includes running sage-location and generating .pyc files). See #11926.

[jdemeyer]

Regarding

# hmm, who is going to create sage_started if SAGE_ETC is readonly?

This isn't going to happen since this file should be written only by the build process. At that point, SAGE_ETC should be writable.

So please remove this comment.

[jdemeyer]

What is the point of this?

    # Do nothing if the file already exists
    if os.path.isfile(started_file):
        return

make expects the timestamp to be updated.

[felixs]

sage_started is now in #21510

[mkoeppe]

OK, best to remove anything about sage_started from the present ticket's branch then.

[felixs]

sage_location now is #21511.

[mkoeppe]

See also #21591.

[embray]

Definitely agree that none of Sage's external dependencies should be assumed to live under the same prefix. I think it's fine for the sake of using Sage-the-distribution to have defaults that assume everything is under $SAGE_LOCAL, but all these paths should be easy for system integrator to change (it might nice to even have a simple config file read at build time).

[embray]

I rebased this properly on top of develop, rather than trying to merge develop into it, if anyone would like to have a look. I think I got everything right but I might be missing one or two things. I'm not sure what to do with the new SAGE_MAXIMA_INIT this adds.

[felixs]

Replying to embray:

I rebased this properly on top of develop

thanks. can you update the "Branch" field? (is it supposed to work like that?)

I'm not sure what to do with the new SAGE_MAXIMA_INIT this adds.

iirc, this variable was needed to point to some maxima fragment, which is in different places, depending on where sage is run from. obviously there must be a better solution (including one that does not use a file like this one).

[fbissey]

Replying to felixs:

Replying to embray:

I rebased this properly on top of develop

thanks. can you update the "Branch" field? (is it supposed to work like that?)

The branch definitely needs to be changed unless Erik has the right to write to felixs' branches ;) Cannot comment until I see the new branch.

[embray]

I can change the branch to mine, but only if it's ok with felixs. I think I need to pull out the SAGE_MAXIMA_INIT stuff which isn't ready, but otherwise I think it brings it most other changes from the old version of his branch that are still relevant.

[felixs]

Replying to embray:

.. if it's ok with felixs.

sure it is. please do!

[embray]

Here's my version of Felix's branch, rebased on a recent develop branch. I think I preserved all the changes between the original version of his branch, and the more recent version from ​https://git.sagemath.org/sage.git/commit/?id=4846a73aad7cf7361c69a6ae43628b8cae821b9f So this removed many changes from the original branch, such as the change to how sage-maxima.lisp is located.

It would still be good to have a solution for that. But for now, this will let us look at what changes this branch does make (that are still relevant).

---

New commits:

​8cb6f9f	unhardwire runtime paths in various cases
​75cb534	fix qepcadrc path
​c484537	don't rely on $SAGE_LOCAL/bin/python.
​ffed9de	cython.py: don't rely in $SAGE_LOCAL
​4278025	python-sage: remove sage-location check
​807588e	initialize SAGE_ETC (from environ) in sage-location before use

[fbissey]

In src/sage/databases/sql_db.py

-    from sage.env import SAGE_SHARE
+    from sage.misc.misc import GRAPHS_DATA_DIR

Go back to using sage.env rather than sage.misc.misc.

[felixs]

sage.env is the wrong place for this. (not saying that, sage.misc.misc is any better, i hardly remember why it ended up there.).

GRAPHS_DATA_DIR certainly belongs to the part of code that uses graphs data... not to the core library (which i think env.py belongs to).

for the transition it might make sense to open a to be sorted section in env.py... if you agree that doing it right[tm] is not within the scope of this ticket.

[fbissey]

Historically, variables where stored in sage.misc.misc. When we started to setup sage.env as the (more comprehensive) variable store we deprecated the use of sage.misc.misc.

I initially needed such a store in sage-on-gentoo anyway because my shell environment didn't contain them. It allowed me to do import sage.all from the system python prompt amongst other things.

Now I am curious as to why sage.env is not the right place for runtime variables - I would agree that now we should split the build time only variables somewhere else. But I am interested in your reasoning. Bear in mind that any answer from me will be several hours away (need to catch some zzzz).

[embray]

Ah, I thought I replaced most of the sage.misc.misc to with sage.env bug I guess I missed that one.

I don't think there's really any problem with it being in sage.env. If I were to do anything different it would be toward the goal of keeping environment variable definitions close to the code that actually uses them. It might be nice if a subpackage can have a sage.<foo>.env describing variable specifically relevant to that sub-package, but still allow all variables to imported from sage.env (lazily evaluated if necessary).

But I think that's all beyond the scope of this ticket--for now I think throwing everything in sage.env is appropriate.

[felixs]

Now I am curious as to why sage.env is not the right place for runtime variables

because it breaks modularity. the core library should not have to know about extensions.

or the other way: there should be a way to extend sage/sagelib by merely installing something. then GRAPH_DATA (as well as some others listed in sage.env), should use that mechanism.

another remark: this is not just about environment variables. an extension should possibly add *anything*. but environment might be a good example, then generalize.

for now .. throwing everything in sage.env is appropriate.

agreed.

[jdemeyer]

You cannot just unilaterally change the path for default.qepcadrc, see 23

[jdemeyer]

In pselect.pyx, maybe it's better to use sys.executable instead of os.path.join(SAGE_LOCAL, 'bin', 'python').

Because now the patch just replaces one hard-coded path by another.

[jdemeyer]

I also don't agree with the removal of the test in src/sage/tests/cmdline.py. The doctest has a function (it checks that sage-location was run), you should not just remove it.

[jdemeyer]

There is #15146 to deal with sage-location and the test in src/sage/tests/cmdline.py

[jdemeyer]

In sage.env, use SAGE_LOCAL instead of os.environ['SAGE_LOCAL'].

[jdemeyer]

The Singular changes will conflict with #17254.

[jdemeyer]

Replace GAP_DATA_DIR with GAP_DIR. It's not a data dir, it's the installation directory of the whole GAP package.

[jdemeyer]

In src/sage/geometry/lattice_polytope.py, instead of assigning data_location = POLYTOPE_DATA_DIR, it is cleaner to just use the variable POLYTOPE_DATA_DIR directly.

[jdemeyer]

Same comment in src/sage/schemes/elliptic_curves/ec_database.py: you can shorten

        from sage.env import ELLCURVE_DATA_DIR
        db = ELLCURVE_DATA_DIR
        data = os.path.join(db,'rank%s'%rank)

to

        from sage.env import ELLCURVE_DATA_DIR
        data = os.path.join(ELLCURVE_DATA_DIR, 'rank%s'%rank)

[jdemeyer]

These seem unused:

# used by cython.py
_add_variable_or_fallback('SINGULAR_INCLUDEDIR', opj('$SAGE_LOCAL','include','singular'))
_add_variable_or_fallback('FACTORY_INCLUDEDIR',  opj('$SAGE_LOCAL','include','factory'))

[jdemeyer]

Replace CONWAY_DATA_DIR by CONWAY_POLYNOMIALS_DATA_DIR because both the package and directory are named conway_polynomials and not just conway.

[felixs]

Replying to jdemeyer:

Replace GAP_DATA_DIR with GAP_DIR. It's not a data dir, it's the installation directory of the whole GAP package.

thanks for the feedback.

what do you mean by 'installation directory of the whole GAP package'? the prefix?

note that the datadir (ought to be platform independent!) usually/generally/commonly not hardcoded to the prefix, hence the different name.

[jdemeyer]

Replying to felixs:

what do you mean by 'installation directory of the whole GAP package'? the prefix?

GAP has an unusual installation process, it doesn't really have a prefix in the usual sense.

[felixs]

Replying to jdemeyer:

GAP has an unusual installation process, it doesn't really have a prefix in the usual sense.

so, it's not prefix (it does not seem relevant, does it?).

paths are subject to the conventions and standards on particular platforms (e.g. FSH), not to an "unusual installation process". package data usually goes to /usr/share/$packagename, and thats where i can find the gap data on my platform. hence, here, /usr/share/gap is GAP_DATA_DIR, and not GAP_DIR as you suggest.

(on a different platform it might be in a different place, but it's still "the place where the gap data is).

[mkoeppe]

For reference, I believe Felix is referring to ​https://www.gnu.org/prep/standards/standards.html#Directory-Variables and ​https://www.gnu.org/software/automake/manual/automake.html#Uniform

[felixs]

yes. and not just GNU, see ​https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard

[jdemeyer]

Replying to felixs:

package data usually goes to /usr/share/$packagename, and thats where i can find the gap data on my platform.

What do you mean with "gap data"?

Anyway, where your distro installs GAP does not matter. We are working on Sage here, and $SAGE_LOCAL/gap/latest is the directory where GAP as a whole is installed. It's not a directory containing data files, so GAP_DATA_DIR is a bad name.

[jdemeyer]

Replying to felixs:

paths are subject to the conventions and standards on particular platforms (e.g. FSH), not to an "unusual installation process".

Accept the current reality: GAP has an "unusual installation process". We should use names which refer to the reality, not to some standard which doesn't apply for GAP.

[felixs]

Replying to jdemeyer:

What do you mean with "gap data"?

the platform independent data files shipped with ("installed by", "included in", "belonging to") the gap package.

[mkoeppe]

I agree with Jeroen that the current practice of the GAP installation cannot be described in terms of standards. Let's call it "unusual" in writing.

Do we know how distributions like Debian handle GAP?

[felixs]

Do we know how distributions like Debian handle GAP?

on debian (and derivatives), you can find the platform independent gap data files in /usr/share/gap.

[mkoeppe]

Jeroen, Felix: I don't have time to look into any specifics with GAP, but I would suggest that the directory variables are defined and named in a way that works both with

• with the status quo of how the Sage spkg installs GAP into $SAGE_LOCAL,
• and with the way how a distribution such as Debian installs GAP into /usr.

On the other hand, if there's a wish to change the way how the Sage spkg install GAP, then that clearly belongs on a different ticket (please open it if it does not exist yet), not on this one.

[jdemeyer]

Replying to felixs:

Replying to jdemeyer:

What do you mean with "gap data"?

the platform independent data files shipped with ("installed by", "included in", "belonging to") the gap package.

Right. With that definition, $SAGE_LOCAL/gap/latest is not the GAP data dir so GAP_DATA_DIR is a bad name.

[felixs]

GAP_DATA_DIR is a bad name.

no it's not. it specifies the location of the (platform indepentdent static, package specific) data shipped with the gap package. it's meant to be used to lead to exactly that. no more, no less.

i really don't see your problem here, Jeroen. can you explain what you want to use GAP_DIR for?

if you intend to use GAP)DIR for something other than the (platform indepentdent static, package specific) gap data, then you will break a standard FHS install. i know you are against standards, but why would you want that terrible implicit breakage here?

mkoeppe: no, nobody said the spkg needs to be changed, and in fact it does not have to.

[jdemeyer]

Replying to felixs:

i know you are against standards

Please read ​https://en.wikipedia.org/wiki/Ad_hominem

I am not against standards, I am against denying reality. If the current reality is that GAP is not installed following the FHS standards, then we should just accept that fact.

[fbissey]

Replying to felixs:

Now I am curious as to why sage.env is not the right place for runtime variables

because it breaks modularity. the core library should not have to know about extensions.

or the other way: there should be a way to extend sage/sagelib by merely installing something. then GRAPH_DATA (as well as some others listed in sage.env), should use that mechanism.

another remark: this is not just about environment variables. an extension should possibly add *anything*. but environment might be a good example, then generalize.

for now .. throwing everything in sage.env is appropriate.

agreed.

OK your new requirements and goals are different from the ones that lead to the emergence of env.py. I can accept that we have to move on to a new state at some point in the future.

[fbissey]

The current gap situation is a hack at best. The problem is that gap doesn't have a real install target. So everything is copied in a versioned folder inSAGE_LOCAL/gap/gap-$version and then it is linked to SAGE_LOCAL/gap/latest. Clean in that it is all in its own container that you can delete afterwards.

I would imagine that debian does something like I do in Gentoo. Shoves most of the stuff under /usr/lib{,64}/gap with appropriate links or script in standard location pointing there. In that scheme GAP_DATA_DIR is /usr/lib{,64}/gap/pkg although some of the databases live under /usr/lib{,64}/gap directly.

There is a lot to do upstream to get gap to install nicely in any kind of fashion. In my latest ebuild I have been quite drastic compared to what sage does or previous version of my own ebuilds. So for the foreseeable future gap will remain a hack and I think GAP_DIR is more apt than GAP_DATA_DIR.

[felixs]

Replying to fbissey:

I would imagine that debian does something like I do in Gentoo.

with all do respect, you got it wrong.

on debian, the (platform indepentdent static, package specific) gap data goes to /usr/share/gap

Shoves most of the stuff under /usr/lib{,64}/gap with appropriate links or script in standard location pointing there. In that scheme GAP_DATA_DIR is /usr/lib{,64}/gap/pkg although some of the databases live under /usr/lib{,64}/gap directly.

not quite. /usr/lib is for platform dependent stuff. that's why you need lib{32,64} but only shared (not shared{32,64}). GAP_DATA_DIR is about *platform independent data*.

There is a lot to do upstream to get gap to install nicely in any kind of fashion.

wrong again. you can install gap any way you like, despite the broken build/install system

I think GAP_DIR is more apt than GAP_DATA_DIR.

now with three wrong premises, you end up on the wrong track.

instead, you could (plan to) fix the gap ebuild to adhere to FHS, as most other gentoo packages do.

Replying to Jeroen

I am not against standards (... but)

i feel the need for a gowdin's style law for this sentence, but it's not there. so i will better stop posting here now.

[fbissey]

I certainly could improve things, thanks for pointing stuff out [although using /usr/share/gap means patching, I guess debian has the patch I am thinking about]. The main point here is that installing gap in a proper way is a lot of hand holding. debian and gentoo have a proper package management system - which track files belonging to a package so upgrading or otherwise changing the package version is clean.

sagemath on the other hand only installs and "uninstall" manually where we can/have to [I guess there are pip packages now and we know what happened with those in another ticket ;) ]. What I am getting at, is that sagemath's packaging system is not sophisticated enough to deal with the hand holding when changing version. Lots of work.

Feel free to improve gap's installation (without forgetting to make provision for upgrade) in another ticket.

Once it is done the discussion between GAP_DATA_DIR and GAP_DIR will be non-sensical. Should we wait until this is ready or should we move on with the current situation?

[embray]

Replying to felixs:

GAP_DATA_DIR is a bad name.

no it's not. it specifies the location of the (platform indepentdent static, package specific) data shipped with the gap package. it's meant to be used to lead to exactly that. no more, no less.

I agree with Felix here. I don't care how gap happens to be installed in Sage. The use that variable is to find where the GAP data lives (which in Sage, which I guess obeys GAP's "unusual" installation, happens to be the directory where all of GAP is installed). On a different platform the two are not necessarily the same.

[jdemeyer]

Replying to embray:

The use [of] that variable is to find where the GAP data lives

Do you have evidence for this claim? Because this statement is the crux of the discussion.

Edit: if the above claim is indeed correct, I totally agree that GAP_DATA_DIR is a good name.

[jdemeyer]

Anyway, back to reality: $SAGE_LOCAL/gap/latest is what GAP calls a ​GAP Root Directory. It seems to be used for everything, not just "data files".

[embray]

Replying to jdemeyer:

Replying to embray:

The use [of] that variable is to find where the GAP data lives

Do you have evidence for this claim? Because this statement is the crux of the discussion.

Edit: if the above claim is indeed correct, I totally agree that GAP_DATA_DIR is a good name.

It seems to me all this argument might be for not much then. It seems to me all it's really used for is in the utility function gap_root(), which itself is not actually used anywhere in Sage (which is not to say nobody uses it for something).

Perhaps we can call it GAP_ROOT_DIR for now and leave it at that. Distros that try to break GAP up into a standard hierarchy would have to have their own definition of what GAP_ROOT_DIR means I guess.

Agreed with all of Jeroen's other comments from before the GAP_DIR argument--I don't at all understand the issue with default.qepcadrc but I'll have a closer look.

[jdemeyer]

Replying to embray:

It seems to me all it's really used for is in the utility function gap_root(), which itself is not actually used anywhere in Sage (which is not to say nobody uses it for something).

It is used to initialize libGAP. From src/sage/libs/gap/util.pyx:

cdef initialize():
    """
    Initialize the GAP library, if it hasn't already been
    initialized.  It is safe to call this multiple times.

    TESTS::

        sage: libgap(123)   # indirect doctest
        123
    """
    global _gap_is_initialized
    if _gap_is_initialized: return

    # Define argv and environ variables, which we will pass in to
    # initialize GAP. Note that we must pass define the memory pool
    # size!
    cdef char* argv[14]
    argv[0] = "sage"
    argv[1] = "-l"
    s = gap_root()
    argv[2] = s

[embray]

That makes sense, obviously. Not sure how my grepping missed that.

[mkoeppe]

Cc'ing some people who worked on libgap (#6391) in the hope that we can settle the GAP_DIR / GAP_ROOT_DIR / GAP_DATA_DIR question and move this ticket forward.

[dimpase]

Replying to mkoeppe:

Cc'ing some people who worked on libgap (#6391) in the hope that we can settle the GAP_DIR / GAP_ROOT_DIR / GAP_DATA_DIR question and move this ticket forward.

OK, so what is your proposal in this regard? GAP suffers from the same "missing install target" illness as Sage....

[mkoeppe]

We are looking for the correct naming of the variable that holds the directory passed to libgap at initialization time. The naming should make sense both for GAP's traditional (upstream) installation and for Debian's or other distributions' cleaned up installation.

[dimpase]

Replying to mkoeppe:

We are looking for the correct naming of the variable that holds the directory passed to libgap at initialization time. The naming should make sense both for GAP's traditional (upstream) installation and for Debian's or other distributions' cleaned up installation.

libGAP is a purely Sage affair, there is no real upstream, so there is no "tradition" in this sense.

[mkoeppe]

but libgap does seem to depend on GAP's installation, no?

[dimpase]

Replying to mkoeppe:

but libgap does seem to depend on GAP's installation, no?

yes it does, sure, as enshrined in build/pkgs/libgap/dependencies

[fbissey]

Replying to mkoeppe:

but libgap does seem to depend on GAP's installation, no?

It absolutely does. In fact it needs to know where gap packages are if it want to use them. So it really need what gap calls GAP_ROOT_DIR.

[mkoeppe]

The Debian/Ubuntu? gap package splits the gap installation into /usr/lib/gap and /usr/share/gap.

root@2f38a5ed7694:/src# ls -l /usr/lib/gap
total 8
drwxr-xr-x 2 root root 4096 Oct 27 23:24 bin
lrwxrwxrwx 1 root root   17 Jun 18  2015 src -> ../../include/gap
-rw-r--r-- 1 root root  110 Jun 18  2015 sysinfo.gap
root@2f38a5ed7694:/src# ls -l /usr/share/gap
total 56
drwxr-xr-x 5 root root  4096 Oct 27 23:24 doc
drwxr-xr-x 2 root root  4096 Oct 27 23:24 etc
drwxr-xr-x 2 root root  4096 Oct 27 23:24 grp
drwxr-xr-x 2 root root 20480 Oct 27 23:24 lib
drwxr-xr-x 3 root root  4096 Oct 27 23:24 pkg
drwxr-xr-x 3 root root  4096 Oct 27 23:24 prim
drwxr-xr-x 4 root root  4096 Oct 27 23:24 small
lrwxrwxrwx 1 root root    25 Jun 18  2015 sysinfo.gap -> ../../lib/gap/sysinfo.gap
drwxr-xr-x 2 root root 12288 Oct 27 23:24 trans

I did a quick check of building libgap using the gap package on Ubuntu. The relevant directory for libgap seems to be /usr/share/gap.

[mkoeppe]

I've split out some commits to #21783 and #21784.

Rebased the remaining commits on top of 7.5.beta0.

---

New commits:

​437827b

unhardwire runtime paths in various cases

[jdemeyer]

Many of my comments starting with 56 have not been taken into account.

[jdemeyer]

Replying to fbissey:

It absolutely does. In fact it needs to know where gap packages are if it want to use them. So it really need what gap calls GAP_ROOT_DIR.

GAP_ROOT_DIR would indeed be a much better name than the misleading GAP_DATA_DIR.

[jdemeyer]

There are also doctest failures (see patchbot).

Why was this ticket set to needs_review?

[fbissey]

Someone insists on importing variables from sage.misc.misc instead of sage.env ergo doctest failures.

[mkoeppe]

Right, I set needs_review by mistake.

[git]

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

​5db9663	lattice_polytope: Use POLYTOPE_DATA_DIR directly
​b011153	ec_database: Use ELLCURVE_DATA_DIR directly
​171e139	Rename CONWAY_DATA_DIR to CONWAY_POLYNOMIALS_DATA_DIR
​a222fc9	Rename GAP_DATA_DIR to GAP_ROOT_DIR
​a80df36	Import GRAPHS_DATA_DIR from sage.env, not sage.misc.misc

[git]

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

​41b40fd	unhardwire runtime paths in various cases
​6e0738e	lattice_polytope: Use POLYTOPE_DATA_DIR directly
​82f9882	ec_database: Use ELLCURVE_DATA_DIR directly
​0d56551	Rename CONWAY_DATA_DIR to CONWAY_POLYNOMIALS_DATA_DIR
​f7a462e	Rename GAP_DATA_DIR to GAP_ROOT_DIR
​cf810a2	Import GRAPHS_DATA_DIR from sage.env, not sage.misc.misc

[git]

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

​a2c3798	GraphClasses: fixup use of GRAPHS_DATA_DIR
​65a00a5	Remove unused variables SINGULAR_INCLUDEDIR, FACTORY_INCLUDEDIR

[mkoeppe]

Replying to fbissey:

using /usr/share/gap means patching, I guess debian has the patch I am thinking about

I've created ticket #21796 for adopting Debian's layout of GAP.

[mkoeppe]

The present ticket now passes doctests (on my machine), and I believe I have taken care of the various comments.

Needs review.

[jdemeyer]

Replying to mkoeppe:

The present ticket now passes doctests (on my machine), and I believe I have taken care of the various comments.

Two more details:

1. in src/sage/env.py, remove

   UNAME = os.uname()[0]
   

as UNAME is actually one of the standard variables.

2. In src/sage/libs/gap/util.pyx, remove gapdir = GAP_ROOT_DIR and just use GAP_ROOT_DIR instead of gapdir.

If you fix these and all tests pass, then this can be set to positive_review.

[git]

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

​ae20d15	Remove duplicate assignment to UNAME
​45c5dd0	gap_root: slight simplification

[felixs]

thank you guys for making that happen.

[jdemeyer]

Replying to felixs:

thank you guys for making that happen.

Well, we did postpone all controversial changes to other tickets :-)

[jdemeyer]

Let me mention one more follow-up: #21821

[fbissey]

Now that it has made it onto Volker's branch and I can see easily how it interact with sage-on-gentoo, I feel sorry about all that GAP_ROOT_DIR business.

We should just have eliminated it because it is totally unnecessary. The variable is only used in sage/libs/gap/util.pyx in the function gap_root which basically return the value of GAP_ROOT_DIR

def gap_root():
    """
    Find the location of the GAP root install which is stored in the gap
    startup script.
    EXAMPLES::
        sage: from sage.libs.gap.util import gap_root
        sage: gap_root()   # random output
        '/home/vbraun/opt/sage-5.3.rc0/local/gap/latest'
    """
    import os.path
    if os.path.exists(GAP_ROOT_DIR):
        return GAP_ROOT_DIR
    print('The gap-4.5.5.spkg (or later) seems to be not installed!')
    gap_sh = open(os.path.join(SAGE_LOCAL, 'bin', 'gap')).read().splitlines()
    gapdir = filter(lambda dir:dir.strip().startswith('GAP_DIR'), gap_sh)[0]
    gapdir = gapdir.split('"')[1]
    gapdir = gapdir.replace('$SAGE_LOCAL', SAGE_LOCAL)
    return gapdir

So if GAP_ROOT_DIR is not defined, a mwarning is issued and then the gap script is looked over for GAP_DIR which gives the right result both in Gentoo [sage-on-gentoo relies solely on the automated procedure] and in vanilla sage. Not sure about debian, I must say.

But I feel we were silly because the automated procedure works well so we don't really need the variable. And if we decide to use the variable, what is the point of keeping the function gap_root? Apart from searching for all the instances of gap_root to replace them that is.

Anyway, that's a potential clean up for another ticket.

[mkoeppe]

Replying to fbissey:

[...] I feel sorry about all that GAP_ROOT_DIR business.

We should just have eliminated it because it is totally unnecessary. The variable is only used in sage/libs/gap/util.pyx in the function gap_root which basically return the value of GAP_ROOT_DIR [...] So if GAP_ROOT_DIR is not defined, a mwarning is issued and then the gap script is looked over for GAP_DIR which gives the right result both in Gentoo [sage-on-gentoo relies solely on the automated procedure] and in vanilla sage.

Yes, I saw this code. I think it does not belong into sagelib proper, but we should move it to compile-time configuration of sagelib (which would set the GAP_ROOT_DIR variable). So I do think that introducing the GAP_ROOT_DIR variable was a good step.

[fbissey]

A lot of stuff should be moved to "configuration" time proper. Definitely for another day.

But is there a point in having a variable when you can just query its value from the application itself? Plus that means we can further decouple gap from sagelib. We don't need to know anything about where gap is really installed, we get the right value so long as gap is in the PATH. In short eliminating that variable makes it easier to switch to a system install of gap (cross fingers about debian).

[fbissey]

OK at the moment we still need to know where the gap executable is located to populate gap_sh but that could be dealt with.

[mkoeppe]

I wouldn't call parsing that script "querying the application"...

[mkoeppe]

And, really, actually it belongs into libgap configuration.

[fbissey]

Have to agree, it is a libgap problem. Once sorted in libgap it is all moot.

[mkoeppe]

I recently created a pull request for libgap in this direction: ​https://bitbucket.org/vbraun/libgap/pull-requests/9

[embray]

Replying to fbissey:

A lot of stuff should be moved to "configuration" time proper. Definitely for another day.

But is there a point in having a variable when you can just query its value from the application itself?

Yes. I can think of at least two reasons:

1) Helpful for some system (not that I have one in mind) where the search code doesn't work--it's always kind to provide some way to configure an alternate path.

2) For testing against alternate GAP installs.

[fbissey]

Replying to embray:

Replying to fbissey:

A lot of stuff should be moved to "configuration" time proper. Definitely for another day.

But is there a point in having a variable when you can just query its value from the application itself?

Yes. I can think of at least two reasons:

1) Helpful for some system (not that I have one in mind) where the search code doesn't work--it's always kind to provide some way to configure an alternate path.

2) For testing against alternate GAP installs.

1) a good case is probably cross compilation of some kind, where things will be available at runtime but not when you build.

2) well I was thinking of asking gap directly (I have tested this) so the only requirement is for the gap executable to be in the PATH. However technically there is more than one valid path that should be looked up and libgap only look at one (the default system one).