Re: [PATCH 0/5] Multiple hook support

On Wed, Apr 24, 2019 at 12:49:43AM +0000, brian m. carlson wrote:

I think I'm some people. :)

I agree with the thoughts that Jonathan pointed out in [1], 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:

       [hook "pre-receive"]
       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
      semantics.

      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 config-based first:

  - 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

-Peff

[1] https://public-inbox.org/git/20190424023438.GE98980@google.com/