Re: [PATCH 1/1] Add preparing state to reference-transaction hook
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 13, 2026, 21:20 UTC
- Message-ID
- <xmqqpl572zq2.fsf@gitster.g>
- In-Reply-To
- <20260313193537.62827-2-eric.peijian@gmail.com>
eric.peijian@gmail.com writes:
> From: Eric Ju <eric.peijian@gmail.com> > > From: Eric Ju <eju@gitlab.com>
This is curious. The former matches the sign-off, but I somehow suspect that the @gitlab.com identity may be what you want to use for both of them, if this is a company sponsored work by an employee? I dunno.
Also the commit title deviates from the established "<area>: <what is done>" format.
Subject: [PATCH] refs: add 'preparing" phase to the transaction hook
or something?
Other than that, both the cover letter and the proposed log message very well explain the motivation behind the new feature. I wish everybody wrote their log messages as clearly as this one.
Show 7 quoted lines
> The "reference-transaction" hook is invoked multiple times during a ref > transaction. Each invocation corresponds to a different phase: > > - The "prepared" phase indicates that references have been locked. > - The "commit" phase indicates that all updates have been written to disk. > - The "abort" phase indicates that the transaction has been aborted and that > all changes have been rolled back.
"commit" -> "committed" and "abort" -> "aborted", if the existing documentation is to be trusted.
Show 11 quoted lines
> This hook can be used to learn about the updates that Git wants to perform. > For example, forges use it to coordinate reference updates across multiple > nodes. > > However, the phases are insufficient for some specific use cases. The earliest > observable phase in the "reference-transaction" hook is "prepared", at which > point Git has already taken exclusive locks on every affected reference. This > makes it suitable for last-chance validation, but not for serialization. So by > the time a hook sees the "prepared" phase, it has no way to defer locking, and > thus it cannot rearrange multiple concurrent ref transactions relative to one > another.
I cannot quite picture how "rearrangement" would happen, though.
Would the hook notice "ah there is a preparing hook invocation incoming", stall the caller by not immediately returning and instead wait for a different Git process to invoke the same ref-transaction hook "preparing" invocation, and somehow decide to let the latter go first before releasing the former?
Show 8 quoted lines
> Introduce a new "preparing" phase that runs before the "prepared" phase, that > is before Git acquires any reference lock on disk. This gives callers a > well-defined window to perform validation, enable higher-level ordering of > concurrent transactions, or reject the transaction entirely, all without > interfering with the locking state. > > This change is strictly speaking not backwards compatible. Existing hook > scripts that do not know to handle unknown phases handle the "preparing" state
"know to handle unknown phrases handle"?
Show 9 quoted lines
> string will encounter an unknown phase, and that might cause them to return an > error now. But the hook is considered to expose internal implementation details > of how Git works, and as such we have been a bit more lenient with changing its > exact semantics, like for example in a8ae923f85 (refs: support symrefs in > 'reference-transaction' hook, 2024-05-07). > > An alternative would be to introduce a "reference-transaction-v2" hook that > knows about the new phase. This feels like a rather heavy-weight option though, > and was thus discarded.
And documenting the design alternatives and decision like these two paragraphs is very much appreciated.
The insertion of a new hook invocation itself is at a very much expected place in the code path. Well written.
Will queue. Thanks.