Merge branch 'ej/ref-transaction-hook-preparing' into seen

The reference-transaction hook was taught to be triggered before
taking locks on references in the "preparing" phase.

Comments?

* ej/ref-transaction-hook-preparing:
  refs: add 'preparing' phase to the reference-transaction hook

Author: gitster

Documentation/githooks.adoc

@@ -484,13 +484,16 @@ reference-transaction
 ~~~~~~~~~~~~~~~~~~~~~
 
 This hook is invoked by any Git command that performs reference
-updates. It executes whenever a reference transaction is prepared,
-committed or aborted and may thus get called multiple times. The hook
-also supports symbolic reference updates.
+updates. It executes whenever a reference transaction is preparing,
+prepared, committed or aborted and may thus get called multiple times.
+The hook also supports symbolic reference updates.
 
 The hook takes exactly one argument, which is the current state the
 given reference transaction is in:
 
+    - "preparing": All reference updates have been queued to the
+      transaction but references are not yet locked on disk.
+
     - "prepared": All reference updates have been queued to the
       transaction and references were locked on disk.
 
@@ -511,16 +514,18 @@ ref and `<ref-name>` is the full name of the ref. When force updating
 the reference regardless of its current value or when the reference is
 to be created anew, `<old-value>` is the all-zeroes object name. To
 distinguish these cases, you can inspect the current value of
-`<ref-name>` via `git rev-parse`.
+`<ref-name>` via `git rev-parse`. During the "preparing" state, symbolic
+references are not resolved: `<ref-name>` will reflect the symbolic reference
+itself rather than the object it points to.
 
 For symbolic reference updates the `<old_value>` and `<new-value>`
 fields could denote references instead of objects. A reference will be
 denoted with a 'ref:' prefix, like `ref:<ref-target>`.
 
 The exit status of the hook is ignored for any state except for the
-"prepared" state. In the "prepared" state, a non-zero exit status will
-cause the transaction to be aborted. The hook will not be called with
-"aborted" state in that case.
+"preparing" and "prepared" states. In these states, a non-zero exit
+status will cause the transaction to be aborted. The hook will not be
+called with "aborted" state in that case.
 
 push-to-checkout
 ~~~~~~~~~~~~~~~~

refs.c

@@ -2656,14 +2656,21 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
 	if (ref_update_reject_duplicates(&transaction->refnames, err))
 		return REF_TRANSACTION_ERROR_GENERIC;
 
+	/* Preparing checks before locking references */
+	ret = run_transaction_hook(transaction, "preparing");
+	if (ret) {
+		ref_transaction_abort(transaction, err);
+		die(_("ref updates aborted by the reference-transaction hook at its %s state"), "preparing");
+	}
+
 	ret = refs->be->transaction_prepare(refs, transaction, err);
 	if (ret)
 		return ret;
 
 	ret = run_transaction_hook(transaction, "prepared");
 	if (ret) {
 		ref_transaction_abort(transaction, err);
-		die(_("ref updates aborted by hook"));
+		die(_("ref updates aborted by the reference-transaction hook at its %s state"), "prepared");
 	}
 
 	return 0;

t/t1416-ref-transaction-hooks.sh

@@ -20,13 +20,26 @@ test_expect_success 'hook allows updating ref if successful' '
 		echo "$*" >>actual
 	EOF
 	cat >expect <<-EOF &&
+		preparing
 		prepared
 		committed
 	EOF
 	git update-ref HEAD POST &&
 	test_cmp expect actual
 '
 
+test_expect_success 'hook aborts updating ref in preparing state' '
+	git reset --hard PRE &&
+	test_hook reference-transaction <<-\EOF &&
+		if test "$1" = preparing
+		then
+			exit 1
+		fi
+	EOF
+	test_must_fail git update-ref HEAD POST 2>err &&
+	test_grep "ref updates aborted by the reference-transaction hook at its preparing state" err
+'
+
 test_expect_success 'hook aborts updating ref in prepared state' '
 	git reset --hard PRE &&
 	test_hook reference-transaction <<-\EOF &&
@@ -36,7 +49,7 @@ test_expect_success 'hook aborts updating ref in prepared state' '
 		fi
 	EOF
 	test_must_fail git update-ref HEAD POST 2>err &&
-	test_grep "ref updates aborted by hook" err
+	test_grep "ref updates aborted by the reference-transaction hook at its prepared state" err
 '
 
 test_expect_success 'hook gets all queued updates in prepared state' '
@@ -121,6 +134,7 @@ test_expect_success 'interleaving hook calls succeed' '
 	cat >expect <<-EOF &&
 		hooks/update refs/tags/PRE $ZERO_OID $PRE_OID
 		hooks/update refs/tags/POST $ZERO_OID $POST_OID
+		hooks/reference-transaction preparing
 		hooks/reference-transaction prepared
 		hooks/reference-transaction committed
 	EOF
@@ -143,6 +157,8 @@ test_expect_success 'hook captures git-symbolic-ref updates' '
 	git symbolic-ref refs/heads/symref refs/heads/main &&
 
 	cat >expect <<-EOF &&
+	preparing
+	$ZERO_OID ref:refs/heads/main refs/heads/symref
 	prepared
 	$ZERO_OID ref:refs/heads/main refs/heads/symref
 	committed
@@ -171,14 +187,20 @@ test_expect_success 'hook gets all queued symref updates' '
 	# In the files backend, "delete" also triggers an additional transaction
 	# update on the packed-refs backend, which constitutes additional reflog
 	# entries.
+	cat >expect <<-EOF &&
+	preparing
+	ref:refs/heads/main $ZERO_OID refs/heads/symref
+	ref:refs/heads/main $ZERO_OID refs/heads/symrefd
+	$ZERO_OID ref:refs/heads/main refs/heads/symrefc
+	ref:refs/heads/main ref:refs/heads/branch refs/heads/symrefu
+	EOF
+
 	if test_have_prereq REFFILES
 	then
-		cat >expect <<-EOF
+		cat >>expect <<-EOF
 		aborted
 		$ZERO_OID $ZERO_OID refs/heads/symrefd
 		EOF
-	else
-		>expect
 	fi &&
 
 	cat >>expect <<-EOF &&

t/t5510-fetch.sh

@@ -469,12 +469,17 @@ test_expect_success 'fetch --atomic executes a single reference transaction only
 	head_oid=$(git rev-parse HEAD) &&
 
 	cat >expected <<-EOF &&
+		preparing
+		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
+		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
 		prepared
 		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
 		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
 		committed
 		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
 		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
+		preparing
+		$ZERO_OID ref:refs/remotes/origin/main refs/remotes/origin/HEAD
 	EOF
 
 	rm -f atomic/actual &&
@@ -497,7 +502,7 @@ test_expect_success 'fetch --atomic aborts all reference updates if hook aborts'
 	head_oid=$(git rev-parse HEAD) &&
 
 	cat >expected <<-EOF &&
-		prepared
+		preparing
 		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-1
 		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-2
 		$ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-3