`sage -n` fails when current directory is $SAGE_ROOT/devel/sage

[kini]

Traceback as follows:

fs@zhenghe /opt/sage/devel/sage $ ../../sage -n
----------------------------------------------------------------------
| Sage Version 4.7.2.alpha3, Release Date: 2011-09-28                |
| 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/sage-notebook", line 9, in <module>
    from sage.server.notebook.all import notebook
  File "/opt/sage-4.7.2.alpha3/devel/sage-main/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/sage-4.7.2.alpha3/devel/sage-main/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 sage-env. 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_11914-sage-env-pythonpath.patch to $SAGE_ROOT/local/bin

[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's PYTHONPATH fix.

[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's PYTHONPATH fix.

The subdirectory ./sage was my guess...

[leif]

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's PYTHONPATH 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.

[ppurka]

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/sage-4.7/tmp
ln -s ../devel/sage/sage .
../sage -n

[jhpalmieri]

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/site-packages/sage/__init__.py, say by the install script in devel/sage) when necessary? Or is that a bad, or a too complicated, idea?

[kini]

But $SAGE_ROOT/local/lib/python/site-packages/sage is a symlink to $SAGE_ROOT/local/lib/python/site-packages/../../../../devel/sage/build/sage, which already contains an __init__.py. Or am I misunderstanding what you propose to do?

[kini]

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.

[jhpalmieri]

Or perhaps this patch to the script sage-notebook:

sage-notebook

@@ -2 +2 @@
 
 import os, sys, socket
 
+CUR=os.path.abspath(os.curdir)
+if os.path.samefile(CUR, os.path.join(os.environ['SAGE_ROOT'], 'devel', 'sage')):
+    try:
+        sys.path.remove(CUR)
+    except ValueError:
+        pass
+
 print open(os.environ['SAGE_ROOT'] + '/local/bin/sage-banner').read()
 
 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.

[kini]

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: 2011-09-28                |
| 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/sage-notebook", line 9, in <module>
    from sage.server.notebook.all import notebook
ImportError: No module named server.notebook.all

[leif]

Pffffffff...

How about this:

sage-env

@@ -186 +186 @@
 fi
 
 if [ -d "$SAGE_ROOT/local/lib/python" ]; then
-    PYTHONPATH="$SAGE_PATH:$SAGE_ROOT/local/lib/python"   && export PYTHONPATH
+    if [ -n "$SAGE_PATH" ]; then
+        PYTHONPATH="$SAGE_PATH:$SAGE_ROOT/local/lib/python"
+    else
+        PYTHONPATH="$SAGE_ROOT/local/lib/python"
+    fi
+    export PYTHONPATH
     PYTHONHOME="$SAGE_ROOT/local" && export PYTHONHOME
 fi
 

We could make it more nice of course.

This disturbed me for a while now...

[leif]

Replying to leif:

We could make it more nice of course.

Like this for example:

sage-env

@@ -186 +186 @@
 fi
 
 if [ -d "$SAGE_ROOT/local/lib/python" ]; then
-    PYTHONPATH="$SAGE_PATH:$SAGE_ROOT/local/lib/python"   && export PYTHONPATH
-    PYTHONHOME="$SAGE_ROOT/local" && export PYTHONHOME
+    PYTHONPATH="$SAGE_ROOT/local/lib/python"
+    if [ -n "$SAGE_PATH" ]; then
+        PYTHONPATH="$SAGE_PATH:$PYTHONPATH"
+    fi
+    PYTHONHOME="$SAGE_ROOT/local"
+    export PYTHONPATH
+    export PYTHONHOME
 fi
 
 if [ -z "${SAGE_ORIG_LD_LIBRARY_PATH_SET}" ]; then

[leif]

(I don't know what potentially weird shells might source sage-env, otherwise I'd use export FOO="bar" or export FOO BAR etc.)

[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.

[leif]

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 (".").

[ppurka]

Replying to leif:

(I don't know what potentially weird shells might source sage-env, otherwise I'd use export FOO="bar" or export FOO BAR etc.)

Is sage-env 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.

[kini]

If they are mostly written as POSIX-compatible, 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.

[kini]

apply to $SAGE_ROOT/local/bin

[kini]

I'll leave formal review to others who know shell scripting better than I, but it looks fine to me, fwiw.

[leif]

Replying to kini:

If they are mostly written as POSIX-compatible, 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/shells.

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.

[kini]

Well, I'll run a doctest, though I doubt this will catch anything relevant...

[ppurka]

I don't think anything should break because the following two commands returned no output:

...lations/sage-4.7.2 [130] > LC_ALL="C" grep -re 'from [^\.]+[[:space:]]+import' *
...allations/sage-4.7.2 [1] > LC_ALL="C" grep -re '^[[:space:]]+import [^\.]+' *
...allations/sage-4.7.2 [1] > 

[ppurka]

Neglect above post. Really weird. It should have returned some output.

[kini]

All tests pass on sage.math.washington.edu, as expected.

[leif]

Added a reference to #10192.

[jdemeyer]

Milestone sage-4.7.3 deleted