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

[RFC] Scripts under .git/hooks/ may contain calls to undefined commands #617

Open
phansys opened this issue Aug 8, 2016 · 8 comments
Open
Labels

Comments

@phansys
Copy link
Member

phansys commented Aug 8, 2016

I have a prepare-commit-msg hook, which has #!/bin/bash as shebang. Obviously, as the container image doesn't includes bash, it doesn't work and the merge commit is aborted.
Even if in my case, the situation could be resolved adding bash to container dependencies, I think there could be other corner cases where commit hooks invokes commands that aren't present at the containerized gush environment.
How we could proceed in front of these scenarios? I can imagine just a possible solution: Check if there is any of the hooks enabled, delete them and warn the user about that limitation.

@sstok sstok added the RFC label Aug 9, 2016
@sstok
Copy link
Contributor

sstok commented Aug 9, 2016

Hmm, hooks should not be executed when using Gush as local operations are only done to bypass some API limitations. Is there no command option in Git to skip/disable this for the operation?

Edit. Typo.

@phansys
Copy link
Member Author

phansys commented Aug 9, 2016

@sstok, there are some options such --no-verify for commit, which bypass the pre-commit and commit-msg. Additionally, all hooks can be disabled removing the executable bit:

Hooks that don’t have the executable bit set are ignored.

Ref: https://git-scm.com/docs/githooks#_description

@cordoval
Copy link
Member

cordoval commented Aug 9, 2016

yeah the problem is with the commands that are not in there, but this could include any tools and be any commands so i don't think you can include satisfactorily all cases here. The only gush commands that commit are when rebasing or a minimum set. I think you should use git on local for using your hooks and then just use gush for the main api stuff.

@phansys
Copy link
Member Author

phansys commented Aug 9, 2016

@cordoval , the RFC is about how to address the situation. I think executing chmod -x $GIT_DIR/hooks/* and later restore the original permissions could be a good solution in order to cover all the possibilities.

@cordoval
Copy link
Member

oh you mean because the hooks are not seen as executables? is that what you mean?

@phansys
Copy link
Member Author

phansys commented Aug 10, 2016

No @cordoval, I mean we should disable them at gush execution time (temporarily) with some mechanism that applies for all of them. I'm opening the game here in order to hear thoughts about how to achieve that goal.

@cordoval
Copy link
Member

i think we should include bash to the image, are you sure it does not contain sh?

@phansys
Copy link
Member Author

phansys commented Sep 30, 2016

Yes @cordoval, it contains sh but not bash. But I think adding bash to container dependencies is just a workaround for my specific scenario, other users can have any other dependencies on their hooks. BTW, I agree that majority of hooks just relies on simple sh or bash commands, but we should prevent the "border" cases too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants