#11707 closed defect (duplicate)
Remove `readlink -n` and `realpath` from $SAGE_ROOT/sage
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | scripts | Keywords: | SAGE_ROOT |
Cc: | Merged in: | ||
Authors: | Reviewers: | Jeroen Demeyer | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The file `$SAGE_ROOT/sage' contains the lines
if [ "$SAGE_ROOT" = "....." ]; then SAGE_ROOT=`readlink -n "$0" 2> /dev/null` || \ SAGE_ROOT=`realpath "$0" 2> /dev/null` || \ SAGE_ROOT="$0"
However, readlink -n
certainly does not do what is intended:
- It only works when
$0
(the sage executable itself) is a symbolic link - If the sage executable is a symbolic link, then
readlink -n
returns the link itself, not the canonicalized name. Example: if/usr/local/sage-4.7.1/sage
is a symbolic link tosagefoo
, thenSAGE_ROOT
would becomesagefoo
when'/usr/local/sage-4.7.1/sagefoo
is intended.
Maybe, the whole readlink
/realpath
logic should be removed.
Change History (15)
comment:1 follow-up: ↓ 2 Changed 11 years ago by
comment:2 in reply to: ↑ 1 ; follow-up: ↓ 3 Changed 11 years ago by
Replying to leif:
Haha, had the same idea just yesterday.
IMHO we should use (the more portable and perhaps simpler)
(cd "$WHEREVER"; NORMALIZED_PATH=`pwd -P`)to get a canonical form in shell scripts.
I'm not sure if this also fixes the apparently sometimes prepended automount prefix (cf.
sage-ptest
etc.).
I guess you mean
NORMALIZED_PATH=`cd "$WHEREVER"; pwd -P`
because otherwise the "NORMALIZED_PATH" from the subshell will not be seen by the main shell.
comment:3 in reply to: ↑ 2 Changed 11 years ago by
Replying to jdemeyer:
I guess you mean
NORMALIZED_PATH=`cd "$WHEREVER"; pwd -P`
because otherwise the "NORMALIZED_PATH" from the subshell will not be seen by the main shell.
Yes, indeed I meant something like:
MY_PATH=${0%/*} CANON_PATH=`cd "$MY_PATH" && pwd -P`
comment:4 Changed 11 years ago by
P.S.: sage-env
should also get changed when guessing SAGE_ROOT
(cf. also #11687).
comment:5 Changed 11 years ago by
I mean we should also simplify / use pwd -P
in sage-env
's
# GUESS SAGE_ROOT from pwd SAVEDIR="`pwd`" if [ -f sage -a -d spkg ]; then GUESSED_SAGE_ROOT="`pwd`" else if [ -f ../../sage -a -d ../../spkg ]; then cd ../../ GUESSED_SAGE_ROOT="`pwd`" else GUESSED_SAGE_ROOT="" fi fi cd "$SAVEDIR"
Sorry, currently doing four things at the same time. 8/
comment:6 in reply to: ↑ description ; follow-up: ↓ 7 Changed 11 years ago by
Replying to jdemeyer:
The file `$SAGE_ROOT/sage' contains the lines
...
However,
readlink -n
certainly does not do what is intended:
...
Using readlink -n
is definitely not "the right thing". However:
- Using it makes a symlink work, provided it is a full path symlink (and there are no recursive symlinks, etc)
- One way to properly expand the symlink is using
readlink -f
. That was proposed as a fix for #5852, it was applied, and later unapplied because using fully canonicalized paths causes other issues with doctesting. It just happens that usingreadlink -n
is a middle-ground that works in some cases but usingreadlink -f
exposes the doctesting issue in some common installs.
Thus, removing the use of readlink -n
will break some common usage while not fixing anything. Replacing it by readlink -f
fixes the canonicalization issue but will cause a regression by exposing a bug the way doctesting detects library code (see #5852 comment 12).
Additional remark: note that using readlink -f
is not completely portable; neither is using realpath
. Using both covers a majority of systems but not all of them. A more portable solution was proposed in #6146 but it doesn't make sense to push it until #5852 is resolved.
comment:7 in reply to: ↑ 6 Changed 11 years ago by
Replying to tornaria:
Thus, removing the use of
readlink -n
will break some common usage while not fixing
The common usage I refer to is the following:
- I install sage x.y in a directory
~/sage/sage-x.y
- I make a symlink
~/bin/sage -> ~/sage/sage-x.y/sage
so that the sage command is available in my path [note the full path symlink].
This works fine right now, and would break if you remove "readlink -n".
NOTE: When "~/bin/sage" is a symlink with a relative path symlink, this doesn't work, and "readlink -f" would work. However, this leads to other breakup (this is the reason why #5852 was reverted and #6146 was not developed further). If you want to fix this you should start with #5852 (comment 12).
comment:8 follow-up: ↓ 11 Changed 11 years ago by
It is not immediately clear why using pwd -P
, which fully expands symbolic links, and therefore would canonicalize $SAGE_ROOT
as well, should break doctesting.
Even if it did somehow, this should be addressed in the corresponding scripts, sage-{test,ptest,doctest}
, which are currently under work anyway, and have certainly changed since #5852 was opened (or last modified).
Taking the canonical path of both the files to test and $SAGE_ROOT
(or, more precisely, of the current branch of its Sage library), there shouldn't be any ambiguity in distinguishing Sage library code from other code. We also meanwhile have an option -force_lib
to sage -t
and its friends.
Slashs, dashs etc. from paths of files to test should never end up in an import
statement, and I don't think they currently could, as we copy source files anyway.
comment:9 follow-up: ↓ 10 Changed 11 years ago by
I attach a shell script which should be portable (however, I have not yet tested this) and behaves like "readlink -f" if combined with ( cd $dir && pwd )
. I guess this would solve all problems mentioned here. I still have to think about the best way of using this from $SAGE_ROOT/sage
.
comment:10 in reply to: ↑ 9 ; follow-up: ↓ 13 Changed 11 years ago by
Replying to jdemeyer:
I attach a shell script which should be portable (however, I have not yet tested this) and behaves like "readlink -f" if combined with
( cd $dir && pwd )
. I guess this would solve all problems mentioned here. I still have to think about the best way of using this from$SAGE_ROOT/sage
.
I'll suggest for a last time that you check out #6146. There is a script there that is simpler, shorter, and has some testing. Portability is not so easy... your script hangs on my first try on SunOS 5.10 (fulvia). Note also that because we are assuming bash, we can portably use "pwd -P" as a bash builtin.
If you can figure out a way to make "readlink -f" work (when available), i.e. finish fixing #5852, then #6146 already gives a solution for systems where "readlink -f" is not available.
comment:11 in reply to: ↑ 8 Changed 11 years ago by
Replying to leif:
It is not immediately clear why using
pwd -P
, which fully expands symbolic links, and therefore would canonicalize$SAGE_ROOT
as well, should break doctesting.
The issue is described in comment:ticket:5852:12. Summary:
- there's a symlink
/home/wstein/sage-build -> /tmp/wstein
- sage is untarred and compiled inside
/home/wstein/sage-build/sage-x.y/
- doctesting as
sage -t /home/wstein/sage-x.y/...
The last step fails if SAGE_ROOT is fully canonicalized to /tmp/wstein/sage-x.y/...
because in that case the doctesting code didn't realize /home/wstein/sage-x.y/...
is library code. IOW, that workflow worked for William because canonicalization was broken!
I presume the fix is that the comparision done for detecting library code be done with using canonical paths, etc.
Even if it did somehow, this should be addressed in the corresponding scripts,
sage-{test,ptest,doctest}
, which are currently under work anyway, and have certainly changed since #5852 was opened (or last modified).
I agree. I haven't followed the development of these scripts, so I wouldn't know if the issue was addressed or not. If it is fixed, then #5852 can be reapplied and then #6164 can easily be turned into a patch that extends canonicalization to other systems where readlink -f is not available.
comment:12 Changed 11 years ago by
Yesterday after reading some of this, I posted a question on #5852: has the main obstacle there been resolved? The relevant code in sage-doctest seems to resolve all symbolic links in the various paths before comparing them, so whatever we do elsewhere shouldn't cause problems with doctesting.
comment:13 in reply to: ↑ 10 Changed 11 years ago by
Replying to tornaria:
your script hangs on my first try on SunOS 5.10 (fulvia).
Fixed that.
If you can figure out a way to make "readlink -f" work (when available)
I don't think we should make "readlink -f" work when available. I think either your or my bash script should do the job without making any assumptions on availability of realpath
or readlink -f
.
comment:14 Changed 11 years ago by
- Resolution set to duplicate
- Reviewers set to Jeroen Demeyer
- Status changed from new to closed
Let's move the whole discussion to #5852.
comment:15 Changed 11 years ago by
- Milestone changed from sage-4.7.2 to sage-duplicate/invalid/wontfix
Haha, had the same idea just yesterday.
IMHO we should use (the more portable and perhaps simpler)
to get a canonical form in shell scripts.
I'm not sure if this also fixes the apparently sometimes prepended automount prefix (cf.
sage-ptest
etc.).