Skip to content

Use /usr/bin/env bash instead of /bin/bash in gh extension create#4203

Merged
mislav merged 1 commit intocli:trunkfrom
kidonng:patch-1
Aug 26, 2021
Merged

Use /usr/bin/env bash instead of /bin/bash in gh extension create#4203
mislav merged 1 commit intocli:trunkfrom
kidonng:patch-1

Conversation

@kidonng
Copy link
Contributor

@kidonng kidonng commented Aug 25, 2021

Not being too lazy to explain the reason, but this SO answer pretty much said it: https://stackoverflow.com/a/16365367

As for myself, the motivation is that NixOS doesn't have /bin/bash (only /bin/sh).

Some other places that may need to update:

P.S. I'm not sure when a new release will come out, but before this PR gets merged (if even) and everyone update to the new version, there will be a lot of new plugins using /bin/sh and none of them will work without modification.

I probably won't bother to make a PR to change the shebang for every plugin out there, gh should set a good example, even if the current way works for most people.

@cliAutomation
Copy link
Collaborator

Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message.

@mislav
Copy link
Contributor

mislav commented Aug 26, 2021

P.S. I'm not sure when a new release will come out, but before this PR gets merged (if even) and everyone update to the new version, there will be a lot of new plugins using /bin/sh and none of them will work without modification.

You're right. But we do control the execution mechanism for these scripts. So before executing a script, we could read the first line from it, and if it's #!/bin/bash and we're on an OS where that doesn't exist, we could find bash in PATH and execute it manually with /path/to/bash /extension/script/here. Just an idea if we wish to help portability on systems like NixOS.

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