#12627 closed defect (fixed)
The spkg/bin/sage script engraves paths to executables
Reported by:  Snark  Owned by:  leif 

Priority:  major  Milestone:  sage6.1 
Component:  scripts  Keywords:  rd2 
Cc:  kini  Merged in:  
Authors:  Julien Puydt, Volker Braun  Reviewers:  R. Andrew Ohana 
Report Upstream:  N/A  Work issues:  
Branch:  u/vbraun/use_path_for_binaries (Commits)  Commit:  44c4cf1d27b8562200c5b1978d29ad7b70b2f687 
Dependencies:  Stopgaps: 
Description
There is no need to make full paths explicit  it is even an hindrance when one moves things around.
Since sageenv already sets PATH correctly, let's just use that.
The use of 'command' to test for the presence of an executable in the path is portable, as documented in this stackoverflow question.
Change History (42)
comment:1 Changed 7 years ago by
 Status changed from new to needs_review
comment:2 Changed 7 years ago by
comment:3 Changed 7 years ago by
Done ; it wasn't just a case of applying the same patch, as now 'R' can be called through the script.
comment:4 Changed 7 years ago by
This patch doesn't seem like a bad idea, but I don't know if it's actually useful for anything. Using the environment variable SAGE_LOCAL
is supposed to make these paths portable already, so I'm not sure I understand the comment about the current situation being "an hindrance when one moves things around". Another way to say it: sageenv sets the PATH
by using SAGE_LOCAL
, so with or without this patch, if SAGE_LOCAL
is right, these commands will work, while if it is wrong, they won't.
comment:5 Changed 7 years ago by
With the current situation, sageenv makes sure the rest of sage has a good environment to work, but sagesage decides it knows better and shortcuts it.
This is one of the situations I denounced on the sagedevel mailinglist, where sage is supposed to be modular but in fact isn't, because the modules don't just depend on each other's work : there are hidden, subtle places where they do their own thing.
I have a few patches up my sleeve to move things around ; they're not supposed to go upstream anytime soon. But they make some hidden things appear, so I fix them, and as it's interesting for upstream, I report immediately, with a proper fix, independent of my experiments.
To be fully honest, the goal of my notforupstream experiments is precisely to find modularity problems and fix them...
comment:6 Changed 7 years ago by
 Keywords rd2 added
 Reviewers set to R. Andrew Ohana
 Status changed from needs_review to positive_review
The patch looks good to me and works properly for me.
comment:7 followup: ↓ 10 Changed 7 years ago by
Julien, I'm not sure whether you're aware of that, but your patch changes in at least some situations the fully intentional behavior.
For example, a couple of Sage components / programs are optional, like e.g. kash
or Macaulay2
, and it is not intended to run a systemwide / nonSage version of kash
(that might be found along PATH
) if one types sage kash
. (Although I admit that the current response in case Sage's version isn't installed  "File not found"  is quite odd, i.e., not very userfriendly.)
In other cases a path is prepended to make sure we really run the correct instance of a Sage script, namely that of the "current" Sage installation, given by SAGE_ROOT
(or SAGE_LOCAL
), or to make sure our assumptions about the current directory (or other variable settings) are correct  especially since different people develop things, sometimes perhaps too independently. Not very pythonic, but similar to assert()
, which at least signals errors [before things get worse and untraceable].
Redundancy is necessary to achieve some robustness, catch (and hopefully appropriately handle) errors when they occur, or as early as possible, e.g. if a script or program doesn't exist where it is supposed to be located, since we cannot rule out an installation gets messed up  probably by Sage itself, due to bugs, if not by a user...
comment:8 Changed 7 years ago by
Oh, forgot, the PATH
issue reminds me of this... (Web 3.0)
comment:9 Changed 7 years ago by
@leif: I did check sageenv prepended to PATH before proposing the patch, so I can confirm that sage will run what it wants when it wants something, and perhaps still run something even if it isn't sure what it wants. And when one moves things around, that is useful. So, positive review?
comment:10 in reply to: ↑ 7 Changed 7 years ago by
 Status changed from positive_review to needs_info
