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

Use /bin/sh as command line interpreter #265

Merged

Conversation

manicmaniac
Copy link
Member

@manicmaniac manicmaniac commented Aug 25, 2019

ShellExecutor considers $SHELL environment variable to decide which shell should be used.
Actually POSIX permits to set $SHELL to any kind of command line interpreter even it doesn't conform to the specification for shell command interpreter.

https://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
SHELL
This variable shall represent a pathname of the user's preferred command language interpreter. If this interpreter does not conform to the Shell Command Language in the Shell and Utilities volume of IEEE Std 1003.1-2001, Chapter 2, Shell Command Language, utilities may behave differently from those described in IEEE Std 1003.1-2001.

Potentially users even can do sudo chsh -s /usr/bin/python and in that case, danger-swift doesn't work properly.

Therefore I changed ShellExecutor not to depend on the environment variable but just use POSIX standard /bin/sh.
/bin/sh is guaranteed to be there and it must be a bsh-compatible shell script language interpreter.

https://pubs.opengroup.org/onlinepubs/7908799/xcu/sh.html

@f-meloni
Copy link
Member

Nice! Thank you! :)

@f-meloni f-meloni merged commit 24dcf88 into danger:master Aug 25, 2019
@peril-staging
Copy link
Contributor

peril-staging bot commented Aug 25, 2019

Thanks for the PR @manicmaniac.

This PR has been shipped in v2.0.2 - CHANGELOG.

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.

2 participants