#11914 closed defect (fixed)
`sage n` fails when current directory is $SAGE_ROOT/devel/sage
Reported by:  kini  Owned by:  

Priority:  major  Milestone:  sage4.8 
Component:  scripts  Keywords:  notebook SageNB startup, crash, sageenv PYTHONPATH 
Cc:  ppurka, leif  Merged in:  sage4.8.alpha0 
Authors:  Leif Leonhardy  Reviewers:  Keshav Kini, John Palmieri 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Traceback as follows:
fs@zhenghe /opt/sage/devel/sage $ ../../sage n   Sage Version 4.7.2.alpha3, Release Date: 20110928   Type notebook() for the GUI, and license() for information.   ********************************************************************** * * * Warning: this is a prerelease version, and it may be unstable. * * * ********************************************************************** Please wait while the Sage Notebook server starts... Traceback (most recent call last): File "/opt/sage/local/bin/sagenotebook", line 9, in <module> from sage.server.notebook.all import notebook File "/opt/sage4.7.2.alpha3/devel/sagemain/sage/server/notebook/all.py", line 22, in <module> from sagenb.notebook.all import * File "/opt/sage/devel/sagenb/sagenb/notebook/all.py", line 18, in <module> from interact import interact, input_box, slider, range_slider, selector, checkbox, input_grid, text_control, color_selector File "/opt/sage/devel/sagenb/sagenb/notebook/interact.py", line 158, in <module> from sage.misc.cachefunc import cached_method File "/opt/sage4.7.2.alpha3/devel/sagemain/sage/misc/cachefunc.py", line 22, in <module> from function_mangling import ArgumentFixer ImportError: No module named function_mangling fs@zhenghe /opt/sage/devel/sage $
ppurka suggests prepending $SAGE_ROOT/devel/sage/build/sage/
to PYTHONPATH
in sageenv
. This indeed works, but I'm not sure what else it might break. Advice?
Related: #10192 (SageNB is broken if ".
" is explicitly contained in PYTHONPATH
during installation of it.)
Apply trac_11914sageenvpythonpath.patch to $SAGE_ROOT/local/bin
Attachments (1)
Change History (32)
comment:1 Changed 11 years ago by
 Cc ppurka added
comment:2 followup: ↓ 3 Changed 11 years ago by
comment:3 in reply to: ↑ 2 ; followup: ↓ 4 Changed 11 years ago by
 Cc leif added
Replying to kini:
To clarify, it seems that Python is trying to load the sage library from
$SAGE_ROOT/devel/sage/sage/
(which equals./sage/
in this context) rather than from$SAGE_ROOT/devel/sage/build/sage/
, thus ppurka'sPYTHONPATH
fix.
The subdirectory ./sage
was my guess...
comment:4 in reply to: ↑ 3 ; followup: ↓ 5 Changed 11 years ago by
Replying to leif:
Replying to kini:
To clarify, it seems that Python is trying to load the sage library from
$SAGE_ROOT/devel/sage/sage/
(which equals./sage/
in this context) rather than from$SAGE_ROOT/devel/sage/build/sage/
, thus ppurka'sPYTHONPATH
fix.The subdirectory
./sage
was my guess...
$ cd $SAGE_ROOT $ (mkdir p tmp/sage/sage && cd tmp/sage && ../../sage n)
in contrast works for me.
comment:5 in reply to: ↑ 4 Changed 11 years ago by
Replying to leif:
#!sh $ cd $SAGE_ROOT $ (mkdir p tmp/sage/sage && cd tmp/sage && ../../sage n)
in contrast works for me.
But the following doesn't work:
cd Installations/sage4.7/tmp ln s ../devel/sage/sage . ../sage n
comment:6 Changed 11 years ago by
If I delete the file SAGE_ROOT/devel/sage/sage/__init__.py
(and the corresponding .pyc file if present), then I don't see this problem. Should that file be autogenerated (as SAGE_ROOT/local/lib/python/sitepackages/sage/__init__.py
, say by the install
script in devel/sage) when necessary? Or is that a bad, or a too complicated, idea?
comment:7 Changed 11 years ago by
But $SAGE_ROOT/local/lib/python/sitepackages/sage
is a symlink to $SAGE_ROOT/local/lib/python/sitepackages/../../../../devel/sage/build/sage
, which already contains an __init__.py
. Or am I misunderstanding what you propose to do?
comment:8 Changed 11 years ago by
Never mind, I guess you mean we should remove __init__.py
from $SAGE_ROOT/devel/sage/sage
. I don't think this is a good idea, as it is part of the Sage library.
comment:9 Changed 11 years ago by
Or perhaps this patch to the script sagenotebook:

sagenotebook
diff git a/sagenotebook b/sagenotebook
a b 2 2 3 3 import os, sys, socket 4 4 5 CUR=os.path.abspath(os.curdir) 6 if os.path.samefile(CUR, os.path.join(os.environ['SAGE_ROOT'], 'devel', 'sage')): 7 try: 8 sys.path.remove(CUR) 9 except ValueError: 10 pass 11 5 12 print open(os.environ['SAGE_ROOT'] + '/local/bin/sagebanner').read() 6 13 7 14 print "Please wait while the Sage Notebook server starts..."
This looks safer than my first suggestion, although it could be a bit more refined: search sys.path not just for CUR, but for anything which is equivalent to CUR.
comment:10 Changed 11 years ago by
That raises the question of whether we want to fix this too:
fs@zhenghe ~ $ mkdir sage fs@zhenghe ~ $ touch sage/__init__.py fs@zhenghe ~ $ sage n   Sage Version 4.7.2.alpha3, Release Date: 20110928   Type notebook() for the GUI, and license() for information.   ********************************************************************** * * * Warning: this is a prerelease version, and it may be unstable. * * * ********************************************************************** Please wait while the Sage Notebook server starts... Traceback (most recent call last): File "/opt/sage/local/bin/sagenotebook", line 9, in <module> from sage.server.notebook.all import notebook ImportError: No module named server.notebook.all
comment:11 followup: ↓ 12 Changed 11 years ago by
Pffffffff...
How about this:

sageenv
diff git a/sageenv b/sageenv
a b 186 186 fi 187 187 188 188 if [ d "$SAGE_ROOT/local/lib/python" ]; then 189 PYTHONPATH="$SAGE_PATH:$SAGE_ROOT/local/lib/python" && export PYTHONPATH 189 if [ n "$SAGE_PATH" ]; then 190 PYTHONPATH="$SAGE_PATH:$SAGE_ROOT/local/lib/python" 191 else 192 PYTHONPATH="$SAGE_ROOT/local/lib/python" 193 fi 194 export PYTHONPATH 190 195 PYTHONHOME="$SAGE_ROOT/local" && export PYTHONHOME 191 196 fi 192 197
We could make it more nice of course.
This disturbed me for a while now...
comment:12 in reply to: ↑ 11 Changed 11 years ago by
Replying to leif:
We could make it more nice of course.
Like this for example:

sageenv
diff git a/sageenv b/sageenv
a b 186 186 fi 187 187 188 188 if [ d "$SAGE_ROOT/local/lib/python" ]; then 189 PYTHONPATH="$SAGE_PATH:$SAGE_ROOT/local/lib/python" && export PYTHONPATH 190 PYTHONHOME="$SAGE_ROOT/local" && export PYTHONHOME 189 PYTHONPATH="$SAGE_ROOT/local/lib/python" 190 if [ n "$SAGE_PATH" ]; then 191 PYTHONPATH="$SAGE_PATH:$PYTHONPATH" 192 fi 193 PYTHONHOME="$SAGE_ROOT/local" 194 export PYTHONPATH 195 export PYTHONHOME 191 196 fi 192 197 193 198 if [ z "${SAGE_ORIG_LD_LIBRARY_PATH_SET}" ]; then
comment:13 followup: ↓ 16 Changed 11 years ago by
(I don't know what potentially weird shells might source sageenv
, otherwise I'd use export FOO="bar"
or export FOO BAR
etc.)
comment:14 followup: ↓ 15 Changed 11 years ago by
What, seriously!? The cause of this is just the colon at the beginning of PYTHONPATH
? Well then your solution is simple and obviously the correct way to fix this, it seems to me.
comment:15 in reply to: ↑ 14 Changed 11 years ago by
Replying to kini:
What, seriously!? The cause of this is just the colon at the beginning of
PYTHONPATH
? Well then your solution is simple and obviously the correct way to fix this, it seems to me.
Python interprets the empty entry as the current working directory (".
").
comment:16 in reply to: ↑ 13 Changed 11 years ago by
Replying to leif:
(I don't know what potentially weird shells might source
sageenv
, otherwise I'd useexport FOO="bar"
orexport FOO BAR
etc.)
Is sageenv even called from anywhere outside Sage scripts? It seems weird that all the sage scripts are called as #!/usr/bin/env bash
but the scripts themselves are not written (most of the time) as bash scripts. It should probably be like the $SAGE_ROOT/sage
script should be POSIX shell compatible and then the rest should be proper bash scripts, since they anyway invoke bash. It will make the scripts nicer and easier to read.
Thanks for the PYTHONPATH fix. Didn't know that empty paths resolve to current directory.
comment:17 followup: ↓ 20 Changed 11 years ago by
If they are mostly written as POSIXcompatible, why not just go all the way and make them all call sh
instead, and remove bashisms? But this is getting off topic, I think...
I'll make an hg patch out of leif's diff.
comment:18 Changed 11 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:19 Changed 11 years ago by
I'll leave formal review to others who know shell scripting better than I, but it looks fine to me, fwiw.
comment:20 in reply to: ↑ 17 Changed 11 years ago by
Replying to kini:
If they are mostly written as POSIXcompatible, why not just go all the way and make them all call
sh
instead, and remove bashisms? But this is getting off topic, I think...
Hmmm. In principle we should avoid bashisms whenever possible. (One of the very few exceptions would IMHO be set o pipefail
, which is supported by bashs >=3.0, but we still officially require just 2.04 since this version is shipped with MacOS X 10.4 [which is a mess to support anyway], <flame> and its users are incapable of installing a more recent version </flame>.)
The main reason we use bash
is that we can to some extent rely on it being functional; there are too many broken Bourne shells, or at least /bin/sh
ells.
I'll make an hg patch out of leif's diff.
Nice service, thanks.
I just wonder how many parts of Sage (unconsciously exploiting it) break if we fix this bug.
comment:21 Changed 11 years ago by
Well, I'll run a doctest, though I doubt this will catch anything relevant...
comment:22 Changed 11 years ago by
I don't think anything should break because the following two commands returned no output:
...lations/sage4.7.2 [130] > LC_ALL="C" grep re 'from [^\.]+[[:space:]]+import' * ...allations/sage4.7.2 [1] > LC_ALL="C" grep re '^[[:space:]]+import [^\.]+' * ...allations/sage4.7.2 [1] >
comment:23 Changed 11 years ago by
Neglect above post. Really weird. It should have returned some output.
comment:24 Changed 11 years ago by
All tests pass on sage.math.washington.edu
, as expected.
comment:26 Changed 11 years ago by
 Keywords notebook SageNB PYTHONPATH added
comment:27 Changed 11 years ago by
 Reviewers set to Keshav Kini, John Palmieri
 Status changed from needs_review to positive_review
comment:28 Changed 11 years ago by
 Milestone changed from sage4.7.2 to sage4.7.3
comment:29 Changed 11 years ago by
 Merged in set to sage4.7.3.alpha0
 Resolution set to fixed
 Status changed from positive_review to closed
comment:31 Changed 11 years ago by
 Merged in changed from sage4.7.3.alpha0 to sage4.8.alpha0
 Milestone set to sage4.8
To clarify, it seems that Python is trying to load the sage library from
$SAGE_ROOT/devel/sage/sage/
(which equals./sage/
in this context) rather than from$SAGE_ROOT/devel/sage/build/sage/
, thus ppurka'sPYTHONPATH
fix.