doctest unnecessarily assumes SAGE_VENV/bin contains sage binary

[tornaria]

In my setup SAGE_VENV is left unset so it defaults to sys.prefix = '/usr'; in this way, sage uses system python packages (and everything works).

However, the sage binary is not necessarily installed as /usr/bin/sage. For example:

• I build just sagelib from source, so that the sage python packages are placed in .../pkgs/sagemath-standard/build/lib.linux-x86_64-3.10/ and the sage scripts are placed in .../pkgs/sagemath-standard/build/scripts-3.10/. I want to use and doctest this sagelib without installing (which can be done easily by setting PATH and PYTHONPATH to those directories).

Now /usr/bin/sage may or may not exist (depends on whether I have the system sagemath installed -- this is never true in github CI where the void linux package for sagemath is built and tested in a clean container)

In any case, the directory where the actual sage script is located can be obtained as os.path.dirname(sys.argv[0]) (this will normally be the same as SAGE_VENV/bin).

Component: doctest framework

Author: Gonzalo Tornaría

Branch/Commit: 0c1ca80

Reviewer: Matthias Koeppe

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

[tornaria]

comment:1

Doctest failure is:

**********************************************************************
File "pkgs/sagemath-standard/build/lib.linux-x86_64-3.10/sage/misc/sage_ostools.pyx", line 36, in sage.misc.sage_ostools.have_program
Failed example:
    have_program('sage', os.path.join(SAGE_VENV, 'bin'))
Expected:
    True
Got:
    False
**********************************************************************

[tornaria]

Branch: u/tornaria/33624

[tornaria]

Author: Gonzalo Tornaría

[tornaria]

Commit: 6ec4ee7

[tornaria]

New commits:

6ec4ee7

Trac 33624: fix doctest when sage scripts are not in SAGE_VENV/bin

[mkoeppe]

comment:3

Executing stuff out of the build/... directories is not a supported operation.
This is an internal directory of the setuptools build system of the Sage library and is not user-facing.

[tornaria]

comment:4

Replying to @mkoeppe:

Executing stuff out of the build/... directories is not a supported operation.
This is an internal directory of the setuptools build system of the Sage library and is not user-facing.

That's quite unfortunate, since it works very smoothly.

In any case, all the issues here and in #33625 would be the same if sagelib is installed to a custom directory (i.e. not /usr) while using /usr for system python packages.

---

PS: in void templates we try to run all tests and checks from upstream as much as possible. However, the standard way is to build and then check from build directory, this is done before packaging. There are no good ways to check packages after installation, I've seen that if a package requires installation to check the void maintainers tend to just disable the check step.

[mkoeppe]

comment:5

Note that this is not Sage-specific.
Python packaging has deprecated the whole user-facing interface of setuptools.
The process is to build a wheel, to install the wheel (possibly in a virtual env) and to test it there.

[tornaria]

comment:6

Replying to @mkoeppe:

Note that this is not Sage-specific.
Python packaging has deprecated the whole user-facing interface of setuptools.
The process is to build a wheel, to install the wheel (possibly in a virtual env) and to test it there.

I know, but it does work ok in sagelib right now and it would be a pity to loose it.

Still, it seems not quite right to use sys.prefix to know the directory where the sage scripts are installed. The sage script goes to lengths in order to figure out this directory, let's just use that.

In our void package we use --install-scripts=/usr/lib/sagemath/bin for the sage scripts, with /usr/bin/sage being a symlink. I'm actually thinking of using /usr/lib/sagemath/VERSION/bin so that different versions of sagemath can coexist in the same system.

As I said, all these issues are orthogonal to installing/not installing. Currently my workaround is to delete sage-venv-config so that SAGE_VENV is left unset (which I can't do in sage_conf as explained in #33625). And I still have to apply the patch in this ticket.

[tornaria]

comment:7

Here's a different approach:

--- a/src/sage/env.py
+++ b/src/sage/env.py
@@ -167,6 +167,7 @@ SAGE_VERSION_BANNER = var("SAGE_VERSION_BANNER", version.banner)
 
 # virtual environment where sagelib is installed
 SAGE_VENV = var("SAGE_VENV", os.path.abspath(sys.prefix))
+SAGE_BIN = var("SAGE_BIN", join(SAGE_VENV, "bin"))
 SAGE_LIB = var("SAGE_LIB", os.path.dirname(os.path.dirname(sage.__file__)))
 SAGE_EXTCODE = var("SAGE_EXTCODE", join(SAGE_LIB, "sage", "ext_data"))
 SAGE_VENV_SPKG_INST = var("SAGE_VENV_SPKG_INST", join(SAGE_VENV, "var", "lib", "sage", "installed"))
diff --git a/src/sage/misc/sage_ostools.pyx b/src/sage/misc/sage_ostools.pyx
index 57c8db69664..18330f94352 100644
--- a/src/sage/misc/sage_ostools.pyx
+++ b/src/sage/misc/sage_ostools.pyx
@@ -32,12 +32,12 @@ def have_program(program, path=None):
         True
         sage: have_program('there_is_not_a_program_with_this_name')
         False
-        sage: from sage.env import SAGE_VENV
-        sage: have_program('sage', os.path.join(SAGE_VENV, 'bin'))
+        sage: from sage.env import SAGE_BIN
+        sage: have_program('sage', SAGE_BIN)
         True
         sage: have_program('sage', '/there_is_not_a_path_with_this_name')
         False
-        sage: have_program('there_is_not_a_program_with_this_name', os.path.join(SAGE_VENV, 'bin'))
+        sage: have_program('there_is_not_a_program_with_this_name', SAGE_BIN)
         False
     """
     if path is None:

What do you think? This is now really a no-op, except now I can easily override SAGE_BIN in sage_conf.py. And there are other places that could be using SAGE_BIN (cf #33625).

[mkoeppe]

comment:8

have_program is on its way out. See #32957. We can replace these nonsense doctests for presence of the sage binary by something else on that ticket.

[mkoeppe]

comment:9

The whole practice of using SAGE_VENV/bin explicitly in the Sage library is also on its way out. See #31296 and tickets listed there. Help is welcome to take care of the remaining uses

[sagetrac-git]

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

0c1ca80

Trac 33624: don't assume SAGE_VENV/bin contains sage

[sagetrac-git]

Changed commit from 6ec4ee7 to 0c1ca80

[tornaria]

comment:11

Replying to @mkoeppe:

have_program is on its way out. See #32957. We can replace these nonsense doctests for presence of the sage binary by something else on that ticket.

I redid the patch with that in mind. Now we test have_program("sh", "/bin") and have_program('there_is_not_a_program_with_this_name', "/bin").

Since sage ships some scripts with /bin/sh in the shebang, I think we can assume /bin/sh to always exist.

[mkoeppe]

Reviewer: Matthias Koeppe

[mkoeppe]

comment:12

LGTM

[vbraun]

Changed branch from u/tornaria/33624 to 0c1ca80