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

Already on GitHub? Sign in to your account

Use bash, not sh for install scritps #450

Merged
merged 1 commit into from Jul 3, 2014

Conversation

Projects
None yet
3 participants
Contributor

mgol commented Jul 2, 2014

Install scripts don't work in pure sh; they may work on some systems where
the /bin/sh binary actually implements more than the pure Bourne Shell but
fail on other ones (e.g. Ubuntu).

Just using bash works.

Use bash, not sh for install scritps
Install scripts don't work in pure sh; they may work on some systems where
the /bin/sh binary actually implements more than the pure Bourne Shell but
fail on other ones (e.g. Ubuntu).

Just using bash works.
Contributor

koenpunt commented Jul 2, 2014

Any insights on what does not work in sh?

Contributor

mgol commented Jul 2, 2014

I don't have an Ubuntu box nearby at the moment except on a colleague machine where it broke and I don't want to break it for him again. ;) Ubuntu is pretty popular, though.

If you want to keep it working in pure sh everywhere, you should test it on a shell that tries to emulate it as close as possible, e.g. dash which is used by Ubuntu.

That said, your install script already states it should be run in bash: https://github.com/creationix/nvm/blob/master/install.sh#L1

Collaborator

ljharb commented Jul 3, 2014

@mzgol the shebang at the top of the install script should autorun it in bash, shouldn't it? or does that only apply when it's executed directly.

Contributor

mgol commented Jul 3, 2014

@mzgol the shebang at the top of the install script should autorun it in bash, shouldn't it? or does that only apply when it's executed directly.

It doesn't. :) You'd have to run it directly but then you'll need an executable flag so it's easier to just pipe it to bash.

ljharb added a commit that referenced this pull request Jul 3, 2014

Merge pull request #450 from mzgol/install-script
Use bash, not sh for install scritps

@ljharb ljharb merged commit d016fe0 into creationix:master Jul 3, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@mgol mgol deleted the mgol:install-script branch Jul 3, 2014

Contributor

mgol commented Jul 3, 2014

Thanks!

Contributor

mgol commented Jul 10, 2014

There's one left-over reference, search for curl ... | NVM_DIR=/usr/local/nvm sh.

EDIT: PR: #463.

mgol added a commit to mgol/nvm that referenced this pull request Jul 10, 2014

ljharb added a commit that referenced this pull request Jul 10, 2014

Merge pull request #463 from mzgol/bash
Use bash, not sh for install scritps - followup to #450
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment