sagemath/sage

Put SAGE_ROOT/src/bin in PATH only when invoked by SAGE_ROOT/sage or sage-build-env

Closed this issue · 14 comments

Only when invoked as SAGE_ROOT/sage or SAGE_ROOT/src/bin/sage, then $SAGE_ROOT/src/bin should be put in the front of PATH.

When the installed sage script, SAGE_VENV/bin/sage, is invoked directly, it should not put $SAGE_ROOT/src/bin on the PATH.

CC: @kwankyu @jhpalmieri

Component: scripts

Author: Matthias Koeppe

Branch/Commit: 7519048

Reviewer: Kwankyu Lee

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

New commits:

7519048src/bin/sage[-env]: Put SAGE_ROOT/src/bin in front of path only if run out of this directory

Author: Matthias Koeppe

Description changed:

--- 
+++ 
@@ -1,3 +1,5 @@
-The installed `sage` script should only use the installed helper scripts. 
+Only when invoked as `SAGE_ROOT/sage` or `SAGE_ROOT/src/bin/sage`, then `$SAGE_ROOT/src/bin` should be put in the front of `PATH`.
+
+When the installed `sage` script, `SAGE_VENV/bin/sage`, is invoked directly, it should not put `$SAGE_ROOT/src/bin` on the `PATH`.
 
 

Commit: 7519048

comment:3

Is this supposed to solve a particular problem, or is it general cleanup?

comment:4

It fixes a problem when sagemath-standard is installed in a user-defined venv. In this case, before this ticket, the installed sage script would actually end up running sage from SAGE_ROOT/venv.

We ran into this in #29865. The distribution sagemath-objects, as of #29865, also installs the sage script, and it had this behavior.

comment:5

Replying to @mkoeppe:

We ran into this in #29865. The distribution sagemath-objects, as of #29865, also installs the sage script, and it had this behavior.

As this is merged with #29865, we can set the status when we do so for #29865. Is this okay?

comment:6

That's fine, but the idea with opening this separate ticket was that it can be reviewed separately.

comment:7

Replying to @mkoeppe:

That's fine, but the idea with opening this separate ticket was that it can be reviewed separately.

Of course, it is okay that someone else, perhaps John, reviews this, earlier than #29865.

comment:8

Already merged with #29865

comment:9

It's better to merge it, so that the ticket description is included in the commit message.

Reviewer: Kwankyu Lee