On 24/04/2019 09:10, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Apr 24 2019, brian m. carlson wrote: > >> Oftentimes, people want to use multiple of the same kind of hook. This >> may be because software or a project they use requires a given hook, but >> they would also like to have a custom hook, or because they're using >> multiple pieces of software that both require involvement with the same >> hook. >> >> This series introduces support for multiple hooks by using a ".d" >> directory. For example, instead of using a single >> ".git/hooks/post-checkout" file, you'd use multiple files within >> ".git/hooks/post-checkout.d". This is the standard Unix paradigm for >> multiple files and should be familiar to most people. Hooks are executed >> in sorted order. > > I think it's interesting if people can chime in with current in-the-wild > implementations of this. > > I know GitLab's the best, not because I was in any way involved in it, > I've just dealt with writing hooks for it: > https://docs.gitlab.com/ee/administration/custom_hooks.html#chained-hooks-support > > There: > > 1. The instance has a 'hooks' dir in the .git repo that's a symlink to > /my/global/hooks. They could also use a /etc/gitconfig > core.hooksPath for this part, but whatever. > > 2. That /my/global/hooks has e.g. a /my/global/hooks/pre-receive that > git itself runs, which is a trampoline script that runs all over the > place and executes global/per-project hooks (which live in > .git/custom_hooks/). Having global and local hooks could be useful. I've got some tooling that rewrites multiple branches and has hooks to prevent committing and rebasing in worktrees where the branch is being rewritten by a rewrite in another worktree. I'm using core.hooksPath at the moment, but if I ever work with a project that uses its own hooks I'll have to add some sort of trampoline like gitlab. Best Wishes Phillip > > 3. "The hooks of the same type are executed in order and execution > stops on the first script exiting with a non-zero value." > > I wonder if the eventual goal of this facility would be to get such > users on board with using git's native feature for this. This series is > most of the way there for GitLab's case, but not quite. > >> To preserve backwards compatibility, we don't run the hooks in the ".d" >> directory if the single file is a valid hook (i.e. it exists and is >> executable). This is because some people already have multiple hook >> scripts configured, and if we ran them both, we'd run the hooks twice. >> This would be bad for e.g. the prepare-commit-msg hook. This is also the >> least surprising behavior. >> >> We check each hook for its exit status, even if the hook normally >> ignores exit status, and if it fails, we abort executing further hooks. >> This provides an easy way to reason about what the exit status is when a >> hook fails; we need not consider how to handle multiple failing hooks. >> It's also consistent among all hooks, whether they care about exit >> status or not. > > Others have replied to this already. I linked to the 2016 discussion of > my RFC for this in > https://public-inbox.org/git/87wojjsv9p.fsf@xxxxxxxxxxxxxxxxxxx where I > made the same choice. Some points on that: > > * There was the mention of "but what if someone wants to run them all", > e.g. for logging to N logging systems where one fails, that's been > brought up again this time around by others. > > * The case I find more interesting is the ability to run the hooks in > parallel. Saying "glob order and exit on first fail" categorically > closes the door to that. > >> I've talked with some people about this approach, and they've indicated >> they would prefer a configuration-based approach. I've tried to make the >> series such that it can be replaced with such an approach if that's the >> decision we make. It should be easy enough to simply replace find_hooks >> with an appropriate implementation and update the test framework. > > I think whatever opinions we all have on the current implementation it's > OK to get this in in *some* form and just made it configurable or > whatever later. > > Most of the work is the ability to run N hooks, how exactly that happens > can be tweaked later, and if this series lands and someone isn't 100% > happy with the semantics they're no worse off than they are now. > > I.e. we could get something like this in its current form, and later have: > > core.hooksOrder = glob | random > core.hooksHaltOnError = true | never > > Where we'd say that what this series does is core.hooksOrder=glob and > core.hooksHaltOnError=true, but that e.g. parallel "run them all" could > be done in the future with core.hooksOrder=random & > core.hooksHaltOnError=never >
