Sage: Ticket #11707: Remove `readlink -n` and `realpath` from $SAGE_ROOT/sage
https://trac.sagemath.org/ticket/11707
<p>
The file `$SAGE_ROOT/sage' contains the lines
</p>
<pre class="wiki">if [ "$SAGE_ROOT" = "....." ]; then
SAGE_ROOT=`readlink -n "$0" 2> /dev/null` || \
SAGE_ROOT=`realpath "$0" 2> /dev/null` || \
SAGE_ROOT="$0"
</pre><p>
However, <code>readlink -n</code> certainly does not do what is intended:
</p>
<ul><li>It only works when <code>$0</code> (the sage executable itself) is a symbolic link
</li><li>If the sage executable is a symbolic link, then <code>readlink -n</code> returns the link itself, not the canonicalized name. Example: if <code>/usr/local/sage-4.7.1/sage</code> is a symbolic link to <code>sagefoo</code>, then <code>SAGE_ROOT</code> would become <code>sagefoo</code> when <code>'/usr/local/sage-4.7.1/sagefoo</code> is intended.
</li></ul><p>
Maybe, the whole <code>readlink</code>/<code>realpath</code> logic should be removed.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/11707
Trac 1.1.6leifThu, 18 Aug 2011 20:37:25 GMT
https://trac.sagemath.org/ticket/11707#comment:1
https://trac.sagemath.org/ticket/11707#comment:1
<p>
Haha, had the same idea just yesterday.
</p>
<p>
IMHO we should use (the more portable and perhaps simpler)
</p>
<div class="wiki-code"><div class="code"><pre><span class="o">(</span><span class="nb">cd</span> <span class="s2">"$WHEREVER"</span>; <span class="nv">NORMALIZED_PATH</span><span class="o">=</span><span class="sb">`</span><span class="nb">pwd</span> -P<span class="sb">`</span><span class="o">)</span>
</pre></div></div><p>
to get a canonical form in shell scripts.
</p>
<p>
I'm not sure if this also fixes the apparently sometimes prepended automount prefix (cf. <code>sage-ptest</code> etc.).
</p>
TicketjdemeyerThu, 18 Aug 2011 20:44:10 GMT
https://trac.sagemath.org/ticket/11707#comment:2
https://trac.sagemath.org/ticket/11707#comment:2
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11707#comment:1" title="Comment 1">leif</a>:
</p>
<blockquote class="citation">
<p>
Haha, had the same idea just yesterday.
</p>
<p>
IMHO we should use (the more portable and perhaps simpler)
</p>
<div class="wiki-code"><div class="code"><pre><span class="o">(</span><span class="nb">cd</span> <span class="s2">"$WHEREVER"</span>; <span class="nv">NORMALIZED_PATH</span><span class="o">=</span><span class="sb">`</span><span class="nb">pwd</span> -P<span class="sb">`</span><span class="o">)</span>
</pre></div></div><p>
to get a canonical form in shell scripts.
</p>
<p>
I'm not sure if this also fixes the apparently sometimes prepended automount prefix (cf. <code>sage-ptest</code> etc.).
</p>
</blockquote>
<p>
I guess you mean
</p>
<pre class="wiki">NORMALIZED_PATH=`cd "$WHEREVER"; pwd -P`
</pre><p>
because otherwise the "NORMALIZED_PATH" from the subshell will not be seen by the main shell.
</p>
TicketleifThu, 18 Aug 2011 20:51:45 GMT
https://trac.sagemath.org/ticket/11707#comment:3
https://trac.sagemath.org/ticket/11707#comment:3
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11707#comment:2" title="Comment 2">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
I guess you mean
</p>
</blockquote>
<pre class="wiki">NORMALIZED_PATH=`cd "$WHEREVER"; pwd -P`
</pre><blockquote class="citation">
<p>
because otherwise the "NORMALIZED_PATH" from the subshell will not be seen by the main shell.
</p>
</blockquote>
<p>
Yes, indeed I meant something like:
</p>
<div class="wiki-code"><div class="code"><pre><span class="nv">MY_PATH</span><span class="o">=</span><span class="k">${</span><span class="nv">0</span><span class="p">%/*</span><span class="k">}</span>
<span class="nv">CANON_PATH</span><span class="o">=</span><span class="sb">`</span><span class="nb">cd</span> <span class="s2">"$MY_PATH"</span> <span class="o">&&</span> <span class="nb">pwd</span> -P<span class="sb">`</span>
</pre></div></div>
TicketleifThu, 18 Aug 2011 20:59:06 GMT
https://trac.sagemath.org/ticket/11707#comment:4
https://trac.sagemath.org/ticket/11707#comment:4
<p>
P.S.: <code>sage-env</code> should also get changed when guessing <code>SAGE_ROOT</code> (cf. also <a class="closed ticket" href="https://trac.sagemath.org/ticket/11687" title="defect: Sanitize `sage-env` (closed: fixed)">#11687</a>).
</p>
TicketleifThu, 18 Aug 2011 21:06:37 GMT
https://trac.sagemath.org/ticket/11707#comment:5
https://trac.sagemath.org/ticket/11707#comment:5
<p>
I mean we should also simplify / use <code>pwd -P</code> in <code>sage-env</code>'s
</p>
<div class="wiki-code"><div class="code"><pre>
<span class="c"># GUESS SAGE_ROOT from pwd
</span><span class="nv">SAVEDIR</span><span class="o">=</span><span class="s2">"`pwd`"</span>
<span class="k">if</span> <span class="o">[</span> -f sage -a -d spkg <span class="o">]</span>; <span class="k">then
</span><span class="nv">GUESSED_SAGE_ROOT</span><span class="o">=</span><span class="s2">"`pwd`"</span>
<span class="k">else
if</span> <span class="o">[</span> -f ../../sage -a -d ../../spkg <span class="o">]</span>; <span class="k">then
</span><span class="nb">cd</span> ../../
<span class="nv">GUESSED_SAGE_ROOT</span><span class="o">=</span><span class="s2">"`pwd`"</span>
<span class="k">else
</span><span class="nv">GUESSED_SAGE_ROOT</span><span class="o">=</span><span class="s2">""</span>
<span class="k">fi
fi
</span><span class="nb">cd</span> <span class="s2">"$SAVEDIR"</span>
</pre></div></div><p>
Sorry, currently doing four things at the same time. 8/
</p>
TickettornariaFri, 19 Aug 2011 02:10:26 GMT
https://trac.sagemath.org/ticket/11707#comment:6
https://trac.sagemath.org/ticket/11707#comment:6
<p>
Replying to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11707" title="defect: Remove `readlink -n` and `realpath` from $SAGE_ROOT/sage (closed: duplicate)">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
The file `$SAGE_ROOT/sage' contains the lines
</p>
</blockquote>
<blockquote class="citation">
<p>
...
</p>
</blockquote>
<blockquote class="citation">
<p>
However, <code>readlink -n</code> certainly does not do what is intended:
</p>
</blockquote>
<blockquote class="citation">
<p>
...
</p>
</blockquote>
<p>
Using <code>readlink -n</code> is definitely not "the right thing". However:
</p>
<ol class="loweralpha"><li>Using it makes a symlink work, <em>provided</em> it is a full path symlink (and there are no recursive symlinks, etc)
</li><li>One way to properly expand the symlink is using <code>readlink -f</code>. That was proposed as a fix for <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a>, it was applied, and later unapplied because using fully canonicalized paths causes other issues with doctesting. It just happens that using <code>readlink -n</code> is a middle-ground that works in some cases but using <code>readlink -f</code> exposes the doctesting issue in some common installs.
</li></ol><p>
Thus, removing the use of <code>readlink -n</code> will break some common usage while not fixing anything. Replacing it by <code>readlink -f</code> fixes the canonicalization issue but will cause a regression by exposing a bug the way doctesting detects library code (see <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a> comment 12).
</p>
<hr />
<p>
Additional remark: note that using <code>readlink -f</code> is not completely portable; neither is using <code>realpath</code>. Using both covers a majority of systems but not all of them. A more portable solution was proposed in <a class="closed ticket" href="https://trac.sagemath.org/ticket/6146" title="defect: the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand ... (closed: duplicate)">#6146</a> but it doesn't make sense to push it until <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a> is resolved.
</p>
TickettornariaFri, 19 Aug 2011 02:22:34 GMT
https://trac.sagemath.org/ticket/11707#comment:7
https://trac.sagemath.org/ticket/11707#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11707#comment:6" title="Comment 6">tornaria</a>:
</p>
<blockquote class="citation">
<p>
Thus, removing the use of <code>readlink -n</code> will break some common usage while not fixing
</p>
</blockquote>
<p>
The common usage I refer to is the following:
</p>
<ol><li>I install sage x.y in a directory <code>~/sage/sage-x.y</code>
</li><li>I make a symlink <code>~/bin/sage -> ~/sage/sage-x.y/sage</code> so that the sage command is available in my path [note the full path symlink].
</li></ol><p>
This works fine right now, and would break if you remove "readlink -n".
</p>
<hr />
<p>
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 <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a> was reverted and <a class="closed ticket" href="https://trac.sagemath.org/ticket/6146" title="defect: the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand ... (closed: duplicate)">#6146</a> was not developed further). If you want to fix this you should start with <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a> (comment 12).
</p>
TicketleifFri, 19 Aug 2011 09:14:50 GMT
https://trac.sagemath.org/ticket/11707#comment:8
https://trac.sagemath.org/ticket/11707#comment:8
<p>
It is not immediately clear why using <code>pwd -P</code>, which fully expands symbolic links, and therefore would canonicalize <code>$SAGE_ROOT</code> as well, should break doctesting.
</p>
<p>
Even if it did somehow, this should be addressed in the corresponding scripts, <code>sage-{test,ptest,doctest}</code>, which are currently under work anyway, and have certainly changed since <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a> was opened (or last modified).
</p>
<p>
Taking the canonical path of both the files to test and <code>$SAGE_ROOT</code> (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 <code>-force_lib</code> to <code>sage -t</code> and its friends.
</p>
<p>
Slashs, dashs etc. from paths of files to test should never end up in an <code>import</code> statement, and I don't think they currently could, as we copy source files anyway.
</p>
TicketjdemeyerFri, 19 Aug 2011 14:12:37 GMT
https://trac.sagemath.org/ticket/11707#comment:9
https://trac.sagemath.org/ticket/11707#comment:9
<p>
I attach a shell script which should be portable (however, I have not yet tested this) and behaves like "readlink -f" if combined with <code>( cd $dir && pwd )</code>. I guess this would solve all problems mentioned here. I still have to think about the best way of using this from <code>$SAGE_ROOT/sage</code>.
</p>
TickettornariaFri, 19 Aug 2011 14:55:59 GMT
https://trac.sagemath.org/ticket/11707#comment:10
https://trac.sagemath.org/ticket/11707#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11707#comment:9" title="Comment 9">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
I attach a shell script which should be portable (however, I have not yet tested this) and behaves like "readlink -f" if combined with <code>( cd $dir && pwd )</code>. I guess this would solve all problems mentioned here. I still have to think about the best way of using this from <code>$SAGE_ROOT/sage</code>.
</p>
</blockquote>
<p>
I'll suggest for a last time that you check out <a class="closed ticket" href="https://trac.sagemath.org/ticket/6146" title="defect: the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand ... (closed: duplicate)">#6146</a>. 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.
</p>
<p>
If you can figure out a way to make "readlink -f" work (when available), i.e. finish fixing <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a>, then <a class="closed ticket" href="https://trac.sagemath.org/ticket/6146" title="defect: the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand ... (closed: duplicate)">#6146</a> already gives a solution for systems where "readlink -f" is not available.
</p>
TickettornariaFri, 19 Aug 2011 15:24:18 GMT
https://trac.sagemath.org/ticket/11707#comment:11
https://trac.sagemath.org/ticket/11707#comment:11
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11707#comment:8" title="Comment 8">leif</a>:
</p>
<blockquote class="citation">
<p>
It is not immediately clear why using <code>pwd -P</code>, which fully expands symbolic links, and therefore would canonicalize <code>$SAGE_ROOT</code> as well, should break doctesting.
</p>
</blockquote>
<p>
The issue is described in <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852#comment:12" title="Comment 12 for Ticket #5852">comment:ticket:5852:12</a>. Summary:
</p>
<ul><li>there's a symlink <code>/home/wstein/sage-build -> /tmp/wstein</code>
</li><li>sage is untarred and compiled inside <code>/home/wstein/sage-build/sage-x.y/</code>
</li><li>doctesting as <code>sage -t /home/wstein/sage-x.y/...</code>
</li></ul><p>
The last step fails if SAGE_ROOT is fully canonicalized to <code>/tmp/wstein/sage-x.y/...</code> because in that case the doctesting code didn't realize <code>/home/wstein/sage-x.y/...</code> is library code. IOW, that workflow worked for William <em>because</em> canonicalization was broken!
</p>
<p>
I presume the fix is that the comparision done for detecting library code be done with using canonical paths, etc.
</p>
<blockquote class="citation">
<p>
Even if it did somehow, this should be addressed in the corresponding scripts, <code>sage-{test,ptest,doctest}</code>, which are currently under work anyway, and have certainly changed since <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a> was opened (or last modified).
</p>
</blockquote>
<p>
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 <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a> can be reapplied and then <a class="closed ticket" href="https://trac.sagemath.org/ticket/6164" title="enhancement: [with patch, positive review] Phan's Mini-AES for educational purposes (closed: fixed)">#6164</a> can easily be turned into a patch that extends canonicalization to other systems where readlink -f is not available.
</p>
TicketjhpalmieriFri, 19 Aug 2011 16:07:49 GMT
https://trac.sagemath.org/ticket/11707#comment:12
https://trac.sagemath.org/ticket/11707#comment:12
<p>
Yesterday after reading some of this, I posted a question on <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a>: 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.
</p>
TicketjdemeyerFri, 19 Aug 2011 17:29:46 GMT
https://trac.sagemath.org/ticket/11707#comment:13
https://trac.sagemath.org/ticket/11707#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11707#comment:10" title="Comment 10">tornaria</a>:
</p>
<blockquote class="citation">
<blockquote>
<p>
your script hangs on my first try on SunOS 5.10 (fulvia).
</p>
</blockquote>
</blockquote>
<p>
Fixed that.
</p>
<blockquote class="citation">
<p>
If you can figure out a way to make "readlink -f" work (when available)
</p>
</blockquote>
<p>
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 <code>realpath</code> or <code>readlink -f</code>.
</p>
TicketjdemeyerFri, 19 Aug 2011 17:32:04 GMTstatus changed; reviewer, resolution set
https://trac.sagemath.org/ticket/11707#comment:14
https://trac.sagemath.org/ticket/11707#comment:14
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>reviewer</strong>
set to <em>Jeroen Demeyer</em>
</li>
<li><strong>resolution</strong>
set to <em>duplicate</em>
</li>
</ul>
<p>
Let's move the whole discussion to <a class="closed ticket" href="https://trac.sagemath.org/ticket/5852" title="defect: Properly canonicalize $SAGE_ROOT (closed: fixed)">#5852</a>.
</p>
TicketmvnguMon, 29 Aug 2011 23:54:32 GMTmilestone changed
https://trac.sagemath.org/ticket/11707#comment:15
https://trac.sagemath.org/ticket/11707#comment:15
<ul>
<li><strong>milestone</strong>
changed from <em>sage-4.7.2</em> to <em>sage-duplicate/invalid/wontfix</em>
</li>
</ul>
Ticket