Re: Client exit whilst running pre-receive hook : commit accepted but no post-receive hook ran
- From
Jeff King <peff@peff.net>
- Date
- Aug 3, 2016, 19:30 UTC
- Message-ID
- <20160803193018.ydhmxntikhyowmjz@sigill.intra.peff.net>
- In-Reply-To
- <a3f64a09-3094-eee1-0050-9960f0674036@nokia.com>
On Tue, Aug 02, 2016 at 12:01:57PM -0400, Stephen Morton wrote:
> Sounds tricky. I do think it's important, almost a 'data integrity' issue, > that IF a commit is received, THEN the post-receive hook must be run. Too > much mission-critical stuff is based on post-receive hooks.
I agree it would be a good property to have. I think it's hard to do atomically, though. Certainly we can wait to tell the other side "your push has been recorded" until after the hook is run. But we would already have updated the refs locally at that point (and we must -- that is part of the interface to the post-receive hooks, that the refs are already in place). So would we roll-back the ref update then? Even that suffers from power failures, etc.
So I'm not sure if making it truly atomic is all the feasible. However, we could certainly make things more robust than they are now.
The simplest thing may be to just bump the post-receive hook before the status report. That opens up the question of whether clients are actually waiting already for the post-receive to finish. Looking at the code in send-pack, it looks like the network is hooked up to the sideband demuxer thread, which will read until EOF on the network. So we are waiting either way for the post-receive to run. It doesn't really matter if it happens before or after the report to the client.
So I _think_ something like this would work:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 15c323a..91d01f0 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1767,9 +1767,9 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix) execute_commands(commands, unpack_status, &si); if (pack_lockfile) unlink_or_warn(pack_lockfile); + run_receive_hook(commands, "post-receive", 1); if (report_status) report(commands, unpack_status); - run_receive_hook(commands, "post-receive", 1); run_update_post_hook(commands); if (auto_gc) { const char *argv_gc_auto[] = { but maybe there are other gotchas. -Peff