public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Adrian Ratiu <adrian.ratiu@collabora•com>
To: Jeff King <peff@peff•net>, Wesley Schwengle <wesleys@opperschaap•net>
Cc: git@vger•kernel.org
Subject: Re: git hook question
Date: Wed, 03 Jun 2026 16:07:15 +0300	[thread overview]
Message-ID: <874ijjojr0.fsf@gentoo.mail-host-address-is-not-set> (raw)
In-Reply-To: <20260529210049.GC2628906@coredump.intra.peff.net>

On Fri, 29 May 2026, Jeff King <peff@peff•net> 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.

  reply	other threads:[~2026-06-03 13:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-29  5:01 git hook question Wesley Schwengle
2026-05-29  5:21 ` Jeff King
2026-05-29 16:11   ` Wesley Schwengle
2026-05-29 16:22     ` Wesley
2026-05-29 17:52     ` Ben Knoble
2026-05-29 19:23     ` Jeff King
     [not found]       ` <4d938e1e-fdd3-42d6-a879-4d394ee8c00d@opperschaap.net>
2026-05-29 21:00         ` Jeff King
2026-06-03 13:07           ` Adrian Ratiu [this message]
2026-06-01  5:33   ` Junio C Hamano
2026-06-01  5:55     ` 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

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

  git send-email \
    --in-reply-to=874ijjojr0.fsf@gentoo.mail-host-address-is-not-set \
    --to=adrian.ratiu@collabora$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    --cc=wesleys@opperschaap$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox