commercialhaskell / stack

The Haskell Tool Stack
http://haskellstack.org
BSD 3-Clause "New" or "Revised" License
3.95k stars 842 forks source link

get-stack.sh: offer to extend path for user #6609

Closed mathematicalmichael closed 2 weeks ago

mathematicalmichael commented 3 weeks ago

I saw a todo while inspecting the shell script that I could handle, having done this for other installers before.

tested on zsh and bash by setting PATH to something short, removing current installation, and running the shell script. Did not test fish but used what I believe is the default there.

mpilgrem commented 3 weeks ago

@mathematicalmichael, many thanks! I'll have a look.

mpilgrem commented 3 weeks ago

I tested this on Windows with sh get-stack.sh (in ...\etc\scripts, in the Stack-supplied MSYS2 environment), and got the following output (extract). Pressing ENTER was not recognised as the default. C:\Users\mike\.bashrc does not exist:

Stack has been installed to: /bin/stack

WARNING: '/bin' is not on your PATH.

WARNING: '/c/Users/mike/.local/bin' is not on your PATH.
    Stack will place the binaries it builds in '/c/Users/mike/.local/bin' so
    for best results, please add it to the beginning of PATH in your profile.

    You can do this by running the following command:
    echo 'export PATH="/c/Users/mike/.local/bin:$PATH"' >> "/c/Users/mike/.bashrc"
    (You may need to restart your shell for this to take effect.)

Would you like this installer to add it to your PATH in '/c/Users/mike/.bashrc'?
    (this will be done by adding export PATH="/c/Users/mike/.local/bin:$PATH" to it)
    (you may need to restart your shell for this to take effect)
    [y] Yes, prepend  [n] No (default)

Please answer 'y' or 'n'
Would you like this installer to add it to your PATH in '/c/Users/mike/.bashrc'?
    (this will be done by adding export PATH="/c/Users/mike/.local/bin:$PATH" to it)
    (you may need to restart your shell for this to take effect)
    [y] Yes, prepend  [n] No (default)
n
Not updating PATH in '/c/Users/mike/.bashrc'

I am still investigating why.

(EDIT: As an aside, the existing get-stack.sh script outputs in the same conditions:

Stack has been installed to: /bin/stack

WARNING: '/bin' is not on your PATH.

WARNING: '/c/Users/mike/.local/bin' is not on your PATH.
    Stack will place the binaries it builds in '/c/Users/mike/.local/bin' so
    for best results, please add it to the beginning of PATH in your profile.

That is not great, but that is not something this pull request is intended to, or needs to, address.)

mpilgrem commented 2 weeks ago

I now understand the Windows behaviour: when you run a script with the MSYS2-supplied sh tool, the BASH_VERSION variable is set and available to the script.

mpilgrem commented 2 weeks ago

After testing on Ubuntu, I discoved that POSIX-compliant read does not support -e or -t options. They were giving rise to errors:

get-stack.sh: 813: read: Illegal option -e
get-stack.sh: 813: read: Illegal option -t

So, I have added a commit to remove them and the idea of timeout and will rely on the has_ci_environment guard.

mpilgrem commented 2 weeks ago

@mathematicalmichael, I am going to tidy-up the commit history in preparation for merging.

mpilgrem commented 2 weeks ago

@mathematicalmichael, many thanks. That should now be live.

mathematicalmichael commented 2 weeks ago

thanks! sorry about the radio silence, and thanks for accepting the contribution