Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#11914 closed defect (fixed)

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

Reported by: kini Owned by:
Priority: major Milestone: sage-4.8
Component: scripts Keywords: notebook SageNB startup, crash, sage-env PYTHONPATH
Cc: ppurka, leif Merged in: sage-4.8.alpha0
Authors: Leif Leonhardy Reviewers: Keshav Kini, John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by leif)

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

Attachments (1)

trac_11914-sage-env-pythonpath.patch (771 bytes) - added by kini 10 years ago.
apply to $SAGE_ROOT/local/bin

Download all attachments as: .zip

Change History (32)

comment:1 Changed 10 years ago by ppurka

  • Cc ppurka added

comment:2 follow-up: Changed 10 years ago by 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.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 10 years ago by leif

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

The subdirectory ./sage was my guess...

comment:4 in reply to: ↑ 3 ; follow-up: Changed 10 years ago by 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.

comment:5 in reply to: ↑ 4 Changed 10 years ago by 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

comment:6 Changed 10 years ago by 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?

comment:7 Changed 10 years ago by 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?

comment:8 Changed 10 years ago by 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.

comment:9 Changed 10 years ago by jhpalmieri

Or perhaps this patch to the script sage-notebook:

  • sage-notebook

    diff --git a/sage-notebook b/sage-notebook
    a b  
    22
    33import os, sys, socket
    44
     5CUR=os.path.abspath(os.curdir)
     6if 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
    512print open(os.environ['SAGE_ROOT'] + '/local/bin/sage-banner').read()
    613
    714print "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 10 years ago by 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

comment:11 follow-up: Changed 10 years ago by leif

Pffffffff...

How about this:

  • sage-env

    diff --git a/sage-env b/sage-env
    a b  
    186186fi
    187187
    188188if [ -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
    190195    PYTHONHOME="$SAGE_ROOT/local" && export PYTHONHOME
    191196fi
    192197

We could make it more nice of course.

This disturbed me for a while now...

comment:12 in reply to: ↑ 11 Changed 10 years ago by leif

Replying to leif:

We could make it more nice of course.

Like this for example:

  • sage-env

    diff --git a/sage-env b/sage-env
    a b  
    186186fi
    187187
    188188if [ -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
    191196fi
    192197
    193198if [ -z "${SAGE_ORIG_LD_LIBRARY_PATH_SET}" ]; then

comment:13 follow-up: Changed 10 years ago by 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.)

comment:14 follow-up: Changed 10 years ago by 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.

comment:15 in reply to: ↑ 14 Changed 10 years ago by 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 (".").

comment:16 in reply to: ↑ 13 Changed 10 years ago by 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.

comment:17 follow-up: Changed 10 years ago by 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.

Changed 10 years ago by kini

apply to $SAGE_ROOT/local/bin

comment:18 Changed 10 years ago by kini

  • Authors set to Leif Leonhardy
  • Description modified (diff)
  • Status changed from new to needs_review

comment:19 Changed 10 years ago by kini

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 10 years ago by 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.

comment:21 Changed 10 years ago by kini

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

comment:22 Changed 10 years ago by 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] > 

comment:23 Changed 10 years ago by ppurka

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

comment:24 Changed 10 years ago by kini

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

comment:25 Changed 10 years ago by leif

  • Description modified (diff)

Added a reference to #10192.

comment:26 Changed 10 years ago by leif

  • Keywords notebook SageNB PYTHONPATH added

comment:27 Changed 10 years ago by jhpalmieri

  • Reviewers set to Keshav Kini, John Palmieri
  • Status changed from needs_review to positive_review

comment:28 Changed 10 years ago by jdemeyer

  • Milestone changed from sage-4.7.2 to sage-4.7.3

comment:29 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.3.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:30 Changed 10 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:31 Changed 10 years ago by jdemeyer

  • Merged in changed from sage-4.7.3.alpha0 to sage-4.8.alpha0
  • Milestone set to sage-4.8
Note: See TracTickets for help on using tickets.