sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.36k stars 462 forks source link

Problems when running without SAGE_VENV #33625

Open tornaria opened 2 years ago

tornaria commented 2 years ago

Example context:

Say $SAGE_LIB contains the sage python library and $SAGE_BIN contains the scripts. I can run this sagelib by setting

$ export PATH=$SAGE_BIN:$PATH
$ export PYTHONPATH=$SAGE_LIB

which will pick this sagelib (overriding a system sagelib if installed, both in scripts and python packages).

However, there are a few issues with this, since: a. sage-env uses sage-venv-config to SAGE_VENV to sys.prefix b. there are a few places where it is assumed that the sage scripts live in SAGE_VENV/bin, thus breaking stuff. c. in particular sage-env itself prepends SAGE_VENV/bin to PATH.

Problem 1: If I don't have any system sage installed in /usr/bin/sage. This causes a doctest failure (#33624, very easy fix).

Problem 2: If I have a system sage installed in /usr/bin/sage but it is a different version than the one I'm trying to run. This causes trouble because after sage-env prepends PATH with SAGE_VENV/bin the sage script in SAGE_BIN will be overrided (for example, doctesting src/sage/doctest/test.py causes trouble).

Other problems? Maybe (untested) a. #33627 - sage.doctest.control uses SAGE_VENV/bin/sage-gdb-commands so it won't work b. sage.features looks for binaries in SAGE_VENV/bin c. sage.repl.ipython_kernel.install uses SAGE_VENV/bin/sage when configuring jupyter kernel so it seems it will end up running the system sage.

CC: @mkoeppe

Component: scripts

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

tornaria commented 2 years ago
comment:1

For problem 2 (sage-env messes PATH), this could be solved by setting SAGE_VENV= However, the way this is set in sage-venv-config makes it impossible.

If the line from sys import config as SAGE_VENV is moved before from sage_conf import *, then the value of SAGE_VENV could be overrided there.

I'm not sure that's really useful but it's harmless if unused.

tornaria commented 2 years ago
comment:2

For the other issues, maybe it would be wise to introduce sage.env.SAGE_BIN that is configured to point to the location of the sage scripts (similar to sage.env.SAGE_LIB. Then we can figure out how to actually compute SAGE_BIN, but at least it would be possible to configure in sage_conf.

I think, similar to my fix for #33624, that using os.path.dirname(sys.argv[0]) for the default for SAGE_BIN would be very reasonable, akin to the default for SAGE_LIB.

I think the sage script does a fair amount of work to ensure that ${SELF} has a good path (resolving symlinks, etc) and that will end up being the path in sys.argv[0].

CC: mkoeppe for RFC

mkoeppe commented 2 years ago
comment:3

Note SAGE_LIB is on its way out, see #33037.

mkoeppe commented 2 years ago

Replying to @tornaria:

c. sage.repl.ipython_kernel.install uses SAGE_VENV/bin/sage when configuring jupyter kernel so it seems it will end up running the system sage.

We may want to determine the correct path using https://docs.python.org/3/library/importlib.metadata.html#module-importlib.metadata

mkoeppe commented 2 years ago

Replying to @tornaria:

b. sage.features looks for binaries in SAGE_VENV/bin

It only uses it (as part of a search path) when also SAGE_LOCAL is set.

mkoeppe commented 2 years ago
comment:6

Replying to @mkoeppe:

Replying to @tornaria:

c. sage.repl.ipython_kernel.install uses SAGE_VENV/bin/sage when configuring jupyter kernel so it seems it will end up running the system sage.

We may want to determine the correct path using https://docs.python.org/3/library/importlib.metadata.html#module-importlib.metadata

Relevant: https://discuss.python.org/t/proper-way-to-determine-install-location-of-console-scripts/7188

mkoeppe commented 2 years ago
comment:7

This works:

sage: from importlib.metadata import distribution
sage: distribution("sagemath_standard").files
tornaria commented 2 years ago
comment:8

Replying to @mkoeppe:

This works:

sage: from importlib.metadata import distribution
sage: distribution("sagemath_standard").files

Hmmm... not really. It seems to be a list of source files:

$ sage-9.5 
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 9.5, Release Date: 2022-01-30                     │
│ Using Python 3.10.4. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
sage: from importlib.metadata import distribution
sage: distribution("sagemath_standard").files[0:20]
[PackagePath('LICENSE.txt'),
 PackagePath('MANIFEST.in'),
 PackagePath('README.rst'),
 PackagePath('VERSION.txt'),
 PackagePath('pyproject.toml'),
 PackagePath('requirements.txt'),
 PackagePath('setup.cfg'),
 PackagePath('setup.py'),
 PackagePath('bin/math-readline'),
 PackagePath('bin/sage'),
 PackagePath('bin/sage-cachegrind'),
 PackagePath('bin/sage-callgrind'),
 PackagePath('bin/sage-cleaner'),
 PackagePath('bin/sage-coverage'),
 PackagePath('bin/sage-cython'),
 PackagePath('bin/sage-env'),
 PackagePath('bin/sage-eval'),
 PackagePath('bin/sage-fixdoctests'),
 PackagePath('bin/sage-gdb-commands'),
 PackagePath('bin/sage-grep')]

This is in my system install of sagemath, which was installed using --install-scripts=/usr/lib/sagemath/bin, but there's no info.

I assume importlib.metadata obtains the info from /usr/lib/python3.10/site-packages/sagemath_standard-9.5-py3.10.egg-info/ but a quick grep -r shows that there's nothing in there that reveals the /usr/lib/sagemath/bin path.

One easy way to obtain the path to the scripts directory would be as dirname $SELF in the main sage script. At least for setting PATH that would be better than using $SAGE_VENV/bin as is currently done.

However, there doesn't seem to be any way to make this work when using sage as a python module. Unless this is saved somewhere in the sage package or in the sagemath_standard egg-info at install time.

mkoeppe commented 2 years ago
comment:9

Replying to @tornaria:

This is in my system install of sagemath, which was installed using --install-scripts=/usr/lib/sagemath/bin

Similar to what I said in #33624 comment:5, I think it's best to give up on using the legacy setuptools command options such as setup.py install --install-scripts DIR.

mkoeppe commented 2 years ago
comment:10

I do agree though that using SAGE_VENV/bin can be problematic, for example if the Sage library is installed using pip install --user ....

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -20,7 +20,7 @@
 Problem 2: If I have a system sage installed in `/usr/bin/sage` but it is a different version than the one I'm trying to run. This causes trouble because after `sage-env` prepends PATH with `SAGE_VENV/bin` the `sage` script in `SAGE_BIN` will be overrided (for example, doctesting `src/sage/doctest/test.py` causes trouble).

 Other problems? Maybe (untested)
-a. `sage.doctest.control` uses `SAGE_VENV/bin/sage-gdb-commands` so it won't work
+a. #33627 - `sage.doctest.control` uses `SAGE_VENV/bin/sage-gdb-commands` so it won't work
 b. `sage.features` looks for binaries in `SAGE_VENV/bin` 
 c. `sage.repl.ipython_kernel.install` uses `SAGE_VENV/bin/sage` when configuring jupyter kernel so it seems it will end up running the system sage.
tornaria commented 2 years ago
comment:12

Replying to @mkoeppe:

Replying to @tornaria:

This is in my system install of sagemath, which was installed using --install-scripts=/usr/lib/sagemath/bin

Similar to what I said in #33624 comment:5, I think it's best to give up on using the legacy setuptools command options such as setup.py install --install-scripts DIR.

TIL that, since there's no indication anywhere that --install-scripts is legacy/deprecated/unsupported:

$ python setup.py install --help
...
  --install-scripts   installation directory for Python scripts
...

What about --exec-prefix, is that also deprecated? (btw, is --exec-prefix supported in a "normal" installation of sage-the-distribution using ./configure, etc.?)

The main reason I'm using --install-scripts is because sagelib installs a lot of scripts which don't seem necessary in PATH. I prefer to "hide" them and just have one symlink to the main sage script. In a "normal" installation of sage this is not an issue because everything is already "hidden" in SAGE_ROOT and the only thing exposed is the symlink to sage.

Maybe these non-user scripts can be placed in a different location. I don't know if there's a standard location for this purpose. Other packages that install "hidden" binaries, e.g. git installs a bunch of binaries in /usr/libexec/git-core/, maxima installs binaries in /usr/lib/maxima/VERSION/..., etc.

Would it be outrageous to place them at .../sage/ext_data/script/ or something like that? The advantage is that they would be tied together with the corresponding python modules.

mkoeppe commented 2 years ago
comment:13

Replying to @tornaria:

Replying to @mkoeppe:

Similar to what I said in #33624 comment:5, I think it's best to give up on using the legacy setuptools command options such as setup.py install --install-scripts DIR.

What about --exec-prefix, is that also deprecated?

All of setup.py install is deprecated

mkoeppe commented 2 years ago
comment:14

Replying to @tornaria:

btw, is --exec-prefix supported in a "normal" installation of sage-the-distribution using ./configure, etc.?

No, this is not passed on to any of the packages that sage-the-distribution installs.

The option is there because of the (unused) build/make/Makefile-auto.am.

mkoeppe commented 2 years ago
comment:15

Replying to @tornaria:

Maybe these non-user scripts can be placed in a different location.

Yes, at least some of these have a "libexec" flavor.

I don't know if there's a standard location for this purpose.

I'm not aware of one in Python packaging.

Would it be outrageous to place them at .../sage/ext_data/script/ or something like that? The advantage is that they would be tied together with the corresponding python modules.

ext_data is also on its way out (see #31306), but yes, some of this can be moved to package_data somewhere.

See #33627 for a step in this direction

tornaria commented 6 months ago

@mkoeppe help here. The fault is caused by this code in src/bin/sage-env:

if [ -n "$SAGE_VENV" ]; then
    export PATH="$SAGE_VENV/bin:$PATH"
fi

which was added by 1a518c0 in #30013.

I assume removing this will regress on #30013 and I don't understand what is going on there. IMHO, nothing in sagelib should be touching the PATH at all.

mkoeppe commented 6 months ago

Thanks for the reminder. I agree this needs to be fixed. I'll take a look in the next days.