Replying to leif:
Julien, I'm not sure whether you're aware of that, but your patch changes in at least some situations the fully intentional behavior.
For example, a couple of Sage components / programs are optional, like e.g.
kash
orMacaulay2
, and it is not intended to run a systemwide / nonSage version ofkash
(that might be found alongPATH
) if one typessage kash
. (Although I admit that the current response in case Sage's version isn't installed  "File not found"  is quite odd, i.e., not very userfriendly.)
I think this still needs an answer.
comment:11 Changed 7 years ago by
 Summary changed from The sagesage script engraves paths to executables to The spkg/bin/sage script engraves paths to executables
comment:12 followups: ↓ 13 ↓ 15 ↓ 28 Changed 7 years ago by
Well, in fact I did answer, but apparently I wasn't clear enough ; let's rephrase it with arrays.
The current situation is :
insage  outofsage  what runs? 
yes  yes  insage 
yes  no  insage 
no  yes  error 
no  no  error 
with my patch :
insage  outofsage  what runs? 
yes  yes  insage 
yes  no  insage 
no  yes  outofsage 
no  no  error 
and additionnally, using PATH instead of putting the PATH inside removes implicit dependencies, which makes sage really more modular (things can move around and that won't break).
comment:13 in reply to: ↑ 12 Changed 7 years ago by
I think the current behaviour is better. I certainly would not expect
sage kash
to run some random kash
which happens to be installed on my system. I expect sage kash
to run the kash
from sage
and give an error message when kash
is not installed in Sage.
Replying to Snark:
and additionnally, using PATH instead of putting the PATH inside removes implicit dependencies, which makes sage really more modular (things can move around and that won't break).
I really don't see the need for this. Maybe you should be more explicit about the patches on your sleeve.
comment:14 Changed 7 years ago by
If there is a kash on your system, there shouldn't be any problem using it. Just because it doesn't come from sage doesn't mean it's wrong.
My patch makes sage less fragile (less error cases) and more portable (use of 'command' to test the presence of an executable) and less surprising (don't go against what sageenv says it PATH). It doesn't have the side effect of running something outofsage when it's available insage. What exactly is the problem?
[I can't make more patches public : I already made public those that looked good (see #12623, #12624, #12639 in addition of this one), and the rest were too hackish and are probably obsolete now. Besides, this one patch here has enough merits of its own.]
comment:15 in reply to: ↑ 12 Changed 7 years ago by
Replying to Snark:
using PATH instead of putting the PATH inside removes implicit dependencies, which makes sage really more modular (things can move around and that won't break).
I don't really understand what you want to "move around".
As mentioned, the explicit paths, although (or because they are  in a perfect world) redundant, are for safety reasons, since they lead to errors if a script is missing, i.e., not where it is supposed to be located. That's IMHO just fine. And I don't see what you want to make "more modular" there.
W.r.t. sage PROGRAM
:
If you want to run a systemwide installed version of R, type (outside of a Sage subshell) R
.
If you want to run Sage's version of R, type sage R
, or type R
from within a Sage subshell.
Same for optional components. The following part of your patch is IMHO plain wrong:

spkg/bin/sage
diff r 959bec9e01d9 r a46cac822f1e spkg/bin/sage
a b 92 92 echo " ipython [...]  run Sage's IPython using the default environment (not" 93 93 echo " Sage), passing additional options to IPython" 94 94 echo " kash [...]  run Sage's Kash with given arguments" 95 test x "$SAGE_LOCAL/bin/kash" \95 command v kash &>/dev/null  \ 96 96 echo " (not installed currently, run sage i kash)" 97 97 echo " lisp [...]  run Lisp interpreter included with Sage" 98 98 echo " M2 [...]  run Sage's Macaulay2 with given arguments" 99 test x "$SAGE_LOCAL/bin/M2" \99 command v M2 &>/dev/null  \ 100 100 echo " (not installed currently, run sage i macaulay2)" 101 101 echo " maxima [...]  run Sage's Maxima with given arguments" 102 102 echo " mwrank [...]  run Sage's mwrank with given arguments"
along with

spkg/bin/sage
diff r 959bec9e01d9 r a46cac822f1e spkg/bin/sage
a b 417 417 if [ "$1" = 'kash' o "$1" = 'kash' ]; then 418 418 cd "$CUR" 419 419 shift 420 "$SAGE_LOCAL/bin/kash""$@"420 kash "$@" 421 421 exit $? 422 422 fi 423 423
and

spkg/bin/sage
diff r 959bec9e01d9 r a46cac822f1e spkg/bin/sage
a b 445 445 if [ "$1" = 'M2' o "$1" = 'M2' ]; then 446 446 cd "$CUR" 447 447 shift 448 "$SAGE_LOCAL/bin/M2""$@"448 M2 "$@" 449 449 exit $? 450 450 fi 451 451
(Instead, we should use test x "$SAGE_LOCAL"/bin/<program>
there, too, and tell the user how to install it in case it's not. You could of course also use case "`command v <program>`" in ...
, although the latter might give unexpected results with symlinks; I'm not 100% sure about that.)
Similar for standard components (and other scripts) of Sage, since if  for whatever reason  $SAGE_LOCAL/bin/<program>
is missing, which is an error, no other program along the path which happens to have the same name should be run; instead, an appropriate error message should be printed.
If you want to "move things around", why not create symbolic links in $SAGE_LOCAL/bin/
?
Or you have to be more explicit with what you want to move, or what should be more customizable.
Slightly related:
To install small scripts to directly run Sage's versions of GAP, the PARI/GP interpreter, Maxima, or Singular etc. (by typing e.g. just 'gap' or 'gp') into a standard 'bin' directory, start Sage by typing 'sage' (or './sage') and enter something like install_scripts('/usr/local/bin') at the Sage command prompt ('sage:').
(Type install_scripts?
at the Sage prompt for more information, i.e., options to it.)
comment:16 followup: ↓ 17 Changed 7 years ago by
Sigh.
First, you still mention "test x" although I pointed out it's not the recommended way to check for an executable.
Second, as I already pointed out, my patch makes sage run sage versions by default if available. And if not available in sage, if they are available on the system, use that. The only error case is the one where indeed, it is really not available. That makes sense.
There seem to be a high dose of paranoia in sage developers about everything that doesn't come out of sage.
You still have to show me why I should be more cautious about running /usr/bin/kash : after all, sage runs on the system bash and uses the system libc! And for that last one, there are definitive reports where that gives problems and failed doctests  I know it well, it's on ubuntu/ARM and I did the port!
My point is simple : from a sagewhateverbeta_n to sagewhateverbeta_{n+1}, 90% of the spkg are the same. So if I could make foo3.14.spkg install in $HOME/sage/foo3.14 and make the rest of sage use that (this is modularity  the contrary of a oneblock where nothing can move an hair's width without everything collapsing), then that means I can gain 90% of build time. And I have the room to have more versions of sage installed simultaneously.
And maintaining a big chunk of symlinks is a bad solution to a wrong problem.
comment:17 in reply to: ↑ 16 ; followup: ↓ 21 Changed 7 years ago by
Replying to Snark:
Sigh.
First, you still mention "test x" although I pointed out it's not the recommended way to check for an executable.
You "pointed out" without saying why. I think test x
is used perfectly here.
Second, as I already pointed out, my patch makes sage run sage versions by default if available. And if not available in sage, if they are available on the system, use that. The only error case is the one where indeed, it is really not available.
As I (and leif, for once I agree with him) have said, sage COMMAND is supposed to run the Sage version of COMMAND.
There seem to be a high dose of paranoia in sage developers about everything that doesn't come out of sage.
That's not really the point. Within Sage, we have more control over the versions of the optional packages. Also, nothing prevents you from running in a shell (either a Sage shell or not):
$ kash
It's as easy as that.
My point is simple : from a sagewhateverbeta_n to sagewhateverbeta_{n+1}, 90% of the spkg are the same. So if I could make foo3.14.spkg install in $HOME/sage/foo3.14 and make the rest of sage use that (this is modularity  the contrary of a oneblock where nothing can move an hair's width without everything collapsing), then that means I can gain 90% of build time. And I have the room to have more versions of sage installed simultaneously.
You could play with environment variables to solve this: apply patch #12647 and write in your .sage/sagerc
:
export PATH="$PATH:$HOME/sage/foo3.14/local/bin" export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$HOME/sage/foo3.14/local/lib"
or something like this.
comment:18 followup: ↓ 19 Changed 7 years ago by
 the .sagerc trick depends on setting PATH, but my point is precisely that spkg/bin/sage doesn't use PATH, but instead points directly to where it insists things must be (one of my dirty definitelynotforupstream patch was precisely modifying sageenv to set PATH, LD_LIBRARY_PATH and the rest...) ;
 "test x" is nice when you know exactly where things must be ; but if they can be in another place, then that doesn't fly. "command v" is portable and will find things in PATH.
comment:19 in reply to: ↑ 18 Changed 7 years ago by
Why do you insist on running
sage kash
as opposed to
kash
comment:20 followup: ↓ 22 Changed 7 years ago by
I don't insist on running "sage kash".
I ran "make ptestlong" and it tried to run things using "sage something".
Kash wasn't the problem ; I don't remember if it was ecl or scons... or something else... Of course the test failed : even though I made sure in sageenv it had what it takes in the PATH, the script didn't even see it!
I claim sageasitis is broken because it can't accept a small change in a place without breaking in an unrelated place for no serious reason.
My patch doesn't break sageasitis, and makes it more easy to modify in the future: tomorrow, you'll decide to put the executables in $SAGE/whatever/bin instead of $SAGE_LOCAL/bin. Of course, you'll modify sageenv to cope with the change. And it will still not work. Why? Because that script doesn't care about sageenv, it does it's own thing... and I claim that is wrong.
My patch makes it possible to change where sage finds executables in one sensible place instead of several.
I'm not telling you: "There is a theoretical issue, drop everything you do and work on it for me!". I'm telling you : "I made private changes, hit a practical issue, and even though you might not have met it yet, there's a chance you will, and here is a simple nice fix."
Does it sound that unreasonable?
comment:21 in reply to: ↑ 17 Changed 7 years ago by
Replying to jdemeyer:
Replying to Snark:
First, you still mention "test x" although I pointed out it's not the recommended way to check for an executable.
That's simply bu**sh**. First of all, I did mention command v
as well, and more importantly, test x
and command
have different use cases. Nobody claimed that command
wasn't portable; we use it in many places of Sage  where it is appropriate.
You "pointed out" without saying why. I think
test x
is used perfectly here.
It is.
Second, as I already pointed out, my patch makes sage run sage versions by default if available. And if not available in sage, if they are available on the system, use that. The only error case is the one where indeed, it is really not available.
As I (and leif, for once I agree with him)
:)
have said, sage COMMAND is supposed to run the Sage version of COMMAND.
And if sage PROGRAM
should run external versions at all, then the help messages would have to be changed as well, which your patch doesn't, although it changes the (btw. intuitive) behavior:
echo " kash [...]  run Sage's Kash with given arguments" ... echo " M2 [...]  run Sage's Macaulay2 with given arguments"
(And if there happen to be systemwide versions along the path, there's no mention that Sage's optional packages could be installed [in case they aren't already]. People frequently have completely outdated, incompatible or broken versions installed  sometimes even not to their knowledge, which is one reason Sage ships such a lot, or at least offers such components as optional packages.)
There seem to be a high dose of paranoia in sage developers about everything that doesn't come out of sage.
Sometimes for good reason; see above.
And as mentioned, it's better to not blindly trust even the Sage installation itself, since its integrity is not necessarily an invariant, especially if users (and developers, which most people of the Sage community intentionally don't differentiate) "play" with it.
comment:22 in reply to: ↑ 20 Changed 7 years ago by
Replying to Snark:
I ran "make ptestlong" and it tried to run things using "sage something".
Kash wasn't the problem ; I don't remember if it was ecl or scons... or something else... Of course the test failed : even though I made sure in sageenv it had what it takes in the PATH, the script didn't even see it!
I claim sageasitis is broken because it can't accept a small change in a place without breaking in an unrelated place for no serious reason.
Not in an "unrelated place", nor for no reason: ptestlong
checks whether the installation went (or is still) ok, and testing whether programs installed by Sage are where they belong (or have been installed), and whether they work, is just part of that.
If you change their locations, you'd obviously also have to change these tests.
comment:23 Changed 7 years ago by
Oh, dear, that was the thing I missed to understand!
Ok, so when "make ptestlong" runs "sage something", it really checks :
 things are at the right place ;
 things work ;
 things work correctly.
I was only considering points 2 and 3 since the beginning! My patch does indeed break point 1.
Now, I still think there are things to fix :
 checking that something was installed at the right place should be done by something.spkg, not in somewhere unrelated  that will cover 1 where it should be covered ;
 and then my patch should be applied, because now the implicit constraint will be satisfied in an explicit way by a proper test somewhere.
comment:24 Changed 7 years ago by
How about introducing SAGE_BINARIES_PATH
? (to be optionally set by the user; containing a colonseparated list of directories to search for Sage programs and scripts)
(sageenv
would probably prepend e.g. "$SAGE_LOCAL/bin:"
if SAGE_BINARIES_PATH
is nonempty, otherwise set it to the former.)
Then we could use (logically)
PATH="$SAGE_BINARIES_PATH" command v FOO >/dev/null  echo "Sage's FOO is not installed..."
or
PATH="$SAGE_BINARIES_PATH" command FOO # exit status 127 would (usually) tell that FOO wasn't found (along $SAGE_BINARIES_PATH)
and similar.
A potential problem being that FOO would inherit the modified PATH
in the second example; we could therefore also use
prog=`PATH="$SAGE_BINARIES_PATH" command v FOO` if [[ z $prog ]]; then # FOO isn't installed anywhere along $SAGE_BINARIES_PATH else # found; for example run FOO: $prog fi
Better names appreciated. SAGE_PATH
is unfortunately already used for a slightly different purpose.
comment:25 followup: ↓ 26 Changed 7 years ago by
I'm not sure the idea is that neat, but SAGE_EXEC_PATH could be a better name for it ;)
"PATH" does it already... doesn't it?
comment:26 in reply to: ↑ 25 Changed 7 years ago by
Replying to Snark:
"PATH" does it already... doesn't it?
PATH
contains arbitrary things... (and we still don't want to remove parts from it ;) )
comment:27 Changed 7 years ago by
 Cc kini added
comment:28 in reply to: ↑ 12 Changed 6 years ago by
The build system proposed in http://wiki.sagemath.org/BuildSystemSEP (#14796), provides options to disable packages (and falling back to already installed things outside of sage). also sage (the library) needs to work without the distribution.
Finally, under some circumstances, sage PROGRAM needs to just use plain $PATH. And it always should.
So lets solve this problem at its root:
 sage (the distribution) places a dummy into $SAGE_LOCAL/bin/p for all enabled packages p, if such a file doesnt exist (or more dummies if needed)
 installation of a package will replace these dummies
 sage (the distribution) clears these dummies upon disabling a package (which also triggers uninstall)
 sage (the environment) just sets PATH=$(SAGE_LOCAL)/bin:$PATH (as it currently does)
 sage (the library) works outside sage (the distribution) without headache
To deal with even more paranoia, sage (the distribution) could also include a list of header and library locations for each package. Placed in a similar way, respective files could make sure gcc won't fall back to /usr/include or /usr/lib without your consent.
comment:29 Changed 6 years ago by
This is now implemented in #14796 (Component: 'build'). Here, Sage (the distribution) creates/removes dummies depending on how it is configured. $PATH
is made to contain SAGE_LOCAL/bin
, PATH where the dummies are
, the system PATH
in that order.
With this mechanism, sage PROGRAM
may safely use $PATH
without accidentally running into system executables from within sage (the distribution).
comment:30 Changed 6 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:31 Changed 6 years ago by
 Status changed from needs_info to needs_review
comment:32 Changed 6 years ago by
Please rebase this:
(sagesh) Nav1ZWFD@compute4a:bin$ hg import trac12627.patch applying trac12627.patch patching file spkg/bin/sage Hunk #2 FAILED at 265 Hunk #3 FAILED at 367 Hunk #4 FAILED at 402 Hunk #5 FAILED at 430 Hunk #6 FAILED at 465 Hunk #7 FAILED at 548 6 out of 7 hunks FAILED  saving rejects to file spkg/bin/sage.rej abort: patch failed to apply
There's exec's at the beginning of many of the lines in sage, which cause the patch to be rejected. Otherwise, I think this should go into Sage soonish, since the change makes sense, and will help with the OS X port ticket (e.g., because there we use the systemwide sqlite3, but we still want sage sqlite3
to be portable and work).
comment:33 Changed 6 years ago by
 Status changed from needs_review to needs_work
comment:34 followup: ↓ 35 Changed 6 years ago by
comment:35 in reply to: ↑ 34 Changed 6 years ago by
Replying to jhpalmieri:
Is it subsumed by #14796?
yes, in #14796, '$PATH' is used to locate executables. it is a good idea to to so, whether or not a dummy wall or a configuarable toplevel build system exists.
... and while you/someone are/is at it: the code may be reworded to
arg=${1//} case $arg in axiomcombinatgapgpsingularsqlite3twistdecl\ kashmaximamwrankM2sconspythonipythonR) shift exec "${arg#}" "$@" ;; lisp) shift exec "ecl" "$@" ;; esac
, so it will even fit on a single screen.
comment:36 Changed 6 years ago by
 Branch set to u/vbraun/use_path_for_binaries
 Commit set to 44c4cf1d27b8562200c5b1978d29ad7b70b2f687
New commits:
44c4cf1  Use foo from path when running sage foo 
comment:37 Changed 6 years ago by
 Status changed from needs_work to needs_review
IMHO the whole sage foo
script is a mistake. If you want to use Sage's version of binaries, you put them into your path, should be as easy as that. Of course this assumes proper rpaths for libraries instead of the LD_LIBRARY_PATH
hack.
In any case, until then we should just implement the proposed workaround.
comment:38 Changed 6 years ago by
 Status changed from needs_review to positive_review
Passes doctests.
comment:39 Changed 6 years ago by
 Milestone changed from sage5.13 to sage6.0
comment:40 Changed 6 years ago by
 Milestone changed from sage6.0 to sage6.1
comment:41 Changed 6 years ago by
 Resolution set to fixed
 Status changed from positive_review to closed
comment:42 Changed 6 years ago by
I'm not sure the issues in comment 7 were ever really resolved. Rather than merging this right away, it might be good to discuss it on sagedevel first. There are at least two issues:
 if you run
sage kash
, should it run a system version of kash if Sage's optional kash spkg hasn't been installed, or not?  in the old version there was some redundancy: the path to
gap
, for example, was explicit, to make sure that Sage's version of GAP was used. Is this redundancy important?
Also, the positive review is not appropriate, since the issue in comment 21 was not fixed: the message for sage help
says that sage M2
(for example) runs "Sage's Macaulay2", but this is no longer accurate with the current patch.
This needs to be rebased to Sage 5.0:
sagesage
no longer exists; the relevant script is$SAGE_ROOT/spkg/bin/sage
and is in the root repository.