Jeff King <peff@xxxxxxxx> writes:
> 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.
As long as the requirement is that post- hook must see the updated
ref in place, I do not think it is feasible to give "the hook always
runs once" guarantee, without cooperation by other parts of the flow
(think: pulling the power at an arbitrary point in the process).
A receiving repository can implement it all in the userland, I would
think, though:
* A pre-receive hook records the intention to update a ref (from
what old commit to what new commit), and does not return until
that record is securely in a database;
* A post-receive hook checks the entry in the database above (it
_must_ find one), and atomically does its thing and marks the
entry "done";
* A separate sweeper scans the database for entries not yet marked
as "done", sees if the ref has been already updated, and
atomically does its thing and marks the entry "done" (the same
can be done as part of a post-receive for previously pushed
commit that pre-receive recorded but did not manage to run
post-receive before the power was pulled or the user did \C-c).
As you originally described, the non-atomicity is not new; as long
as we have necessary hooks in place on the git-core side for
repositories that want a stronger guarantee, I do not think there is
any more thing we need to do on this topic. If we can narrow the
window in a non-intrusive way, that would be a good thing to do,
though.
> However,
> we could certainly make things more robust than they are now.
And this change may to the "narrowing the window in a non-intrusive
way" (I wonder if we also need to lift the post-update hook the same
way, though).
But that would still not guarantee "the hook always runs once".
What we have is "the hook runs at most once".
Thanks.
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html