Opened 9 years ago

Closed 8 years ago

Last modified 4 years ago

#10822 closed defect (fixed)

sage -sh doesn't set the path right if default shell is zsh on OSX

Reported by: jason Owned by:
Priority: major Milestone: sage-5.0.1
Component: scripts Keywords: sd40.5
Cc: Merged in: sage-5.0.1.rc1
Authors: John Palmieri, Jason Grout Reviewers: Ivan Andrus, Jeroen Demeyer, Punarbasu Purkayastha
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11866, #11790 Stopgaps:

Description (last modified by chapoton)

On OSX (10.6.6), when starting zsh, the /etc/zshenv file is always executed (even if the -f option is used to start zsh). This file:

# system-wide environment settings for zsh(1)
if [ -x /usr/libexec/path_helper ]; then
        eval `/usr/libexec/path_helper -s`
fi

modifies the path to put things like /usr/bin, etc., in at the *start* of the path (even though the documentation for path_helper says that it *appends* things to the PATH variable).

So in the end, I get:

bash-3.2$ ~/sage/sage -sh
Detected SAGE64 flag
Building Sage on OS X in 64-bit mode

Starting subshell with Sage environment variables set.
Be sure to exit when you are done and do not do anything
with other copies of Sage!

Bypassing shell configuration files ...

SAGE_ROOT=/Users/grout/sage
(sage subshell) tiny:~/sage/local/bin grout$ echo $PATH    
/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/git/bin:/usr/texbin:/usr/X11/bin:/Users/grout/sage/local/Frameworks/Python.framework/Versions/2.5/bin:/Users/grout/sage:/Users/grout/sage/local/bin:.
SAGE_ROOT=/Users/grout/sage
(sage subshell) tiny:~/sage/local/bin grout$ 


Notice that the Sage paths are at the end of the PATH variable. This causes all sorts of problems when trying to do anything with sage -sh.

apply: trac-10822-zsh-path.patch to sage root repository

Attachments (2)

trac_10822-sage-sh.patch (1.6 KB) - added by jhpalmieri 8 years ago.
scripts repo
trac-10822-zsh-path.patch (1.1 KB) - added by jason 8 years ago.
apply instead of previous patch

Download all attachments as: .zip

Change History (39)

comment:1 Changed 9 years ago by jhpalmieri

First of all, I think path_helper might be working properly. On my machine, the documentation says that it adds paths found in the files in /etc/paths.d/, which on my machine just adds /usr/texbin and /usr/X11/bin. In particular, I think that /usr/bin, etc. are already at the start of the path before path_helper is run.

Second, I don't see this problem, but maybe I'm not doing things right. My default shell is bash, but if I do

$ zsh
jpalmieri538% sage -sh

Starting subshell with Sage environment variables set.
Be sure to exit when you are done and do not do anything
with other copies of Sage!

Bypassing shell configuration files ...

SAGE_ROOT=/Applications/sage
(sage subshell) jpalmieri538:~ palmieri$ echo $PATH
/Applications/sage/local/Frameworks/Python.framework/Versions/2.5/bin:/Applications/sage:/Applications/sage/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/texbin:/usr/X11/bin:/Users/palmieri/bin:/Applications/sage
SAGE_ROOT=/Applications/sage

(Also, are you sure that this has to do with zsh? Your first line says bash-3.2$.)

To troubleshoot this, try applying this patch to sage-sage in SAGE_ROOT/local/bin:

  • sage-sage

    diff -r e7a865848991 sage-sage
    a b if [ "$1" = '-sh' -o "$1" = '--sh' ]; t 
    476476    SHELL_NAME=`basename $SHELL`
    477477    echo "Bypassing shell configuration files ..."
    478478    echo
     479    echo "PATH is set to $PATH"
    479480    # We must start a new shell with no .profile or .bashrc files
    480481    # processed, so that we know our path is correct
    481482    PS1="SAGE_ROOT=${SAGE_ROOT}\n(sage subshell) \h:\W \u\$ "

Then it will print the PATH before it even starts the new shell.

comment:2 follow-up: Changed 9 years ago by jason

At first I thought it was peculiar to zsh, but then in my example above I tried it with bash, so it's not particular to zsh on my computer.

