* [PATCH v2 0/9] Fix issues of refx-txn hook for various git commands
2022-08-05 1:41 ` Jiang Xin
@ 2022-08-19 3:21 ` Jiang Xin
2022-08-19 3:21 ` [PATCH v2 1/9] t1416: more testcases for reference-transaction hook Jiang Xin
` (8 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Before Patrick introduced the reference-transaction hook in git 2.28,
We had implemented some internal transaction hooks "pre-txn" and
"post-txn" for our internal git infrastructure. The "pre-txn" hook is
used to check our site-wide lockfile to disable write services for
one or multiple repositories. The "post-txn" hook is used to update
the timestamp and checksum of the repository. Recently I wanted to
migrate our internal implementation to use ref-txn hooks, but I ran
into the same issues reported by Michael and Bryan:
* https://lore.kernel.org/git/CAGyf7-GYYA2DOnAVYW--=QEc2WjSHzUhp2OQyuyOr3EOtFDm6g@mail.gmail.com/
* https://lore.kernel.org/git/pull.1228.git.1651676435634.gitgitgadget@gmail.com/
This patch series try to fix the issues I encountered.
Patch 1/9 adds more testcases in t1416:
* Some commands failed because the expected "<old-oid>" was
"<zero-oid>". Such as: "git update-ref -d <ref>".
* Some commands failed because the "reference-transaction committed"
command was repeated multiple times for the same changes. Such as:
"git update-ref -d <ref>" and "git tag -d <tag>".
* Some commands should not trigger the "reference-transaction" hook
because no real changes have occurred to the repository. Such as:
"git pack-refs --all" and "git gc".
* Some commands did not execute the "reference-transaction" hook at
all. E.g.: "git branch -c <src> <dest>", "git branch -m <old> <new>".
Patch 2/9 propagates real old-oid found in lockfile to the update
entries for transaction, so we can get proper old-oid in "prepared" and
"committed" stage for refs-txn hook.
Patch 3/9 adds a new flag in transaction, and we can turn off running
the entire hook for git-pack-refs in patch 4/9. We can also turn off the
"committed" and "aborted" stages of the ref-txn hook for
"packed-ref-store", but we can still run "prepared" stage of the hook.
See patch 5/9.
Patch 6/9 and 7/9 create an extended function
"ref_transaction_update_extended()"
to be used in patch 8/9 to reimplement branch copy and rename.
Patch 9/9 reimplents "files_delete_refs()" to fix failed testcases for
"git branch -d" and "git tag -d".
## Commit log changed since v1:
1: bd0ceab4a2 ! 1: 2860af63ee t1416: more testcases for reference-transaction hook
@@ Commit message
* git tag <new-tag> <oid> # create new tag
* git update-ref --stdin # create new refs
* git update-ref <ref> <oid> # create new ref
+ * git update-ref <ref> <new-oid> <old-oid> # update ref
- But 16 testcases failed.
+ But 17 testcases failed.
Some commands failed because the expected "<old-oid>" became
"<zero-oid>". E.g.:
@@ Commit message
* git tag -d <tag>
* git update-ref --stdin # update/delete refs
* git update-ref -d <ref>
- * git update-ref <ref> <new-oid> [<old-oid>] # update ref
+ * git update-ref <ref> <new-oid> # update ref
Some commands failed because the "reference-transaction committed"
command was repeated multiple times for the same changes. E.g.:
@@ Commit message
We will fix the failed testcases in later commits.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
+ Helped-by: Eric Sunshine <sunshine@sunshineco.com>
2: 915ff63007 ! 2: 1dbd4251aa refs: update missing old-oid in transaction from lockfile
@@ Commit message
in any case, get proper old_oid from the lock file and propagate it to
the corresponding update entry of a transaction.
- The behavior of the following git commands and four testcases have been
+ The behavior of the following git commands and five testcases have been
fixed in t1416:
- * git branch [-f] <ref> <new-oid> # update branch
+ * git branch [-f] <ref> <new-oid> # update branch
* git cherry-pick <oid>
* git rebase
* git tag -d <tag>
- * git update-ref --stdin # update refs
+ * git update-ref --stdin # update refs
* git update-ref -d <ref>
- * git update-ref <ref> <new-oid> [<old-oid>] # update ref
+ * git update-ref <ref> <new-oid> # update ref
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
3: cf7be214c6 ! 3: 58658c782d refs: add new field in transaction for running transaction hook
@@ Commit message
and aborted. To update a reference, git may create two seperate
transactions, one for loose reference and one for packed ref-store. This
may cause duplicate running of the hook for same references. The new
- field "hook_flags" in the transaction can turn off running of a specific
+ field "hook_flags" in the transaction can turn off running a specific
transaction. In some scenarios, we may only want to turn off certain
states of a transaction, such as "committed" and "aborted", but want to
turn on the "prepared" state of the hook to do some pre-checks, so the
@@ Commit message
By calling the "ref_store_transaction_begin()" function, all the flags
of the "hook_flags" field for the new initialized transaction will be
turned on. The new function "ref_store_transaction_begin_extended()"
- will be used in later commits to custom the "hook_flags" field for a
- new initialized transaction.
+ will be used in later commits with a specific "hook_flags" field to
+ initialize a new transaction.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
4: 34f6a5803a ! 4: ec0c6a1aed refs: do not run transaction hook for git-pack-refs
@@ Metadata
## Commit message ##
refs: do not run transaction hook for git-pack-refs
- git-pack-refs will call "files_pack_refs()" to pack loose references
- into the "packed-refs" file, and there are no real changes to the
- repository. Therefore, by initializing a transaction with an empty
- "hook_flags" field, the "reference-transaction" hook will not run.
+ The git-pack-refs command will call "files_pack_refs()" to pack loose
+ references into the "packed-refs" file, and there are no real changes
+ to the repository. Therefore, by initializing a transaction with an
+ empty "hook_flags" field, the "reference-transaction" hook will not run.
The "prune_refs()" and "prune_ref()" functions are called from
"file_pack_refs()", and they are used to prune loose refereces which
5: 649a5734c3 ! 5: d04425af29 refs: avoid duplicate running of the reference-transaction hook
@@ Commit message
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 references), and another for the second ref-store (i.e.
+ 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" for some pre-checks, such as
- terminate unwanted transaction for the "packed-refs" file.
+ 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:
6: 9c5ae8e592 ! 6: 7443764f6f refs: add reflog_info to hold more fields for reflog entry
@@ Commit message
a new structure "reflog_info" to hold more custom fields for the new
reflog entry, and add two additional extended version functions.
- We will use this extension in a later commit to reimplement
- "files_copy_or_rename_ref()" using "refs_update_ref_extended()" to
- create new reference in a transaction and add proper reflog entry.
+ We will use this extension in a later commit to reimplement the function
+ "files_copy_or_rename_ref()" by calling "refs_update_ref_extended()" to
+ create a new reference in a transaction, while adding proper reflog
+ entry.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
7: 602deefb6d ! 7: 0347885acc refs: get error message via refs_update_ref_extended()
@@ Metadata
## Commit message ##
refs: get error message via refs_update_ref_extended()
- The last parameter for "refs_update_ref_extended()" is an enum
- "action_on_err", and we can not use this function to get the specific
- error message. Extend this function again to get error message.
+ The last parameter of the function "refs_update_ref()" is an enum
+ "action_on_err", and we can not use this function to get the error
+ message. Extend this function to get error message.
We will use the function "refs_update_ref_extended()" to reimplement
the function "files_copy_or_rename_ref()" in later commit.
8: 16f3c34db1 ! 8: 3c41c0ac8d refs: reimplement files_copy_or_rename_ref() to run hook
@@ Metadata
Author: Jiang Xin <zhiyou.jx@alibaba-inc.com>
## Commit message ##
- refs: reimplement files_copy_or_rename_ref() to run hook
+ refs: reimplement files_copy_or_rename_ref() to run refs-txn hook
When copying or renaming a branch, the "reference-transaction" hook is
- not executed. This is because we used to reinvent the wheel in function
- "files_copy_or_rename_ref()". By calling "refs_update_ref_extended()" to
- reimplement "files_copy_or_rename_ref()", the "reference-transaction"
- hook will run correctly.
+ not executed. This is because we called two low-level functions
+ "lock_ref_oid_basic()" and "write_ref_to_lockfile()", and reinvented
+ the wheel in "commit_ref_update()" to update the reference instead of
+ implementing "files_copy_or_rename_ref()" by calling "refs_update_ref()"
+ to update a reference in a transaction. The reason for this is that we
+ want to create a proper reflog for the newly copied reference.
+
+ Refactor "files_copy_or_rename_ref()" by calling the extended version
+ of "refs_update_ref", i.e. "refs_update_ref_extended()", so we can
+ create the target branch for copying or renaming a branch and generate
+ a correct reflog file at the same time.
The behavior of the following git commands and two testcases have been
fixed in t1416:
9: c206059145 ! 9: f201436741 refs: reimplement refs_delete_refs() and run hook once
@@ Metadata
## Commit message ##
refs: reimplement refs_delete_refs() and run hook once
- When delete references using "git branch -d" or "git tag -d", there will
- be duplicate call of "reference-transaction committed" for same refs.
- This is because "refs_delete_refs()" is called twice, once for
- files-backend and once for packed-backend, and we used to reinvented the
- wheel in "files_delete_refs()" and "packed_delete_refs()". By removing
- "packed_delete_refs()" and reimplement "files_delete_refs()", the
- "reference-transaction" hook will run only once for deleted branches and
- tags.
+ When using "git branch -d" or "git tag -d" to delete one or multiple
+ references, command "reference-transaction committed" will be called
+ repeatedly for the same references. This is because the function
+ "refs_delete_refs()" is called twice, once for loose ref-store and once
+ for packed ref-store.
+
+ The old story starts when running the function "refs_delete_refs()" on a
+ loose ref-store:
+
+ 1. Try to remove the references from packed ref-store.
+
+ 1.1. Lock the packed-ref-store by calling "packed_refs_lock()" in
+ "files_delete_refs()".
+
+ 1.2. Call "refs_delete_refs()" on packed-ref-store, and then call
+ "packed_delete_refs()".
+
+ 1.3. Create a transaction for packed-ref-store in function
+ "packed_delete_refs()" by calling the function
+ "ref_store_transaction_begin()".
+
+ 2.2. Add update entries for all the references to be removed into
+ this transaction by calling "ref_transaction_delete()".
+
+ 2.3. Call "ref_transaction_commit()" to commit the transaction.
+
+ 2.4. Unlock the packed-ref-store.
+
+ 2. Try to remove the references one by one by calling the function
+ "refs_delete_ref()".
+
+ 2.1. Create a new transaction on loose-ref-store by calling
+ "ref_store_transaction_begin()".
+
+ 2.2. Call "ref_transaction_delete()" to add a update entry
+ for the reference to be deleted into the transaction.
+
+ 2.3. In "ref_transaction_commit()", it will call functions
+ "files_transaction_prepare()" and "files_transaction_finish()"
+ to commit the transaction.
+
+ 2.3.1. Lock the loose reference.
+
+ 2.3.2. Create a new packed-transaction, and add a new update
+ entry to this packed-transaction. The previous step 1
+ makes this operation unnecessary.
+
+ 2.3.3. Lock the packed-ref-store and call fucntion
+ "is_packed_transaction_needed()" to check whether it
+ is necessary to commit the transaction, and then
+ abort the transaction because the reference is already
+ removed from the packed-ref-store in step 1.
+
+ 2.3.4. Remove the reflog and the loose reference file for
+ the reference to be deleted.
+
+ 2.3.4. Unlock the loose reference.
+
+ From the above steps, we can see that "refs_delete_refs()" is not an
+ atomic operation, but a semi-atomic operation. The operation is atomic
+ if all references to be deleted are in the packed ref-store, but not
+ if some references are loose references because we delete the loose
+ references one by one by calling "refs_delete_ref()" .
+
+ Refactored function "files_delete_refs()" to delete references within a
+ transaction, so the "reference-transaction" hook will only run once for
+ deleted branches and tags.
The behavior of the following git commands and the last two testcases
have been fixed in t1416:
## Source code changed since v1:
2: 915ff63007 ! 2: 1dbd4251aa refs: update missing old-oid in transaction from lockfile
## refs/files-backend.c ##
@@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *refs,
- goto out;
- }
+ update->backend_data = lock;
+ if (update->type & REF_ISSYMREF) {
++ const char *ref;
++
+ /*
-+ * Propagate old_oid from the lock to the update entry, so we can
-+ * provide a real old-oid of to the "reference-transaction" hook.
++ * Read the referent even though we won't use it as part
++ * of the transaction, because we want to set a proper
++ * old_oid for this symref using the oid we got.
+ */
-+ if (!(update->flags & REF_HAVE_OLD)) {
-+ oidcpy(&update->old_oid, &lock->old_oid);
-+ update->flags |= REF_HAVE_OLD;
-+ }
++ ref = refs_resolve_ref_unsafe(&refs->base,
++ referent.buf, 0,
++ &lock->old_oid, NULL);
++
+ if (update->flags & REF_NO_DEREF) {
+ /*
+ * We won't be reading the referent as part of
+- * the transaction, so we have to read it here
+- * to record and possibly check old_oid:
++ * the transaction, so we may need to check
++ * old_oid here:
+ */
+- if (!refs_resolve_ref_unsafe(&refs->base,
+- referent.buf, 0,
+- &lock->old_oid, NULL)) {
++ if (!ref) {
+ if (update->flags & REF_HAVE_OLD) {
+ strbuf_addf(err, "cannot lock ref '%s': "
+ "error reading reference",
+@@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *refs,
+ }
+ }
+
++ /*
++ * Propagate old_oid from the lock to the update entry, so we can
++ * provide a proper old-oid of to the "reference-transaction" hook.
++ */
++ if (!(update->flags & REF_HAVE_OLD) && !is_null_oid(&lock->old_oid)) {
++ oidcpy(&update->old_oid, &lock->old_oid);
++ update->flags |= REF_HAVE_OLD;
++ }
+
- /*
- * If this update is happening indirectly because of a
- * symref update, record the old OID in the parent
+ if ((update->flags & REF_HAVE_NEW) &&
+ !(update->flags & REF_DELETING) &&
+ !(update->flags & REF_LOG_ONLY)) {
6: 9c5ae8e592 ! 6: 7443764f6f refs: add reflog_info to hold more fields for reflog entry
@@ refs.c: struct ref_update *ref_transaction_add_update(
oidcpy(&update->old_oid, old_oid);
- update->msg = normalize_reflog_message(msg);
+ if (reflog_info) {
-+ update->reflog_info = xmalloc(sizeof(*reflog_info));
++ update->reflog_info = xcalloc(1, sizeof(*reflog_info));
+ update->reflog_info->msg = normalize_reflog_message(reflog_info->msg);
+ if (reflog_info->old_oid)
+ update->reflog_info->old_oid = oiddup(reflog_info->old_oid);
## Testcases changed since v1 are not list here
--
Jiang Xin (9):
t1416: more testcases for reference-transaction hook
refs: update missing old-oid in transaction from lockfile
refs: add new field in transaction for running transaction hook
refs: do not run transaction hook for git-pack-refs
refs: avoid duplicate running of the reference-transaction hook
refs: add reflog_info to hold more fields for reflog entry
refs: get error message via refs_update_ref_extended()
refs: reimplement files_copy_or_rename_ref() to run refs-txn hook
refs: reimplement refs_delete_refs() and run hook once
refs.c | 99 ++-
refs.h | 23 +
refs/debug.c | 2 +-
refs/files-backend.c | 177 ++---
refs/packed-backend.c | 51 +-
refs/refs-internal.h | 25 +-
t/t1416-ref-transaction-hooks.sh | 1115 +++++++++++++++++++++++++++++-
t/t5510-fetch.sh | 16 +
8 files changed, 1318 insertions(+), 190 deletions(-)
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 1/9] t1416: more testcases for reference-transaction hook
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 ` Jiang Xin
2022-08-19 3:21 ` [PATCH v2 2/9] refs: update missing old-oid in transaction from lockfile Jiang Xin
` (7 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin, Eric Sunshine
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Append more testcases in t1416 for various git commands that may trigger
the "reference-transaction" hook.
In order to create a common "reference-transaction" hook, we define the
common hook path using config variable "core.hooksPath", and create the
"reference-transaction" hook in the path.
Some commands trigger the "reference-transaction" hook properly and get
the expected output. E.g.:
* git branch <new-branch> <oid> # create new branch
* git clone
* git commit [--amend]
* git fetch
* git merge
* git push [--atomic]
* git reset --hard
* git tag [-f] <tag> <oid> # update tag
* git tag <new-tag> <oid> # create new tag
* git update-ref --stdin # create new refs
* git update-ref <ref> <oid> # create new ref
* git update-ref <ref> <new-oid> <old-oid> # update ref
But 17 testcases failed.
Some commands failed because the expected "<old-oid>" became
"<zero-oid>". E.g.:
* git branch [-f] <ref> <new-oid> # update branch
* git cherry-pick <oid>
* git rebase
* git tag -d <tag>
* git update-ref --stdin # update/delete refs
* git update-ref -d <ref>
* git update-ref <ref> <new-oid> # update ref
Some commands failed because the "reference-transaction committed"
command was repeated multiple times for the same changes. E.g.:
* git cherry-pick
* git rebase
* git revert
* git tag -d <tag>
* git update-ref -d <ref>
* git update-ref --stdin # delete refs
Some commands should not trigger the "reference-transaction" hook
because no real changes have occurred to the repository. E.g.:
* git gc
* git pack-refs --all
Some commands did not execute the "reference-transaction" hook at all.
E.g.:
* git branch -c <src> <dest> # copy branch
* git branch -m <old> <new> # rename branch
Some commands ran unexpected command "reference-transaction aborted".
E.g.:
* git branch -d <branch> # delete branch
* git branch -m <old> <new> # rename branch
* git cherr-pick <oid>
* git rebase
* git revert
* git tag -d <tag> # delete tag
* git update-ref -d <ref> # delete ref
We will fix the failed testcases in later commits.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
---
t/t1416-ref-transaction-hooks.sh | 1484 ++++++++++++++++++++++++++++++
1 file changed, 1484 insertions(+)
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 27731722a5..84509cb6a4 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -133,4 +133,1488 @@ test_expect_success 'interleaving hook calls succeed' '
test_cmp expect target-repo.git/actual
'
+HOOK_OUTPUT=hook-output
+
+# Create commits in <repo> and assign each commit's oid to shell variables
+# given in the arguments (A, B, and C). E.g.:
+#
+# create_commits_in <repo> A B C
+#
+# NOTE: Never calling this function from a subshell since variable
+# assignments will disappear when subshell exits.
+create_commits_in () {
+ local repo="$1" &&
+ shift &&
+ while test $# -gt 0
+ do
+ local name=$1 &&
+ shift &&
+ test_commit -C "$repo" --no-tag "$name" &&
+ local rev=$(git -C "$repo" rev-parse HEAD) &&
+ eval "$name=$rev" || return 1
+ done
+}
+
+get_abbrev_oid () {
+ local oid=$1 &&
+ local suffix=${oid#???????} &&
+ oid=${oid%$suffix} &&
+ if test -n "$oid"
+ then
+ echo "$oid"
+ else
+ echo "undefined-oid"
+ fi
+}
+
+# Format the output of git-push, git-show-ref and other commands to make a
+# user-friendly and stable text. We can easily prepare the expect text
+# without having to worry about future changes of the commit ID.
+make_user_friendly_and_stable_output () {
+ sed \
+ -e "s/$(get_abbrev_oid $A)[0-9a-f]*/<COMMIT-A>/g" \
+ -e "s/$(get_abbrev_oid $B)[0-9a-f]*/<COMMIT-B>/g" \
+ -e "s/$(get_abbrev_oid $C)[0-9a-f]*/<COMMIT-C>/g" \
+ -e "s/$(get_abbrev_oid $D)[0-9a-f]*/<COMMIT-D>/g" \
+ -e "s/$(get_abbrev_oid $E)[0-9a-f]*/<COMMIT-E>/g" \
+ -e "s/$(get_abbrev_oid $F)[0-9a-f]*/<COMMIT-F>/g" \
+ -e "s/$(get_abbrev_oid $G)[0-9a-f]*/<COMMIT-G>/g" \
+ -e "s/$(get_abbrev_oid $H)[0-9a-f]*/<COMMIT-H>/g" \
+ -e "s/$(get_abbrev_oid $I)[0-9a-f]*/<COMMIT-I>/g" \
+ -e "s/$ZERO_OID/<ZERO-OID>/g"
+}
+
+test_cmp_heads_and_tags () {
+ local indir= expect actual &&
+ while test $# != 0
+ do
+ case "$1" in
+ -C)
+ indir="$2" &&
+ shift
+ ;;
+ *)
+ break
+ ;;
+ esac &&
+ shift
+ done &&
+ expect=${1:-expect} &&
+ actual=${2:-actual-heads-and-tags} &&
+ indir=${indir:+"$indir"/} &&
+ test_path_is_file "$expect" &&
+ test_when_finished "rm -f \"$actual\"" &&
+ git ${indir:+ -C "$indir"} show-ref --heads --tags |
+ make_user_friendly_and_stable_output >"$actual" &&
+ test_cmp "$expect" "$actual"
+}
+
+test_expect_success 'setup git config and hook' '
+ git config --global core.hooksPath "$HOME/test-hooks" &&
+ git config --global core.abbrev 7 &&
+ mkdir "test-hooks" &&
+ write_script "test-hooks/reference-transaction" <<-EOF
+ exec >>"$HOME/$HOOK_OUTPUT"
+ printf "## Call hook: reference-transaction %9s ##\n" "\$@"
+ while read -r line
+ do
+ echo "\$line"
+ done
+ EOF
+'
+
+test_expect_success "setup base repository" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> HEAD
+ <ZERO-OID> <COMMIT-A> refs/heads/main
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> HEAD
+ <ZERO-OID> <COMMIT-A> refs/heads/main
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-B> HEAD
+ <COMMIT-A> <COMMIT-B> refs/heads/main
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-B> HEAD
+ <COMMIT-A> <COMMIT-B> refs/heads/main
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-B> <COMMIT-C> HEAD
+ <COMMIT-B> <COMMIT-C> refs/heads/main
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-B> <COMMIT-C> HEAD
+ <COMMIT-B> <COMMIT-C> refs/heads/main
+ EOF
+
+ git init base &&
+ create_commits_in base A B C &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ EOF
+ test_cmp_heads_and_tags -C base expect
+'
+
+test_expect_success "update-ref: setup workdir using git-clone" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/remotes/origin/main
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/remotes/origin/main
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> HEAD
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> HEAD
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ EOF
+
+ git clone base workdir &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "update-ref: create new refs" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic1
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic1
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic2
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic2
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic3
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic3
+ EOF
+
+ (
+ cd workdir &&
+ git update-ref refs/heads/topic1 $A &&
+ git update-ref refs/heads/topic2 $A &&
+ git update-ref refs/heads/topic3 $A
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-A> refs/heads/topic2
+ <COMMIT-A> refs/heads/topic3
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Failed because the old-oids for the default branch and
+# HEAD which points to the default branch were not the
+# expected old-oids, but <ZERO-OID>.
+#
+# The differences are as follows:
+#
+# @@ -5,8 +5,8 @@
+# <COMMIT-A> <COMMIT-B> refs/heads/topic1
+# <COMMIT-A> <COMMIT-B> HEAD
+# ## Call hook: reference-transaction prepared ##
+# -<COMMIT-B> <COMMIT-A> refs/heads/topic1
+# -<COMMIT-B> <COMMIT-A> HEAD
+# +<ZERO-OID> <COMMIT-A> refs/heads/topic1
+# +<ZERO-OID> <COMMIT-A> HEAD
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-B> <COMMIT-A> refs/heads/topic1
+# -<COMMIT-B> <COMMIT-A> HEAD
+# +<ZERO-OID> <COMMIT-A> refs/heads/topic1
+# +<ZERO-OID> <COMMIT-A> HEAD
+test_expect_failure "update-ref: update default branch" '
+ test_when_finished "git switch main; rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-B> refs/heads/topic1
+ <COMMIT-A> <COMMIT-B> HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-B> refs/heads/topic1
+ <COMMIT-A> <COMMIT-B> HEAD
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-B> <COMMIT-A> refs/heads/topic1
+ <COMMIT-B> <COMMIT-A> HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-B> <COMMIT-A> refs/heads/topic1
+ <COMMIT-B> <COMMIT-A> HEAD
+ EOF
+
+ (
+ cd workdir &&
+ git switch topic1 &&
+ git update-ref refs/heads/topic1 $B $A &&
+ git update-ref refs/heads/topic1 $A
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-A> refs/heads/topic2
+ <COMMIT-A> refs/heads/topic3
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Failed because the old-oids for HEAD and the ref that the HEAD points
+# to were not the expected old-oids, but <ZERO-OID>.
+#
+# The differences are as follows:
+#
+# @@ -5,8 +5,8 @@
+# <COMMIT-A> <COMMIT-B> HEAD
+# <COMMIT-A> <COMMIT-B> refs/heads/topic1
+# ## Call hook: reference-transaction prepared ##
+# -<COMMIT-B> <COMMIT-A> HEAD
+# -<COMMIT-B> <COMMIT-A> refs/heads/topic1
+# +<ZERO-OID> <COMMIT-A> HEAD
+# +<ZERO-OID> <COMMIT-A> refs/heads/topic1
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-B> <COMMIT-A> HEAD
+# -<COMMIT-B> <COMMIT-A> refs/heads/topic1
+# +<ZERO-OID> <COMMIT-A> HEAD
+# +<ZERO-OID> <COMMIT-A> refs/heads/topic1
+test_expect_failure "update-ref: update HEAD" '
+ test_when_finished "git switch main; rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-B> HEAD
+ <COMMIT-A> <COMMIT-B> refs/heads/topic1
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-B> HEAD
+ <COMMIT-A> <COMMIT-B> refs/heads/topic1
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-B> <COMMIT-A> HEAD
+ <COMMIT-B> <COMMIT-A> refs/heads/topic1
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-B> <COMMIT-A> HEAD
+ <COMMIT-B> <COMMIT-A> refs/heads/topic1
+ EOF
+
+ (
+ cd workdir &&
+ git switch topic1 &&
+ git update-ref HEAD $B $A &&
+ git update-ref HEAD $A
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-A> refs/heads/topic2
+ <COMMIT-A> refs/heads/topic3
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Failed because the reference-transaction hook was executed even
+# though no refs were changed by running git-pack-refs.
+test_expect_failure "update-ref: prepare packed_ref_store using pack-refs" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+ git -C workdir pack-refs --all &&
+ test_path_is_file workdir/.git/packed-refs &&
+ test_path_is_missing $HOOK_OUTPUT
+'
+
+# Failed because the old-oid was not the expected old-oid, but
+# <ZERO-OID> for updating a reference using git-update-refs
+# command without providing the old-oid parameter.
+#
+# The differences are as follows:
+#
+# @@ -3,14 +3,14 @@
+# ## Call hook: reference-transaction committed ##
+# <COMMIT-A> <COMMIT-B> refs/heads/topic2
+# ## Call hook: reference-transaction prepared ##
+# -<COMMIT-A> <COMMIT-C> refs/heads/topic3
+# +<ZERO-OID> <COMMIT-C> refs/heads/topic3
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-A> <COMMIT-C> refs/heads/topic3
+# +<ZERO-OID> <COMMIT-C> refs/heads/topic3
+# ## Call hook: reference-transaction prepared ##
+# <ZERO-OID> <COMMIT-A> refs/heads/topic4
+# ## Call hook: reference-transaction committed ##
+# <ZERO-OID> <COMMIT-A> refs/heads/topic4
+# ## Call hook: reference-transaction prepared ##
+# -<COMMIT-A> <COMMIT-C> refs/heads/topic4
+# +<ZERO-OID> <COMMIT-C> refs/heads/topic4
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-A> <COMMIT-C> refs/heads/topic4
+# +<ZERO-OID> <COMMIT-C> refs/heads/topic4
+test_expect_failure "update-ref: update refs already in packed_ref_store" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-B> refs/heads/topic2
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-B> refs/heads/topic2
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-C> refs/heads/topic3
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-C> refs/heads/topic3
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic4
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic4
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-C> refs/heads/topic4
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-C> refs/heads/topic4
+ EOF
+
+ (
+ cd workdir &&
+ git update-ref refs/heads/topic2 $B $A &&
+ git update-ref refs/heads/topic3 $C &&
+ git update-ref refs/heads/topic4 $A &&
+ git update-ref refs/heads/topic4 $C
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-B> refs/heads/topic2
+ <COMMIT-C> refs/heads/topic3
+ <COMMIT-C> refs/heads/topic4
+ EOF
+ 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.
+#
+# * The old-oid was not the expected old-oid, but <ZERO-OID> when
+# deleting a reference without providing the old-oid parameter.
+#
+# * 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,14 +13,20 @@
+# ## 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
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# +## Call hook: reference-transaction committed ##
+# +<ZERO-OID> <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
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-C> <ZERO-OID> refs/heads/topic4
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
+test_expect_failure "update-ref: remove refs with mixed ref_stores" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <ZERO-OID> refs/heads/topic1
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <ZERO-OID> refs/heads/topic1
+ <COMMIT-A> <ZERO-OID> HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <ZERO-OID> refs/heads/topic1
+ <COMMIT-A> <ZERO-OID> HEAD
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <ZERO-OID> refs/heads/topic2
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-B> <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 ##
+ <COMMIT-C> <ZERO-OID> refs/heads/topic3
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-C> <ZERO-OID> refs/heads/topic4
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-C> <ZERO-OID> refs/heads/topic4
+ EOF
+
+ (
+ cd workdir &&
+ git update-ref -d refs/heads/topic1 $A &&
+ git update-ref -d refs/heads/topic2 $B &&
+ git update-ref -d refs/heads/topic3 &&
+ git update-ref -d refs/heads/topic4
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "update-ref --stdin: create new refs" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic1
+ <ZERO-OID> <COMMIT-A> refs/heads/topic2
+ <ZERO-OID> <COMMIT-A> refs/heads/topic3
+ <ZERO-OID> <COMMIT-A> HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic1
+ <ZERO-OID> <COMMIT-A> refs/heads/topic2
+ <ZERO-OID> <COMMIT-A> refs/heads/topic3
+ <ZERO-OID> <COMMIT-A> HEAD
+ EOF
+
+ (
+ cd workdir &&
+ git update-ref --stdin <<-EOF
+ create refs/heads/topic1 $A
+ create refs/heads/topic2 $A
+ create refs/heads/topic3 $A
+ EOF
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-A> refs/heads/topic2
+ <COMMIT-A> refs/heads/topic3
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "update-ref --stdin: prepare packed_ref_store using pack-refs" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+ git -C workdir pack-refs --all
+'
+
+# Failed because the old-oid was not the expected old-oid, but
+# <ZERO-OID> when running "git update-ref --stdin" to update a
+# reference without providing an old-oid.
+#
+# The differences are as follows:
+#
+# @@ -1,8 +1,8 @@
+# ## Call hook: reference-transaction prepared ##
+# <COMMIT-A> <COMMIT-B> refs/heads/topic2
+# -<COMMIT-A> <COMMIT-C> refs/heads/topic3
+# +<ZERO-OID> <COMMIT-C> refs/heads/topic3
+# <ZERO-OID> <COMMIT-C> refs/heads/topic4
+# ## Call hook: reference-transaction committed ##
+# <COMMIT-A> <COMMIT-B> refs/heads/topic2
+# -<COMMIT-A> <COMMIT-C> refs/heads/topic3
+# +<ZERO-OID> <COMMIT-C> refs/heads/topic3
+# <ZERO-OID> <COMMIT-C> refs/heads/topic4
+test_expect_failure "update-ref --stdin: update refs" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-B> refs/heads/topic2
+ <COMMIT-A> <COMMIT-C> refs/heads/topic3
+ <ZERO-OID> <COMMIT-C> refs/heads/topic4
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-B> refs/heads/topic2
+ <COMMIT-A> <COMMIT-C> refs/heads/topic3
+ <ZERO-OID> <COMMIT-C> refs/heads/topic4
+ EOF
+
+ (
+ cd workdir &&
+ git update-ref --stdin <<-EOF
+ start
+ update refs/heads/topic2 $B $A
+ update refs/heads/topic3 $C
+ create refs/heads/topic4 $C
+ prepare
+ commit
+ EOF
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-B> refs/heads/topic2
+ <COMMIT-C> refs/heads/topic3
+ <COMMIT-C> refs/heads/topic4
+ EOF
+ 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 old-oid was not the expected old-oid, but <ZERO-OID> when
+# deleting a ref without providing the old-oid parameter.
+#
+# The differences are as follows:
+#
+# @@ -4,14 +4,19 @@
+# <ZERO-OID> <ZERO-OID> refs/heads/topic3
+# <ZERO-OID> <ZERO-OID> refs/heads/topic4
+# ## Call hook: reference-transaction prepared ##
+# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
+# <COMMIT-B> <ZERO-OID> refs/heads/topic2
+# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
+# -<COMMIT-C> <ZERO-OID> refs/heads/topic4
+# -<COMMIT-A> <ZERO-OID> HEAD
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
+# +<ZERO-OID> <ZERO-OID> HEAD
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
+# +<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 ##
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
+# <COMMIT-B> <ZERO-OID> refs/heads/topic2
+# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
+# -<COMMIT-C> <ZERO-OID> refs/heads/topic4
+# -<COMMIT-A> <ZERO-OID> HEAD
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
+# +<ZERO-OID> <ZERO-OID> HEAD
+test_expect_failure "update-ref --stdin: delete refs" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <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 prepared ##
+ <COMMIT-A> <ZERO-OID> refs/heads/topic1
+ <COMMIT-B> <ZERO-OID> refs/heads/topic2
+ <COMMIT-C> <ZERO-OID> refs/heads/topic3
+ <COMMIT-C> <ZERO-OID> refs/heads/topic4
+ <COMMIT-A> <ZERO-OID> HEAD
+ ## 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
+ <COMMIT-C> <ZERO-OID> refs/heads/topic4
+ <COMMIT-A> <ZERO-OID> HEAD
+ EOF
+
+ (
+ cd workdir &&
+ git update-ref --stdin <<-EOF
+ start
+ delete refs/heads/topic1
+ delete refs/heads/topic2 $B
+ delete refs/heads/topic3
+ delete refs/heads/topic4
+ prepare
+ commit
+ EOF
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "branch: setup workdir using git-fetch" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/remotes/origin/main
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/remotes/origin/main
+ EOF
+
+ rm -rf workdir &&
+ git init workdir &&
+ git -C workdir remote add origin ../base &&
+ git -C workdir fetch origin &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ <ZERO-OID> <COMMIT-C> HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ <ZERO-OID> <COMMIT-C> HEAD
+ EOF
+
+ rm $HOOK_OUTPUT &&
+ git -C workdir switch -c main origin/main &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "branch: create new branches" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic1
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic1
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic2
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic2
+ EOF
+
+ (
+ cd workdir &&
+ git branch topic1 $A &&
+ git branch topic2 $A
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-A> refs/heads/topic2
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Failed because the reference-transaction hook was executed even
+# though no refs were changed by running git-gc.
+test_expect_failure "branch: prepare packed_ref_store using gc" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+ git -C workdir gc &&
+ test_path_is_file workdir/.git/packed-refs &&
+ test_path_is_missing $HOOK_OUTPUT
+'
+
+# Failed because the old-oid was not the expected old-oid, but
+# <ZERO-OID> when running git-branch to update a branch without
+# providing an old-oid.
+#
+# The differences are as follows:
+#
+# @@ -1,7 +1,7 @@
+# ## Call hook: reference-transaction prepared ##
+# -<COMMIT-A> <COMMIT-B> refs/heads/topic2
+# +<ZERO-OID> <COMMIT-B> refs/heads/topic2
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-A> <COMMIT-B> refs/heads/topic2
+# +<ZERO-OID> <COMMIT-B> refs/heads/topic2
+# ## Call hook: reference-transaction prepared ##
+# <ZERO-OID> <COMMIT-C> refs/heads/topic3
+# ## Call hook: reference-transaction committed ##
+test_expect_failure "branch: update branch without old-oid" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-B> refs/heads/topic2
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-B> refs/heads/topic2
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic3
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic3
+ EOF
+
+ (
+ cd workdir &&
+ git branch -f topic2 $B &&
+ git branch topic3 $C
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-B> refs/heads/topic2
+ <COMMIT-C> refs/heads/topic3
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Failed because the reference-transaction hook was not executed at all
+# when copying a branch using "git branch -c".
+test_expect_failure "branch: copy branches" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-B> refs/heads/topic4
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-B> refs/heads/topic4
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic5
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic5
+ EOF
+
+ (
+ cd workdir &&
+ git branch -c topic2 topic4 &&
+ git branch -c topic3 topic5
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-B> refs/heads/topic2
+ <COMMIT-C> refs/heads/topic3
+ <COMMIT-B> refs/heads/topic4
+ <COMMIT-C> refs/heads/topic5
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Mismatched hook output for "git branch -m":
+#
+# * 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
+# ## Call hook: reference-transaction committed ##
+# <COMMIT-B> <ZERO-OID> refs/heads/topic4
+# -## 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 ##
+# <COMMIT-C> <ZERO-OID> refs/heads/topic5
+# ## Call hook: reference-transaction committed ##
+# <COMMIT-C> <ZERO-OID> refs/heads/topic5
+# -## Call hook: reference-transaction prepared ##
+# -<ZERO-OID> <COMMIT-C> refs/heads/topic7
+# -## Call hook: reference-transaction committed ##
+# -<ZERO-OID> <COMMIT-C> refs/heads/topic7
+test_expect_failure "branch: rename branches" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-B> <ZERO-OID> refs/heads/topic4
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-B> <ZERO-OID> refs/heads/topic4
+ ## 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 prepared ##
+ <COMMIT-C> <ZERO-OID> refs/heads/topic5
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-C> <ZERO-OID> refs/heads/topic5
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic7
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic7
+ EOF
+
+ (
+ cd workdir &&
+ git branch -m topic4 topic6 &&
+ git branch -m topic5 topic7
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/heads/topic1
+ <COMMIT-B> refs/heads/topic2
+ <COMMIT-C> refs/heads/topic3
+ <COMMIT-B> refs/heads/topic6
+ <COMMIT-C> refs/heads/topic7
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Mismatched hook output for "git branch -d":
+#
+# * The old-oid was not the expected old-oid, but <ZERO-OID> when
+# deleting a branch without providing the old-oid parameter.
+#
+# * The delete branches operation should be treated as one transaction,
+# but was splitted into several transactions on loose references,
+# 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 @@
+# <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 committed ##
+# +<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 ##
+# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
+# -<COMMIT-B> <ZERO-OID> refs/heads/topic2
+# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
+# ## 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
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
+# +## Call hook: reference-transaction aborted ##
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
+# +## Call hook: reference-transaction prepared ##
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
+# +## Call hook: reference-transaction committed ##
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
+# +## Call hook: reference-transaction aborted ##
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# +## Call hook: reference-transaction prepared ##
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# +## Call hook: reference-transaction committed ##
+# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+test_expect_failure "branch: remove branches" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <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 prepared ##
+ <COMMIT-A> <ZERO-OID> refs/heads/topic1
+ <COMMIT-B> <ZERO-OID> refs/heads/topic2
+ <COMMIT-C> <ZERO-OID> refs/heads/topic3
+ ## 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
+ EOF
+
+ (
+ cd workdir &&
+ git branch -d topic1 topic2 topic3
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-B> refs/heads/topic6
+ <COMMIT-C> refs/heads/topic7
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "tag: setup workdir using git-push" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ <ZERO-OID> <COMMIT-C> HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ <ZERO-OID> <COMMIT-C> HEAD
+ EOF
+
+ rm -rf workdir &&
+ git init workdir &&
+ git -C workdir config receive.denyCurrentBranch ignore &&
+ git -C base push ../workdir "+refs/heads/*:refs/heads/*" &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ EOF
+ test_cmp_heads_and_tags -C workdir expect &&
+
+ git -C workdir restore --staged -- . &&
+ git -C workdir restore -- .
+'
+
+test_expect_success "tag: create new tags" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/tags/v1
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/tags/v1
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/tags/v2
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/tags/v2
+ EOF
+
+ (
+ cd workdir &&
+ git tag v1 $A &&
+ git tag v2 $A
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/tags/v1
+ <COMMIT-A> refs/tags/v2
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Failed because the reference-transaction hook was executed even
+# though no refs were changed by running git-pack-refs.
+test_expect_failure "tag: prepare packed_ref_store using pack-refs" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+ git -C workdir pack-refs --all &&
+ test_path_is_file workdir/.git/packed-refs &&
+ test_path_is_missing $HOOK_OUTPUT
+'
+
+test_expect_success "tag: update refs to create loose refs" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-B> refs/tags/v2
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-B> refs/tags/v2
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/tags/v3
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/tags/v3
+ EOF
+
+ (
+ cd workdir &&
+ git tag -f v2 $B &&
+ git tag v3 $C
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-A> refs/tags/v1
+ <COMMIT-B> refs/tags/v2
+ <COMMIT-C> refs/tags/v3
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Mismatched hook output for "git tag -d":
+#
+# * The old-oid was not the expected old-oid, but <ZERO-OID> when
+# deleting a tag without providing the old-oid parameter.
+#
+# * The delete tags operation should be treated as one transaction,
+# but was splitted into several transactions on loose references,
+# 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 @@
+# <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 committed ##
+# +<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 ##
+# -<COMMIT-A> <ZERO-OID> refs/tags/v1
+# -<COMMIT-B> <ZERO-OID> refs/tags/v2
+# -<COMMIT-C> <ZERO-OID> refs/tags/v3
+# +<ZERO-OID> <ZERO-OID> refs/tags/v1
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-A> <ZERO-OID> refs/tags/v1
+# -<COMMIT-B> <ZERO-OID> refs/tags/v2
+# -<COMMIT-C> <ZERO-OID> refs/tags/v3
+# +<ZERO-OID> <ZERO-OID> refs/tags/v1
+# +## Call hook: reference-transaction aborted ##
+# +<ZERO-OID> <ZERO-OID> refs/tags/v2
+# +## Call hook: reference-transaction prepared ##
+# +<ZERO-OID> <ZERO-OID> refs/tags/v2
+# +## Call hook: reference-transaction committed ##
+# +<ZERO-OID> <ZERO-OID> refs/tags/v2
+# +## Call hook: reference-transaction aborted ##
+# +<ZERO-OID> <ZERO-OID> refs/tags/v3
+# +## Call hook: reference-transaction prepared ##
+# +<ZERO-OID> <ZERO-OID> refs/tags/v3
+# +## Call hook: reference-transaction committed ##
+# +<ZERO-OID> <ZERO-OID> refs/tags/v3
+test_expect_failure "tag: remove tags with mixed ref_stores" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <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 prepared ##
+ <COMMIT-A> <ZERO-OID> refs/tags/v1
+ <COMMIT-B> <ZERO-OID> refs/tags/v2
+ <COMMIT-C> <ZERO-OID> refs/tags/v3
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <ZERO-OID> refs/tags/v1
+ <COMMIT-B> <ZERO-OID> refs/tags/v2
+ <COMMIT-C> <ZERO-OID> refs/tags/v3
+ EOF
+
+ (
+ cd workdir &&
+ git tag -d v1 v2 v3
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "worktree: setup workdir using push --atomic" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ <ZERO-OID> <COMMIT-C> HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ <ZERO-OID> <COMMIT-C> HEAD
+ EOF
+
+ rm -rf workdir &&
+ git init --bare repo.git &&
+ git -C base push --atomic --mirror ../repo.git &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+ rm $HOOK_OUTPUT &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/remotes/origin/main
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/remotes/origin/main
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> HEAD
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> HEAD
+ <ZERO-OID> <COMMIT-C> refs/heads/main
+ EOF
+
+ git clone --no-local repo.git workdir &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "worktree: topic1: commit --amend" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic1
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic1
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-C> <COMMIT-D> HEAD
+ <COMMIT-C> <COMMIT-D> refs/heads/topic1
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-C> <COMMIT-D> HEAD
+ <COMMIT-C> <COMMIT-D> refs/heads/topic1
+ EOF
+
+ (
+ cd workdir &&
+ git checkout -b topic1 &&
+ git commit --amend -m "C (amend)"
+ ) &&
+ D=$(git -C workdir rev-parse HEAD) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-D> refs/heads/topic1
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "worktree: topic2: merge" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic2
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic2
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> ORIG_HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> ORIG_HEAD
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-E> HEAD
+ <COMMIT-A> <COMMIT-E> refs/heads/topic2
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-E> HEAD
+ <COMMIT-A> <COMMIT-E> refs/heads/topic2
+ EOF
+
+ (
+ cd workdir &&
+ git checkout -b topic2 $A &&
+ git merge --no-ff main &&
+ test_path_is_file B.t &&
+ test_path_is_file C.t
+ ) &&
+ E=$(git -C workdir rev-parse HEAD) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-D> refs/heads/topic1
+ <COMMIT-E> refs/heads/topic2
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Mismatched hook output for git-cherry-pick:
+#
+# * The old-oid was not the expected old-oid, but <ZERO-OID>.
+#
+# * Unexpected execution of the "reference-transaction abort" command.
+#
+# The differences are as follows:
+#
+# @@ -12,7 +12,9 @@
+# ## 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
+# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-C> <ZERO-OID> CHERRY_PICK_HEAD
+# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
+test_expect_failure "worktree: topic3: cherry-pick" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic3
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-A> refs/heads/topic3
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> CHERRY_PICK_HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> CHERRY_PICK_HEAD
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-F> HEAD
+ <COMMIT-A> <COMMIT-F> refs/heads/topic3
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-F> HEAD
+ <COMMIT-A> <COMMIT-F> refs/heads/topic3
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-C> <ZERO-OID> CHERRY_PICK_HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-C> <ZERO-OID> CHERRY_PICK_HEAD
+ EOF
+
+ (
+ cd workdir &&
+ git checkout -b topic3 $A &&
+ git cherry-pick $C &&
+ test_path_is_file C.t &&
+ test_path_is_missing B.t
+ ) &&
+ F=$(git -C workdir rev-parse HEAD) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-D> refs/heads/topic1
+ <COMMIT-E> refs/heads/topic2
+ <COMMIT-F> refs/heads/topic3
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+# Mismatched hook output for git-rebase:
+#
+# * The old-oid was not the expected old-oid, but <ZERO-OID>.
+#
+# * 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,10 +20,12 @@
+# <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
+# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
+# ## Call hook: reference-transaction committed ##
+# -<COMMIT-G> <ZERO-OID> CHERRY_PICK_HEAD
+# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
+# ## Call hook: reference-transaction prepared ##
+# <COMMIT-G> <COMMIT-H> refs/heads/topic4
+# ## Call hook: reference-transaction committed ##
+test_expect_failure "worktree: topic4: rebase" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-A> <COMMIT-G> ORIG_HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-A> <COMMIT-G> ORIG_HEAD
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-G> <COMMIT-C> HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-G> <COMMIT-C> HEAD
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <ZERO-OID> REBASE_HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <ZERO-OID> REBASE_HEAD
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-G> CHERRY_PICK_HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-G> CHERRY_PICK_HEAD
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-C> <COMMIT-H> HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-C> <COMMIT-H> HEAD
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-G> <ZERO-OID> CHERRY_PICK_HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-G> <ZERO-OID> CHERRY_PICK_HEAD
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-G> <COMMIT-H> refs/heads/topic4
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-G> <COMMIT-H> refs/heads/topic4
+ EOF
+
+ git -C workdir checkout -b topic4 $A &&
+ create_commits_in workdir G &&
+ rm $HOOK_OUTPUT &&
+ git -C workdir rebase main &&
+ H=$(git -C workdir rev-parse HEAD) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-D> refs/heads/topic1
+ <COMMIT-E> refs/heads/topic2
+ <COMMIT-F> refs/heads/topic3
+ <COMMIT-H> refs/heads/topic4
+ EOF
+ 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_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic5
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic5
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-C> <COMMIT-I> HEAD
+ <COMMIT-C> <COMMIT-I> refs/heads/topic5
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-C> <COMMIT-I> HEAD
+ <COMMIT-C> <COMMIT-I> refs/heads/topic5
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
+ EOF
+
+ (
+ cd workdir &&
+ git checkout -b topic5 $C &&
+ git revert HEAD &&
+ test_path_is_file B.t &&
+ test_path_is_missing C.t
+ ) &&
+ I=$(git -C workdir rev-parse HEAD) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-D> refs/heads/topic1
+ <COMMIT-E> refs/heads/topic2
+ <COMMIT-F> refs/heads/topic3
+ <COMMIT-H> refs/heads/topic4
+ <COMMIT-I> refs/heads/topic5
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
+test_expect_success "worktree: topic6: reset" '
+ test_when_finished "rm -f $HOOK_OUTPUT" &&
+
+ cat >expect <<-\EOF &&
+ ## Call hook: reference-transaction prepared ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic6
+ ## Call hook: reference-transaction committed ##
+ <ZERO-OID> <COMMIT-C> refs/heads/topic6
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-G> <COMMIT-C> ORIG_HEAD
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-G> <COMMIT-C> ORIG_HEAD
+ ## Call hook: reference-transaction prepared ##
+ <COMMIT-C> <COMMIT-B> HEAD
+ <COMMIT-C> <COMMIT-B> refs/heads/topic6
+ ## Call hook: reference-transaction committed ##
+ <COMMIT-C> <COMMIT-B> HEAD
+ <COMMIT-C> <COMMIT-B> refs/heads/topic6
+ EOF
+
+ (
+ cd workdir &&
+ git checkout -b topic6 $C &&
+ git reset --hard $B
+ ) &&
+ make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ <COMMIT-C> refs/heads/main
+ <COMMIT-D> refs/heads/topic1
+ <COMMIT-E> refs/heads/topic2
+ <COMMIT-F> refs/heads/topic3
+ <COMMIT-H> refs/heads/topic4
+ <COMMIT-I> refs/heads/topic5
+ <COMMIT-B> refs/heads/topic6
+ EOF
+ test_cmp_heads_and_tags -C workdir expect
+'
+
test_done
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 2/9] refs: update missing old-oid in transaction from lockfile
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 ` Jiang Xin
2022-08-19 3:21 ` [PATCH v2 3/9] refs: add new field in transaction for running transaction hook Jiang Xin
` (6 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
For commands that update a reference without providing an old-oid, the
"reference-transaction" hook will receive a zero-oid instead of the
correct old-oid.
In order to provide the "reference-transaction" hook with a real old-oid
in any case, get proper old_oid from the lock file and propagate it to
the corresponding update entry of a transaction.
The behavior of the following git commands and five testcases have been
fixed in t1416:
* git branch [-f] <ref> <new-oid> # update branch
* git cherry-pick <oid>
* git rebase
* git tag -d <tag>
* git update-ref --stdin # update refs
* git update-ref -d <ref>
* git update-ref <ref> <new-oid> # update ref
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
refs/files-backend.c | 28 +++-
t/t1416-ref-transaction-hooks.sh | 231 ++++++-------------------------
2 files changed, 65 insertions(+), 194 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8db7882aac..7c1c25a25c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2523,15 +2523,24 @@ static int lock_ref_for_update(struct files_ref_store *refs,
update->backend_data = lock;
if (update->type & REF_ISSYMREF) {
+ const char *ref;
+
+ /*
+ * Read the referent even though we won't use it as part
+ * of the transaction, because we want to set a proper
+ * old_oid for this symref using the oid we got.
+ */
+ ref = refs_resolve_ref_unsafe(&refs->base,
+ referent.buf, 0,
+ &lock->old_oid, NULL);
+
if (update->flags & REF_NO_DEREF) {
/*
* We won't be reading the referent as part of
- * the transaction, so we have to read it here
- * to record and possibly check old_oid:
+ * the transaction, so we may need to check
+ * old_oid here:
*/
- if (!refs_resolve_ref_unsafe(&refs->base,
- referent.buf, 0,
- &lock->old_oid, NULL)) {
+ if (!ref) {
if (update->flags & REF_HAVE_OLD) {
strbuf_addf(err, "cannot lock ref '%s': "
"error reading reference",
@@ -2578,6 +2587,15 @@ static int lock_ref_for_update(struct files_ref_store *refs,
}
}
+ /*
+ * Propagate old_oid from the lock to the update entry, so we can
+ * provide a proper old-oid of to the "reference-transaction" hook.
+ */
+ if (!(update->flags & REF_HAVE_OLD) && !is_null_oid(&lock->old_oid)) {
+ oidcpy(&update->old_oid, &lock->old_oid);
+ update->flags |= REF_HAVE_OLD;
+ }
+
if ((update->flags & REF_HAVE_NEW) &&
!(update->flags & REF_DELETING) &&
!(update->flags & REF_LOG_ONLY)) {
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 84509cb6a4..1ae601a73d 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -52,8 +52,8 @@ test_expect_success 'hook gets all queued updates in prepared state' '
fi
EOF
cat >expect <<-EOF &&
- $ZERO_OID $POST_OID HEAD
- $ZERO_OID $POST_OID refs/heads/main
+ $PRE_OID $POST_OID HEAD
+ $PRE_OID $POST_OID refs/heads/main
EOF
git update-ref HEAD POST <<-EOF &&
update HEAD $ZERO_OID $POST_OID
@@ -75,8 +75,8 @@ test_expect_success 'hook gets all queued updates in committed state' '
fi
EOF
cat >expect <<-EOF &&
- $ZERO_OID $POST_OID HEAD
- $ZERO_OID $POST_OID refs/heads/main
+ $PRE_OID $POST_OID HEAD
+ $PRE_OID $POST_OID refs/heads/main
EOF
git update-ref HEAD POST &&
test_cmp expect actual
@@ -320,26 +320,7 @@ test_expect_success "update-ref: create new refs" '
test_cmp_heads_and_tags -C workdir expect
'
-# Failed because the old-oids for the default branch and
-# HEAD which points to the default branch were not the
-# expected old-oids, but <ZERO-OID>.
-#
-# The differences are as follows:
-#
-# @@ -5,8 +5,8 @@
-# <COMMIT-A> <COMMIT-B> refs/heads/topic1
-# <COMMIT-A> <COMMIT-B> HEAD
-# ## Call hook: reference-transaction prepared ##
-# -<COMMIT-B> <COMMIT-A> refs/heads/topic1
-# -<COMMIT-B> <COMMIT-A> HEAD
-# +<ZERO-OID> <COMMIT-A> refs/heads/topic1
-# +<ZERO-OID> <COMMIT-A> HEAD
-# ## Call hook: reference-transaction committed ##
-# -<COMMIT-B> <COMMIT-A> refs/heads/topic1
-# -<COMMIT-B> <COMMIT-A> HEAD
-# +<ZERO-OID> <COMMIT-A> refs/heads/topic1
-# +<ZERO-OID> <COMMIT-A> HEAD
-test_expect_failure "update-ref: update default branch" '
+test_expect_success "update-ref: update default branch" '
test_when_finished "git switch main; rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -375,25 +356,7 @@ test_expect_failure "update-ref: update default branch" '
test_cmp_heads_and_tags -C workdir expect
'
-# Failed because the old-oids for HEAD and the ref that the HEAD points
-# to were not the expected old-oids, but <ZERO-OID>.
-#
-# The differences are as follows:
-#
-# @@ -5,8 +5,8 @@
-# <COMMIT-A> <COMMIT-B> HEAD
-# <COMMIT-A> <COMMIT-B> refs/heads/topic1
-# ## Call hook: reference-transaction prepared ##
-# -<COMMIT-B> <COMMIT-A> HEAD
-# -<COMMIT-B> <COMMIT-A> refs/heads/topic1
-# +<ZERO-OID> <COMMIT-A> HEAD
-# +<ZERO-OID> <COMMIT-A> refs/heads/topic1
-# ## Call hook: reference-transaction committed ##
-# -<COMMIT-B> <COMMIT-A> HEAD
-# -<COMMIT-B> <COMMIT-A> refs/heads/topic1
-# +<ZERO-OID> <COMMIT-A> HEAD
-# +<ZERO-OID> <COMMIT-A> refs/heads/topic1
-test_expect_failure "update-ref: update HEAD" '
+test_expect_success "update-ref: update HEAD" '
test_when_finished "git switch main; rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -438,32 +401,7 @@ test_expect_failure "update-ref: prepare packed_ref_store using pack-refs" '
test_path_is_missing $HOOK_OUTPUT
'
-# Failed because the old-oid was not the expected old-oid, but
-# <ZERO-OID> for updating a reference using git-update-refs
-# command without providing the old-oid parameter.
-#
-# The differences are as follows:
-#
-# @@ -3,14 +3,14 @@
-# ## Call hook: reference-transaction committed ##
-# <COMMIT-A> <COMMIT-B> refs/heads/topic2
-# ## Call hook: reference-transaction prepared ##
-# -<COMMIT-A> <COMMIT-C> refs/heads/topic3
-# +<ZERO-OID> <COMMIT-C> refs/heads/topic3
-# ## Call hook: reference-transaction committed ##
-# -<COMMIT-A> <COMMIT-C> refs/heads/topic3
-# +<ZERO-OID> <COMMIT-C> refs/heads/topic3
-# ## Call hook: reference-transaction prepared ##
-# <ZERO-OID> <COMMIT-A> refs/heads/topic4
-# ## Call hook: reference-transaction committed ##
-# <ZERO-OID> <COMMIT-A> refs/heads/topic4
-# ## Call hook: reference-transaction prepared ##
-# -<COMMIT-A> <COMMIT-C> refs/heads/topic4
-# +<ZERO-OID> <COMMIT-C> refs/heads/topic4
-# ## Call hook: reference-transaction committed ##
-# -<COMMIT-A> <COMMIT-C> refs/heads/topic4
-# +<ZERO-OID> <COMMIT-C> refs/heads/topic4
-test_expect_failure "update-ref: update refs already in packed_ref_store" '
+test_expect_success "update-ref: update refs already in packed_ref_store" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -510,9 +448,6 @@ test_expect_failure "update-ref: update refs already in packed_ref_store" '
# * The "reference-transaction committed" command was executed twice,
# once for packed ref-store, and once for loose ref-store.
#
-# * The old-oid was not the expected old-oid, but <ZERO-OID> when
-# deleting a reference without providing the old-oid parameter.
-#
# * Unexpected execution of the "reference-transaction abort" command.
#
# The differences are as follows:
@@ -526,7 +461,7 @@ test_expect_failure "update-ref: update refs already in packed_ref_store" '
# <COMMIT-A> <ZERO-OID> refs/heads/topic1
# <COMMIT-A> <ZERO-OID> HEAD
# ## Call hook: reference-transaction prepared ##
-# @@ -11,14 +13,20 @@
+# @@ -11,13 +13,19 @@
# ## Call hook: reference-transaction prepared ##
# <COMMIT-B> <ZERO-OID> refs/heads/topic2
# ## Call hook: reference-transaction committed ##
@@ -536,21 +471,16 @@ test_expect_failure "update-ref: update refs already in packed_ref_store" '
# ## Call hook: reference-transaction prepared ##
# <ZERO-OID> <ZERO-OID> refs/heads/topic3
# ## Call hook: reference-transaction prepared ##
-# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# <COMMIT-C> <ZERO-OID> refs/heads/topic3
# ## Call hook: reference-transaction committed ##
-# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
# +## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# <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
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
+# <COMMIT-C> <ZERO-OID> refs/heads/topic4
# ## Call hook: reference-transaction committed ##
-# -<COMMIT-C> <ZERO-OID> refs/heads/topic4
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
test_expect_failure "update-ref: remove refs with mixed ref_stores" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
@@ -638,24 +568,7 @@ test_expect_success "update-ref --stdin: prepare packed_ref_store using pack-ref
git -C workdir pack-refs --all
'
-# Failed because the old-oid was not the expected old-oid, but
-# <ZERO-OID> when running "git update-ref --stdin" to update a
-# reference without providing an old-oid.
-#
-# The differences are as follows:
-#
-# @@ -1,8 +1,8 @@
-# ## Call hook: reference-transaction prepared ##
-# <COMMIT-A> <COMMIT-B> refs/heads/topic2
-# -<COMMIT-A> <COMMIT-C> refs/heads/topic3
-# +<ZERO-OID> <COMMIT-C> refs/heads/topic3
-# <ZERO-OID> <COMMIT-C> refs/heads/topic4
-# ## Call hook: reference-transaction committed ##
-# <COMMIT-A> <COMMIT-B> refs/heads/topic2
-# -<COMMIT-A> <COMMIT-C> refs/heads/topic3
-# +<ZERO-OID> <COMMIT-C> refs/heads/topic3
-# <ZERO-OID> <COMMIT-C> refs/heads/topic4
-test_expect_failure "update-ref --stdin: update refs" '
+test_expect_success "update-ref --stdin: update refs" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -699,39 +612,20 @@ test_expect_failure "update-ref --stdin: update refs" '
# * The "reference-transaction committed" command was executed twice,
# once for packed ref-store, and once for loose ref-store.
#
-# * The old-oid was not the expected old-oid, but <ZERO-OID> when
-# deleting a ref without providing the old-oid parameter.
-#
# The differences are as follows:
#
-# @@ -4,14 +4,19 @@
-# <ZERO-OID> <ZERO-OID> refs/heads/topic3
-# <ZERO-OID> <ZERO-OID> refs/heads/topic4
-# ## Call hook: reference-transaction prepared ##
-# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
-# <COMMIT-B> <ZERO-OID> refs/heads/topic2
-# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
-# -<COMMIT-C> <ZERO-OID> refs/heads/topic4
-# -<COMMIT-A> <ZERO-OID> HEAD
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
-# +<ZERO-OID> <ZERO-OID> HEAD
+# @@ -10,6 +10,11 @@
+# <COMMIT-C> <ZERO-OID> refs/heads/topic4
+# <COMMIT-A> <ZERO-OID> HEAD
# ## Call hook: reference-transaction committed ##
-# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
# +<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 ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
+# <COMMIT-A> <ZERO-OID> refs/heads/topic1
# <COMMIT-B> <ZERO-OID> refs/heads/topic2
-# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
-# -<COMMIT-C> <ZERO-OID> refs/heads/topic4
-# -<COMMIT-A> <ZERO-OID> HEAD
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic4
-# +<ZERO-OID> <ZERO-OID> HEAD
+# <COMMIT-C> <ZERO-OID> refs/heads/topic3
test_expect_failure "update-ref --stdin: delete refs" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
@@ -852,23 +746,7 @@ test_expect_failure "branch: prepare packed_ref_store using gc" '
test_path_is_missing $HOOK_OUTPUT
'
-# Failed because the old-oid was not the expected old-oid, but
-# <ZERO-OID> when running git-branch to update a branch without
-# providing an old-oid.
-#
-# The differences are as follows:
-#
-# @@ -1,7 +1,7 @@
-# ## Call hook: reference-transaction prepared ##
-# -<COMMIT-A> <COMMIT-B> refs/heads/topic2
-# +<ZERO-OID> <COMMIT-B> refs/heads/topic2
-# ## Call hook: reference-transaction committed ##
-# -<COMMIT-A> <COMMIT-B> refs/heads/topic2
-# +<ZERO-OID> <COMMIT-B> refs/heads/topic2
-# ## Call hook: reference-transaction prepared ##
-# <ZERO-OID> <COMMIT-C> refs/heads/topic3
-# ## Call hook: reference-transaction committed ##
-test_expect_failure "branch: update branch without old-oid" '
+test_expect_success "branch: update branch without old-oid" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -1007,9 +885,6 @@ test_expect_failure "branch: rename branches" '
# Mismatched hook output for "git branch -d":
#
-# * The old-oid was not the expected old-oid, but <ZERO-OID> when
-# deleting a branch without providing the old-oid parameter.
-#
# * The delete branches operation should be treated as one transaction,
# but was splitted into several transactions on loose references,
# and the "reference-transaction committed" command was executed
@@ -1029,28 +904,25 @@ test_expect_failure "branch: rename branches" '
# +<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
+# <COMMIT-B> <ZERO-OID> refs/heads/topic2
# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
# ## 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
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic1
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
-# +## Call hook: reference-transaction prepared ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
-# +## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic2
+# <COMMIT-B> <ZERO-OID> refs/heads/topic2
# +## Call hook: reference-transaction aborted ##
# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
# +## Call hook: reference-transaction prepared ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# +<COMMIT-C> <ZERO-OID> refs/heads/topic3
# +## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/heads/topic3
+# <COMMIT-C> <ZERO-OID> refs/heads/topic3
test_expect_failure "branch: remove branches" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
@@ -1184,9 +1056,6 @@ test_expect_success "tag: update refs to create loose refs" '
# Mismatched hook output for "git tag -d":
#
-# * The old-oid was not the expected old-oid, but <ZERO-OID> when
-# deleting a tag without providing the old-oid parameter.
-#
# * The delete tags operation should be treated as one transaction,
# but was splitted into several transactions on loose references,
# and the "reference-transaction committed" command was executed
@@ -1206,28 +1075,25 @@ test_expect_success "tag: update refs to create loose refs" '
# +<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
+# <COMMIT-B> <ZERO-OID> refs/tags/v2
# -<COMMIT-C> <ZERO-OID> refs/tags/v3
-# +<ZERO-OID> <ZERO-OID> refs/tags/v1
# ## Call hook: reference-transaction committed ##
# -<COMMIT-A> <ZERO-OID> refs/tags/v1
-# -<COMMIT-B> <ZERO-OID> refs/tags/v2
-# -<COMMIT-C> <ZERO-OID> refs/tags/v3
-# +<ZERO-OID> <ZERO-OID> refs/tags/v1
-# +## Call hook: reference-transaction aborted ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v2
-# +## Call hook: reference-transaction prepared ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v2
-# +## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v2
+# <COMMIT-B> <ZERO-OID> refs/tags/v2
# +## Call hook: reference-transaction aborted ##
# +<ZERO-OID> <ZERO-OID> refs/tags/v3
# +## Call hook: reference-transaction prepared ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v3
+# +<COMMIT-C> <ZERO-OID> refs/tags/v3
# +## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v3
+# <COMMIT-C> <ZERO-OID> refs/tags/v3
test_expect_failure "tag: remove tags with mixed ref_stores" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
@@ -1374,24 +1240,19 @@ test_expect_success "worktree: topic2: merge" '
# Mismatched hook output for git-cherry-pick:
#
-# * The old-oid was not the expected old-oid, but <ZERO-OID>.
-#
# * Unexpected execution of the "reference-transaction abort" command.
#
# The differences are as follows:
#
-# @@ -12,7 +12,9 @@
+# @@ -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
-# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
+# <COMMIT-C> <ZERO-OID> CHERRY_PICK_HEAD
# ## Call hook: reference-transaction committed ##
-# -<COMMIT-C> <ZERO-OID> CHERRY_PICK_HEAD
-# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
test_expect_failure "worktree: topic3: cherry-pick" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
@@ -1438,8 +1299,6 @@ test_expect_failure "worktree: topic3: cherry-pick" '
# Mismatched hook output for git-rebase:
#
-# * The old-oid was not the expected old-oid, but <ZERO-OID>.
-#
# * Unexpected execution of the "reference-transaction abort" command.
#
# The differences are as follows:
@@ -1453,20 +1312,14 @@ test_expect_failure "worktree: topic3: cherry-pick" '
# ## Call hook: reference-transaction prepared ##
# <ZERO-OID> <ZERO-OID> REBASE_HEAD
# ## Call hook: reference-transaction committed ##
-# @@ -18,10 +20,12 @@
+# @@ -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
-# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
-# ## Call hook: reference-transaction committed ##
-# -<COMMIT-G> <ZERO-OID> CHERRY_PICK_HEAD
-# +<ZERO-OID> <ZERO-OID> CHERRY_PICK_HEAD
-# ## Call hook: reference-transaction prepared ##
-# <COMMIT-G> <COMMIT-H> refs/heads/topic4
+# <COMMIT-G> <ZERO-OID> CHERRY_PICK_HEAD
# ## Call hook: reference-transaction committed ##
test_expect_failure "worktree: topic4: rebase" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 3/9] refs: add new field in transaction for running transaction hook
2022-08-05 1:41 ` Jiang Xin
` (2 preceding siblings ...)
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 ` Jiang Xin
2022-08-19 3:21 ` [PATCH v2 4/9] refs: do not run transaction hook for git-pack-refs Jiang Xin
` (5 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Add a new field "hook_flags" in transaction, and only run the
"reference-transaction" hook if the specific flag is turned on.
The "reference-transaction" hook has three states: prepared, committed,
and aborted. To update a reference, git may create two seperate
transactions, one for loose reference and one for packed ref-store. This
may cause duplicate running of the hook for same references. The new
field "hook_flags" in the transaction can turn off running a specific
transaction. In some scenarios, we may only want to turn off certain
states of a transaction, such as "committed" and "aborted", but want to
turn on the "prepared" state of the hook to do some pre-checks, so the
"hook_flags" field has three bits to control running of the three states
of the hook.
By calling the "ref_store_transaction_begin()" function, all the flags
of the "hook_flags" field for the new initialized transaction will be
turned on. The new function "ref_store_transaction_begin_extended()"
will be used in later commits with a specific "hook_flags" field to
initialize a new transaction.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
refs.c | 25 +++++++++++++++++++++++--
refs.h | 3 +++
refs/refs-internal.h | 8 ++++++++
3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 90bcb27168..48b69460e2 100644
--- a/refs.c
+++ b/refs.c
@@ -998,17 +998,27 @@ int read_ref_at(struct ref_store *refs, const char *refname,
return 1;
}
-struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
- struct strbuf *err)
+struct ref_transaction *ref_store_transaction_begin_extended(struct ref_store *refs,
+ unsigned int hook_flags,
+ struct strbuf *err)
{
struct ref_transaction *tr;
assert(err);
CALLOC_ARRAY(tr, 1);
tr->ref_store = refs;
+ tr->hook_flags = hook_flags;
return tr;
}
+struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
+ struct strbuf *err)
+{
+ return ref_store_transaction_begin_extended(refs,
+ REF_TRANSACTION_RUN_ALL_HOOKS,
+ err);
+}
+
struct ref_transaction *ref_transaction_begin(struct strbuf *err)
{
return ref_store_transaction_begin(get_main_ref_store(the_repository), err);
@@ -2074,6 +2084,17 @@ static int run_transaction_hook(struct ref_transaction *transaction,
const char *hook;
int ret = 0, i;
+ if (!strcmp(state, "prepared")) {
+ if (!(transaction->hook_flags & REF_TRANSACTION_RUN_PREPARED_HOOK))
+ return 0;
+ } else if (!strcmp(state, "committed")) {
+ if (!(transaction->hook_flags & REF_TRANSACTION_RUN_COMMITTED_HOOK))
+ return 0;
+ } else if (!strcmp(state, "aborted")) {
+ if (!(transaction->hook_flags & REF_TRANSACTION_RUN_ABORTED_HOOK))
+ return 0;
+ }
+
hook = find_hook("reference-transaction");
if (!hook)
return ret;
diff --git a/refs.h b/refs.h
index 47cb9edbaa..715127ab58 100644
--- a/refs.h
+++ b/refs.h
@@ -570,6 +570,9 @@ enum action_on_err {
* Begin a reference transaction. The reference transaction must
* be freed by calling ref_transaction_free().
*/
+struct ref_transaction *ref_store_transaction_begin_extended(struct ref_store *refs,
+ unsigned int hook_flags,
+ struct strbuf *err);
struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
struct strbuf *err);
struct ref_transaction *ref_transaction_begin(struct strbuf *err);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 69f93b0e2a..5220d1980d 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -201,6 +201,13 @@ enum ref_transaction_state {
REF_TRANSACTION_CLOSED = 2
};
+#define REF_TRANSACTION_RUN_PREPARED_HOOK (1 << 0)
+#define REF_TRANSACTION_RUN_COMMITTED_HOOK (1 << 1)
+#define REF_TRANSACTION_RUN_ABORTED_HOOK (1 << 2)
+#define REF_TRANSACTION_RUN_ALL_HOOKS \
+ (REF_TRANSACTION_RUN_PREPARED_HOOK | \
+ REF_TRANSACTION_RUN_COMMITTED_HOOK | \
+ REF_TRANSACTION_RUN_ABORTED_HOOK)
/*
* Data structure for holding a reference transaction, which can
* consist of checks and updates to multiple references, carried out
@@ -212,6 +219,7 @@ struct ref_transaction {
size_t alloc;
size_t nr;
enum ref_transaction_state state;
+ unsigned int hook_flags;
void *backend_data;
};
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 4/9] refs: do not run transaction hook for git-pack-refs
2022-08-05 1:41 ` Jiang Xin
` (3 preceding siblings ...)
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 ` Jiang Xin
2022-08-19 3:21 ` [PATCH v2 5/9] refs: avoid duplicate running of the reference-transaction hook Jiang Xin
` (4 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
The git-pack-refs command will call "files_pack_refs()" to pack loose
references into the "packed-refs" file, and there are no real changes
to the repository. Therefore, by initializing a transaction with an
empty "hook_flags" field, the "reference-transaction" hook will not run.
The "prune_refs()" and "prune_ref()" functions are called from
"file_pack_refs()", and they are used to prune loose refereces which
have already been packed into "packed-refs". The transaction instance
in "prune_ref()" should also be initialized with an empty "hook_flags"
field.
The behavior of the following git commands and three testcases have been
fixed in t1416:
* git gc
* git pack-refs
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
refs/files-backend.c | 10 ++++++++--
t/t1416-ref-transaction-hooks.sh | 12 +++---------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7c1c25a25c..f0947c9dca 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1136,7 +1136,8 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r)
if (check_refname_format(r->name, 0))
return;
- transaction = ref_store_transaction_begin(&refs->base, &err);
+ /* Called by "files_pack_refs()" and should not run any hooks. */
+ transaction = ref_store_transaction_begin_extended(&refs->base, 0, &err);
if (!transaction)
goto cleanup;
ref_transaction_add_update(
@@ -1207,7 +1208,12 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
struct strbuf err = STRBUF_INIT;
struct ref_transaction *transaction;
- transaction = ref_store_transaction_begin(refs->packed_ref_store, &err);
+ /*
+ * No real changes have occurred to the repository and no hooks
+ * should be run for this transaction.
+ */
+ transaction = ref_store_transaction_begin_extended(refs->packed_ref_store,
+ 0, &err);
if (!transaction)
return -1;
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 1ae601a73d..b7ffc9415b 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -392,9 +392,7 @@ test_expect_success "update-ref: update HEAD" '
test_cmp_heads_and_tags -C workdir expect
'
-# Failed because the reference-transaction hook was executed even
-# though no refs were changed by running git-pack-refs.
-test_expect_failure "update-ref: prepare packed_ref_store using pack-refs" '
+test_expect_success "update-ref: prepare packed_ref_store using pack-refs" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
git -C workdir pack-refs --all &&
test_path_is_file workdir/.git/packed-refs &&
@@ -737,9 +735,7 @@ test_expect_success "branch: create new branches" '
test_cmp_heads_and_tags -C workdir expect
'
-# Failed because the reference-transaction hook was executed even
-# though no refs were changed by running git-gc.
-test_expect_failure "branch: prepare packed_ref_store using gc" '
+test_expect_success "branch: prepare packed_ref_store using gc" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
git -C workdir gc &&
test_path_is_file workdir/.git/packed-refs &&
@@ -1014,9 +1010,7 @@ test_expect_success "tag: create new tags" '
test_cmp_heads_and_tags -C workdir expect
'
-# Failed because the reference-transaction hook was executed even
-# though no refs were changed by running git-pack-refs.
-test_expect_failure "tag: prepare packed_ref_store using pack-refs" '
+test_expect_success "tag: prepare packed_ref_store using pack-refs" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
git -C workdir pack-refs --all &&
test_path_is_file workdir/.git/packed-refs &&
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 5/9] refs: avoid duplicate running of the reference-transaction hook
2022-08-05 1:41 ` Jiang Xin
` (4 preceding siblings ...)
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
2022-08-19 3:21 ` [PATCH v2 6/9] refs: add reflog_info to hold more fields for reflog entry Jiang Xin
` (3 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 6/9] refs: add reflog_info to hold more fields for reflog entry
2022-08-05 1:41 ` Jiang Xin
` (5 preceding siblings ...)
2022-08-19 3:21 ` [PATCH v2 5/9] refs: avoid duplicate running of the reference-transaction hook Jiang Xin
@ 2022-08-19 3:21 ` Jiang Xin
2022-08-19 3:21 ` [PATCH v2 7/9] refs: get error message via refs_update_ref_extended() Jiang Xin
` (2 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
The parameter "msg" of the functions "ref_transaction_add_update()" and
"refs_update_ref()" is used as a comment for creating a new reflog
entry. For some cases, like copying or renaming a branch, we may need
more custom fields for the new reflog entry, such as old-oid which is
different from the oid we get from the lock file. Therefore, we create
a new structure "reflog_info" to hold more custom fields for the new
reflog entry, and add two additional extended version functions.
We will use this extension in a later commit to reimplement the function
"files_copy_or_rename_ref()" by calling "refs_update_ref_extended()" to
create a new reference in a transaction, while adding proper reflog
entry.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
refs.c | 54 +++++++++++++++++++++++++++++++++++++++-----
refs.h | 20 ++++++++++++++++
refs/debug.c | 2 +-
refs/files-backend.c | 14 ++++++++----
refs/refs-internal.h | 17 ++++++++++++--
5 files changed, 94 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index 48b69460e2..15130f09b9 100644
--- a/refs.c
+++ b/refs.c
@@ -1045,7 +1045,11 @@ void ref_transaction_free(struct ref_transaction *transaction)
}
for (i = 0; i < transaction->nr; i++) {
- free(transaction->updates[i]->msg);
+ if (transaction->updates[i]->reflog_info) {
+ free(transaction->updates[i]->reflog_info->msg);
+ free(transaction->updates[i]->reflog_info->old_oid);
+ free(transaction->updates[i]->reflog_info);
+ }
free(transaction->updates[i]);
}
free(transaction->updates);
@@ -1057,7 +1061,7 @@ struct ref_update *ref_transaction_add_update(
const char *refname, unsigned int flags,
const struct object_id *new_oid,
const struct object_id *old_oid,
- const char *msg)
+ const struct reflog_info *reflog_info)
{
struct ref_update *update;
@@ -1074,7 +1078,12 @@ struct ref_update *ref_transaction_add_update(
oidcpy(&update->new_oid, new_oid);
if (flags & REF_HAVE_OLD)
oidcpy(&update->old_oid, old_oid);
- update->msg = normalize_reflog_message(msg);
+ if (reflog_info) {
+ update->reflog_info = xcalloc(1, sizeof(*reflog_info));
+ update->reflog_info->msg = normalize_reflog_message(reflog_info->msg);
+ if (reflog_info->old_oid)
+ update->reflog_info->old_oid = oiddup(reflog_info->old_oid);
+ }
return update;
}
@@ -1084,6 +1093,23 @@ int ref_transaction_update(struct ref_transaction *transaction,
const struct object_id *old_oid,
unsigned int flags, const char *msg,
struct strbuf *err)
+{
+ struct reflog_info reflog_info;
+
+ reflog_info.msg = (char *)msg;
+ reflog_info.old_oid = NULL;
+ return ref_transaction_update_extended(transaction,
+ refname, new_oid, old_oid,
+ flags, &reflog_info, err);
+}
+
+int ref_transaction_update_extended(struct ref_transaction *transaction,
+ const char *refname,
+ const struct object_id *new_oid,
+ const struct object_id *old_oid,
+ unsigned int flags,
+ const struct reflog_info *reflog_info,
+ struct strbuf *err)
{
assert(err);
@@ -1109,7 +1135,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0);
ref_transaction_add_update(transaction, refname, flags,
- new_oid, old_oid, msg);
+ new_oid, old_oid, reflog_info);
return 0;
}
@@ -1157,6 +1183,22 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
const char *refname, const struct object_id *new_oid,
const struct object_id *old_oid, unsigned int flags,
enum action_on_err onerr)
+{
+ struct reflog_info reflog_info;
+
+ reflog_info.msg = (char *)msg;
+ reflog_info.old_oid = NULL;
+ return refs_update_ref_extended(refs, refname, new_oid, old_oid,
+ flags, &reflog_info, onerr);
+}
+
+int refs_update_ref_extended(struct ref_store *refs,
+ const char *refname,
+ const struct object_id *new_oid,
+ const struct object_id *old_oid,
+ unsigned int flags,
+ const struct reflog_info *reflog_info,
+ enum action_on_err onerr)
{
struct ref_transaction *t = NULL;
struct strbuf err = STRBUF_INIT;
@@ -1164,8 +1206,8 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
t = ref_store_transaction_begin(refs, &err);
if (!t ||
- ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,
- &err) ||
+ ref_transaction_update_extended(t, refname, new_oid, old_oid,
+ flags, reflog_info, &err) ||
ref_transaction_commit(t, &err)) {
ret = 1;
ref_transaction_free(t);
diff --git a/refs.h b/refs.h
index 715127ab58..0f21ba259f 100644
--- a/refs.h
+++ b/refs.h
@@ -10,6 +10,7 @@ struct strbuf;
struct string_list;
struct string_list_item;
struct worktree;
+struct reflog_info;
/*
* Resolve a reference, recursively following symbolic refererences.
@@ -677,6 +678,18 @@ int ref_transaction_update(struct ref_transaction *transaction,
const struct object_id *old_oid,
unsigned int flags, const char *msg,
struct strbuf *err);
+/*
+ * Extended version of ref_transaction_update() that allows us to
+ * provide more fields (in reflog_info) to custom reflog, such
+ * as msg and old_oid.
+ */
+int ref_transaction_update_extended(struct ref_transaction *transaction,
+ const char *refname,
+ const struct object_id *new_oid,
+ const struct object_id *old_oid,
+ unsigned int flags,
+ const struct reflog_info *reflog_info,
+ struct strbuf *err);
/*
* Add a reference creation to transaction. new_oid is the value that
@@ -806,6 +819,13 @@ void ref_transaction_free(struct ref_transaction *transaction);
int refs_update_ref(struct ref_store *refs, const char *msg, const char *refname,
const struct object_id *new_oid, const struct object_id *old_oid,
unsigned int flags, enum action_on_err onerr);
+int refs_update_ref_extended(struct ref_store *refs,
+ const char *refname,
+ const struct object_id *new_oid,
+ const struct object_id *old_oid,
+ unsigned int flags,
+ const struct reflog_info *reflog_info,
+ enum action_on_err onerr);
int update_ref(const char *msg, const char *refname,
const struct object_id *new_oid, const struct object_id *old_oid,
unsigned int flags, enum action_on_err onerr);
diff --git a/refs/debug.c b/refs/debug.c
index eed8bc94b0..1e60507249 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -79,7 +79,7 @@ static void print_transaction(struct ref_transaction *transaction)
for (i = 0; i < transaction->nr; i++) {
struct ref_update *u = transaction->updates[i];
print_update(i, u->refname, &u->old_oid, &u->new_oid, u->flags,
- u->type, u->msg);
+ u->type, u->reflog_info? u->reflog_info->msg : NULL);
}
trace_printf_key(&trace_refs, "}\n");
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9ec4c23a16..336384daa0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2340,7 +2340,7 @@ static int split_head_update(struct ref_update *update,
transaction, "HEAD",
update->flags | REF_LOG_ONLY | REF_NO_DEREF,
&update->new_oid, &update->old_oid,
- update->msg);
+ update->reflog_info);
/*
* Add "HEAD". This insertion is O(N) in the transaction
@@ -2403,7 +2403,7 @@ static int split_symref_update(struct ref_update *update,
new_update = ref_transaction_add_update(
transaction, referent, new_flags,
&update->new_oid, &update->old_oid,
- update->msg);
+ update->reflog_info);
new_update->parent_update = update;
@@ -2902,9 +2902,15 @@ static int files_transaction_finish(struct ref_store *ref_store,
update->flags & REF_LOG_ONLY) {
if (files_log_ref_write(refs,
lock->ref_name,
- &lock->old_oid,
+ update->reflog_info &&
+ update->reflog_info->old_oid ?
+ update->reflog_info->old_oid :
+ &lock->old_oid,
&update->new_oid,
- update->msg, update->flags,
+ update->reflog_info ?
+ update->reflog_info->msg :
+ NULL,
+ update->flags,
err)) {
char *old_msg = strbuf_detach(err, NULL);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 5220d1980d..782cf5fa78 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -104,6 +104,19 @@ enum peel_status {
*/
enum peel_status peel_object(const struct object_id *name, struct object_id *oid);
+/*
+ * When using refs_update_ref() to copy or rename a branch, the old-oid
+ * for the new created branch is null_oid, but the old_oid for the new
+ * appended log entry for the reflog file which is copied from the
+ * original reflog should be the same as the new_oid for the target
+ * branch. Use "reflog_info" to hold log message and old_oid for the
+ * new reflog entry.
+ */
+struct reflog_info {
+ struct object_id *old_oid;
+ char *msg;
+};
+
/**
* Information needed for a single ref update. Set new_oid to the new
* value or to null_oid to delete the ref. To check the old value
@@ -133,7 +146,7 @@ struct ref_update {
void *backend_data;
unsigned int type;
- char *msg;
+ struct reflog_info *reflog_info;
/*
* If this ref_update was split off of a symref update via
@@ -174,7 +187,7 @@ struct ref_update *ref_transaction_add_update(
const char *refname, unsigned int flags,
const struct object_id *new_oid,
const struct object_id *old_oid,
- const char *msg);
+ const struct reflog_info *reflog_info);
/*
* Transaction states.
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 7/9] refs: get error message via refs_update_ref_extended()
2022-08-05 1:41 ` Jiang Xin
` (6 preceding siblings ...)
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 ` 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
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
The last parameter of the function "refs_update_ref()" is an enum
"action_on_err", and we can not use this function to get the error
message. Extend this function to get error message.
We will use the function "refs_update_ref_extended()" to reimplement
the function "files_copy_or_rename_ref()" in later commit.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
refs.c | 50 +++++++++++++++++++++++++-------------------------
refs.h | 2 +-
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/refs.c b/refs.c
index 15130f09b9..a528473a46 100644
--- a/refs.c
+++ b/refs.c
@@ -1185,11 +1185,29 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
enum action_on_err onerr)
{
struct reflog_info reflog_info;
+ struct strbuf err = STRBUF_INIT;
+ int ret;
reflog_info.msg = (char *)msg;
reflog_info.old_oid = NULL;
- return refs_update_ref_extended(refs, refname, new_oid, old_oid,
- flags, &reflog_info, onerr);
+ ret = refs_update_ref_extended(refs, refname, new_oid, old_oid,
+ flags, &reflog_info, &err);
+ if (ret) {
+ const char *str = _("update_ref failed for ref '%s': %s");
+
+ switch (onerr) {
+ case UPDATE_REFS_MSG_ON_ERR:
+ error(str, refname, err.buf);
+ break;
+ case UPDATE_REFS_DIE_ON_ERR:
+ die(str, refname, err.buf);
+ break;
+ case UPDATE_REFS_QUIET_ON_ERR:
+ break;
+ }
+ }
+ strbuf_release(&err);
+ return ret;
}
int refs_update_ref_extended(struct ref_store *refs,
@@ -1198,37 +1216,19 @@ int refs_update_ref_extended(struct ref_store *refs,
const struct object_id *old_oid,
unsigned int flags,
const struct reflog_info *reflog_info,
- enum action_on_err onerr)
+ struct strbuf *err)
{
struct ref_transaction *t = NULL;
- struct strbuf err = STRBUF_INIT;
- int ret = 0;
- t = ref_store_transaction_begin(refs, &err);
+ t = ref_store_transaction_begin(refs, err);
if (!t ||
ref_transaction_update_extended(t, refname, new_oid, old_oid,
- flags, reflog_info, &err) ||
- ref_transaction_commit(t, &err)) {
- ret = 1;
+ flags, reflog_info, err) ||
+ ref_transaction_commit(t, err)) {
ref_transaction_free(t);
- }
- if (ret) {
- const char *str = _("update_ref failed for ref '%s': %s");
-
- switch (onerr) {
- case UPDATE_REFS_MSG_ON_ERR:
- error(str, refname, err.buf);
- break;
- case UPDATE_REFS_DIE_ON_ERR:
- die(str, refname, err.buf);
- break;
- case UPDATE_REFS_QUIET_ON_ERR:
- break;
- }
- strbuf_release(&err);
return 1;
}
- strbuf_release(&err);
+
if (t)
ref_transaction_free(t);
return 0;
diff --git a/refs.h b/refs.h
index 0f21ba259f..85832c4863 100644
--- a/refs.h
+++ b/refs.h
@@ -825,7 +825,7 @@ int refs_update_ref_extended(struct ref_store *refs,
const struct object_id *old_oid,
unsigned int flags,
const struct reflog_info *reflog_info,
- enum action_on_err onerr);
+ struct strbuf *err);
int update_ref(const char *msg, const char *refname,
const struct object_id *new_oid, const struct object_id *old_oid,
unsigned int flags, enum action_on_err onerr);
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 8/9] refs: reimplement files_copy_or_rename_ref() to run refs-txn hook
2022-08-05 1:41 ` Jiang Xin
` (7 preceding siblings ...)
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 ` Jiang Xin
2022-08-19 3:21 ` [PATCH v2 9/9] refs: reimplement refs_delete_refs() and run hook once Jiang Xin
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
When copying or renaming a branch, the "reference-transaction" hook is
not executed. This is because we called two low-level functions
"lock_ref_oid_basic()" and "write_ref_to_lockfile()", and reinvented
the wheel in "commit_ref_update()" to update the reference instead of
implementing "files_copy_or_rename_ref()" by calling "refs_update_ref()"
to update a reference in a transaction. The reason for this is that we
want to create a proper reflog for the newly copied reference.
Refactor "files_copy_or_rename_ref()" by calling the extended version
of "refs_update_ref", i.e. "refs_update_ref_extended()", so we can
create the target branch for copying or renaming a branch and generate
a correct reflog file at the same time.
The behavior of the following git commands and two testcases have been
fixed in t1416:
* git branch -c <src> <dest> # copy branch
* git branch -m <old> <new> # rename branch
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
refs/files-backend.c | 97 +++-----------------------------
t/t1416-ref-transaction-hooks.sh | 28 +--------
2 files changed, 9 insertions(+), 116 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 336384daa0..e029f5a885 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1376,10 +1376,6 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname)
static int write_ref_to_lockfile(struct ref_lock *lock,
const struct object_id *oid,
int skip_oid_verification, struct strbuf *err);
-static int commit_ref_update(struct files_ref_store *refs,
- struct ref_lock *lock,
- const struct object_id *oid, const char *logmsg,
- struct strbuf *err);
/*
* Emit a better error message than lockfile.c's
@@ -1418,13 +1414,13 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
struct object_id orig_oid;
int flag = 0, logmoved = 0;
- struct ref_lock *lock;
struct stat loginfo;
struct strbuf sb_oldref = STRBUF_INIT;
struct strbuf sb_newref = STRBUF_INIT;
struct strbuf tmp_renamed_log = STRBUF_INIT;
int log, ret;
struct strbuf err = STRBUF_INIT;
+ struct reflog_info reflog_info;
files_reflog_path(refs, &sb_oldref, oldrefname);
files_reflog_path(refs, &sb_newref, newrefname);
@@ -1510,8 +1506,10 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
logmoved = log;
- lock = lock_ref_oid_basic(refs, newrefname, &err);
- if (!lock) {
+ reflog_info.msg = (char *)logmsg;
+ reflog_info.old_oid = &orig_oid;
+ if (refs_update_ref_extended(ref_store, newrefname, &orig_oid, NULL,
+ REF_NO_DEREF, &reflog_info, &err)) {
if (copy)
error("unable to copy '%s' to '%s': %s", oldrefname, newrefname, err.buf);
else
@@ -1519,36 +1517,20 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
strbuf_release(&err);
goto rollback;
}
- oidcpy(&lock->old_oid, &orig_oid);
-
- if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) ||
- commit_ref_update(refs, lock, &orig_oid, logmsg, &err)) {
- error("unable to write current sha1 into %s: %s", newrefname, err.buf);
- strbuf_release(&err);
- goto rollback;
- }
ret = 0;
goto out;
rollback:
- lock = lock_ref_oid_basic(refs, oldrefname, &err);
- if (!lock) {
- error("unable to lock %s for rollback: %s", oldrefname, err.buf);
- strbuf_release(&err);
- goto rollbacklog;
- }
-
flag = log_all_ref_updates;
log_all_ref_updates = LOG_REFS_NONE;
- if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) ||
- commit_ref_update(refs, lock, &orig_oid, NULL, &err)) {
+ if (refs_update_ref_extended(ref_store, oldrefname, &orig_oid, NULL,
+ REF_NO_DEREF, NULL, &err)) {
error("unable to write current sha1 into %s: %s", oldrefname, err.buf);
strbuf_release(&err);
}
log_all_ref_updates = flag;
- rollbacklog:
if (logmoved && rename(sb_newref.buf, sb_oldref.buf))
error("unable to restore logfile %s from %s: %s",
oldrefname, newrefname, strerror(errno));
@@ -1815,71 +1797,6 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
return 0;
}
-/*
- * Commit a change to a loose reference that has already been written
- * to the loose reference lockfile. Also update the reflogs if
- * necessary, using the specified lockmsg (which can be NULL).
- */
-static int commit_ref_update(struct files_ref_store *refs,
- struct ref_lock *lock,
- const struct object_id *oid, const char *logmsg,
- struct strbuf *err)
-{
- files_assert_main_repository(refs, "commit_ref_update");
-
- clear_loose_ref_cache(refs);
- if (files_log_ref_write(refs, lock->ref_name,
- &lock->old_oid, oid,
- logmsg, 0, err)) {
- char *old_msg = strbuf_detach(err, NULL);
- strbuf_addf(err, "cannot update the ref '%s': %s",
- lock->ref_name, old_msg);
- free(old_msg);
- unlock_ref(lock);
- return -1;
- }
-
- if (strcmp(lock->ref_name, "HEAD") != 0) {
- /*
- * Special hack: If a branch is updated directly and HEAD
- * points to it (may happen on the remote side of a push
- * for example) then logically the HEAD reflog should be
- * updated too.
- * A generic solution implies reverse symref information,
- * but finding all symrefs pointing to the given branch
- * would be rather costly for this rare event (the direct
- * update of a branch) to be worth it. So let's cheat and
- * check with HEAD only which should cover 99% of all usage
- * scenarios (even 100% of the default ones).
- */
- int head_flag;
- const char *head_ref;
-
- head_ref = refs_resolve_ref_unsafe(&refs->base, "HEAD",
- RESOLVE_REF_READING,
- NULL, &head_flag);
- if (head_ref && (head_flag & REF_ISSYMREF) &&
- !strcmp(head_ref, lock->ref_name)) {
- struct strbuf log_err = STRBUF_INIT;
- if (files_log_ref_write(refs, "HEAD",
- &lock->old_oid, oid,
- logmsg, 0, &log_err)) {
- error("%s", log_err.buf);
- strbuf_release(&log_err);
- }
- }
- }
-
- if (commit_ref(lock)) {
- strbuf_addf(err, "couldn't set '%s'", lock->ref_name);
- unlock_ref(lock);
- return -1;
- }
-
- unlock_ref(lock);
- return 0;
-}
-
static int create_ref_symlink(struct ref_lock *lock, const char *target)
{
int ret = -1;
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 6ba7ba746c..77996017d7 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -715,9 +715,7 @@ test_expect_success "branch: update branch without old-oid" '
test_cmp_heads_and_tags -C workdir expect
'
-# Failed because the reference-transaction hook was not executed at all
-# when copying a branch using "git branch -c".
-test_expect_failure "branch: copy branches" '
+test_expect_success "branch: copy branches" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -750,29 +748,7 @@ test_expect_failure "branch: copy branches" '
test_cmp_heads_and_tags -C workdir expect
'
-# Mismatched hook output for "git branch -m":
-#
-# * The "reference-transaction committed" command was not executed
-# for the target branch.
-#
-# The differences are as follows:
-#
-# @@ -3,14 +3,6 @@
-# ## Call hook: reference-transaction committed ##
-# <COMMIT-B> <ZERO-OID> refs/heads/topic4
-# ## 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 prepared ##
-# <COMMIT-C> <ZERO-OID> refs/heads/topic5
-# ## Call hook: reference-transaction committed ##
-# <COMMIT-C> <ZERO-OID> refs/heads/topic5
-# -## Call hook: reference-transaction prepared ##
-# -<ZERO-OID> <COMMIT-C> refs/heads/topic7
-# -## Call hook: reference-transaction committed ##
-# -<ZERO-OID> <COMMIT-C> refs/heads/topic7
-test_expect_failure "branch: rename branches" '
+test_expect_success "branch: rename branches" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 9/9] refs: reimplement refs_delete_refs() and run hook once
2022-08-05 1:41 ` Jiang Xin
` (8 preceding siblings ...)
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 ` Jiang Xin
9 siblings, 0 replies; 27+ messages in thread
From: Jiang Xin @ 2022-08-19 3:21 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt, Michael Heemskerk, Git List
Cc: Jiang Xin, Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
When using "git branch -d" or "git tag -d" to delete one or multiple
references, command "reference-transaction committed" will be called
repeatedly for the same references. This is because the function
"refs_delete_refs()" is called twice, once for loose ref-store and once
for packed ref-store.
The old story starts when running the function "refs_delete_refs()" on a
loose ref-store:
1. Try to remove the references from packed ref-store.
1.1. Lock the packed-ref-store by calling "packed_refs_lock()" in
"files_delete_refs()".
1.2. Call "refs_delete_refs()" on packed-ref-store, and then call
"packed_delete_refs()".
1.3. Create a transaction for packed-ref-store in function
"packed_delete_refs()" by calling the function
"ref_store_transaction_begin()".
2.2. Add update entries for all the references to be removed into
this transaction by calling "ref_transaction_delete()".
2.3. Call "ref_transaction_commit()" to commit the transaction.
2.4. Unlock the packed-ref-store.
2. Try to remove the references one by one by calling the function
"refs_delete_ref()".
2.1. Create a new transaction on loose-ref-store by calling
"ref_store_transaction_begin()".
2.2. Call "ref_transaction_delete()" to add a update entry
for the reference to be deleted into the transaction.
2.3. In "ref_transaction_commit()", it will call functions
"files_transaction_prepare()" and "files_transaction_finish()"
to commit the transaction.
2.3.1. Lock the loose reference.
2.3.2. Create a new packed-transaction, and add a new update
entry to this packed-transaction. The previous step 1
makes this operation unnecessary.
2.3.3. Lock the packed-ref-store and call fucntion
"is_packed_transaction_needed()" to check whether it
is necessary to commit the transaction, and then
abort the transaction because the reference is already
removed from the packed-ref-store in step 1.
2.3.4. Remove the reflog and the loose reference file for
the reference to be deleted.
2.3.4. Unlock the loose reference.
From the above steps, we can see that "refs_delete_refs()" is not an
atomic operation, but a semi-atomic operation. The operation is atomic
if all references to be deleted are in the packed ref-store, but not
if some references are loose references because we delete the loose
references one by one by calling "refs_delete_ref()" .
Refactored function "files_delete_refs()" to delete references within a
transaction, so the "reference-transaction" hook will only run once for
deleted branches and tags.
The behavior of the following git commands and the last two testcases
have been fixed in t1416:
* git branch -d <branch>
* git tag -d <tag>
A testcase in t5510 is broken because we used to call the function
"packed_refs_lock()", but it is not necessary if the deleted reference
is not in the "packed-refs" file.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
refs/files-backend.c | 21 +++++-----
refs/packed-backend.c | 51 +-----------------------
t/t1416-ref-transaction-hooks.sh | 68 +-------------------------------
t/t5510-fetch.sh | 16 ++++++++
4 files changed, 28 insertions(+), 128 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index e029f5a885..8f3deddc71 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1268,31 +1268,27 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
static int files_delete_refs(struct ref_store *ref_store, const char *msg,
struct string_list *refnames, unsigned int flags)
{
- struct files_ref_store *refs =
- files_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
+ struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
int i, result = 0;
if (!refnames->nr)
return 0;
- if (packed_refs_lock(refs->packed_ref_store, 0, &err))
- goto error;
-
- if (refs_delete_refs(refs->packed_ref_store, msg, refnames, flags)) {
- packed_refs_unlock(refs->packed_ref_store);
+ transaction = ref_store_transaction_begin(ref_store, &err);
+ if (!transaction)
goto error;
- }
-
- packed_refs_unlock(refs->packed_ref_store);
for (i = 0; i < refnames->nr; i++) {
const char *refname = refnames->items[i].string;
-
- if (refs_delete_ref(&refs->base, msg, refname, NULL, flags))
+ if (ref_transaction_delete(transaction, refname, NULL,
+ flags, msg, &err))
result |= error(_("could not remove reference %s"), refname);
}
+ if (ref_transaction_commit(transaction, &err))
+ goto error;
+ ref_transaction_free(transaction);
strbuf_release(&err);
return result;
@@ -1309,6 +1305,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
else
error(_("could not delete references: %s"), err.buf);
+ ref_transaction_free(transaction);
strbuf_release(&err);
return -1;
}
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 97b6837767..fdb7a0a52c 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1519,55 +1519,6 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store,
return ref_transaction_commit(transaction, err);
}
-static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
- struct string_list *refnames, unsigned int flags)
-{
- struct packed_ref_store *refs =
- packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
- struct strbuf err = STRBUF_INIT;
- struct ref_transaction *transaction;
- struct string_list_item *item;
- int ret;
-
- (void)refs; /* We need the check above, but don't use the variable */
-
- if (!refnames->nr)
- return 0;
-
- /*
- * Since we don't check the references' old_oids, the
- * individual updates can't fail, so we can pack all of the
- * updates into a single transaction.
- */
-
- transaction = ref_store_transaction_begin(ref_store, &err);
- if (!transaction)
- return -1;
-
- for_each_string_list_item(item, refnames) {
- if (ref_transaction_delete(transaction, item->string, NULL,
- flags, msg, &err)) {
- warning(_("could not delete reference %s: %s"),
- item->string, err.buf);
- strbuf_reset(&err);
- }
- }
-
- ret = ref_transaction_commit(transaction, &err);
-
- if (ret) {
- if (refnames->nr == 1)
- error(_("could not delete reference %s: %s"),
- refnames->items[0].string, err.buf);
- else
- error(_("could not delete references: %s"), err.buf);
- }
-
- ref_transaction_free(transaction);
- strbuf_release(&err);
- return ret;
-}
-
static int packed_pack_refs(struct ref_store *ref_store, unsigned int flags)
{
/*
@@ -1595,7 +1546,7 @@ struct ref_storage_be refs_be_packed = {
.pack_refs = packed_pack_refs,
.create_symref = NULL,
- .delete_refs = packed_delete_refs,
+ .delete_refs = NULL,
.rename_ref = NULL,
.copy_ref = NULL,
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 77996017d7..3d39e1634a 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -789,39 +789,7 @@ test_expect_success "branch: rename branches" '
test_cmp_heads_and_tags -C workdir expect
'
-# Mismatched hook output for "git branch -d":
-#
-# * The delete branches operation should be treated as one transaction,
-# but was splitted into several transactions on loose references,
-# and the "reference-transaction committed" command was executed
-# redundantly on the packed-ref-store.
-#
-# The differences are as follows:
-#
-# @@ -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
-# +## 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
-# +## 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 prepared ##
-# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
-# <COMMIT-B> <ZERO-OID> refs/heads/topic2
-# -<COMMIT-C> <ZERO-OID> refs/heads/topic3
-# ## Call hook: reference-transaction committed ##
-# -<COMMIT-A> <ZERO-OID> refs/heads/topic1
-# <COMMIT-B> <ZERO-OID> refs/heads/topic2
-# +## Call hook: reference-transaction prepared ##
-# +<COMMIT-C> <ZERO-OID> refs/heads/topic3
-# +## Call hook: reference-transaction committed ##
-# <COMMIT-C> <ZERO-OID> refs/heads/topic3
-test_expect_failure "branch: remove branches" '
+test_expect_success "branch: remove branches" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
@@ -950,39 +918,7 @@ test_expect_success "tag: update refs to create loose refs" '
test_cmp_heads_and_tags -C workdir expect
'
-# Mismatched hook output for "git tag -d":
-#
-# * The delete tags operation should be treated as one transaction,
-# but was splitted into several transactions on loose references,
-# and the "reference-transaction committed" command was executed
-# redundantly on the packed-ref-store.
-#
-# The differences are as follows:
-#
-# @@ -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
-# +## Call hook: reference-transaction committed ##
-# +<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 prepared ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v1
-# +## Call hook: reference-transaction committed ##
-# +<ZERO-OID> <ZERO-OID> refs/tags/v1
-# ## Call hook: reference-transaction prepared ##
-# -<COMMIT-A> <ZERO-OID> refs/tags/v1
-# <COMMIT-B> <ZERO-OID> refs/tags/v2
-# -<COMMIT-C> <ZERO-OID> refs/tags/v3
-# ## Call hook: reference-transaction committed ##
-# -<COMMIT-A> <ZERO-OID> refs/tags/v1
-# <COMMIT-B> <ZERO-OID> refs/tags/v2
-# +## Call hook: reference-transaction prepared ##
-# +<COMMIT-C> <ZERO-OID> refs/tags/v3
-# +## Call hook: reference-transaction committed ##
-# <COMMIT-C> <ZERO-OID> refs/tags/v3
-test_expect_failure "tag: remove tags with mixed ref_stores" '
+test_expect_success "tag: remove tags with mixed ref_stores" '
test_when_finished "rm -f $HOOK_OUTPUT" &&
cat >expect <<-\EOF &&
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index b45879a760..dfdab09600 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -168,6 +168,8 @@ test_expect_success REFFILES 'fetch --prune fails to delete branches' '
cd "$D" &&
git clone . prune-fail &&
cd prune-fail &&
+ git update-ref refs/remotes/origin/extrabranch main~ &&
+ git pack-refs --all &&
git update-ref refs/remotes/origin/extrabranch main &&
: this will prevent --prune from locking packed-refs for deleting refs, but adding loose refs still succeeds &&
>.git/packed-refs.new &&
@@ -175,6 +177,20 @@ test_expect_success REFFILES 'fetch --prune fails to delete branches' '
test_must_fail git fetch --prune origin
'
+test_expect_success REFFILES 'fetch --prune ok for loose refs not in locked packed-refs' '
+ cd "$D" &&
+ git clone . prune-ok-ref-not-packed &&
+ (
+ cd prune-ok-ref-not-packed &&
+ git update-ref refs/remotes/origin/extrabranch main &&
+ : for loose refs not in packed-refs, we can delete them even the packed-refs is locked &&
+ :>.git/packed-refs.new &&
+
+ git fetch --prune origin &&
+ test_must_fail git rev-parse refs/remotes/origin/extrabranch --
+ )
+'
+
test_expect_success 'fetch --atomic works with a single branch' '
test_when_finished "rm -rf \"$D\"/atomic" &&
--
2.36.1.25.gc87d5ad63a.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread