From: Adrian Ratiu Date: Wed, 03 Jun 2026 13:07:15 GMT Subject: Re: git hook question Message-ID: <874ijjojr0.fsf@gentoo.mail-host-address-is-not-set> In-Reply-To: <20260529210049.GC2628906@coredump.intra.peff.net> On Fri, 29 May 2026, Jeff King 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.