Can you try the following simple test?

bash-3.2$ PATH='.';export PATH
bash-3.2$ /usr/libexec/path_helper -s
PATH="/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/git/bin:/usr/texbin:/usr/X11/bin:."; export PATH;

Do you see "." at the end or the start of PATH in the output of path_helper?

comment:3 Changed 9 years ago by jason

Also, just to make sure, could you check the md5 of your path_helper binary?

$ md5 /usr/libexec/path_helper 
MD5 (/usr/libexec/path_helper) = 66b40636fd8adb430b0b8667135bcefe

comment:4 in reply to: ↑ 2 Changed 9 years ago by jason

Replying to jason:

At first I thought it was peculiar to zsh, but then in my example above I tried it with bash, so it's not particular to zsh on my computer.

Can you try the following simple test?

bash-3.2$ PATH='.';export PATH
bash-3.2$ /usr/libexec/path_helper -s
PATH="/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/git/bin:/usr/texbin:/usr/X11/bin:."; export PATH;

Do you see "." at the end or the start of PATH in the output of path_helper?

I just tried the simple test above with the guest account on my computer and I got the same results---path_helper prepended its paths instead of appending them.

comment:5 Changed 9 years ago by jhpalmieri

I get the same as you in both cases. Now I see I didn't read all of the path_helper documentation: in addition to adding paths from /etc/paths.d/, it also says

Prior to reading these directories, default PATH and MANPATH values are obtained from the files
/etc/paths and /etc/manpaths respectively.

Note that there are no promises about appending anything.

However, I still don't have the problem you're having with "sage -sh". Indeed, I would guess that your PATH is set incorrectly whenever you run Sage, and then I'm surprised that anything works right. What happens if you do this in Sage:

sage: import os
sage: os.environ.get('PATH')

Are the Sage directories at the beginning or end?

comment:6 Changed 9 years ago by jason

John, when I apply your patch, I get:

{{{% sage -sh Detected SAGE64 flag Building Sage on OS X in 64-bit mode

Starting subshell with Sage environment variables set. Be sure to exit when you are done and do not do anything with other copies of Sage!

Bypassing shell configuration files ...

PATH is set to /Users/grout/sage/local/Frameworks/Python.framework/Versions/2.5/bin:/Users/grout/sage:/Users/grout/sage/local/bin:/Users/grout/sage:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/git/bin:/usr/texbin:/usr/X11/bin:/opt/local/bin:/Users/grout/bin:/opt/local/bin }}}

So the path is set correctly before starting up the shell (well, except that I don't know what's going on with the 2.5 version framework, but regardless, sage is at the start of the path).

Jason

comment:7 Changed 9 years ago by jason

From within Sage:

sage: os.environ.get('PATH')
'/Users/grout/sage/local/Frameworks/Python.framework/Versions/2.5/bin:/Users/grout/sage:/Users/grout/sage/local/bin:/Users/grout/sage:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/git/bin:/usr/texbin:/usr/X11/bin:/opt/local/bin:/Users/grout/bin:/opt/local/bin'

So it's set correctly in there. I'm positive it's also set correctly when I'm installing spkgs.

comment:8 follow-up: Changed 9 years ago by jason

Okay, so if I use chsh to change my shell to bash, then sage -sh has the right $PATH. John, if you change your shell to zsh using chsh, do you have the right $PATH?

Thanks, Jason

comment:9 Changed 9 years ago by jason

  • Summary changed from sage -sh doesn't work on OSX to sage -sh doesn't set the path right if default shell is zsh on OSX

comment:10 in reply to: ↑ 8 Changed 9 years ago by jhpalmieri

Replying to jason:

Okay, so if I use chsh to change my shell to bash, then sage -sh has the right $PATH. John, if you change your shell to zsh using chsh, do you have the right $PATH?

No, the Sage stuff comes at the end. Also, the prompt is bad:

Macintosh% /Applications/sage/sage -sh

Starting subshell with Sage environment variables set.
Be sure to exit when you are done and do not do anything
with other copies of Sage!

Bypassing shell configuration files ...

