Opened 12 years ago

Closed 10 years ago

Last modified 7 years ago

#10822 closed defect (fixed)

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

Reported by: Jason Grout 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:

Status badges

Description (last modified by Frédéric 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 John Palmieri 11 years ago.
scripts repo
trac-10822-zsh-path.patch (1.1 KB) - added by Jason Grout 10 years ago.
apply instead of previous patch

Download all attachments as: .zip

Change History (39)

comment:1 Changed 12 years ago by John Palmieri

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 Changed 12 years ago by Jason Grout

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 12 years ago by Jason Grout

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 12 years ago by Jason Grout

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 12 years ago by John Palmieri

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 12 years ago by Jason Grout

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 12 years ago by Jason Grout

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 Changed 12 years ago by Jason Grout

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 12 years ago by Jason Grout

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

comment:10 in reply to:  8 Changed 12 years ago by John Palmieri

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 12 years ago by John Palmieri

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 12 years ago by John Palmieri

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

comment:13 Changed 12 years ago by John Palmieri

Authors: John Palmieri
Status: newneeds_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 12 years ago by John Palmieri

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 11 years ago by John Palmieri

Dependencies: #11866, #11790

Rebased to #11866 and #11790.

Changed 11 years ago by John Palmieri

Attachment: trac_10822-sage-sh.patch added

scripts repo

comment:16 Changed 11 years ago by Martin Albrecht

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 Changed 11 years ago by Punarbasu Purkayastha

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 11 years ago by Punarbasu Purkayastha

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 ; Changed 11 years ago by John Palmieri

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 11 years ago by Punarbasu Purkayastha

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

Status: needs_reviewneeds_work

This obviously needs to be rebased...

comment:22 Changed 10 years ago by Jeroen Demeyer

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

comment:23 Changed 10 years ago by Jason Grout

Authors: John PalmieriJohn Palmieri, Jason Grout
Description: modified (diff)
Milestone: sage-5.0
Status: needs_workneeds_review

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

comment:24 Changed 10 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

This still needs to be rebased to a recent beta.

comment:25 Changed 10 years ago by Jason Grout

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

Changed 10 years ago by Jason Grout

Attachment: trac-10822-zsh-path.patch added

apply instead of previous patch

comment:26 Changed 10 years ago by Jason Grout

I rebased the patch to beta12

comment:27 Changed 10 years ago by Jason Grout

Status: needs_workneeds_review

comment:28 Changed 10 years ago by Jeroen Demeyer

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

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

Keywords: sd40.5 added

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

comment:31 Changed 10 years ago by William Stein

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

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

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

Reviewers: Ivan Andrus, Jeroen Demeyer, Punarbasu Purkayastha
Status: needs_reviewpositive_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 10 years ago by Jeroen Demeyer

Milestone: sage-5.1sage-5.0.1

comment:36 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.0.1.rc1
Resolution: fixed
Status: positive_reviewclosed

comment:37 Changed 7 years ago by Frédéric Chapoton

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