Skip to content
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

update shebang to reflect bashisms #1269

Merged
merged 1 commit into from
May 27, 2018
Merged

update shebang to reflect bashisms #1269

merged 1 commit into from
May 27, 2018

Conversation

mcandre
Copy link

@mcandre mcandre commented Oct 22, 2017

Update shebang, given that local is not a POSIX semantic. Discovered this when trying to run the rustup installer on Oracle Solaris, where /bin/sh is a symlink to ksh93.

Note that the rustup.rs website should also pipe to bash, not sh.

Alternatively, we can rewrite this script in pure POSIX sh, though I leave that to cooler heads.

Update shebang, given that `local` is not a POSIX semantic.
@mcandre
Copy link
Author

mcandre commented Oct 22, 2017

Looks like one of the macOS CI builds stalled for this tweak, dunno why?

https://travis-ci.org/rust-lang-nursery/rustup.rs/jobs/290991285

@mati865
Copy link
Contributor

mati865 commented Oct 29, 2017

If changing to bash using #!/usr/bin/env bash should be preferred.

@mcandre
Copy link
Author

mcandre commented Nov 10, 2017

@mati865 Eh, if the shell were ash, dash, or posh, or relied on alpha features from Homebrew bash tip, then /usr/bin/env would definitely help to port the script to varied systems. But for common shell languages like bash, sh, ksh, zsh, the /usr/bin/env prefix just slows things down unnecessarily.

@Diggsey
Copy link
Contributor

Diggsey commented May 27, 2018

Thanks for the PR!

@Diggsey Diggsey merged commit 09fb0f5 into rust-lang:master May 27, 2018
@Diggsey Diggsey mentioned this pull request May 27, 2018
AJ-Ianozi pushed a commit to AJ-Ianozi/getada-download that referenced this pull request Oct 12, 2023
update shebang to reflect bashisms
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants