Skip to content
This repository has been archived by the owner on Mar 19, 2022. It is now read-only.

explicitly execute bash script in bash #351

Merged
merged 3 commits into from Feb 26, 2014
Merged

explicitly execute bash script in bash #351

merged 3 commits into from Feb 26, 2014

Conversation

robacarp
Copy link
Contributor

Fix for Issue #350

I put the if/else block on one line to give the best possibility of whatever random shell on the client passing the whole command string on to bash. In my testing, csh seems to have the most trouble with multiline strings. I'm not familiar enough with it to know if there is a cleaner way to send the command string through to bash.

I hardcoded /bin/sh as the shell because the opscode install.sh script already depends on /bin/sh.

Robert L. Carpenter added 2 commits February 23, 2014 00:54
@tmatilai
Copy link
Collaborator

Thanks!

Could you test just adding the semicolons after the curl and wget lines? Or if the newline character really is the issue, we can eat it with a backslash at the end of lines. I think that one-liner is not very readable.

@robacarp
Copy link
Contributor Author

It is amazing what a few hours of sleep will do...I had previously tried the backslashes and found an incompatibility with fishshell. However, adding semicolons after curl/wget and backslashes for the newlines passes through every shell I could find. Good suggestion.

I tested both execution paths (curl and wget) by duplicating the if block and forcing fail on the second one, and running knife solo prepare somebox while manually iterating through csh, sh, dash, bash, and fish. csh and fish being the keys as they're not necessarily bash compatible.

matschaffer added a commit that referenced this pull request Feb 26, 2014
explicitly execute bash script in bash
@matschaffer matschaffer merged commit 511c973 into matschaffer:master Feb 26, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants