typicode / husky

Git hooks made easy 🐶 woof!
https://typicode.github.io/husky
MIT License
31.75k stars 999 forks source link

Use /usr/bin/env sh instead of direct path of sh #1051

Closed skhaz closed 2 years ago

skhaz commented 2 years ago

According with the best practices of shell scripting, the right way to invoke the shebang is #!/usr/bin/env shell

Reference: https://www.cyberciti.biz/tips/finding-bash-perl-python-portably-using-env.html

Maxim-Mazurok commented 2 years ago

This doesn't work in my Ubuntu 20 (WSL):

$ /usr/bin/env shell
/usr/bin/env: ‘shell’: No such file or directory
Maxim-Mazurok commented 2 years ago

I wanted to use bash, this worked for me:

# Bash sets the BASH environment variable, so if it is not set, then we
# are running in a different shell, so manually run ourselves in BASH.
# source: https://github.com/typicode/husky/issues/971#issuecomment-846431179
if [ -z "${BASH:-}" ]; then
  exec bash "$0" "$@"
fi

Credit: https://github.com/typicode/husky/issues/971#issuecomment-846431179

nathandao commented 2 years ago

This doesn't work in my Ubuntu 20 (WSL):

$ /usr/bin/env shell
/usr/bin/env: ‘shell’: No such file or directory

hmmm, does using /usr/bin/env sh instead of shell work for you? :thinking:

nathandao commented 2 years ago

@skhaz since you're already on it, would it be possible to update other parts of the code that are using #!/bin/sh as well? The files I found are:

.husky/commit-msg
.husky/pre-commit
docs/README.md
./husky.sh
Maxim-Mazurok commented 2 years ago

This doesn't work in my Ubuntu 20 (WSL):

$ /usr/bin/env shell
/usr/bin/env: ‘shell’: No such file or directory

hmmm, does using /usr/bin/env sh instead of shell work for you? 🤔

Yes, it does. Opens up sh for me. Same with /usr/bin/env bash - opens up bash for me.

Maxim-Mazurok commented 2 years ago

Also, I can see that in the linked article they mention #!/usr/bin/env bash not #!/usr/bin/env shell:

Let us find out why is it a good idea to use #!/usr/bin/env bash instead of #!/bin/bash as shebang? (source)

So, I guess that it was a typo, @skhaz?

skhaz commented 2 years ago

@Maxim-Mazurok do you want bash instead of sh? I am not sure if macOS cames with bash, sh is a more secure option

skhaz commented 2 years ago

@nathandao done, changed all references of /bin/sh

Maxim-Mazurok commented 2 years ago

@Maxim-Mazurok do you want bash instead of sh? I am not sure if macOS cames with bash, sh is more secure option

Yes, I want bash because I am letting developers configure if they want to lint and unit test their code in pre-commit hook. I'm storing scrips in array, and then passing this array of scripts to npm-run-all. Didn't find an easy way to do this with sh. Also, I'm only concerned about Ubuntu and Windows. And windows git client comes with git bash, so I guess it's pretty safe for me. Mac may think differently ;)

Maxim-Mazurok commented 2 years ago

This is my code:

scripts=("npm:sort -- --check" "npm:spell-check" "npm:prettier:check")
[[ "$PRE_COMMIT_LINT" == true ]] && scripts+=("npm:test:lint")
[[ "$PRE_COMMIT_TEST_UNIT" == true ]] && scripts+=("npm:test:unit")
npx -y concurrently --kill-others-on-fail "${scripts[@]}"

But sh/bash is not related to this issue PR, so never mind :)

skhaz commented 2 years ago

@Maxim-Mazurok you can use bash by invoking the script inside the hook, for example:

Your hook

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

your_script.sh || exit

And in your_script.sh you have bash specific things:

#!/bin/bash

your code for bash

Or... Manually change to bash your hook

Maxim-Mazurok commented 2 years ago

Or... Manually change to bash your hook

This doesn't work (changing shebang in hook), because husky.sh resets shell to be sh.

@Maxim-Mazurok you can use bash by invoking the script inside the hook, for example:

Your hook

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

your_script.sh || exit

And in your_script.sh you have bash specific things:

#!/bin/bash

your code for bash

This is a good idea, thanks

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Maxim-Mazurok commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Occuring an activity

skhaz commented 2 years ago

Bump

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Maxim-Mazurok commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Occuring an activity

typicode commented 2 years ago

Thank you for the PR 👍