Subject: Re: git hook question

From: Adrian Ratiu <[email-masked]>
Date: Wed, 03 Jun 2026 16:07:15 +0300

On Fri, 29 May 2026, Jeff King <[email-masked]> wrote:
> [re-adding list cc; let's let everyone benefit from the discussion]
>
> On Fri, May 29, 2026 at 04:14:33PM -0400, Wesley Schwengle wrote:
>
>> > I don't think the hooks themselves should need to be aware. If somebody
>> > is calling "git hook run pre-push" without providing arguments, they are
>> > breaking the contract to the hooks. You can get away with it if you know
>> > your particular hooks do not care about those arguments, but in the
>> > general case, what should a pre-push hook that _does_ care about the
>> > remote name do when it doesn't get any arguments? It's an error.
>> 
>> Are they? The manual says this:
>> 
>> git hook run has been designed to make it easy for tools which wrap Git to
>> configure and execute hooks using the Git hook infrastructure.  It is
>> possible to provide arguments and stdin via the command line, as well as
>> specifying parallel or series execution if the user has provided multiple
>> hooks.
>> 
>>      Assuming your wrapper wants to support a hook named
>> "mywrapper-start-tests", you can have your users specify their hooks like
>> so:
>> 
>>          [hook "setup-test-dashboard"]
>>            event = mywrapper-start-tests
>>            command = ~/mywrapper/setup-dashboard.py --tap
>> 
>>      Then, in your mywrapper tool, you can invoke any users' configured
>> hooks by running:
>> 
>>          git hook run --allow-unknown-hook-name mywrapper-start-tests \
>>            # providing something to stdin
>>            --stdin some-tempfile-123 \
>>            # execute multiple hooks in parallel
>>            --jobs 3 \
>>            # plus some arguments of your own...
>>            -- \
>>            --testname bar \
>>            baz
>> 
>> There is nothing about the contract of the hook, in fact, the way it is
>> written there isn't really a contract.
>
> This is a made-up hook, so it is up to the person defining
> mywrapper-start-tests to define that contract. And in this example,
> implicitly it takes whatever is in some-tempfile-123 on stdin, and
> --testname as an argument. What those mean would need to be communicated
> between the script invoking "git hook" and whoever is configuring hooks.
>
> I agree that is not made very clear in the documentation, though.
>
>> > So whether you are getting input as arguments or over stdin, it's
>> > probably something the hook needs to deal with (or at least think
>> > about).
>> 
>> Right. I see where this is going. That means I think the examples in the
>> manual are incorrect, no, that's harsh, it could be stated more clearly in
>> git-hook(1).
>> 
>> Examples like this:
>> 
>> > [hook "linter"]
>> >   event = pre-commit
>> >   command = ~/bin/linter --cpp20
>> 
>> seem to indicate: Any script can be run as a hook, the fact it needs to
>> respect the native hook structure isn't mentioned. This is mentioned:
>
> That example is OK-ish, in the sense that pre-commit does not take any
> arguments or receive anything on stdin. So you really can invoke
> whatever program you like (though it needs to understand how to use Git
> commands to look at what is staged in the index). So the details of
> "~/bin/linter" are doing a lot of the heavy lifting here, which is left
> unsaid.
>
> But the later example that adds "event = pre-push" is actively
> misleading. How does the ~/bin/linter script even know in which context
> it's being run? In the real world you are more likely to invoke a script
> that is aware it is a Git hook and can react accordingly.
>
> So I suspect there is a lot of room for expanding the documentation and
> explaining some of these gotchas. +cc Adrian, who wrote these docs, for
> visibility.

Yes, there is a lot of room for improvements everywhere, especially in
the documentation.

Patches are very much welcome to expand on or correct hook-related
issues. :) 

BTW the git hook command is also just a very basic tool for testing, it
needs much attention and more additions. It is obviously not
feature-complete or bug-free.

Some historical context for the curious:

This area of work was blocked for almost a decade because people tried to
find a perfect/complete solution in one go, with complex patch series
reaching even 36-38 review iterations for a single series which went
nowhere, was regressing, was hard to review, you get the idea. 

So I tried to enable a simplified incremental development approach,
reusing existing APIs & mechanisms, to allow more people to contribute
smaller patches which are also easier to review, test and so on.

P.S:
This also reminds me, I don't think it's documented anywhere that the
proc-receive hook is not using hook.[ch], so it cannot be specified via
configs yet like pre-receive and other similar server hooks.

I actually have a collegue at Collabora working on converting
proc-receive so we can remove some deprecated APIs and also clean up
some external hook_exists() calls which are now redundant because they
are handled by the unified hook.c implementation.