Skip to content

Commit d92dd32

Browse files
peijianjugitster
authored andcommitted
refs: add 'preparing' phase to the reference-transaction hook
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 "committed" phase indicates that all updates have been written to disk. - The "aborted" phase indicates that the transaction has been aborted and that all changes have been rolled back. 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. 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 how to handle unknown phases may treat 'preparing' as an error and return non-zero. 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 a8ae923 (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. Helped-by: Patrick Steinhardt <ps@pks.im> Helped-by: Justin Tobler <jltobler@gmail.com> Helped-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 7f19e4e commit d92dd32

4 files changed

Lines changed: 52 additions & 13 deletions

File tree

Documentation/githooks.adoc

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -484,13 +484,16 @@ reference-transaction
484484
~~~~~~~~~~~~~~~~~~~~~
485485
486486
This hook is invoked by any Git command that performs reference
487-
updates. It executes whenever a reference transaction is prepared,
488-
committed or aborted and may thus get called multiple times. The hook
489-
also supports symbolic reference updates.
487+
updates. It executes whenever a reference transaction is preparing,
488+
prepared, committed or aborted and may thus get called multiple times.
489+
The hook also supports symbolic reference updates.
490490
491491
The hook takes exactly one argument, which is the current state the
492492
given reference transaction is in:
493493
494+
- "preparing": All reference updates have been queued to the
495+
transaction but references are not yet locked on disk.
496+
494497
- "prepared": All reference updates have been queued to the
495498
transaction and references were locked on disk.
496499
@@ -511,16 +514,18 @@ ref and `<ref-name>` is the full name of the ref. When force updating
511514
the reference regardless of its current value or when the reference is
512515
to be created anew, `<old-value>` is the all-zeroes object name. To
513516
distinguish these cases, you can inspect the current value of
514-
`<ref-name>` via `git rev-parse`.
517+
`<ref-name>` via `git rev-parse`. During the "preparing" state, symbolic
518+
references are not resolved: `<ref-name>` will reflect the symbolic reference
519+
itself rather than the object it points to.
515520
516521
For symbolic reference updates the `<old_value>` and `<new-value>`
517522
fields could denote references instead of objects. A reference will be
518523
denoted with a 'ref:' prefix, like `ref:<ref-target>`.
519524
520525
The exit status of the hook is ignored for any state except for the
521-
"prepared" state. In the "prepared" state, a non-zero exit status will
522-
cause the transaction to be aborted. The hook will not be called with
523-
"aborted" state in that case.
526+
"preparing" and "prepared" states. In these states, a non-zero exit
527+
status will cause the transaction to be aborted. The hook will not be
528+
called with "aborted" state in that case.
524529
525530
push-to-checkout
526531
~~~~~~~~~~~~~~~~

refs.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2655,14 +2655,21 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
26552655
if (ref_update_reject_duplicates(&transaction->refnames, err))
26562656
return REF_TRANSACTION_ERROR_GENERIC;
26572657

2658+
/* Preparing checks before locking references */
2659+
ret = run_transaction_hook(transaction, "preparing");
2660+
if (ret) {
2661+
ref_transaction_abort(transaction, err);
2662+
die(_("ref updates aborted by the reference-transaction hook at its %s state"), "preparing");
2663+
}
2664+
26582665
ret = refs->be->transaction_prepare(refs, transaction, err);
26592666
if (ret)
26602667
return ret;
26612668

26622669
ret = run_transaction_hook(transaction, "prepared");
26632670
if (ret) {
26642671
ref_transaction_abort(transaction, err);
2665-
die(_("ref updates aborted by hook"));
2672+
die(_("ref updates aborted by the reference-transaction hook at its %s state"), "prepared");
26662673
}
26672674

26682675
return 0;

t/t1416-ref-transaction-hooks.sh

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,26 @@ test_expect_success 'hook allows updating ref if successful' '
2020
echo "$*" >>actual
2121
EOF
2222
cat >expect <<-EOF &&
23+
preparing
2324
prepared
2425
committed
2526
EOF
2627
git update-ref HEAD POST &&
2728
test_cmp expect actual
2829
'
2930

31+
test_expect_success 'hook aborts updating ref in preparing state' '
32+
git reset --hard PRE &&
33+
test_hook reference-transaction <<-\EOF &&
34+
if test "$1" = preparing
35+
then
36+
exit 1
37+
fi
38+
EOF
39+
test_must_fail git update-ref HEAD POST 2>err &&
40+
test_grep "ref updates aborted by the reference-transaction hook at its preparing state" err
41+
'
42+
3043
test_expect_success 'hook aborts updating ref in prepared state' '
3144
git reset --hard PRE &&
3245
test_hook reference-transaction <<-\EOF &&
@@ -36,7 +49,7 @@ test_expect_success 'hook aborts updating ref in prepared state' '
3649
fi
3750
EOF
3851
test_must_fail git update-ref HEAD POST 2>err &&
39-
test_grep "ref updates aborted by hook" err
52+
test_grep "ref updates aborted by the reference-transaction hook at its prepared state" err
4053
'
4154

4255
test_expect_success 'hook gets all queued updates in prepared state' '
@@ -121,6 +134,7 @@ test_expect_success 'interleaving hook calls succeed' '
121134
cat >expect <<-EOF &&
122135
hooks/update refs/tags/PRE $ZERO_OID $PRE_OID
123136
hooks/update refs/tags/POST $ZERO_OID $POST_OID
137+
hooks/reference-transaction preparing
124138
hooks/reference-transaction prepared
125139
hooks/reference-transaction committed
126140
EOF
@@ -143,6 +157,8 @@ test_expect_success 'hook captures git-symbolic-ref updates' '
143157
git symbolic-ref refs/heads/symref refs/heads/main &&
144158
145159
cat >expect <<-EOF &&
160+
preparing
161+
$ZERO_OID ref:refs/heads/main refs/heads/symref
146162
prepared
147163
$ZERO_OID ref:refs/heads/main refs/heads/symref
148164
committed
@@ -171,14 +187,20 @@ test_expect_success 'hook gets all queued symref updates' '
171187
# In the files backend, "delete" also triggers an additional transaction
172188
# update on the packed-refs backend, which constitutes additional reflog
173189
# entries.
190+
cat >expect <<-EOF &&
191+
preparing
192+
ref:refs/heads/main $ZERO_OID refs/heads/symref
193+
ref:refs/heads/main $ZERO_OID refs/heads/symrefd
194+
$ZERO_OID ref:refs/heads/main refs/heads/symrefc
195+
ref:refs/heads/main ref:refs/heads/branch refs/heads/symrefu
196+
EOF
197+
174198
if test_have_prereq REFFILES
175199
then
176-
cat >expect <<-EOF
200+
cat >>expect <<-EOF
177201
aborted
178202
$ZERO_OID $ZERO_OID refs/heads/symrefd
179203
EOF
180-
else
181-
>expect
182204
fi &&
183205
184206
cat >>expect <<-EOF &&

t/t5510-fetch.sh

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,12 +469,17 @@ test_expect_success 'fetch --atomic executes a single reference transaction only
469469
head_oid=$(git rev-parse HEAD) &&
470470
471471
cat >expected <<-EOF &&
472+
preparing
473+
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
474+
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
472475
prepared
473476
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
474477
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
475478
committed
476479
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
477480
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
481+
preparing
482+
$ZERO_OID ref:refs/remotes/origin/main refs/remotes/origin/HEAD
478483
EOF
479484
480485
rm -f atomic/actual &&
@@ -497,7 +502,7 @@ test_expect_success 'fetch --atomic aborts all reference updates if hook aborts'
497502
head_oid=$(git rev-parse HEAD) &&
498503
499504
cat >expected <<-EOF &&
500-
prepared
505+
preparing
501506
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-1
502507
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-2
503508
$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-3

0 commit comments

Comments
 (0)