Re: [PATCH 0/5] Multiple hook support
- Date: Tue, 30 Apr 2019 17:39:02 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH 0/5] Multiple hook support
On Wed, Apr 24, 2019 at 12:49:43AM +0000, brian m. carlson wrote:
> I've talked with some people about this approach, and they've indicated
> they would prefer a configuration-based approach.
I think I'm some people. :)
I agree with the thoughts that Jonathan pointed out in , but I wanted
to raise a few points that are more directly related to hook features:
1. Config is resolved at run-time, making it much easier to have
system or user-level hooks (as opposed to our current system of
on-disk files, which require copying or symlinking hooks ahead of
time into each repository you want to impact).
2. Config values let you easily run hooks from multiple sources (e.g.,
a hook specified in /etc/gitconfig, one in ~/.gitconfig, and then a
repo-level hook in .git/config). Even with a "hook.d" feature like
this, you are back to doing lots of symlinks within the ".d"
directory to get this behavior.
I specifically worry that adding ".d" directories is a step in the
wrong direction because our solution will probably make this point
_worse_ than whatever custom trampolines people are already using.
3. A well-designed config schema can leave room for more
configuration. E.g., one of the big questions with multi-hooks is
the error semantics. But what if we had:
command = my-hook-cmd
command = another-hook-cmd
# stop running and return failure at first non-zero exit
errorBehavior = stop-on-first
# ...or run all and return error if _any_ failed
errorBehavior = report-any-error
# ...or run and report if any _accepted_
errorBehavior = report-any-success
Those are just off the top of my head. But my point is that by
staking out a config section for each hook, it gives us a place to
naturally add new config options. And we can do it on a per-hook
basis, which I think will be important since each hook has its own
Now that's not _strictly_ necessary. We could still have
"hook.pre-receive.errorBehavior" and just assume
"hook.pre-receive.command" is "$GIT_DIR/hooks/pre-receive". But I
think doing the whole thing from config makes the behavior simple
and consistent (and the backwards compatibility is easy -- if they
aren't using the new command config, we really do behave "as if"
they had set it to the file in the hooks directory).
So I agree with your general sentiment that the multi-hook support
is conceptually orthogonal to switching to a config-based system. But I
think it's worth considering whether we want to do something
- if we introduce it later, it saves us from having _three_ ways to do
the same thing
- I think it provides a more natural way to express the options that
will inevitably grow once we have multi-hook support