public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jeff King <peff@peff•net>
To: Wesley Schwengle <wesleys@opperschaap•net>
Cc: Adrian Ratiu <adrian.ratiu@collabora•com>, git@vger•kernel.org
Subject: Re: git hook question
Date: Fri, 29 May 2026 17:00:49 -0400	[thread overview]
Message-ID: <20260529210049.GC2628906@coredump.intra.peff.net> (raw)
In-Reply-To: <4d938e1e-fdd3-42d6-a879-4d394ee8c00d@opperschaap.net>

[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.

-Peff

  parent reply	other threads:[~2026-05-29 21:00 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 [this message]
2026-06-03 13:07           ` Adrian Ratiu
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=20260529210049.GC2628906@coredump.intra.peff.net \
    --to=peff@peff$(echo .)net \
    --cc=adrian.ratiu@collabora$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --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