Sage: Ticket #5852: Properly canonicalize $SAGE_ROOT
https://trac.sagemath.org/ticket/5852
<p>
Currently, <code>$SAGE_ROOT/sage</code> uses (first among other alternate methods) <code>readlink -n</code> to detect the directory where the script lives (that's $SAGE_ROOT), but that is broken because
</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><li>The symlink expansion may not be completely done, and <code>$SAGE_ROOT</code> could end up with a non-canonical dirname, which leads to issues with testing.
</li><li>The code to detect <code>SAGE_ROOT</code> inside <code>sage-env</code> does not canonicalize the pathname at all. This should be fixed as well. (The only case where <code>sage-env</code> is run without <code>SAGE_ROOT</code> being set is when testing Sage from the <code>Makefile</code>, i.e. when running <code>make ptest</code> or similar.)
</li></ul><p>
Note that we should do this in a portable way, without using <code>realpath</code>, <code>readlink -f</code> or the likes.
</p>
<p>
See also <a class="closed ticket" href="https://trac.sagemath.org/ticket/11704" title="enhancement: Resolve symbolic links in $HOME/.sage (closed: fixed)">#11704</a>, which solves the same problem for <code>DOT_SAGE</code>.
</p>
<p>
Apply:
</p>
<ol><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/5852/5852_sage_root.patch" title="Attachment '5852_sage_root.patch' in Ticket #5852">5852_sage_root.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/5852/5852_sage_root.patch" title="Download"></a> to <code>SAGE_ROOT</code>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/5852/5852_scripts.patch" title="Attachment '5852_scripts.patch' in Ticket #5852">5852_scripts.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/5852/5852_scripts.patch" title="Download"></a> to <code>local/bin</code>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/5852/5852_doc.patch" title="Attachment '5852_doc.patch' in Ticket #5852">5852_doc.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/5852/5852_doc.patch" title="Download"></a> and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/5852/trac_5852-doc-referee.patch" title="Attachment 'trac_5852-doc-referee.patch' in Ticket #5852">trac_5852-doc-referee.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/5852/trac_5852-doc-referee.patch" title="Download"></a> to <code>devel/sage</code>
</li></ol>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/5852
Trac 1.1.6tornariaWed, 22 Apr 2009 12:46:06 GMTowner, component changed; milestone set
https://trac.sagemath.org/ticket/5852#comment:1
https://trac.sagemath.org/ticket/5852#comment:1
<ul>
<li><strong>owner</strong>
changed from <em>tbd</em> to <em>tornaria</em>
</li>
<li><strong>component</strong>
changed from <em>algebra</em> to <em>misc</em>
</li>
<li><strong>milestone</strong>
set to <em>sage-3.4.2</em>
</li>
</ul>
<p>
Patching <code>$SAGE_ROOT/sage</code> with this:
</p>
<pre class="wiki">--- sage-3.4/sage.orig 2009-04-22 01:45:48.000000000 -0300
+++ sage-3.4/sage 2009-04-22 09:37:27.000000000 -0300
@@ -14,6 +14,7 @@
fi
if [ "$SAGE_ROOT" = "....." ]; then
+ SAGE_ROOT=`readlink -nf "$0" 2> /dev/null` || \
SAGE_ROOT=`readlink -n "$0" 2> /dev/null` || \
SAGE_ROOT=`realpath "$0" 2> /dev/null` || \
SAGE_ROOT="$0"
</pre><p>
fixes the issue, since now <code>$SAGE_ROOT</code> is correct.
</p>
<p>
According to mabshoff, <code>readlink -f</code> doesn't work on some BSD; that's why I left the <code>readlink -n</code> test in the second line, but this should of course be tested on those BSD to make sure it doesn't cause a regression.
</p>
TickettornariaSat, 25 Apr 2009 02:02:27 GMTattachment set
https://trac.sagemath.org/ticket/5852
https://trac.sagemath.org/ticket/5852
<ul>
<li><strong>attachment</strong>
set to <em>trac_5852.patch</em>
</li>
</ul>
<p>
On systems where "readlink -f" is supported, use that so the path for $SAGE_ROOT is fully canonicalized
</p>
TicketnthieryTue, 28 Apr 2009 00:18:36 GMT
https://trac.sagemath.org/ticket/5852#comment:2
https://trac.sagemath.org/ticket/5852#comment:2
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:1" title="Comment 1">tornaria</a>:
</p>
<blockquote class="citation">
<p>
Patching <code>$SAGE_ROOT/sage</code> with this:
</p>
<pre class="wiki">--- sage-3.4/sage.orig 2009-04-22 01:45:48.000000000 -0300
+++ sage-3.4/sage 2009-04-22 09:37:27.000000000 -0300
@@ -14,6 +14,7 @@
fi
if [ "$SAGE_ROOT" = "....." ]; then
+ SAGE_ROOT=`readlink -nf "$0" 2> /dev/null` || \
SAGE_ROOT=`readlink -n "$0" 2> /dev/null` || \
SAGE_ROOT=`realpath "$0" 2> /dev/null` || \
SAGE_ROOT="$0"
</pre><p>
fixes the issue, since now <code>$SAGE_ROOT</code> is correct.
</p>
<p>
According to mabshoff, <code>readlink -f</code> doesn't work on some BSD; that's why I left the <code>readlink -n</code> test in the second line, but this should of course be tested on those BSD to make sure it doesn't cause a regression.
</p>
</blockquote>
<p>
I can confirm that it does not work on MacOS X.10.4.11 (e.g. Anne Schilling's machine)
</p>
<p>
A fix would be most welcome, as this makes sage -t make false reports of broken test files.
</p>
TicketnthieryMon, 04 May 2009 16:16:06 GMT
https://trac.sagemath.org/ticket/5852#comment:3
https://trac.sagemath.org/ticket/5852#comment:3
<p>
The readlink -f workaround is better than nothing, and should not make things worst for systems like BSD. I would vote for including it now, in waiting for a better solution.
Should I set a positive review?
</p>
<p>
Besides, what about adding a switch to sage -t to specify manually that the given file is inside or outside the sage source tree?
</p>
<p>
This would make a workaround for MacOX X, and also be occasionally be useful. For example, I often run tests from one sage source tree with another sage to compare the results.
</p>
TickettornariaSun, 17 May 2009 03:02:28 GMTmilestone changed
https://trac.sagemath.org/ticket/5852#comment:4
https://trac.sagemath.org/ticket/5852#comment:4
<ul>
<li><strong>milestone</strong>
changed from <em>sage-4.0.1</em> to <em>sage-4.0</em>
</li>
</ul>
<p>
Is there some equivalent of <code>readlink -f</code> that works in MacOS X?
</p>
TickettornariaSun, 17 May 2009 03:17:57 GMT
https://trac.sagemath.org/ticket/5852#comment:5
https://trac.sagemath.org/ticket/5852#comment:5
<p>
Note that the version of <code>readlink</code> which is included in fink (in package <code>debianutils</code>) supports the <code>-f</code> switch, so a mac with fink doesn't suffer from this issue (asuming <code>/sw/sbin</code> is before <code>/usr/bin</code> in the search PATH).
</p>
TicketwasThu, 28 May 2009 07:00:31 GMTsummary changed
https://trac.sagemath.org/ticket/5852#comment:6
https://trac.sagemath.org/ticket/5852#comment:6
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs review] the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand symlinks recursively</em> to <em>[with patch, positive review] the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand symlinks recursively (fix this on systems that support readlink -f)</em>
</li>
</ul>
<p>
See <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> for fixing this on systems that don't support readlink -f.
</p>
TicketmhansenThu, 28 May 2009 07:04:07 GMTstatus changed; resolution set
https://trac.sagemath.org/ticket/5852#comment:7
https://trac.sagemath.org/ticket/5852#comment:7
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Merged in 4.0.rc1.
</p>
TicketwasWed, 01 Jul 2009 11:25:00 GMT
https://trac.sagemath.org/ticket/5852#comment:8
https://trac.sagemath.org/ticket/5852#comment:8
<p>
Question. Does
</p>
<pre class="wiki">readlink -n sage
</pre><p>
work on any platform?! It gives an error on *both* OS X and Linux. Why is it even there?!
</p>
<pre class="wiki">OS X
ub243101:s wstein$ readlink -n sage
ub243101:s wstein$ echo $?
1
Linux:
wstein@boxen:~/sage$ readlink -n sage
wstein@boxen:~/sage$ echo $?
1
</pre><p>
I wonder who wrote this weird SAGE_ROOT code in the first place? I wrote something a long time ago, but it bears no resemblance to the current code.
</p>
<p>
By the way, I've had reports of major failures caused by using <code>readlink -nf</code> by one user who has a symlink + nfs mount setup. Their problems are solved by deleting the <code>readlink -nf</code> line. Why don't we use realpath first and only if that doesn't work use something else? It seems like realpath is the right choice, since it's supposed to " converts each filename argument to an absolute pathname, which has no components that are symbolic links or the special
</p>
<blockquote>
<p>
. or .. directory entries... Please note that mostly the same functionality is provided by the ‘-f’ option."
</p>
</blockquote>
<p>
There is no realpath on OS X, but that is ok since readlink doesn't work ever on OS X anyways, so no loss.
</p>
<blockquote>
<p>
-- William
</p>
</blockquote>
TickettornariaWed, 01 Jul 2009 13:00:03 GMT
https://trac.sagemath.org/ticket/5852#comment:9
https://trac.sagemath.org/ticket/5852#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:8" title="Comment 8">was</a>:
</p>
<blockquote class="citation">
<p>
Question. Does
</p>
<pre class="wiki">readlink -n sage
</pre><p>
work on any platform?!
</p>
</blockquote>
<p>
Yes it does: it reads the content of a symbolic link. It succeeds if and only if the argument is actually a symbolic link, e.g.
</p>
<pre class="wiki">~/sandbox$ ls -l
total 0
~/sandbox$ mkdir sage1
~/sandbox$ readlink sage1 ; echo $? ## fails b/c sage1 is not a symlink
1
~/sandbox$ ln -s sage1 sage2
~/sandbox$ readlink sage2 ; echo $? ## ok b/c sage2 is actually a symlink
sage1
0
</pre><p>
The option <code>-n</code> means to not print a trainling newline character; I don't think it really make a difference due to bash usual escaping rules.
</p>
<blockquote class="citation">
<blockquote>
<p>
It gives an error on *both* OS X and Linux. Why is it even there?!
</p>
</blockquote>
</blockquote>
<p>
It was there before the patch in this ticket, so that if <code>$0</code> (the path to the script one is running) is actually a symlink to the real path of the sage script, the detection of <code>SAGE_ROOT</code> works. On systems that support <code>-f</code>, that is a more complete solution, but the fallback was left for the benefit of systems where <code>readlink -f</code> does not work (e.g. OS X).
</p>
<p>
Following my example above, here's an example where <code>-f</code> is needed:
</p>
<pre class="wiki">~/sandbox$ ln -s sage2 sage3
tornaria@bip:~/sandbox$ readlink -n sage3
sage2tornaria@bip:~/sandbox$ readlink sage3
sage2
tornaria@bip:~/sandbox$ readlink -f sage3
/home/tornaria/sandbox/sage1
</pre><p>
The other major case is when there are symlinks in some of the components of the path, those get canonicalized by <code>readlink -f</code>, but not by plain <code>readlink</code> (this leads to failures as shown in the description).
</p>
<blockquote class="citation">
<p>
By the way, I've had reports of major failures caused by using <code>readlink -nf</code> by one user who has a symlink + nfs mount setup. Their problems are solved by deleting the <code>readlink -nf</code> line. Why don't we use realpath first and only if that doesn't work use something else? It seems like realpath is the right choice, since it's supposed to " converts each filename argument to an absolute pathname, which has no components that are symbolic links or the special
</p>
<blockquote>
<p>
. or .. directory entries... Please note that mostly the same functionality is provided by the ‘-f’ option."
</p>
</blockquote>
</blockquote>
<p>
Can you give a pointer to those? Not using <code>readlink -f</code> leads to major failures in testing, as described in the description of the ticket.
</p>
<p>
Do you actually know that in those cases <code>realpath</code> works? It seems to me that both are implemented using <code>realpath(3)</code>, so they should be the same unless I'm missing something.
</p>
<blockquote class="citation">
<p>
There is no realpath on OS X, but that is ok since readlink doesn't work ever on OS X anyways, so no loss.
</p>
</blockquote>
<p>
There is no <code>realpath</code> in most systems I have access to (other than sage.math). In fact, <code>readlink</code> is pretty much standard (possibly POSIX), although <code>-f</code> option is not (a GNUism?). For GNU systems (e.g. linux), it comes bundled in coreutils, which means it will be available everywhere. OTOH, <code>realpath</code> comes in <em>optional</em> package <code>realpath</code>. Do you know of a system where <code>readlink -f</code> doesn't work but <code>realpath(1)</code> is available?
</p>
<p>
OTOH, realpath(3) seems to be a POSIX standard, and it seems to be available on OS X:
</p>
<pre class="wiki">$ nm /usr/lib/libc.dylib | grep realpath
/usr/lib/libc.dylib(realpath.So):
9003f1f0 T _realpath
</pre><p>
so an alternative would be to compile our own <code>realpath</code> binary and somehow use it from the startup script. But we need a path to SAGE_ROOT so we can find SAGE_ROOT/local/bin/realpath... auch... (doesn't need to be canonical, though.... so we could use readlink a few times to get a path to the actual sage script, and then run <code>realpath</code> from there).
</p>
TicketwasWed, 01 Jul 2009 23:32:30 GMT
https://trac.sagemath.org/ticket/5852#comment:10
https://trac.sagemath.org/ticket/5852#comment:10
<p>
It's possible that this ticket should be reverted until a major bug it causes is fixed.
</p>
<p>
The reason for this ticket in the first place was the following, as given in the ticket description:
</p>
<pre class="wiki">/home/sage$ md5sum sage-3.4/sage
4153919efe1edcd34ad7fa193122d679 sage-3.4/sage
/home/sage$ ln -s sage-3.4 sage-3.4-symlink
/home/sage$ ln -sf /home/sage/sage-3.4-symlink/sage /home/tornaria/bin/sage
/home/sage$ type sage
</pre><p>
Notice the symlink of the Sage script
</p>
<pre class="wiki">/home/sage$ ln -sf /home/sage/sage-3.4-symlink/sage /home/tornaria/bin/sage
</pre><p>
For the record, this is *not* how I meant the sage script is meant to be used. I bet this isn't documented, but it should be. The script should never be used that way. Instead one should do
</p>
<pre class="wiki">/home/sage$ cp /home/sage/sage-3.4-symlink/sage /home/tornaria/bin/sage
</pre><p>
and then edit the copied script to explicitly point to the ROOT. It was never my intention for somebody to run the sage script unmodified outside of SAGE_ROOT. Me not intending this means that elsewhere in the Sage build/test system this assumption is made, and the workaround on this ticket actually seriously breaks things for some users.
</p>
<p>
The change in this ticket causes serious breakage for people whose home directory is NFS mounted, and for which their Sage build is on another volume that is symlinked from their home directory. i.e., this sort of setup:
</p>
<pre class="wiki"> cd ~wstein # my home directory is NFS mounted.
mkdir /tmp/wstein # /tmp is a local disk
ln -s /tmp/wstein sage-build
cd sage-build #
# build sage here, doctesting fails completely
</pre><p>
I'm doing a test build for myself to confirm that this happens, and if so and I can't figure out how to fix this promptly (maybe I will be able to), then we have to revert this change, and document that one can't just symlink the sage script out.
</p>
TickettornariaWed, 01 Jul 2009 23:58:20 GMT
https://trac.sagemath.org/ticket/5852#comment:11
https://trac.sagemath.org/ticket/5852#comment:11
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:10" title="Comment 10">was</a>:
</p>
<blockquote class="citation">
<p>
It's possible that this ticket should be reverted until a major bug it causes is fixed.
[...]
For the record, this is *not* how I meant the sage script is meant to be used. I bet this isn't documented, but it should be. The script should never be used that way. Instead one should do
</p>
<pre class="wiki">/home/sage$ cp /home/sage/sage-3.4-symlink/sage /home/tornaria/bin/sage
</pre><p>
and then edit the copied script to explicitly point to the ROOT. It was never my intention for somebody to run the sage script unmodified outside of SAGE_ROOT. Me not intending this means that elsewhere in the Sage build/test system this assumption is made, and the workaround on this ticket actually seriously breaks things for some users.
</p>
</blockquote>
<p>
If you only use the script in *that* way, then the
</p>
<pre class="wiki">if [ "$SAGE_ROOT" = "....." ]; then
</pre><p>
branch would never be taken, and as the patch in this ticket only touches this branch, it can't break anything.
</p>
<p>
In practice, it is much more convenient to just use a symlink to the script, if it can be worked out. Before this patch, it turned out that the real, canonical path for SAGE_ROOT could be identified incorrectly, and *this* causes doctesting to fail.
</p>
<blockquote class="citation">
<p>
The change in this ticket causes serious breakage for people whose home directory is NFS mounted, and for which their Sage build is on another volume that is symlinked from their home directory. i.e., this sort of setup:
</p>
<pre class="wiki"> cd ~wstein # my home directory is NFS mounted.
mkdir /tmp/wstein # /tmp is a local disk
ln -s /tmp/wstein sage-build
cd sage-build #
# build sage here, doctesting fails completely
</pre><p>
I'm doing a test build for myself to confirm that this happens, and if so and I can't figure out how to fix this promptly (maybe I will be able to), then we have to revert this change, and document that one can't just symlink the sage script out.
</p>
</blockquote>
<p>
This sort of setup is *exactly* what used to cause breakage for me, because the <code>SAGE_ROOT</code> was incorrectly computed (to a non-canonical path). What would you expect <code>SAGE_ROOT</code> to be computed to, other than the canonical path? e.g., continuing your example above:
</p>
<pre class="wiki"> cd ~wstein/sage-build
tar xvf sage-nnn.tar
cd sage-nnn
./sage -sh
echo "$SAGE_ROOT"
</pre><p>
Are you expecting SAGE_ROOT above to be "/home/wstein/sage-build/sage-nnn/", or "/tmp/wstein/sage-nnn" ?
</p>
<p>
Before the patch, it was the former, non canonical path; after the patch, it is the latter, which is IMO the correct canonical path. When SAGE_ROOT is non-canonical, running doctests for files in the sage library fails b/c they are not recognized as part of the sage library, etc.
I don't see how the fact that this is NFS mounted could be relevant to the issue.
</p>
TicketwasSat, 04 Jul 2009 11:43:26 GMT
https://trac.sagemath.org/ticket/5852#comment:12
https://trac.sagemath.org/ticket/5852#comment:12
<p>
The problem with this patch isn't that it is "wrong" (which is what you're arguing with me about above). It is that it seriously breaks Sage, hence it must be reverted or the problem it causes must be fixed. I had a look, and the problem is here in local/bin/sage-doctest:
</p>
<pre class="wiki"> library_code = True
ext = os.path.splitext(argv[1])[1]
if ext in ['.spyx', '.sage'] or \
not (SAGE_ROOT.strip('/') + '/devel' in os.path.abspath(argv[1])):
library_code = False
</pre><p>
The problem is that the library_code variable is being set to False for all the code that *is* in the library. It is being set to false because if one does
</p>
<pre class="wiki"> sage -t "/home/wstein/sage-build/sage-nnn/..."
</pre><p>
then argv[1] is not first canonicalized, which messes everything up completely.
</p>
<p>
So for this ticket to be in (which I agree with at some point), one needs to factor out this path caonicalization, and make sure it is applied everywhere (e.g,. in sage-doctest). There could be many other places where subtle problems arise -- I don't know.
</p>
<p>
For now, this needs to be reverted.
</p>
TicketrlmSat, 04 Jul 2009 19:57:42 GMTstatus, component, summary changed; resolution deleted
https://trac.sagemath.org/ticket/5852#comment:13
https://trac.sagemath.org/ticket/5852#comment:13
<ul>
<li><strong>status</strong>
changed from <em>closed</em> to <em>reopened</em>
</li>
<li><strong>resolution</strong>
<em>fixed</em> deleted
</li>
<li><strong>component</strong>
changed from <em>misc</em> to <em>distribution</em>
</li>
<li><strong>summary</strong>
changed from <em>[with patch, positive review] the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand symlinks recursively (fix this on systems that support readlink -f)</em> to <em>[with patch, needs work] the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand symlinks recursively (fix this on systems that support readlink -f)</em>
</li>
</ul>
<p>
I have reverted this patch in sage-4.1.rc0, and I'm reopening the ticket.
</p>
TicketdavidloefflerFri, 16 Apr 2010 15:50:22 GMTmilestone changed; upstream set
https://trac.sagemath.org/ticket/5852#comment:14
https://trac.sagemath.org/ticket/5852#comment:14
<ul>
<li><strong>milestone</strong>
changed from <em>sage-4.0</em> to <em>sage-4.4</em>
</li>
<li><strong>upstream</strong>
set to <em>N/A</em>
</li>
</ul>
TicketjhpalmieriFri, 19 Aug 2011 03:46:08 GMT
https://trac.sagemath.org/ticket/5852#comment:15
https://trac.sagemath.org/ticket/5852#comment:15
<p>
Has the issue with <code>sage-doctest</code> been resolved? The code now says
</p>
<div class="wiki-code"><div class="code"><pre> dev_path <span class="o">=</span> os<span class="o">.</span>path<span class="o">.</span>realpath<span class="p">(</span>os<span class="o">.</span>path<span class="o">.</span>join<span class="p">(</span>SAGE_ROOT<span class="p">,</span> <span class="s">'devel'</span><span class="p">))</span>
our_path <span class="o">=</span> os<span class="o">.</span>path<span class="o">.</span>realpath<span class="p">(</span>argv<span class="p">[</span><span class="mi">1</span><span class="p">])</span>
<span class="k">if</span> <span class="ow">not</span> force_lib <span class="ow">and</span> <span class="p">(</span>ext <span class="ow">in</span> <span class="p">[</span><span class="s">'.spyx'</span><span class="p">,</span> <span class="s">'.sage'</span><span class="p">]</span> <span class="ow">or</span>
<span class="ow">not</span> dev_path <span class="ow">in</span> our_path<span class="p">):</span>
library_code <span class="o">=</span> <span class="bp">False</span>
</pre></div></div><p>
Since <code>os.path.realpath</code> is used twice, shouldn't this be okay? If not, another option is to use <a class="ext-link" href="http://docs.python.org/library/os.path.html#os.path.samefile"><span class="icon"></span>os.path.samefile</a>.
</p>
TicketjdemeyerFri, 19 Aug 2011 17:29:30 GMTdescription, summary changed
https://trac.sagemath.org/ticket/5852#comment:16
https://trac.sagemath.org/ticket/5852#comment:16
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/5852?action=diff&version=16">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>[with patch, needs work] the detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand symlinks recursively (fix this on systems that support readlink -f)</em> to <em>The detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand symlinks recursively</em>
</li>
</ul>
TicketjdemeyerFri, 19 Aug 2011 17:31:34 GMTattachment set
https://trac.sagemath.org/ticket/5852
https://trac.sagemath.org/ticket/5852
<ul>
<li><strong>attachment</strong>
set to <em>realpath_bash.sh</em>
</li>
</ul>
<p>
Shell script replacement for "readlink -f"
</p>
TicketjdemeyerFri, 19 Aug 2011 17:34:35 GMTdescription changed
https://trac.sagemath.org/ticket/5852#comment:17
https://trac.sagemath.org/ticket/5852#comment:17
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/5852?action=diff&version=17">diff</a>)
</li>
</ul>
TicketleifFri, 19 Aug 2011 19:15:39 GMTcc set
https://trac.sagemath.org/ticket/5852#comment:18
https://trac.sagemath.org/ticket/5852#comment:18
<ul>
<li><strong>cc</strong>
<em>leif</em> added
</li>
</ul>
TicketjdemeyerTue, 23 Aug 2011 09:14:28 GMTowner changed
https://trac.sagemath.org/ticket/5852#comment:19
https://trac.sagemath.org/ticket/5852#comment:19
<ul>
<li><strong>owner</strong>
changed from <em>tornaria</em> to <em>jdemeyer</em>
</li>
</ul>
<p>
Why do we set <code>SAGE_ROOT</code> inside <code>sage-env</code>? Given that <code>sage-env</code> is only ever called when we already know <code>SAGE_ROOT</code> (i.e. we do <code>source $SAGE_ROOT/local/bin/sage-env</code>).
</p>
TicketjdemeyerTue, 23 Aug 2011 09:24:49 GMT
https://trac.sagemath.org/ticket/5852#comment:20
https://trac.sagemath.org/ticket/5852#comment:20
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:19" title="Comment 19">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Why do we set <code>SAGE_ROOT</code> inside <code>sage-env</code>? Given that <code>sage-env</code> is only ever called when we already know <code>SAGE_ROOT</code> (i.e. we do <code>source $SAGE_ROOT/local/bin/sage-env</code>).
</p>
</blockquote>
<p>
Okay, I did find one counterexamples (I only looked in local/bin before):
</p>
<ul><li>The top-level Makefile calls <code>sage-env</code> without first setting <code>SAGE_ROOT</code>.
</li></ul><p>
I also noticed that <code>data/extcode/sage/ext/mac-app/start-sage.sh</code> has its own <code>SAGE_ROOT</code>-detecting code but it probably shouldn't and should use <code>sage-env</code> instead.
</p>
TicketjdemeyerTue, 23 Aug 2011 09:25:36 GMTsummary changed
https://trac.sagemath.org/ticket/5852#comment:21
https://trac.sagemath.org/ticket/5852#comment:21
<ul>
<li><strong>summary</strong>
changed from <em>The detection of SAGE_ROOT in $SAGE_ROOT/sage script should expand symlinks recursively</em> to <em>The detection of SAGE_ROOT in $SAGE_ROOT/sage and local/bin/sage-env should expand symlinks recursively</em>
</li>
</ul>
TicketjdemeyerTue, 23 Aug 2011 12:01:55 GMTattachment set
https://trac.sagemath.org/ticket/5852
https://trac.sagemath.org/ticket/5852
<ul>
<li><strong>attachment</strong>
set to <em>resolvelinks.sh</em>
</li>
</ul>
<p>
Shell script replacement for "readlink -f"
</p>
TicketjdemeyerTue, 23 Aug 2011 13:15:26 GMTstatus changed
https://trac.sagemath.org/ticket/5852#comment:22
https://trac.sagemath.org/ticket/5852#comment:22
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketjdemeyerTue, 23 Aug 2011 13:18:22 GMTdescription changed; author set
https://trac.sagemath.org/ticket/5852#comment:23
https://trac.sagemath.org/ticket/5852#comment:23
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/5852?action=diff&version=23">diff</a>)
</li>
<li><strong>author</strong>
set to <em>Jeroen Demeyer</em>
</li>
</ul>
TicketjdemeyerTue, 23 Aug 2011 13:22:14 GMTdescription changed
https://trac.sagemath.org/ticket/5852#comment:24
https://trac.sagemath.org/ticket/5852#comment:24
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/5852?action=diff&version=24">diff</a>)
</li>
</ul>
TicketleifMon, 29 Aug 2011 10:13:47 GMT
https://trac.sagemath.org/ticket/5852#comment:25
https://trac.sagemath.org/ticket/5852#comment:25
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:20" title="Comment 20">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
I also noticed that <code>data/extcode/sage/ext/mac-app/start-sage.sh</code> has its own <code>SAGE_ROOT</code>-detecting code but it probably shouldn't and should use <code>sage-env</code> instead.
</p>
</blockquote>
<p>
It seems the MacOS X app wants just the opposite, i.e. to <strong>not</strong> resolve symbolic links, since the absolute, canonicalized path may frequently change.
</p>
<p>
Therefore it always creates (on start-up) the same, "constant" symbolic link from <code>/tmp/sage-mac-app</code> to the current, volatile <code>$SAGE_ROOT</code>, which can only work if the application is also actually <em>always</em> built in (a real directory) <code>/tmp/sage-mac-app/</code> (such that no change of hardcoded paths is necessary).
</p>
<p>
Cf. <a class="closed ticket" href="https://trac.sagemath.org/ticket/11755" title="defect: Allow running Sage.app by someone other than it was installed by (closed: invalid)">#11755</a>. In that case, the app should also define some special environment variable, such that <code>sage-env</code> (and perhaps also <code>sage</code>) can treat this specifically, namely not canonicalize <code>$SAGE_ROOT</code>.
</p>
TicketjdemeyerMon, 19 Sep 2011 08:52:59 GMT
https://trac.sagemath.org/ticket/5852#comment:26
https://trac.sagemath.org/ticket/5852#comment:26
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:25" title="Comment 25">leif</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:20" title="Comment 20">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
I also noticed that <code>data/extcode/sage/ext/mac-app/start-sage.sh</code> has its own <code>SAGE_ROOT</code>-detecting code but it probably shouldn't and should use <code>sage-env</code> instead.
</p>
</blockquote>
<p>
It seems the MacOS X app wants just the opposite, i.e. to <strong>not</strong> resolve symbolic links, since the absolute, canonicalized path may frequently change.
</p>
<p>
Therefore it always creates (on start-up) the same, "constant" symbolic link from <code>/tmp/sage-mac-app</code> to the current, volatile <code>$SAGE_ROOT</code>, which can only work if the application is also actually <em>always</em> built in (a real directory) <code>/tmp/sage-mac-app/</code> (such that no change of hardcoded paths is necessary).
</p>
</blockquote>
<p>
The question is: why are things done this way? It seems to me that the <code>/tmp/sage-mac-app</code> symlink is an ugly hack around a problem which can probably be solved in a better way.
</p>
TicketjdemeyerMon, 26 Sep 2011 20:55:10 GMTdescription changed
https://trac.sagemath.org/ticket/5852#comment:27
https://trac.sagemath.org/ticket/5852#comment:27
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/5852?action=diff&version=27">diff</a>)
</li>
</ul>
TicketjdemeyerSat, 29 Oct 2011 18:43:34 GMTcomponent, milestone changed
https://trac.sagemath.org/ticket/5852#comment:28
https://trac.sagemath.org/ticket/5852#comment:28
<ul>
<li><strong>component</strong>
changed from <em>distribution</em> to <em>scripts</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-4.7.2</em> to <em>sage-4.7.3</em>
</li>
</ul>
TicketjdemeyerSat, 29 Oct 2011 18:44:31 GMTsummary changed
https://trac.sagemath.org/ticket/5852#comment:29
https://trac.sagemath.org/ticket/5852#comment:29
<ul>
<li><strong>summary</strong>
changed from <em>The detection of SAGE_ROOT in $SAGE_ROOT/sage and local/bin/sage-env should expand symlinks recursively</em> to <em>Properly canonicalize $SAGE_ROOT</em>
</li>
</ul>
TicketkiniSun, 30 Oct 2011 04:24:41 GMTcc changed
https://trac.sagemath.org/ticket/5852#comment:30
https://trac.sagemath.org/ticket/5852#comment:30
<ul>
<li><strong>cc</strong>
<em>kini</em> added
</li>
</ul>
TicketleifSun, 30 Oct 2011 05:58:21 GMT
https://trac.sagemath.org/ticket/5852#comment:31
https://trac.sagemath.org/ticket/5852#comment:31
<p>
What sense does it make to first call <code>resolvelinks()</code> and then finally do
</p>
<div class="wiki-code"><div class="code"><pre><span class="nv">SAGE_ROOT</span><span class="o">=</span><span class="sb">`</span><span class="nb">cd</span> <span class="s2">"$SAGE_ROOT"</span> <span class="o">&&</span> <span class="nb">pwd</span> -P<span class="sb">`</span>
</pre></div></div><p>
?
</p>
<p>
Also, why use all of <code>[ "x$foo" != "x" ]</code> (causing eye cancer), <code>[ -n "$foo" ]</code> and <code>[ "$foo" != "" ]</code>?
</p>
<p>
For <code>sage</code> at least, or any script that's run by <code>bash</code>, <code>[[ -n $foo ]]</code> or <code>[[ $foo != "" ]]</code> does the job, and is by the way both safer and faster.
</p>
TicketjdemeyerSun, 30 Oct 2011 13:00:39 GMT
https://trac.sagemath.org/ticket/5852#comment:32
https://trac.sagemath.org/ticket/5852#comment:32
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:31" title="Comment 31">leif</a>:
</p>
<blockquote class="citation">
<p>
What sense does it make to first call <code>resolvelinks()</code> and then finally do
</p>
<div class="wiki-code"><div class="code"><pre><span class="nv">SAGE_ROOT</span><span class="o">=</span><span class="sb">`</span><span class="nb">cd</span> <span class="s2">"$SAGE_ROOT"</span> <span class="o">&&</span> <span class="nb">pwd</span> -P<span class="sb">`</span>
</pre></div></div><p>
?
</p>
</blockquote>
<p>
Because <code>resolvelinks</code> resolves symbolic links in the <code>sage</code> executable name, which is not a directory (so the <code>cd && pwd</code> trick does not work).
</p>
TicketjdemeyerSun, 30 Oct 2011 13:08:20 GMTattachment set
https://trac.sagemath.org/ticket/5852
https://trac.sagemath.org/ticket/5852
<ul>
<li><strong>attachment</strong>
set to <em>5852_sage_root.patch</em>
</li>
</ul>
<p>
Patch for $SAGE_ROOT/sage, SAGE_ROOT repository
</p>
TicketjdemeyerSun, 30 Oct 2011 13:08:29 GMTattachment set
https://trac.sagemath.org/ticket/5852
https://trac.sagemath.org/ticket/5852
<ul>
<li><strong>attachment</strong>
set to <em>5852_scripts.patch</em>
</li>
</ul>
<p>
Patch for local/bin/sage-env, SCRIPTS repository
</p>
TicketleifSun, 30 Oct 2011 15:19:11 GMT
https://trac.sagemath.org/ticket/5852#comment:33
https://trac.sagemath.org/ticket/5852#comment:33
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:32" title="Comment 32">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:31" title="Comment 31">leif</a>:
</p>
<blockquote class="citation">
<p>
What sense does it make to first call <code>resolvelinks()</code> and then finally do
</p>
</blockquote>
</blockquote>
<div class="wiki-code"><div class="code"><pre><span class="nv">SAGE_ROOT</span><span class="o">=</span><span class="sb">`</span><span class="nb">cd</span> <span class="s2">"$SAGE_ROOT"</span> <span class="o">&&</span> <span class="nb">pwd</span> -P<span class="sb">`</span>
</pre></div></div><blockquote class="citation">
<blockquote class="citation">
<p>
?
</p>
</blockquote>
<p>
Because <code>resolvelinks</code> resolves symbolic links in the <code>sage</code> executable name, which is not a directory (so the <code>cd && pwd</code> trick does not work).
</p>
</blockquote>
<p>
Of course it does. You just have to
</p>
<ul><li>strip the program name, and
</li><li>if no path remains after that, it's the current working directory, i.e. path=".".
</li><li><code>cd</code> to that path and do <code>pwd -P</code>. Doesn't matter whether the path was relative or absolute.
</li></ul>
TicketjdemeyerSun, 30 Oct 2011 17:03:56 GMT
https://trac.sagemath.org/ticket/5852#comment:34
https://trac.sagemath.org/ticket/5852#comment:34
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:33" title="Comment 33">leif</a>:
</p>
<blockquote class="citation">
<p>
You just have to
</p>
<ul><li>strip the program name, and
</li><li>if no path remains after that, it's the current working directory, i.e. path=".".
</li><li><code>cd</code> to that path and do <code>pwd -P</code>. Doesn't matter whether the path was relative or absolute.
</li></ul></blockquote>
<p>
Again, this does not work if the executable itself is a symbolic link.
</p>
<p>
It could very well be that my solution is too complicated, but your solution is certainly too simple.
</p>
TicketjdemeyerThu, 03 Nov 2011 16:14:43 GMTmilestone deleted
https://trac.sagemath.org/ticket/5852#comment:35
https://trac.sagemath.org/ticket/5852#comment:35
<ul>
<li><strong>milestone</strong>
<em>sage-4.7.3</em> deleted
</li>
</ul>
<p>
Milestone sage-4.7.3 deleted
</p>
TicketjdemeyerTue, 15 Nov 2011 09:57:46 GMTdescription changed; milestone set
https://trac.sagemath.org/ticket/5852#comment:36
https://trac.sagemath.org/ticket/5852#comment:36
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/5852?action=diff&version=36">diff</a>)
</li>
<li><strong>milestone</strong>
set to <em>sage-4.8</em>
</li>
</ul>
TicketjhpalmieriThu, 17 Nov 2011 18:18:13 GMT
https://trac.sagemath.org/ticket/5852#comment:37
https://trac.sagemath.org/ticket/5852#comment:37
<p>
How widely available is <code>pwd -P</code>? The GNU version of <code>pwd</code> does not recognize the <code>-P</code> option, but its man page says
</p>
<pre class="wiki"> NOTE: your shell may have its own version of pwd, which usually super‐
sedes the version described here. Please refer to your shell’s docu‐
mentation for details about the options it supports.
</pre><p>
This is what it says on sage.math, for example. I use bash there, and the built-in pwd supports the <code>-P</code> option. But do we need to worry about systems where there is no built-in pwd, and it is relying on the GNU version? I have access to one such machine, and <code>pwd -P</code> doesn't work there, but I've never tried to build Sage on it.
</p>
TicketjhpalmieriThu, 17 Nov 2011 18:19:56 GMT
https://trac.sagemath.org/ticket/5852#comment:38
https://trac.sagemath.org/ticket/5852#comment:38
<p>
(Part of the problem is that on that other machine, I'm using tcsh and it doesn't let me run 'chsh'.)
</p>
TicketjhpalmieriThu, 17 Nov 2011 20:00:10 GMT
https://trac.sagemath.org/ticket/5852#comment:39
https://trac.sagemath.org/ticket/5852#comment:39
<p>
Has this been tested on OS X 10.4? I believe that uses an older version of bash, and so we should make sure that it has the features used in the modifications to the <code>sage</code> shell script.
</p>
TicketjhpalmieriThu, 17 Nov 2011 23:11:10 GMT
https://trac.sagemath.org/ticket/5852#comment:40
https://trac.sagemath.org/ticket/5852#comment:40
<p>
This seems to work for me on various platforms. If someone can test on OS X 10.4, I think we can give it a positive review. (The Changelog I saw for bash doesn't discuss changes for expansions like <code>${pattern%word}</code> between versions 2 and 3 of bash, so I think it should work.)
</p>
TicketjdemeyerFri, 18 Nov 2011 08:28:37 GMT
https://trac.sagemath.org/ticket/5852#comment:41
https://trac.sagemath.org/ticket/5852#comment:41
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5852#comment:39" title="Comment 39">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
Has this been tested on OS X 10.4?
</p>
</blockquote>
<p>
Yes, it works.
</p>
<p>
You are right that <code>/bin/pwd</code> does not always support <code>-P</code>, even on sage.math:
</p>
<pre class="wiki">jdemeyer@sage:~$ /bin/pwd -P
/bin/pwd: invalid option -- P
Try `/bin/pwd --help' for more information.
</pre><p>
But it seems <code>bash</code> always supports <code>pwd -P</code> as shell builtin, so we are safe.
</p>
TicketjdemeyerFri, 18 Nov 2011 09:09:12 GMTattachment set
https://trac.sagemath.org/ticket/5852
https://trac.sagemath.org/ticket/5852
<ul>
<li><strong>attachment</strong>
set to <em>5852_doc.patch</em>
</li>
</ul>
TicketjdemeyerFri, 18 Nov 2011 09:09:49 GMTdescription changed
https://trac.sagemath.org/ticket/5852#comment:42
https://trac.sagemath.org/ticket/5852#comment:42
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/5852?action=diff&version=42">diff</a>)
</li>
</ul>
<p>
Added documentation patch <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/5852/5852_doc.patch" title="Attachment '5852_doc.patch' in Ticket #5852">5852_doc.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/5852/5852_doc.patch" title="Download"></a>
</p>
TicketjdemeyerFri, 18 Nov 2011 09:13:52 GMTdependencies set
https://trac.sagemath.org/ticket/5852#comment:43
https://trac.sagemath.org/ticket/5852#comment:43
<ul>
<li><strong>dependencies</strong>
set to <em>#11926, #11959</em>
</li>
</ul>
TicketjhpalmieriWed, 23 Nov 2011 17:49:57 GMTdescription changed; reviewer set
https://trac.sagemath.org/ticket/5852#comment:44
https://trac.sagemath.org/ticket/5852#comment:44
<ul>
<li><strong>reviewer</strong>
set to <em>John Palmieri, Leif Leonhardy</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/5852?action=diff&version=44">diff</a>)
</li>
</ul>
<p>
This looks good to me. I'm attaching a referee patch for the documentation part. If that's okay, this can be changed to "positive review".
</p>
TicketjhpalmieriWed, 23 Nov 2011 17:50:14 GMTattachment set
https://trac.sagemath.org/ticket/5852
https://trac.sagemath.org/ticket/5852
<ul>
<li><strong>attachment</strong>
set to <em>trac_5852-doc-referee.patch</em>
</li>
</ul>
<p>
main sage repo
</p>
TicketjdemeyerWed, 23 Nov 2011 20:59:31 GMTstatus changed
https://trac.sagemath.org/ticket/5852#comment:45
https://trac.sagemath.org/ticket/5852#comment:45
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketjdemeyerSat, 26 Nov 2011 09:34:28 GMTmerged set
https://trac.sagemath.org/ticket/5852#comment:46
https://trac.sagemath.org/ticket/5852#comment:46
<ul>
<li><strong>merged</strong>
set to <em>sage-4.8.alpha3</em>
</li>
</ul>
TicketjdemeyerSat, 26 Nov 2011 10:31:33 GMTstatus changed; resolution set
https://trac.sagemath.org/ticket/5852#comment:47
https://trac.sagemath.org/ticket/5852#comment:47
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
Ticket