From: Jiang Xin <worldhello.net@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
Patrick Steinhardt <ps@pks.im>,
Michael Heemskerk <mheemskerk@atlassian.com>,
Git List <git@vger.kernel.org>
Cc: Jiang Xin <zhiyou.jx@alibaba-inc.com>,
Jiang Xin <worldhello.net@gmail.com>
Subject: [PATCH v2 5/9] refs: avoid duplicate running of the reference-transaction hook
Date: Fri, 19 Aug 2022 11:21:43 +0800 [thread overview]
Message-ID: <20220819032147.28841-6-worldhello.net@gmail.com> (raw)
In-Reply-To: <CANYiYbFw71bX827akAG87RSKOozPk313Hoe573O9dQ65_U6sLQ@mail.gmail.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
If there are references to be deleted in a transaction, we should remove
each reference from both loose references and the "packed-refs" file.
The "reference-transaction" hook will run twice, once for the primary
ref-store (loose ref-store), and another for the second ref-store (i.e.
packed ref-store).
To avoid duplicate running of the "reference-trancaction" hook, we pass
a special "hook-flags" parameter to initialize the second ref-store.
The "REF_TRANSACTION_RUN_PREPARED_HOOK" bit is preserved for the
transaction of the second ref-store because we may still want to call
command "reference-trancaction prepared" to do some checks. E.g.: We
can create a global lock file (such as "site.lock") to disable writing
permissions for one or multiple repositories.
The behavior of the following git commands and five testcases have been
fixed in t1416:
* git cherry-pick
* git rebase
* git revert
* git update-ref -d <ref>
* git update-ref --stdin # delete refs
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
refs/files-backend.c | 7 +-
t/t1416-ref-transaction-hooks.sh | 156 ++-----------------------------
2 files changed, 15 insertions(+), 148 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f0947c9dca..9ec4c23a16 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2797,8 +2797,11 @@ static int files_transaction_prepare(struct ref_store *ref_store,
* packed-refs if it exists there.
*/
if (!packed_transaction) {
- packed_transaction = ref_store_transaction_begin(
- refs->packed_ref_store, err);
+ packed_transaction = ref_store_transaction_begin_extended(
+ refs->packed_ref_store,
+ transaction->hook_flags &
+ REF_TRANSACTION_RUN_PREPARED_HOOK,
+ err);
if (!packed_transaction) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index b7ffc9415b..6ba7ba746c 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -441,45 +441,7 @@ test_expect_success "update-ref: update refs already in packed_ref_store" '
test_cmp_heads_and_tags -C workdir expect
'
-# Mismatched hook output when deleting refs using "git update-refs -d":
-#
-# * The "reference-transaction committed" command was executed twice,
-# once for packed ref-store, and once for loose ref-store.
-#
-# * Unexpected execution of the "reference-transaction abort" command.
-#
-# The differences are as follows:
-#
-# @@ -4,6 +4,8 @@
-# <COMMIT-A> <ZERO-OID> refs/heads/topic1
-# <COMMIT-A> <ZERO-OID> HEAD
-# ## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
-# +## Call hook: reference-transaction committed ##
-# <COMMIT-A> <ZERO-OID> refs/heads/topic1
-# <COMMIT-A> <ZERO-OID> HEAD
-# ## Call hook: reference-transaction prepared ##
-# @@ -11,13 +13,19 @@
-# ## Call hook: reference-transaction prepared ##
-# <COMMIT-B> <ZERO-OID> refs/heads/topic2
-# ## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
-# +## Call hook: reference-transaction committed ##
-# <COMMIT-B> <ZERO-OID> refs/heads/topic2
-# ## Call hook: reference-transaction prepared ##
-# <ZERO-OID> <ZERO-OID> refs/heads/topic3
-# ## Call hook: reference-transaction prepared ##
-# <COMMIT-C> <ZERO-OID> refs/heads/topic3
-# ## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
-# +## Call hook: reference-transaction committed ##
-# <COMMIT-C> <ZERO-OID> refs/heads/topic3
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
-# ## Call hook: reference-transaction prepared ##
-# <COMMIT-C> <ZERO-OID> refs/heads/topic4
-# ## Call hook: reference-transaction committed ##
-test_expect_failure "update-ref: remove refs with mixed ref_stores" '
+test_expect_success "update-ref: remove refs with mixed ref_stores" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -604,27 +566,7 @@ test_expect_success "update-ref --stdin: update refs" '
test_cmp_heads_and_tags -C workdir expect
'
-# Mismatched hook output when deleting refs using "git update-refs
-# --stdin":
-#
-# * The "reference-transaction committed" command was executed twice,
-# once for packed ref-store, and once for loose ref-store.
-#
-# The differences are as follows:
-#
-# @@ -10,6 +10,11 @@
-# <COMMIT-C> <ZERO-OID> refs/heads/topic4
-# <COMMIT-A> <ZERO-OID> HEAD
-# ## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
-# +## Call hook: reference-transaction committed ##
-# <COMMIT-A> <ZERO-OID> refs/heads/topic1
-# <COMMIT-B> <ZERO-OID> refs/heads/topic2
-# <COMMIT-C> <ZERO-OID> refs/heads/topic3
-test_expect_failure "update-ref --stdin: delete refs" '
+test_expect_success "update-ref --stdin: delete refs" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -813,24 +755,16 @@ test_expect_failure "branch: copy branches" '
# * The "reference-transaction committed" command was not executed
# for the target branch.
#
-# * Unexpected execution of the "reference-transaction abort" command.
-#
# The differences are as follows:
#
-# @@ -1,16 +1,12 @@
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
-# ## Call hook: reference-transaction prepared ##
-# <COMMIT-B> <ZERO-OID> refs/heads/topic4
+# @@ -3,14 +3,6 @@
# ## Call hook: reference-transaction committed ##
# <COMMIT-B> <ZERO-OID> refs/heads/topic4
-# -## Call hook: reference-transaction prepared ##
+# ## Call hook: reference-transaction prepared ##
# -<ZERO-OID> <COMMIT-B> refs/heads/topic6
# -## Call hook: reference-transaction committed ##
# -<ZERO-OID> <COMMIT-B> refs/heads/topic6
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic5
-# ## Call hook: reference-transaction prepared ##
+# -## Call hook: reference-transaction prepared ##
# <COMMIT-C> <ZERO-OID> refs/heads/topic5
# ## Call hook: reference-transaction committed ##
# <COMMIT-C> <ZERO-OID> refs/heads/topic5
@@ -886,11 +820,9 @@ test_expect_failure "branch: rename branches" '
# and the "reference-transaction committed" command was executed
# redundantly on the packed-ref-store.
#
-# * Unexpected execution of the "reference-transaction abort" command.
-#
# The differences are as follows:
#
-# @@ -2,11 +2,25 @@
+# @@ -2,11 +2,19 @@
# <ZERO-OID> <ZERO-OID> refs/heads/topic1
# <ZERO-OID> <ZERO-OID> refs/heads/topic2
# <ZERO-OID> <ZERO-OID> refs/heads/topic3
@@ -898,14 +830,10 @@ test_expect_failure "branch: rename branches" '
# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
# +## Call hook: reference-transaction prepared ##
# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
# +## Call hook: reference-transaction committed ##
# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
# ## Call hook: reference-transaction prepared ##
# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
# <COMMIT-B> <ZERO-OID> refs/heads/topic2
@@ -913,8 +841,6 @@ test_expect_failure "branch: rename branches" '
# ## Call hook: reference-transaction committed ##
# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
# <COMMIT-B> <ZERO-OID> refs/heads/topic2
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
# +## Call hook: reference-transaction prepared ##
# +<COMMIT-C> <ZERO-OID> refs/heads/topic3
# +## Call hook: reference-transaction committed ##
@@ -1055,11 +981,9 @@ test_expect_success "tag: update refs to create loose refs" '
# and the "reference-transaction committed" command was executed
# redundantly on the packed-ref-store.
#
-# * Unexpected execution of the "reference-transaction abort" command.
-#
# The differences are as follows:
#
-# @@ -2,11 +2,25 @@
+# @@ -2,11 +2,19 @@
# <ZERO-OID> <ZERO-OID> refs/tags/v1
# <ZERO-OID> <ZERO-OID> refs/tags/v2
# <ZERO-OID> <ZERO-OID> refs/tags/v3
@@ -1067,14 +991,10 @@ test_expect_success "tag: update refs to create loose refs" '
# +<ZERO-OID> <ZERO-OID> refs/tags/v1
# +<ZERO-OID> <ZERO-OID> refs/tags/v2
# +<ZERO-OID> <ZERO-OID> refs/tags/v3
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v1
# +## Call hook: reference-transaction prepared ##
# +<ZERO-OID> <ZERO-OID> refs/tags/v1
# +## Call hook: reference-transaction committed ##
# +<ZERO-OID> <ZERO-OID> refs/tags/v1
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v2
# ## Call hook: reference-transaction prepared ##
# -<COMMIT-A> <ZERO-OID> refs/tags/v1
# <COMMIT-B> <ZERO-OID> refs/tags/v2
@@ -1082,8 +1002,6 @@ test_expect_success "tag: update refs to create loose refs" '
# ## Call hook: reference-transaction committed ##
# -<COMMIT-A> <ZERO-OID> refs/tags/v1
# <COMMIT-B> <ZERO-OID> refs/tags/v2
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v3
# +## Call hook: reference-transaction prepared ##
# +<COMMIT-C> <ZERO-OID> refs/tags/v3
# +## Call hook: reference-transaction committed ##
@@ -1232,22 +1150,7 @@ test_expect_success "worktree: topic2: merge" '
test_cmp_heads_and_tags -C workdir expect
'
-# Mismatched hook output for git-cherry-pick:
-#
-# * Unexpected execution of the "reference-transaction abort" command.
-#
-# The differences are as follows:
-#
-# @@ -12,6 +12,8 @@
-# ## Call hook: reference-transaction committed ##
-# <COMMIT-A> <COMMIT-F> HEAD
-# <COMMIT-A> <COMMIT-F> refs/heads/topic3
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
-# ## Call hook: reference-transaction prepared ##
-# <COMMIT-C> <ZERO-OID> CHERRY_PICK_HEAD
-# ## Call hook: reference-transaction committed ##
-test_expect_failure "worktree: topic3: cherry-pick" '
+test_expect_success "worktree: topic3: cherry-pick" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -1291,31 +1194,7 @@ test_expect_failure "worktree: topic3: cherry-pick" '
test_cmp_heads_and_tags -C workdir expect
'
-# Mismatched hook output for git-rebase:
-#
-# * Unexpected execution of the "reference-transaction abort" command.
-#
-# The differences are as follows:
-#
-# @@ -6,6 +6,8 @@
-# <COMMIT-G> <COMMIT-C> HEAD
-# ## Call hook: reference-transaction committed ##
-# <COMMIT-G> <COMMIT-C> HEAD
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> REBASE_HEAD
-# ## Call hook: reference-transaction prepared ##
-# <ZERO-OID> <ZERO-OID> REBASE_HEAD
-# ## Call hook: reference-transaction committed ##
-# @@ -18,6 +20,8 @@
-# <COMMIT-C> <COMMIT-H> HEAD
-# ## Call hook: reference-transaction committed ##
-# <COMMIT-C> <COMMIT-H> HEAD
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
-# ## Call hook: reference-transaction prepared ##
-# <COMMIT-G> <ZERO-OID> CHERRY_PICK_HEAD
-# ## Call hook: reference-transaction committed ##
-test_expect_failure "worktree: topic4: rebase" '
+test_expect_success "worktree: topic4: rebase" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -1367,22 +1246,7 @@ test_expect_failure "worktree: topic4: rebase" '
test_cmp_heads_and_tags -C workdir expect
'
-# Mismatched hook output for git-revert:
-#
-# * Unexpected execution of the "reference-transaction abort" command.
-#
-# The differences are as follows:
-#
-# @@ -8,6 +8,8 @@
-# ## Call hook: reference-transaction committed ##
-# <COMMIT-C> <COMMIT-I> HEAD
-# <COMMIT-C> <COMMIT-I> refs/heads/topic5
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
-# ## Call hook: reference-transaction prepared ##
-# <ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
-# ## Call hook: reference-transaction committed ##
-test_expect_failure "worktree: topic5: revert" '
+test_expect_success "worktree: topic5: revert" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
--
2.36.1.25.gc87d5ad63a.dirty
next prev parent reply other threads:[~2022-08-19 3:22 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-29 10:12 [PATCH 0/9] Fix issues of reference-transaction hook for various git commands Jiang Xin
2022-07-29 10:12 ` [PATCH 1/9] t1416: more testcases for reference-transaction hook Jiang Xin
2022-07-30 6:44 ` Eric Sunshine
2022-07-31 3:25 ` Jiang Xin
2022-07-29 10:12 ` [PATCH 2/9] refs: update missing old-oid in transaction from lockfile Jiang Xin
2022-07-29 10:12 ` [PATCH 3/9] refs: add new field in transaction for running transaction hook Jiang Xin
2022-07-29 10:12 ` [PATCH 4/9] refs: do not run transaction hook for git-pack-refs Jiang Xin
2022-07-29 10:12 ` [PATCH 5/9] refs: avoid duplicate running of the reference-transaction hook Jiang Xin
2022-08-02 12:18 ` Michael Heemskerk
2022-08-05 1:41 ` Jiang Xin
2022-08-19 3:21 ` [PATCH v2 0/9] Fix issues of refx-txn hook for various git commands Jiang Xin
2022-08-19 3:21 ` [PATCH v2 1/9] t1416: more testcases for reference-transaction hook Jiang Xin
2022-08-19 3:21 ` [PATCH v2 2/9] refs: update missing old-oid in transaction from lockfile Jiang Xin
2022-08-19 3:21 ` [PATCH v2 3/9] refs: add new field in transaction for running transaction hook Jiang Xin
2022-08-19 3:21 ` [PATCH v2 4/9] refs: do not run transaction hook for git-pack-refs Jiang Xin
2022-08-19 3:21 ` Jiang Xin [this message]
2022-08-19 3:21 ` [PATCH v2 6/9] refs: add reflog_info to hold more fields for reflog entry Jiang Xin
2022-08-19 3:21 ` [PATCH v2 7/9] refs: get error message via refs_update_ref_extended() Jiang Xin
2022-08-19 3:21 ` [PATCH v2 8/9] refs: reimplement files_copy_or_rename_ref() to run refs-txn hook Jiang Xin
2022-08-19 3:21 ` [PATCH v2 9/9] refs: reimplement refs_delete_refs() and run hook once Jiang Xin
2022-07-29 10:12 ` [PATCH 6/9] refs: add reflog_info to hold more fields for reflog entry Jiang Xin
2022-08-01 11:32 ` Jiang Xin
2022-07-29 10:12 ` [PATCH 7/9] refs: get error message via refs_update_ref_extended() Jiang Xin
2022-07-29 10:12 ` [PATCH 8/9] refs: reimplement files_copy_or_rename_ref() to run hook Jiang Xin
2022-07-29 10:12 ` [PATCH 9/9] refs: reimplement refs_delete_refs() and run hook once Jiang Xin
2022-08-02 12:42 ` Michael Heemskerk
2022-08-09 11:05 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220819032147.28841-6-worldhello.net@gmail.com \
--to=worldhello.net@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mheemskerk@atlassian.com \
--cc=ps@pks.im \
--cc=zhiyou.jx@alibaba-inc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.