SAGE_ROOT=/Applications/sage\n(sage subshell) \h:\W \u$ echo $PATH
/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/texbin:/usr/X11/bin:/Applications/sage/local/Frameworks/Python.framework/Versions/2.5/bin:/Applications/sage:/Applications/sage/local/bin

comment:11 Changed 9 years ago by jhpalmieri

I have one idea how to fix this, but it doesn't seem ideal: first, change the SHELL_OPTS for zsh to " -d". Then create a .zshrc file, say in SAGE_ROOT/local/bin, containing

export PATH=$SAGE_ROOT:$SAGE_LOCAL/bin:$PATH

and then tell zsh to read this file when starting up. The following patch to sage-sage does it, and also fixes the prompt:

  • sage-sage

    diff -r e7a865848991 sage-sage
    a b if [ "$1" = '-sh' -o "$1" = '--sh' ]; t 
    478478    echo
    479479    # We must start a new shell with no .profile or .bashrc files
    480480    # processed, so that we know our path is correct
    481     PS1="SAGE_ROOT=${SAGE_ROOT}\n(sage subshell) \h:\W \u\$ "
    482     export PS1
    483481    case $SHELL_NAME in
    484482        bash)
     483            PS1="SAGE_ROOT=${SAGE_ROOT}\n(sage subshell) \h:\W \u\$ "
     484            export PS1
    485485            SHELL_OPTS=" --norc"
    486486            ;;
    487487        csh)
    488488            SHELL_OPTS=" -f"
    489489            ;;
    490490        ksh)
     491            PS1="""SAGE_ROOT=${SAGE_ROOT}
     492(sage subshell) `hostname -s`:\${PWD##*/} $USER$ """
     493            export PS1
    491494            SHELL_OPTS=" -p"
    492495            ;;
    493496        sh)
    if [ "$1" = '-sh' -o "$1" = '--sh' ]; t 
    497500            SHELL_OPTS=" -f"
    498501            ;;
    499502        zsh)
    500             SHELL_OPTS=" -f -d"
     503            PS1="""SAGE_ROOT=${SAGE_ROOT}
     504(sage subshell) %m:%1~ %n$ """
     505            export PS1
     506            ZDOTDIR=${SAGE_ROOT}/local/bin/
     507            export ZDOTDIR
     508            SHELL_OPTS=" -d"
    501509            ;;
    502510        *)
    503511            echo >&2 "Unknown shell: $SHELL!"

I think that setting the prompt for csh and tcsh might require a similar trick, since their prompt doesn't seem to be governed by an environment variable the same way.

comment:12 Changed 9 years ago by jhpalmieri

(We could also generate the file .zshrc on the fly and store it in some temporary directory.)

comment:13 Changed 9 years ago by jhpalmieri

  • Authors set to John Palmieri
  • Status changed from new to needs_review

Here's a patch implementing the idea above. I can't figure out a safe way to deal with csh and tcsh: the only idea I have is to temporarily overwrite the user's .cshrc or .tcshrc file, and this seems risky. So with this patch, the prompts for those shells are not set correctly.

comment:14 Changed 9 years ago by jhpalmieri

It would be good to test this on different platforms. I can test it on my mac, but I don't know if I have access to another system which has zsh installed and on which I can run "chsh" successfully.

comment:15 Changed 8 years ago by jhpalmieri

  • Dependencies set to #11866, #11790

Rebased to #11866 and #11790.

Changed 8 years ago by jhpalmieri

scripts repo

comment:16 Changed 8 years ago by malb

Not an expert on zsh but the patch works for me on Debian GNU/Linux using zsh.

On my machine mktemp -t sage wouldn't work though:

$ mktemp -t sage
mktemp: too few X's in template `sage'
$ mktemp -t sageXXX
/tmp/sage5WO

comment:17 follow-up: Changed 8 years ago by ppurka

What does the -d option do for zsh? I can't find anything at the manual which suggests that that option is available.

Secondly, I believe one of the reasons for creating a new .zshrc is to avoid sourcing user settings (as I read it in sage-sage). In this case, one should invoke zsh as zsh -o noglobalrcs see Files.

A cleaner way to implement the ZDOTDIR would be to then remove /tmp/.zshrc if it is present and echo a newer one. In fact, one could even use the RPROMPT of zsh to get a cleaner prompt. It is less annoying that a massive prompt that is obtained otherwise. :)

