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

Default run.shell to /bin/sh #957

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tobim
Copy link

@tobim tobim commented Jul 11, 2023

/bin/bash is not availabe on some popular distributions, for example NixOS. Defaulting to the more conservative /bin/sh improves portability.

`/bin/bash` is not availabe on some popular distributions, for example
NixOS. Defaulting to the more conservative `/bin/sh` improves
portability.
@tobim tobim force-pushed the default-run-shell-to-bin-sh branch from 6b035b7 to 967a6d1 Compare July 11, 2023 07:33
@kuwv
Copy link
Contributor

kuwv commented Oct 13, 2023

This should be the correct default.

@hydrocat
Copy link

hydrocat commented Aug 2, 2024

Based on the CI results, the test cases are strictly requiring the shell to be bash. We could do like windows and use the SHELL environment variable, which would make sense for NixOS but would perhaps crash depending on the user's own configuration (for example, if they use fish, Oil, zsh, csh .. )

Maybe, we could query the system for bash's path.

@tobim
Copy link
Author

tobim commented Aug 4, 2024

Well, you could use /usr/bin/env to find bash in $PATH.

@kuwv
Copy link
Contributor

kuwv commented Aug 5, 2024

It's not really an issue about locating BASH in the environment.

BASH includes capabilities you don't need with Python/invoke. The fact that it's not included with Alpine or other Busybox-based distros is problematic too.

The default should be '/bin/sh' because it's the right combination of capabilities and compatibility for invoke.

For example if you were to use invoke in a CI you could potentially utilize many different images. Using bash would force you to either bullet proof either incoke or the containers to work instead of just "do the thing".

@hydrocat
Copy link

hydrocat commented Aug 5, 2024 via email

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