git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Duy Nguyen <pclouds@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 0/5] Multiple hook support
Date: Wed, 24 Apr 2019 10:55:37 +0100	[thread overview]
Message-ID: <551bcdfa-d098-48a8-8712-05b7e1516bd8@gmail.com> (raw)
In-Reply-To: <87tvensu1a.fsf@evledraar.gmail.com>

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@evledraar.gmail.com 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
> 


  reply	other threads:[~2019-04-24  9:55 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-24  0:49 [PATCH 0/5] Multiple hook support brian m. carlson
2019-04-24  0:49 ` [PATCH 1/5] run-command: add preliminary support for multiple hooks brian m. carlson
2019-04-24  2:27   ` Junio C Hamano
2019-04-24 18:48     ` Johannes Sixt
2019-04-25  0:55       ` Junio C Hamano
2019-04-25  9:39         ` Ævar Arnfjörð Bjarmason
2019-04-25 10:04           ` Junio C Hamano
2019-04-25 19:40         ` Johannes Sixt
2019-04-26 20:58           ` brian m. carlson
2019-04-26 21:53             ` Johannes Sixt
2019-04-24 22:32     ` brian m. carlson
2019-04-24  0:49 ` [PATCH 2/5] builtin/receive-pack: add " brian m. carlson
2019-04-24  0:49 ` [PATCH 3/5] sequencer: " brian m. carlson
2019-04-24  9:51   ` Phillip Wood
2019-04-24 22:46     ` brian m. carlson
2019-04-25 14:59       ` Phillip Wood
2019-04-24  0:49 ` [PATCH 4/5] builtin/worktree: add support for multiple post-checkout hooks brian m. carlson
2019-04-24  0:49 ` [PATCH 5/5] transport: add support for multiple pre-push hooks brian m. carlson
2019-04-24  2:09 ` [PATCH 0/5] Multiple hook support Junio C Hamano
2019-04-24  2:22   ` brian m. carlson
2019-04-24  2:41     ` Junio C Hamano
2019-04-24  8:14     ` Ævar Arnfjörð Bjarmason
2019-04-24  2:34 ` Jonathan Nieder
2019-04-24  7:43   ` Ævar Arnfjörð Bjarmason
2019-04-24  8:22   ` Ævar Arnfjörð Bjarmason
2019-04-24 23:07   ` brian m. carlson
2019-04-24 23:26     ` Jonathan Nieder
2019-04-25 10:08     ` How to undo previously set configuration? (again) Ævar Arnfjörð Bjarmason
2019-04-25 10:43       ` Duy Nguyen
2019-04-25 11:58         ` Ævar Arnfjörð Bjarmason
2019-04-26 15:18           ` Ævar Arnfjörð Bjarmason
2019-04-25 14:36       ` Jonathan Nieder
2019-04-25 14:43         ` Barret Rhoden
2019-04-25 15:27           ` Ævar Arnfjörð Bjarmason
2019-04-25 15:25         ` Ævar Arnfjörð Bjarmason
2019-04-26  2:13         ` Junio C Hamano
2019-04-26  9:36         ` Duy Nguyen
2019-04-30 21:14           ` Jeff King
2019-05-01 11:41             ` Duy Nguyen
2019-05-01 12:18               ` Ævar Arnfjörð Bjarmason
2019-05-01 12:32                 ` Duy Nguyen
2019-05-01 21:09                   ` Jeff King
2019-05-01 21:15                 ` Jeff King
2019-04-24  8:10 ` [PATCH 0/5] Multiple hook support Ævar Arnfjörð Bjarmason
2019-04-24  9:55   ` Phillip Wood [this message]
2019-04-24 18:29   ` Bryan Turner
2019-04-24  9:49 ` Duy Nguyen
2019-04-24 22:49   ` brian m. carlson
2019-04-24 23:40   ` brian m. carlson
2019-04-25  0:08     ` Duy Nguyen
2019-04-30 21:39 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=551bcdfa-d098-48a8-8712-05b7e1516bd8@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).