Here's a diff of what changes I did to sage-sage.

  • sage-sage

    # HG changeset patch
    # User P Purkayastha <ppurka@gmail.com>
    # Date 1317310855 -28800
    # Node ID 469d05f30083d92990be2a0af692062d9b51c7be
    # Parent  f8f1496d316ce8556a8d7350de61e0f755c23681
    [mq]: trac_10822_fix_zsh_shell.patch
    
    diff --git a/sage-sage b/sage-sage
    a b  
    497497            SHELL_OPTS=" -f"
    498498            ;;
    499499        zsh)
    500             SHELL_OPTS=" -f -d"
     500            FILE="/tmp/.zshrc"
     501            if [ -e "$FILE" ]; then
     502                if ! rm -f "$FILE"; then
     503                    echo "No permission to delete and create new file $FILE"
     504                    exit 1
     505                fi
     506            fi
     507            cat > "$FILE" << EOF
     508PATH="$SAGE_ROOT:$SAGE_LOCAL/bin:$PATH"
     509export PATH
     510PS1="%m:%~ %n$ "
     511export PS1
     512RPROMPT=" #SAGE_ROOT=${SAGE_ROOT} (sage subshell)"
     513export RPROMPT
     514EOF
     515            export ZDOTDIR="/tmp"
     516            SHELL_OPTS="-o noglobalrcs"
    501517            ;;
    502518        *)
    503519            echo >&2 "Unknown shell: $SHELL!"

comment:18 Changed 8 years ago by ppurka

Sorry, I got my exports all backwards.

  • sage-sage

    # HG changeset patch
    # User P Purkayastha <ppurka@gmail.com>
    # Date 1317311589 -28800
    # Node ID 09875d34e10e03692a831a8b5cb9f65693c6a7b3
    # Parent  f8f1496d316ce8556a8d7350de61e0f755c23681
    [mq]: trac_10822_fix_zsh_shell.patch
    
    diff --git a/sage-sage b/sage-sage
    a b  
    497497            SHELL_OPTS=" -f"
    498498            ;;
    499499        zsh)
    500             SHELL_OPTS=" -f -d"
     500            FILE="/tmp/.zshrc"
     501            if [ -e "$FILE" ]; then
     502                if ! rm -f "$FILE"; then
     503                    echo "No permission to delete and create new file $FILE"
     504                    exit 1
     505                fi
     506            fi
     507            cat > "$FILE" << EOF
     508export PATH="$SAGE_ROOT:$SAGE_LOCAL/bin:$PATH"
     509export PS1="%m:%~ %n$ "
     510export RPROMPT=" #SAGE_ROOT=${SAGE_ROOT} (sage subshell)"
     511EOF
     512            ZDOTDIR="/tmp" && export ZDOTDIR
     513            SHELL_OPTS="-o noglobalrcs"
    501514            ;;
    502515        *)
    503516            echo >&2 "Unknown shell: $SHELL!"

comment:19 in reply to: ↑ 17 ; follow-up: Changed 8 years ago by jhpalmieri

Replying to ppurka:

What does the -d option do for zsh? I can't find anything at the manual which suggests that that option is available.

In section 16.2.5 of http://zsh.sourceforge.net/Doc/Release/Options.html#Options, it says

GLOBAL_RCS (-d) <D>
If this option is unset, the startup files /etc/zprofile, /etc/zshrc, /etc/zlogin and /etc/zlogout will not be run. It can be disabled and re-enabled at any time, including inside local startup files (.zshrc, etc.).

I'll take a look at the rest of your changes later (as will other people, I hope).

comment:20 in reply to: ↑ 19 Changed 8 years ago by ppurka

Replying to jhpalmieri:

Replying to ppurka:

What does the -d option do for zsh? I can't find anything at the manual which suggests that that option is available.

In section 16.2.5 of http://zsh.sourceforge.net/Doc/Release/Options.html#Options, it says

GLOBAL_RCS (-d) <D>
If this option is unset, the startup files /etc/zprofile, /etc/zshrc, /etc/zlogin and /etc/zlogout will not be run. It can be disabled and re-enabled at any time, including inside local startup files (.zshrc, etc.).

I'll take a look at the rest of your changes later (as will other people, I hope).

Thanks. It is the same as -o noglobalopts then.

comment:21 Changed 8 years ago by jdemeyer

  • Status changed from needs_review to needs_work

This obviously needs to be rebased...

comment:22 Changed 8 years ago by jdemeyer

Moreover: it's better not to use mktemp -p or mktemp -t: use mktemp TEMPLATE which seems to be portable.

comment:23 Changed 8 years ago by jason

  • Authors changed from John Palmieri to John Palmieri, Jason Grout
  • Description modified (diff)
  • Milestone set to sage-5.0
  • Status changed from needs_work to needs_review

I've uploaded my take on what should be done. Apply patch to root repository.

comment:24 Changed 8 years ago by jdemeyer

  • Status changed from needs_review to needs_work

This still needs to be rebased to a recent beta.

comment:25 Changed 8 years ago by jason

I had it based on beta6. I'm downloading beta12 now.

Changed 8 years ago by jason

apply instead of previous patch

comment:26 Changed 8 years ago by jason

I rebased the patch to beta12

comment:27 Changed 8 years ago by jason

  • Status changed from needs_work to needs_review

comment:28 Changed 8 years ago by jdemeyer

The patch looks okay on visual inspection, but I cannot give positive_review because I don't have zsh to test this.

comment:29 Changed 8 years ago by ppurka

I run zsh as my shell. But I don't have a Mac to test it on. So, I can't give a proper review either.

comment:30 Changed 8 years ago by ppurka

  • Keywords sd40.5 added

This change is quite small and looks good. Just needs someone with a mac to test.

comment:31 Changed 8 years ago by was

  1. Trivial nitpick: it says "create a temporary .zshenv to reset the path" but shouldn't it say "create .zshenv to reset the path.", since the file is in fact not temporary.
  1. After reading the above I still don't get the point of this patch. Before applying it, on OS X 10.7 I get:
    $ zsh
    ...
    $ sage -sh
    $ echo $PATH
    (sage-sh) wstein@blastoff:sage-5.0.beta15$ echo $PATH
    /Users/wstein/sage/build/sage-5.0.beta15/local/Frameworks/Python.framework/Versions/2.5/bin:/Users/wstein/sage/build/sage-5.0.beta15/spkg/bin:/Users/wstein/sage/build/sage-5.0.beta15/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin:/usr/texbin:/Users/wstein/sage/build/sage-5.0.beta15:/Users/wstein/bin:/Users/wstein/local/bin
    

which looks fine to me.

comment:32 Changed 8 years ago by jhpalmieri

Regarding point 2, try instead to use chsh to switch your shell to /bin/zsh, open up a new terminal window (now running zsh) and try

$ sage -sh
$ echo $PATH

again. When I do this, I get things like /usr/bin at the front of the path.

comment:33 Changed 8 years ago by jason

In sage -sh, it has this code to pick the shell run:

    # If $SHELL is unset, default to bash
    if [ -z "$SHELL" ]; then
        export SHELL=bash
    fi

Is your $SHELL variable set to zsh?

comment:34 Changed 8 years ago by iandrus

  • Reviewers set to Ivan Andrus, Jeroen Demeyer, Punarbasu Purkayastha
  • Status changed from needs_review to positive_review

I can confirm on a Mac with Sage 5.0 (having run chsh -s /bin/zsh) that without the patch my PATH starts with /usr/bin:/bin:/usr/sbin:/sbin:..., but after applying the patch it starts with /Users/gvol/SageStuff/sage-5.0.rc0/local/Frameworks/Python.framework/Versions/2.5/bin:/Users/gvol/SageStuff/sage-5.0.rc0/spkg/bin:/Users/gvol/SageStuff/sage-5.0.rc0/local/bin:.

I give it a positive review based on my reading of the comments here that testing on a Mac was all that was required. Looking at the patch I don't see any problems, but then, I don't use zsh either.

comment:35 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.1 to sage-5.0.1

comment:36 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.0.1.rc1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:37 Changed 4 years ago by chapoton

  • Description modified (diff)
Note: See TracTickets for help on using tickets.