Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

mathematicalmichael
Copy link
Contributor

@mathematicalmichael mathematicalmichael commented Jun 9, 2024

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
Copy link
Member

mpilgrem commented Jun 9, 2024

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

@mpilgrem mpilgrem changed the title offer to extend path for user get-stack.sh: offer to extend path for user Jun 9, 2024
@mpilgrem
Copy link
Member

mpilgrem commented Jun 10, 2024

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 added a commit that referenced this pull request Jun 10, 2024
@mpilgrem
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

Also adds has_ci_environment to guard the user prompt
@mpilgrem mpilgrem force-pushed the feature/interactive-path-extend branch from 792daba to 7c519cb Compare June 12, 2024 18:59
@mpilgrem mpilgrem merged commit e3ea155 into commercialhaskell:master Jun 12, 2024
13 checks passed
@mpilgrem
Copy link
Member

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

@mathematicalmichael
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants