sage.env: Remove direct uses of SAGE_LOCAL from the Sage library #32036

mkoeppe · 2021-06-23T00:15:50Z

(from #31338 comment:13)

These changes are preparation for modularization and is also intended to make downstream packaging easier.

CC: @kiwifb @antonio-rojas @dimpase @videlec @jhpalmieri

Component: build: configure

Author: Matthias Koeppe, François Bissey

Branch/Commit: 985af6e

Reviewer: Dima Pasechnik, François Bissey

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

The text was updated successfully, but these errors were encountered:

mkoeppe · 2021-06-23T00:21:55Z

comment:1

There are a few remaining uses of SAGE_SHARE in src/sage that will have to be eliminated though.

mkoeppe · 2021-06-23T00:23:05Z

comment:2

(see #30819 sage.databases: Use sage.feature to define database file system location)

mkoeppe · 2021-06-23T00:24:53Z

comment:3

Perhaps now is also the time that we do #30914 (Meta-ticket: Create upstream repositories, pip-installable packages for database packages)?

kiwifb · 2021-06-23T00:28:26Z

comment:4

Replying to @mkoeppe:

Perhaps now is also the time that we do #30914 (Meta-ticket: Create upstream repositories, pip-installable packages for database packages)?

That would be timely if we could.

Other issue about sage_setup/setenv.py I tried to replace if SAGE_LOCAL: by if SAGE_ROOT: since it is set to None for me in distro. But I still got the linking noise which means that it still accepted that as a positive. So a better test may be needed.

mkoeppe · 2021-06-23T00:31:50Z

comment:5

No, SAGE_ROOT is not the right choice there.

kiwifb · 2021-06-23T00:34:20Z

comment:6

Replying to @kiwifb:

Other issue about sage_setup/setenv.py I tried to replace if SAGE_LOCAL: by if SAGE_ROOT: since it is set to None for me in distro. But I still got the linking noise which means that it still accepted that as a positive. So a better test may be needed.

Scratch that, I think I did myself in at build time for some reasons (which I have top check).

mkoeppe · 2021-06-23T18:10:11Z

Branch: u/mkoeppe/sage_env__remove_default_setting_of_sage_local_from_sage_venv

mkoeppe · 2021-06-23T18:11:24Z

Commit: 95deabd

mkoeppe · 2021-06-23T18:11:24Z

New commits:

`95deabd`	`src/sage/env.py: No fallback for SAGE_LOCAL`

mkoeppe · 2021-06-23T18:11:24Z

Author: Matthias Koeppe

antonio-rojas · 2021-06-23T18:36:00Z

comment:9

> sage
Traceback (most recent call last):
  File "/usr/bin/sage-ipython", line 9, in <module>
    from sage.misc.banner import banner
  File "/usr/lib/python3.9/site-packages/sage/misc/banner.py", line 16, in <module>
    from sage.env import (SAGE_VERSION, SAGE_VERSION_BANNER, SAGE_BANNER)
  File "/usr/lib/python3.9/site-packages/sage/env.py", line 314, in <module>
    SINGULAR_SO = var("SINGULAR_SO", _get_shared_lib_path("Singular", "singular-Singular"))
  File "/usr/lib/python3.9/site-packages/sage/env.py", line 291, in _get_shared_lib_path
    search_directories = [Path(SAGE_LOCAL) / 'lib']
  File "/usr/lib/python3.9/pathlib.py", line 1072, in __new__
    self = cls._from_parts(args, init=False)
  File "/usr/lib/python3.9/pathlib.py", line 697, in _from_parts
    drv, root, parts = self._parse_args(args)
  File "/usr/lib/python3.9/pathlib.py", line 681, in _parse_args
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType

sagetrac-git · 2021-06-23T21:19:29Z

Changed commit from 95deabd to de66da4

sagetrac-git · 2021-06-23T21:19:29Z

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

`de66da4`	`sage.env._get_shared_lib_path: If SAGE_LOCAL is not set, skip it`

mkoeppe · 2021-06-23T21:19:47Z

comment:11

Thanks for catching this!

antonio-rojas · 2021-06-23T21:28:31Z

comment:12

Next issue:

/usr/lib/python3.9/site-packages/sage/interfaces/gap_workspace.py in gap_workspace_file(system='gap', name='workspace', dir='/home/antonio/.sage/gap')
     [...]
     57         sage: assert name1 == name2
     58     """
     59     if dir is None:
     60         dir = os.path.join(DOT_SAGE, 'gap')
     61 
---> 62     h = hashlib.sha1(SAGE_LOCAL.encode('utf-8')).hexdigest()
        h = undefined
        global hashlib.sha1 = <built-in function openssl_sha1>
        global SAGE_LOCAL.encode.hexdigest = undefined
     63     return os.path.join(dir, '%s-%s-%s' % (system, name, h))
     64 
     65 
AttributeError: 'NoneType' object has no attribute 'encode'

and, after bypassing it

/usr/lib/python3.9/site-packages/sage/interfaces/lie.py in __init__(self=LiE Interpreter, maxread=None, script_subdirectory=None, logfile=None, server=None)
    [...]
    329                         # it to be very obfuscated that would be better.   Even
    330                         # better is to use sequence numbers.
    331                         prompt = '> ',
    332 
    333                         # This is the command that starts up your program
--> 334                         command = "bash "+ SAGE_LOCAL + "/bin/lie",
        global command = undefined
        global SAGE_LOCAL = None
    335 
    336                         server=server,
    337                         script_subdirectory = script_subdirectory,
    338 

TypeError: can only concatenate str (not "NoneType") to str

kiwifb · 2021-06-23T21:38:25Z

comment:13

Excluding env.py for a minute those are the instances of SAGE_LOCAL still existing under in the sagelib tree from a simple grep command

src/sage/all.py:from sage.env import SAGE_ROOT, SAGE_SRC, SAGE_DOC_SRC, SAGE_LOCAL, DOT_SAGE, SAGE_ENV
src/sage/all.py:        sage: started_file = os.path.join(SAGE_LOCAL, 'etc', 'sage-started.txt')
src/sage/all.py:    started_file = os.path.join(SAGE_LOCAL, 'etc', 'sage-started.txt')
src/sage/calculus/desolvers.py:                          os.path.join('$SAGE_LOCAL','lib','libTIDES.a') + ' $LDFLAGS '
src/sage/calculus/desolvers.py:                          + os.path.join('-L$SAGE_LOCAL','lib ') +' -lm  -O2 ' +
src/sage/calculus/desolvers.py:                          os.path.join('-I$SAGE_LOCAL','include '),
src/sage/calculus/desolvers.py:                          os.path.join('$SAGE_LOCAL','lib','libTIDES.a') + ' $LDFLAGS '
src/sage/calculus/desolvers.py:                          + os.path.join('-L$SAGE_LOCAL','lib ') + '-lmpfr -lgmp -lm  -O2 -w ' +
src/sage/calculus/desolvers.py:                          os.path.join('-I$SAGE_LOCAL','include ') ,
src/sage/doctest/control.py:        # only have the SAGE_LOCAL install tree but not SAGE_ROOT
src/sage/doctest/control.py:            # only have the SAGE_LOCAL install tree but not SAGE_ROOT
src/sage/interfaces/qepcad.py:    sage: with open(os.path.join(SAGE_LOCAL, 'default.qepcadrc')) as f:  # optional - qepcad
src/sage/interfaces/qepcad.py:from sage.env import SAGE_LOCAL
src/sage/interfaces/qepcad.py:        sage: s == 'env qe=%s qepcad '%SAGE_LOCAL
src/sage/interfaces/qepcad.py:        sage: s == 'env qe=%s qepcad +N8000000'%SAGE_LOCAL
src/sage/interfaces/qepcad.py:    return "env qe=%s qepcad %s"%(SAGE_LOCAL, memcells_arg)
src/sage/interfaces/qepcad.py:    with open(os.path.join(SAGE_LOCAL, 'bin', 'qepcad.help')) as help:
src/sage/interfaces/giac.py:    $SAGE_LOCAL/share/giac/doc/en/cascmd_local/index.html
src/sage/interfaces/giac.py:If you got giac from the spkg then ``$PREFIX`` is ``$SAGE_LOCAL``
src/sage/interfaces/gap_workspace.py:from sage.env import DOT_SAGE, SAGE_LOCAL
src/sage/interfaces/gap_workspace.py:    h = hashlib.sha1(SAGE_LOCAL.encode('utf-8')).hexdigest()
src/sage/interfaces/lie.py:from sage.env import DOT_SAGE, SAGE_LOCAL
src/sage/interfaces/lie.py:                        command = "bash "+ SAGE_LOCAL + "/bin/lie",
src/sage/interfaces/lie.py:            filename = SAGE_LOCAL + "/lib/LiE/" + f
src/sage/interfaces/lie.py:    f = open(os.path.join(SAGE_LOCAL, 'lib', 'LiE', 'INFO.0'))
src/sage/interfaces/maxima.py:from sage.env import DOT_SAGE, SAGE_LOCAL, MAXIMA
src/sage/interfaces/mathematica.py:##            it in <SAGE_LOCAL>/bin/ is an easy way to ensure this).
src/sage/libs/gap/util.pyx:    SAGE_LOCAL = sage.env.SAGE_LOCAL
src/sage/libs/gap/util.pyx:    with open(os.path.join(SAGE_LOCAL, 'bin', 'gap')) as f:
src/sage/libs/gap/util.pyx:    gapdir = gapdir.replace('$SAGE_LOCAL', SAGE_LOCAL)
src/sage/libs/giac/giac.pyx:      - or in :doc:`$SAGE_LOCAL/share/giac/doc/en/cascmd_en/index.html`
src/sage/misc/sage_ostools.pyx:        sage: have_program('there_is_not_a_program_with_this_name', os.path.join(SAGE_LOCAL, 'bin'))
src/sage/misc/citation.pyx:from sage.env import SAGE_LOCAL
src/sage/misc/citation.pyx:    strings = [a[0].replace(SAGE_LOCAL, "") + " " + a[2]
src/sage/misc/cython.py:from sage.env import (SAGE_LOCAL, cython_aliases,
src/sage/misc/cython.py:    standard_libdirs = [os.path.join(SAGE_LOCAL, "lib")] + aliases["CBLAS_LIBDIR"] + aliases["NTL_LIBDIR"]
src/sage/misc/compat.py:from sage.env import SAGE_LOCAL
src/sage/misc/compat.py:        for libdir in [os.path.join(SAGE_LOCAL, 'lib'),
src/sage/misc/compat.py:        sage_local_lib = os.path.join(SAGE_LOCAL, 'lib')
src/sage/misc/compat.py:    lib_dirs = (LDPATH_STR.split(':') if LDPATH_STR else []) + [os.path.join(SAGE_LOCAL, 'lib')]
src/sage/misc/edit_module.py:    'SAGE_LOCAL/lib/python.../site-packages', it replaces this by
src/sage/misc/dist.py:    from sage.env import SAGE_LOCAL
src/sage/misc/dist.py:    SAGE_BIN = os.path.join(SAGE_LOCAL, 'bin')
src/sage/misc/dist.py:    # SAGE_LOCAL/bin from PATH so that we can later check whether
src/sage/plot/plot3d/base.pyx:from sage.env import SAGE_LOCAL
src/sage/repl/interpreter.py:``$SAGE_LOCAL/bin/sage-ipython`` script to start the Sage
src/sage/sandpiles/sandpile.py:from sage.env import SAGE_LOCAL
src/sage/sandpiles/sandpile.py:path_to_zsolve = os.path.join(SAGE_LOCAL, 'bin', 'zsolve')

sage-started could be eliminated without regrets (personal opinion).
The stuff in calculus/desolvers.py should be carefully reviewed - can we still use that stuff and if yes, how?
qepcad.py is long time serial offender.
gap_workspace.py needs to be looked at.
lie.py probably should have specific variable, distro may put the stuff somewhere else (Gentoo does).
SAGE_LOCAL is imported in maxima.py and plot/plot3d/base.pyx but appear to be unused - expunge.
libs/gap/util.pyx would need to be reworked.
cython.py need to be looked at.
What is happening with SAGE_LOCAL in misc/compat.py.
citations.pyx seem to rely on it.
sandpiles/sandpile.py features should be used.

sagetrac-git · 2021-06-23T21:45:20Z

Changed commit from de66da4 to d4b822b

sagetrac-git · 2021-06-23T21:45:20Z

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

`d4b822b`	`src/sage/interfaces/maxima.py: Remove unused imports of DOT_SAGE, SAGE_LOCAL`

sagetrac-git · 2021-06-23T21:54:02Z

Changed commit from d4b822b to 13b780b

sagetrac-git · 2021-06-23T21:54:02Z

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

`13b780b`	`src/sage/interfaces/gap_workspace.py: Use hash of GAP_SO to disambiguate the workspace file, not SAGE_LOCAL`

mkoeppe · 2021-06-23T21:54:58Z

comment:16

Perhaps someone who knows more about GAP workspaces can comment whether this change makes sense.

sagetrac-git · 2021-06-23T21:59:01Z

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

`7ca61a9`	`build/pkgs/lie/distros/*.txt: New`

sagetrac-git · 2021-06-23T21:59:01Z

Changed commit from 13b780b to 7ca61a9

mkoeppe · 2021-06-23T22:02:16Z

comment:18

Replying to @kiwifb:

lie.py probably should have specific variable, distro may put the stuff somewhere else (Gentoo does).

I agree.

Could someone on such a distro take care of lie.py please?

There's no package for homebrew and our SPKG does not compile

sagetrac-git · 2021-06-23T22:06:40Z

Changed commit from 7ca61a9 to f1a9c25

kiwifb · 2021-06-25T10:06:13Z

comment:69

Replying to @dimpase:

Replying to @kiwifb:

Replying to @mkoeppe:

Replying to @dimpase:

I don't get sci-mathematics/lie on my Gentoo box, is it not yet in Gentoo?

No idea, I got this information from https://repology.org/project/lie/versions

It is not in the main tree, but it has been in the science overlay for a while. However, if you want one that will definitely work, you'll have to go with my sage-on-gentoo overlay. I updated it yesterday when I found that tarball has changed location.

I was able to install lie on Gentoo by doing
layman --add science
emerge --ask lie
Should we add
emerge --ask app-portage/layman
layman --add science
to whatever the current place for these per-system advice is (it was moved around...)

Using layman is so outdated - even I moved on (last month :) ). Seriously, it is deprecated.

emerge eselect-repository 
eselect repository enable science 
emaint sync -r science

or emerge --sync, all the repos managed that way will get pulled by emerge --sync or equivalent like eix-sync.

dimpase · 2021-06-25T19:35:37Z

Reviewer: Dima Pasechnik

dimpase · 2021-06-25T19:35:37Z

comment:70

OK

mkoeppe · 2021-06-25T19:45:41Z

comment:71

Thanks!

vbraun · 2021-07-06T06:21:18Z

comment:72

On OSX:

**********************************************************************
File "src/sage/misc/compat.py", line 97, in sage.misc.compat
Failed example:
    find_library('Singular')
Exception raised:
    Traceback (most recent call last):
      File "/Users/buildbot-sage/slave/sage_git/build/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 718, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/buildbot-sage/slave/sage_git/build/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 1137, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.misc.compat[1]>", line 1, in <module>
        find_library('Singular')
      File "/Users/buildbot-sage/slave/sage_git/build/local/lib/python3.9/site-packages/sage/misc/compat.py", line 102, in find_library
        result = _find_library(name)
      File "/Users/buildbot-sage/slave/sage_git/build/local/lib/python3.9/site-packages/sage/misc/compat.py", line 66, in _find_library
        os.environ['DYLD_LIBRARY_PATH'] = sage_local_lib
    NameError: name 'sage_local_lib' is not defined
**********************************************************************

kiwifb · 2021-07-06T08:29:54Z

comment:74

I see, we forgot to replace sage_local_lib which was defined as os.path.join(SAGE_LOCAL, 'lib') by expressions based on the new libdir in the darwin section. That was a bit careless of me not to notice.

sagetrac-git · 2021-07-06T13:12:43Z

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

`985af6e`	`sage.misc.compat._find_library [darwin]: Fix up`

sagetrac-git · 2021-07-06T13:12:43Z

Changed commit from d88c884 to 985af6e

mkoeppe · 2021-07-06T13:14:01Z

comment:76

Yes, looks like I fell asleep in the middle of rewriting this function.

kiwifb · 2021-07-06T21:08:08Z

comment:77

Replying to @mkoeppe:

Yes, looks like I fell asleep in the middle of rewriting this function.

The new code should do the trick. I must say that when we were working on this originally, it felt like you were in my time zone. So, I guess falling asleep would have been fair.

kiwifb · 2021-07-06T21:08:08Z

Changed reviewer from Dima Pasechnik to Dima Pasechnik, François Bissey

mkoeppe · 2021-07-06T21:29:47Z

comment:78

Thanks!

kiwifb · 2021-07-08T02:42:40Z

comment:79

I must say, this is not as useful without setting SAGE_LOCAL to None in distro like Gentoo. I just found out I cannot stop patching misc/cython.py to replace lib by lib64 because it otherwise leaks

    /usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libm.so when searching for -lm
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libm.a when searching for -lm
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libpthread.so when searching for -lpthread
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libpthread.a when searching for -lpthread
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libc.so when searching for -lc
    /usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libc.a when searching for -lc

in a good number of doctests. This wouldn't happen with SAGE_LOCAL being None when tested in cython.py.

As an aside, I tried to fix it by adding a line SAGE_LOCAL = None in sage_conf, which seemed to be the obvious fix. But it didn't work, it still reports SAGE_LOCAL as /usr. What am I doing wrong?

mkoeppe · 2021-07-08T02:50:00Z

comment:80

Didn't we take care of this in #32057?

kiwifb · 2021-07-08T03:56:15Z

comment:81

Replying to @mkoeppe:

Didn't we take care of this in #32057?

#32057 helps with build time but not runtime - as it should, sage_setup is for building stuff. I get those as soon as cython.py is called to compile something at runtime. I have had the little patch for it for awhile. But sagemath/sagetrac-mirror@cd4ccd3 along with sagemath/sagetrac-mirror@95deabd made the patch irrelevant. Unfortunately, we reversed the last one for Antonio who still relies on SAGE_LOCAL being set.

mkoeppe · 2021-07-08T04:09:15Z

comment:82

Right, thanks. So, as discussed in #31338 comment:18, we're back to having to solve the same problem as #31578.

kiwifb · 2021-07-08T04:13:19Z

comment:83

That complaint of mine was about build time. We had the runtime issue forever, it is just the opportunity to squash it is gone again :(

But what I want to know and I'll dig is why setting SAGE_LOCAL to None is sage_conf doesn't work. That would be the ideal fix, while keeping it the way it is for Antonio.

kiwifb · 2021-07-08T23:15:44Z

comment:84

I think I will just adopt the commit you removed for Antonio. sage_conf is not really designed to set a variable to None, that cannot work and it is even probably undesirable.

kiwifb · 2021-07-09T01:24:39Z

comment:85

Replying to @kiwifb:

I think I will just adopt the commit you removed for Antonio. sage_conf is not really designed to set a variable to None, that cannot work and it is even probably undesirable.

Interestingly, adopting it broke docbuilding. So, there is probably more work to do on that subject in a follow up ticket.

vbraun · 2021-07-09T20:23:30Z

Changed branch from u/mkoeppe/sage_local_extinction_path to 985af6e

mkoeppe added this to the sage-9.4 milestone Jun 23, 2021

mkoeppe added c: build: configure labels Jun 23, 2021

mkoeppe added the s: needs review label Jun 23, 2021

dimpase added s: positive review and removed s: needs review labels Jun 25, 2021

vbraun added s: needs work and removed s: positive review labels Jul 6, 2021

mkoeppe added s: needs review and removed s: needs work labels Jul 6, 2021

kiwifb added s: positive review and removed s: needs review labels Jul 6, 2021

vbraun removed the s: positive review label Jul 9, 2021

vbraun closed this as completed in f2fbcac Jul 9, 2021

sage.env: Remove direct uses of SAGE_LOCAL from the Sage library #32036

sage.env: Remove direct uses of SAGE_LOCAL from the Sage library #32036

Comments

mkoeppe commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

kiwifb commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

kiwifb commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

antonio-rojas commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

antonio-rojas commented Jun 23, 2021

kiwifb commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

mkoeppe commented Jun 23, 2021

sagetrac-git mannequin commented Jun 23, 2021

kiwifb commented Jun 25, 2021

dimpase commented Jun 25, 2021

dimpase commented Jun 25, 2021

mkoeppe commented Jun 25, 2021

vbraun commented Jul 6, 2021

kiwifb commented Jul 6, 2021

sagetrac-git mannequin commented Jul 6, 2021

sagetrac-git mannequin commented Jul 6, 2021

mkoeppe commented Jul 6, 2021

kiwifb commented Jul 6, 2021

kiwifb commented Jul 6, 2021

mkoeppe commented Jul 6, 2021

kiwifb commented Jul 8, 2021

mkoeppe commented Jul 8, 2021

kiwifb commented Jul 8, 2021

mkoeppe commented Jul 8, 2021

kiwifb commented Jul 8, 2021

kiwifb commented Jul 8, 2021

kiwifb commented Jul 9, 2021

vbraun commented Jul 9, 2021