From patchwork Fri Aug 19 03:21:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948259 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 25228C00140 for ; Fri, 19 Aug 2022 03:22:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242366AbiHSDWI (ORCPT ); Thu, 18 Aug 2022 23:22:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34108 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238585AbiHSDV4 (ORCPT ); Thu, 18 Aug 2022 23:21:56 -0400 Received: from mail-pj1-x102d.google.com (mail-pj1-x102d.google.com [IPv6:2607:f8b0:4864:20::102d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B7732D7D3C for ; Thu, 18 Aug 2022 20:21:53 -0700 (PDT) Received: by mail-pj1-x102d.google.com with SMTP id o5-20020a17090a3d4500b001ef76490983so3663603pjf.2 for ; Thu, 18 Aug 2022 20:21:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=40j6Ki2xp0sY1M4IqH9kdHAo8c/JjWYdgRK/xZjlVaE=; b=BTR9HubTszmmad7VRKSLU/asT2tk4051vGkvAsMhsn7FggezWYBUKh0YD45F307lFk 3PnjJZfx8ID/90p86OB4MFiTMI1VBC6y93IJqEULt/B03PFKflgoDtE4hQ05bq0edgOq Lnkf/UKugB+wPlnjdFoSS4NsPdNWrgwzexlRl9FXSxN9IsC1eCLsSVTEJtqORJM4KNVi 2j3FJAK1GbHBoK2mH5EJdWMBBMgOxBhGggSgGc6OneKbAAEyX5KJztxYRxgJo/loRc6V R1Ij1uL+X8HCSSAAUiB4liSIS2XpsK2wAOJZiG8Y3R+KN7r4XR7nB03LtV+i6BSYx3+A mcLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=40j6Ki2xp0sY1M4IqH9kdHAo8c/JjWYdgRK/xZjlVaE=; b=y43E7260XcRtRCwu0n1rOGk9jVKbtQrRL6qPEIISmXFzgO6RQDuEoOYQP/G6bvJPYL 4uDpUbcLzKzhvZ2SntX5ZL3S1iyLwY3l/zw05VytgVme2IgB03N6tqAKgd8G/fMZQ0ER 0QAL9zKF9vG6K8WlGD7ETy9wfYWRKhmqJN5LEcmjmVqQM681hfG2hP7JzRb8SE/Y2nAt J1Gkne2SScVX2sWgrlmisHGE38eFg8JwdJdxRJftSQSXn401E33uyReEZci1PMmepPMP wOmBhxeuaXpj4cd/tvCds4Kdw+RHoQx2W+RODRGeRh3hXV6ncs6gLqn2M2IKwdOURwkV 0SGA== X-Gm-Message-State: ACgBeo2d3gAj6Pw3/sAwByVlRZLTwqbVNGbR+fcUaIe7Q4N7KtM4gAUU PBqdZXrQT6DvX4j/+twBqfo= X-Google-Smtp-Source: AA6agR7ErEKAR7/ePX3KXajJYURKK9bTMlm5RLsdA72dzPC8JK/M3XgrcuPISSqiYDLApR1SNheKug== X-Received: by 2002:a17:90b:1e0c:b0:1f5:4e52:4866 with SMTP id pg12-20020a17090b1e0c00b001f54e524866mr11964532pjb.230.1660879312918; Thu, 18 Aug 2022 20:21:52 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.51 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:21:52 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin , Eric Sunshine Subject: [PATCH v2 1/9] t1416: more testcases for reference-transaction hook Date: Fri, 19 Aug 2022 11:21:39 +0800 Message-Id: <20220819032147.28841-2-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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 # create new branch * git clone * git commit [--amend] * git fetch * git merge * git push [--atomic] * git reset --hard * git tag [-f] # update tag * git tag # create new tag * git update-ref --stdin # create new refs * git update-ref # create new ref * git update-ref # update ref But 17 testcases failed. Some commands failed because the expected "" became "". E.g.: * git branch [-f] # update branch * git cherry-pick * git rebase * git tag -d * git update-ref --stdin # update/delete refs * git update-ref -d * git update-ref # 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 * git update-ref -d * 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 # copy branch * git branch -m # rename branch Some commands ran unexpected command "reference-transaction aborted". E.g.: * git branch -d # delete branch * git branch -m # rename branch * git cherr-pick * git rebase * git revert * git tag -d # delete tag * git update-ref -d # delete ref We will fix the failed testcases in later commits. Signed-off-by: Jiang Xin Helped-by: Eric Sunshine --- 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 and assign each commit's oid to shell variables +# given in the arguments (A, B, and C). E.g.: +# +# create_commits_in 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]*//g" \ + -e "s/$(get_abbrev_oid $B)[0-9a-f]*//g" \ + -e "s/$(get_abbrev_oid $C)[0-9a-f]*//g" \ + -e "s/$(get_abbrev_oid $D)[0-9a-f]*//g" \ + -e "s/$(get_abbrev_oid $E)[0-9a-f]*//g" \ + -e "s/$(get_abbrev_oid $F)[0-9a-f]*//g" \ + -e "s/$(get_abbrev_oid $G)[0-9a-f]*//g" \ + -e "s/$(get_abbrev_oid $H)[0-9a-f]*//g" \ + -e "s/$(get_abbrev_oid $I)[0-9a-f]*//g" \ + -e "s/$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 ## + HEAD + refs/heads/main + ## Call hook: reference-transaction committed ## + HEAD + refs/heads/main + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/main + ## Call hook: reference-transaction committed ## + HEAD + refs/heads/main + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/main + ## Call hook: reference-transaction committed ## + HEAD + 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 && + 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 ## + refs/remotes/origin/main + ## Call hook: reference-transaction committed ## + refs/remotes/origin/main + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/main + ## Call hook: reference-transaction committed ## + HEAD + refs/heads/main + EOF + + git clone base workdir && + make_user_friendly_and_stable_output <$HOOK_OUTPUT >actual && + test_cmp expect actual && + + cat >expect <<-\EOF && + 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 ## + refs/heads/topic1 + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + ## Call hook: reference-transaction prepared ## + refs/heads/topic2 + ## Call hook: reference-transaction committed ## + refs/heads/topic2 + ## Call hook: reference-transaction prepared ## + refs/heads/topic3 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + 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 . +# +# The differences are as follows: +# +# @@ -5,8 +5,8 @@ +# refs/heads/topic1 +# HEAD +# ## Call hook: reference-transaction prepared ## +# - refs/heads/topic1 +# - HEAD +# + refs/heads/topic1 +# + HEAD +# ## Call hook: reference-transaction committed ## +# - refs/heads/topic1 +# - HEAD +# + refs/heads/topic1 +# + 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 ## + refs/heads/topic1 + HEAD + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + HEAD + ## Call hook: reference-transaction prepared ## + refs/heads/topic1 + HEAD + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + 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 . +# +# The differences are as follows: +# +# @@ -5,8 +5,8 @@ +# HEAD +# refs/heads/topic1 +# ## Call hook: reference-transaction prepared ## +# - HEAD +# - refs/heads/topic1 +# + HEAD +# + refs/heads/topic1 +# ## Call hook: reference-transaction committed ## +# - HEAD +# - refs/heads/topic1 +# + HEAD +# + 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 ## + HEAD + refs/heads/topic1 + ## Call hook: reference-transaction committed ## + HEAD + refs/heads/topic1 + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/topic1 + ## Call hook: reference-transaction committed ## + HEAD + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + 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 +# 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 ## +# refs/heads/topic2 +# ## Call hook: reference-transaction prepared ## +# - refs/heads/topic3 +# + refs/heads/topic3 +# ## Call hook: reference-transaction committed ## +# - refs/heads/topic3 +# + refs/heads/topic3 +# ## Call hook: reference-transaction prepared ## +# refs/heads/topic4 +# ## Call hook: reference-transaction committed ## +# refs/heads/topic4 +# ## Call hook: reference-transaction prepared ## +# - refs/heads/topic4 +# + refs/heads/topic4 +# ## Call hook: reference-transaction committed ## +# - refs/heads/topic4 +# + 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 ## + refs/heads/topic2 + ## Call hook: reference-transaction committed ## + refs/heads/topic2 + ## Call hook: reference-transaction prepared ## + refs/heads/topic3 + ## Call hook: reference-transaction committed ## + refs/heads/topic3 + ## Call hook: reference-transaction prepared ## + refs/heads/topic4 + ## Call hook: reference-transaction committed ## + refs/heads/topic4 + ## Call hook: reference-transaction prepared ## + refs/heads/topic4 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + 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 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 @@ +# refs/heads/topic1 +# HEAD +# ## Call hook: reference-transaction committed ## +# + refs/heads/topic1 +# +## Call hook: reference-transaction committed ## +# refs/heads/topic1 +# HEAD +# ## Call hook: reference-transaction prepared ## +# @@ -11,14 +13,20 @@ +# ## Call hook: reference-transaction prepared ## +# refs/heads/topic2 +# ## Call hook: reference-transaction committed ## +# + refs/heads/topic2 +# +## Call hook: reference-transaction committed ## +# refs/heads/topic2 +# ## Call hook: reference-transaction prepared ## +# refs/heads/topic3 +# ## Call hook: reference-transaction prepared ## +# - refs/heads/topic3 +# + refs/heads/topic3 +# ## Call hook: reference-transaction committed ## +# - refs/heads/topic3 +# + refs/heads/topic3 +# +## Call hook: reference-transaction committed ## +# + refs/heads/topic3 +# +## Call hook: reference-transaction aborted ## +# + refs/heads/topic4 +# ## Call hook: reference-transaction prepared ## +# - refs/heads/topic4 +# + refs/heads/topic4 +# ## Call hook: reference-transaction committed ## +# - refs/heads/topic4 +# + 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 ## + refs/heads/topic1 + ## Call hook: reference-transaction prepared ## + refs/heads/topic1 + HEAD + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + HEAD + ## Call hook: reference-transaction prepared ## + refs/heads/topic2 + ## Call hook: reference-transaction prepared ## + refs/heads/topic2 + ## Call hook: reference-transaction committed ## + refs/heads/topic2 + ## Call hook: reference-transaction prepared ## + refs/heads/topic3 + ## Call hook: reference-transaction prepared ## + refs/heads/topic3 + ## Call hook: reference-transaction committed ## + refs/heads/topic3 + ## Call hook: reference-transaction prepared ## + refs/heads/topic4 + ## Call hook: reference-transaction committed ## + 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 && + 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 ## + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + HEAD + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + 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 +# 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 ## +# refs/heads/topic2 +# - refs/heads/topic3 +# + refs/heads/topic3 +# refs/heads/topic4 +# ## Call hook: reference-transaction committed ## +# refs/heads/topic2 +# - refs/heads/topic3 +# + refs/heads/topic3 +# 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 ## + refs/heads/topic2 + refs/heads/topic3 + refs/heads/topic4 + ## Call hook: reference-transaction committed ## + refs/heads/topic2 + refs/heads/topic3 + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + 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 when +# deleting a ref without providing the old-oid parameter. +# +# The differences are as follows: +# +# @@ -4,14 +4,19 @@ +# refs/heads/topic3 +# refs/heads/topic4 +# ## Call hook: reference-transaction prepared ## +# - refs/heads/topic1 +# + refs/heads/topic1 +# refs/heads/topic2 +# - refs/heads/topic3 +# - refs/heads/topic4 +# - HEAD +# + refs/heads/topic3 +# + refs/heads/topic4 +# + HEAD +# ## Call hook: reference-transaction committed ## +# - refs/heads/topic1 +# + refs/heads/topic1 +# + refs/heads/topic2 +# + refs/heads/topic3 +# + refs/heads/topic4 +# +## Call hook: reference-transaction committed ## +# + refs/heads/topic1 +# refs/heads/topic2 +# - refs/heads/topic3 +# - refs/heads/topic4 +# - HEAD +# + refs/heads/topic3 +# + refs/heads/topic4 +# + HEAD +test_expect_failure "update-ref --stdin: delete refs" ' + test_when_finished "rm -f $HOOK_OUTPUT" && + + cat >expect <<-\EOF && + ## Call hook: reference-transaction prepared ## + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + refs/heads/topic4 + ## Call hook: reference-transaction prepared ## + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + refs/heads/topic4 + HEAD + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + refs/heads/topic4 + 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 && + 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 ## + refs/remotes/origin/main + ## Call hook: reference-transaction committed ## + 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 ## + refs/heads/main + HEAD + ## Call hook: reference-transaction committed ## + refs/heads/main + 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 && + 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 ## + refs/heads/topic1 + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + ## Call hook: reference-transaction prepared ## + refs/heads/topic2 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + 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 +# 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 ## +# - refs/heads/topic2 +# + refs/heads/topic2 +# ## Call hook: reference-transaction committed ## +# - refs/heads/topic2 +# + refs/heads/topic2 +# ## Call hook: reference-transaction prepared ## +# 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 ## + refs/heads/topic2 + ## Call hook: reference-transaction committed ## + refs/heads/topic2 + ## Call hook: reference-transaction prepared ## + refs/heads/topic3 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + 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 ## + refs/heads/topic4 + ## Call hook: reference-transaction committed ## + refs/heads/topic4 + ## Call hook: reference-transaction prepared ## + refs/heads/topic5 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + refs/heads/topic4 + 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 ## +# + refs/heads/topic4 +# ## Call hook: reference-transaction prepared ## +# refs/heads/topic4 +# ## Call hook: reference-transaction committed ## +# refs/heads/topic4 +# -## Call hook: reference-transaction prepared ## +# - refs/heads/topic6 +# -## Call hook: reference-transaction committed ## +# - refs/heads/topic6 +# +## Call hook: reference-transaction aborted ## +# + refs/heads/topic5 +# ## Call hook: reference-transaction prepared ## +# refs/heads/topic5 +# ## Call hook: reference-transaction committed ## +# refs/heads/topic5 +# -## Call hook: reference-transaction prepared ## +# - refs/heads/topic7 +# -## Call hook: reference-transaction committed ## +# - refs/heads/topic7 +test_expect_failure "branch: rename branches" ' + test_when_finished "rm -f $HOOK_OUTPUT" && + + cat >expect <<-\EOF && + ## Call hook: reference-transaction prepared ## + refs/heads/topic4 + ## Call hook: reference-transaction committed ## + refs/heads/topic4 + ## Call hook: reference-transaction prepared ## + refs/heads/topic6 + ## Call hook: reference-transaction committed ## + refs/heads/topic6 + ## Call hook: reference-transaction prepared ## + refs/heads/topic5 + ## Call hook: reference-transaction committed ## + refs/heads/topic5 + ## Call hook: reference-transaction prepared ## + refs/heads/topic7 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + refs/heads/topic6 + 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 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 @@ +# refs/heads/topic1 +# refs/heads/topic2 +# refs/heads/topic3 +# +## Call hook: reference-transaction committed ## +# + refs/heads/topic1 +# + refs/heads/topic2 +# + refs/heads/topic3 +# +## Call hook: reference-transaction aborted ## +# + refs/heads/topic1 +# ## Call hook: reference-transaction prepared ## +# - refs/heads/topic1 +# - refs/heads/topic2 +# - refs/heads/topic3 +# + refs/heads/topic1 +# ## Call hook: reference-transaction committed ## +# - refs/heads/topic1 +# - refs/heads/topic2 +# - refs/heads/topic3 +# + refs/heads/topic1 +# +## Call hook: reference-transaction aborted ## +# + refs/heads/topic2 +# +## Call hook: reference-transaction prepared ## +# + refs/heads/topic2 +# +## Call hook: reference-transaction committed ## +# + refs/heads/topic2 +# +## Call hook: reference-transaction aborted ## +# + refs/heads/topic3 +# +## Call hook: reference-transaction prepared ## +# + refs/heads/topic3 +# +## Call hook: reference-transaction committed ## +# + refs/heads/topic3 +test_expect_failure "branch: remove branches" ' + test_when_finished "rm -f $HOOK_OUTPUT" && + + cat >expect <<-\EOF && + ## Call hook: reference-transaction prepared ## + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + ## Call hook: reference-transaction prepared ## + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + refs/heads/topic2 + 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 && + refs/heads/main + refs/heads/topic6 + 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 ## + refs/heads/main + HEAD + ## Call hook: reference-transaction committed ## + refs/heads/main + 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 && + 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 ## + refs/tags/v1 + ## Call hook: reference-transaction committed ## + refs/tags/v1 + ## Call hook: reference-transaction prepared ## + refs/tags/v2 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/tags/v1 + 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 ## + refs/tags/v2 + ## Call hook: reference-transaction committed ## + refs/tags/v2 + ## Call hook: reference-transaction prepared ## + refs/tags/v3 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/tags/v1 + refs/tags/v2 + 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 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 @@ +# refs/tags/v1 +# refs/tags/v2 +# refs/tags/v3 +# +## Call hook: reference-transaction committed ## +# + refs/tags/v1 +# + refs/tags/v2 +# + refs/tags/v3 +# +## Call hook: reference-transaction aborted ## +# + refs/tags/v1 +# ## Call hook: reference-transaction prepared ## +# - refs/tags/v1 +# - refs/tags/v2 +# - refs/tags/v3 +# + refs/tags/v1 +# ## Call hook: reference-transaction committed ## +# - refs/tags/v1 +# - refs/tags/v2 +# - refs/tags/v3 +# + refs/tags/v1 +# +## Call hook: reference-transaction aborted ## +# + refs/tags/v2 +# +## Call hook: reference-transaction prepared ## +# + refs/tags/v2 +# +## Call hook: reference-transaction committed ## +# + refs/tags/v2 +# +## Call hook: reference-transaction aborted ## +# + refs/tags/v3 +# +## Call hook: reference-transaction prepared ## +# + refs/tags/v3 +# +## Call hook: reference-transaction committed ## +# + 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 ## + refs/tags/v1 + refs/tags/v2 + refs/tags/v3 + ## Call hook: reference-transaction prepared ## + refs/tags/v1 + refs/tags/v2 + refs/tags/v3 + ## Call hook: reference-transaction committed ## + refs/tags/v1 + refs/tags/v2 + 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 && + 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 ## + refs/heads/main + HEAD + ## Call hook: reference-transaction committed ## + refs/heads/main + 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 ## + refs/remotes/origin/main + ## Call hook: reference-transaction committed ## + refs/remotes/origin/main + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/main + ## Call hook: reference-transaction committed ## + HEAD + 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 && + 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 ## + refs/heads/topic1 + ## Call hook: reference-transaction committed ## + refs/heads/topic1 + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/topic1 + ## Call hook: reference-transaction committed ## + HEAD + 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 && + refs/heads/main + 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 ## + refs/heads/topic2 + ## Call hook: reference-transaction committed ## + refs/heads/topic2 + ## Call hook: reference-transaction prepared ## + ORIG_HEAD + ## Call hook: reference-transaction committed ## + ORIG_HEAD + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/topic2 + ## Call hook: reference-transaction committed ## + HEAD + 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 && + refs/heads/main + refs/heads/topic1 + 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 . +# +# * Unexpected execution of the "reference-transaction abort" command. +# +# The differences are as follows: +# +# @@ -12,7 +12,9 @@ +# ## Call hook: reference-transaction committed ## +# HEAD +# refs/heads/topic3 +# +## Call hook: reference-transaction aborted ## +# + CHERRY_PICK_HEAD +# ## Call hook: reference-transaction prepared ## +# - CHERRY_PICK_HEAD +# + CHERRY_PICK_HEAD +# ## Call hook: reference-transaction committed ## +# - CHERRY_PICK_HEAD +# + 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 ## + refs/heads/topic3 + ## Call hook: reference-transaction committed ## + refs/heads/topic3 + ## Call hook: reference-transaction prepared ## + CHERRY_PICK_HEAD + ## Call hook: reference-transaction committed ## + CHERRY_PICK_HEAD + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/topic3 + ## Call hook: reference-transaction committed ## + HEAD + refs/heads/topic3 + ## Call hook: reference-transaction prepared ## + CHERRY_PICK_HEAD + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + 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 . +# +# * Unexpected execution of the "reference-transaction abort" command. +# +# The differences are as follows: +# +# @@ -6,6 +6,8 @@ +# HEAD +# ## Call hook: reference-transaction committed ## +# HEAD +# +## Call hook: reference-transaction aborted ## +# + REBASE_HEAD +# ## Call hook: reference-transaction prepared ## +# REBASE_HEAD +# ## Call hook: reference-transaction committed ## +# @@ -18,10 +20,12 @@ +# HEAD +# ## Call hook: reference-transaction committed ## +# HEAD +# +## Call hook: reference-transaction aborted ## +# + CHERRY_PICK_HEAD +# ## Call hook: reference-transaction prepared ## +# - CHERRY_PICK_HEAD +# + CHERRY_PICK_HEAD +# ## Call hook: reference-transaction committed ## +# - CHERRY_PICK_HEAD +# + CHERRY_PICK_HEAD +# ## Call hook: reference-transaction prepared ## +# 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 ## + ORIG_HEAD + ## Call hook: reference-transaction committed ## + ORIG_HEAD + ## Call hook: reference-transaction prepared ## + HEAD + ## Call hook: reference-transaction committed ## + HEAD + ## Call hook: reference-transaction prepared ## + REBASE_HEAD + ## Call hook: reference-transaction committed ## + REBASE_HEAD + ## Call hook: reference-transaction prepared ## + CHERRY_PICK_HEAD + ## Call hook: reference-transaction committed ## + CHERRY_PICK_HEAD + ## Call hook: reference-transaction prepared ## + HEAD + ## Call hook: reference-transaction committed ## + HEAD + ## Call hook: reference-transaction prepared ## + CHERRY_PICK_HEAD + ## Call hook: reference-transaction committed ## + CHERRY_PICK_HEAD + ## Call hook: reference-transaction prepared ## + refs/heads/topic4 + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + 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 ## +# HEAD +# refs/heads/topic5 +# +## Call hook: reference-transaction aborted ## +# + CHERRY_PICK_HEAD +# ## Call hook: reference-transaction prepared ## +# 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 ## + refs/heads/topic5 + ## Call hook: reference-transaction committed ## + refs/heads/topic5 + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/topic5 + ## Call hook: reference-transaction committed ## + HEAD + refs/heads/topic5 + ## Call hook: reference-transaction prepared ## + CHERRY_PICK_HEAD + ## Call hook: reference-transaction committed ## + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + refs/heads/topic4 + 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 ## + refs/heads/topic6 + ## Call hook: reference-transaction committed ## + refs/heads/topic6 + ## Call hook: reference-transaction prepared ## + ORIG_HEAD + ## Call hook: reference-transaction committed ## + ORIG_HEAD + ## Call hook: reference-transaction prepared ## + HEAD + refs/heads/topic6 + ## Call hook: reference-transaction committed ## + HEAD + 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 && + refs/heads/main + refs/heads/topic1 + refs/heads/topic2 + refs/heads/topic3 + refs/heads/topic4 + refs/heads/topic5 + refs/heads/topic6 + EOF + test_cmp_heads_and_tags -C workdir expect +' + test_done From patchwork Fri Aug 19 03:21:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948257 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E29C9C28B2B for ; Fri, 19 Aug 2022 03:22:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241927AbiHSDWC (ORCPT ); Thu, 18 Aug 2022 23:22:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34100 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235865AbiHSDV4 (ORCPT ); Thu, 18 Aug 2022 23:21:56 -0400 Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8C307D8E0B for ; Thu, 18 Aug 2022 20:21:54 -0700 (PDT) Received: by mail-pj1-x1035.google.com with SMTP id s3-20020a17090a2f0300b001facfc6fdbcso2991633pjd.1 for ; Thu, 18 Aug 2022 20:21:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=5jmcH3OZDb8XZiXAdR7eHucA5ADcneu4RIWfy5rinAo=; b=CglJ66+iRWbXXiv9JsoLr5MsZZjQ19Xs/phEsoG0hcb8B4PQzV3gpIx5Mn/NY1EShi nZxf2CWbiLQ7WbArnay8D+IoBI9zvDLTzlaqW8Dfsj4s1EfKaUHaPVRYWf9/q82L2QMx +TgAYdurFS782ViB1B6AMmyMRPv1hjuGrnD/hdrwJo4alLVD87b/CqzMZeddSin2H9ZH 13jNMTtA4pMckPVeDYg6rgY5sErU5IX6Kbl+PBSEH5zsyQ6EJDX7tcYA6/9l+LEqX2mS LrlMM2yuw4xSzY9uIOcrWesaSBAwEq3b8qD0o6uhs/QrZwiHl+HgfFKHXmFpuT7xfhtB HzyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=5jmcH3OZDb8XZiXAdR7eHucA5ADcneu4RIWfy5rinAo=; b=uSLqCzKSd+AfDVwFnL7J8Hz6jP4WBN7UxOgrQkyrsJkjlyAiDF8KWMbGhghGjhIdfJ prOioBRzdCnavdIVoi6xkeN1g4tAYXZfBKU5DNcskapQBQFBNC+8DNxntIXHkjJFb9EB K8za5NGHB1rmFLk64VsKQkTNqZ1m+z3oefucwe+JoZIYitLH5gb2iekxxsrSWdFiA/7P nN9EJNcaBlVlMJiRoM22yp1LB19j57EdKHaHIFvT51F+9axSu6ACrxLDXDrueInl/9ZY a+Abcrk4niUEmzI+WEIgw5v1YN/eru30JpEL5V1K2OGRTKSFv8/ISzi7qsNURPKNSRSg EiCw== X-Gm-Message-State: ACgBeo1VUFelb//qg2QckKU7ldLac/HoeiT8Ud0gs9kQPWw5NNc6wKWc OzRNOYb0lV/Pxz2QGH5knY8= X-Google-Smtp-Source: AA6agR7rlh4ktu2xodDljqxQHmRyULY8MPAYoh72gWdduV62vX8LcebBOWZl9CELznQIRCB9M8Bz7Q== X-Received: by 2002:a17:902:d2c1:b0:16e:ea4e:36d3 with SMTP id n1-20020a170902d2c100b0016eea4e36d3mr5297849plc.98.1660879313930; Thu, 18 Aug 2022 20:21:53 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.53 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:21:53 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin Subject: [PATCH v2 2/9] refs: update missing old-oid in transaction from lockfile Date: Fri, 19 Aug 2022 11:21:40 +0800 Message-Id: <20220819032147.28841-3-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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] # update branch * git cherry-pick * git rebase * git tag -d * git update-ref --stdin # update refs * git update-ref -d * git update-ref # update ref Signed-off-by: Jiang Xin --- 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 . -# -# The differences are as follows: -# -# @@ -5,8 +5,8 @@ -# refs/heads/topic1 -# HEAD -# ## Call hook: reference-transaction prepared ## -# - refs/heads/topic1 -# - HEAD -# + refs/heads/topic1 -# + HEAD -# ## Call hook: reference-transaction committed ## -# - refs/heads/topic1 -# - HEAD -# + refs/heads/topic1 -# + 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 . -# -# The differences are as follows: -# -# @@ -5,8 +5,8 @@ -# HEAD -# refs/heads/topic1 -# ## Call hook: reference-transaction prepared ## -# - HEAD -# - refs/heads/topic1 -# + HEAD -# + refs/heads/topic1 -# ## Call hook: reference-transaction committed ## -# - HEAD -# - refs/heads/topic1 -# + HEAD -# + 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 -# 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 ## -# refs/heads/topic2 -# ## Call hook: reference-transaction prepared ## -# - refs/heads/topic3 -# + refs/heads/topic3 -# ## Call hook: reference-transaction committed ## -# - refs/heads/topic3 -# + refs/heads/topic3 -# ## Call hook: reference-transaction prepared ## -# refs/heads/topic4 -# ## Call hook: reference-transaction committed ## -# refs/heads/topic4 -# ## Call hook: reference-transaction prepared ## -# - refs/heads/topic4 -# + refs/heads/topic4 -# ## Call hook: reference-transaction committed ## -# - refs/heads/topic4 -# + 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 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" ' # refs/heads/topic1 # HEAD # ## Call hook: reference-transaction prepared ## -# @@ -11,14 +13,20 @@ +# @@ -11,13 +13,19 @@ # ## Call hook: reference-transaction prepared ## # 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 ## # refs/heads/topic3 # ## Call hook: reference-transaction prepared ## -# - refs/heads/topic3 -# + refs/heads/topic3 +# refs/heads/topic3 # ## Call hook: reference-transaction committed ## -# - refs/heads/topic3 # + refs/heads/topic3 # +## Call hook: reference-transaction committed ## -# + refs/heads/topic3 +# refs/heads/topic3 # +## Call hook: reference-transaction aborted ## # + refs/heads/topic4 # ## Call hook: reference-transaction prepared ## -# - refs/heads/topic4 -# + refs/heads/topic4 +# refs/heads/topic4 # ## Call hook: reference-transaction committed ## -# - refs/heads/topic4 -# + 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 -# 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 ## -# refs/heads/topic2 -# - refs/heads/topic3 -# + refs/heads/topic3 -# refs/heads/topic4 -# ## Call hook: reference-transaction committed ## -# refs/heads/topic2 -# - refs/heads/topic3 -# + refs/heads/topic3 -# 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 when -# deleting a ref without providing the old-oid parameter. -# # The differences are as follows: # -# @@ -4,14 +4,19 @@ -# refs/heads/topic3 -# refs/heads/topic4 -# ## Call hook: reference-transaction prepared ## -# - refs/heads/topic1 -# + refs/heads/topic1 -# refs/heads/topic2 -# - refs/heads/topic3 -# - refs/heads/topic4 -# - HEAD -# + refs/heads/topic3 -# + refs/heads/topic4 -# + HEAD +# @@ -10,6 +10,11 @@ +# refs/heads/topic4 +# HEAD # ## Call hook: reference-transaction committed ## -# - refs/heads/topic1 # + refs/heads/topic1 # + refs/heads/topic2 # + refs/heads/topic3 # + refs/heads/topic4 # +## Call hook: reference-transaction committed ## -# + refs/heads/topic1 +# refs/heads/topic1 # refs/heads/topic2 -# - refs/heads/topic3 -# - refs/heads/topic4 -# - HEAD -# + refs/heads/topic3 -# + refs/heads/topic4 -# + HEAD +# 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 -# 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 ## -# - refs/heads/topic2 -# + refs/heads/topic2 -# ## Call hook: reference-transaction committed ## -# - refs/heads/topic2 -# + refs/heads/topic2 -# ## Call hook: reference-transaction prepared ## -# 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 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" ' # + refs/heads/topic3 # +## Call hook: reference-transaction aborted ## # + refs/heads/topic1 +# +## Call hook: reference-transaction prepared ## +# + refs/heads/topic1 +# +## Call hook: reference-transaction committed ## +# + refs/heads/topic1 +# +## Call hook: reference-transaction aborted ## +# + refs/heads/topic2 # ## Call hook: reference-transaction prepared ## # - refs/heads/topic1 -# - refs/heads/topic2 +# refs/heads/topic2 # - refs/heads/topic3 -# + refs/heads/topic1 # ## Call hook: reference-transaction committed ## # - refs/heads/topic1 -# - refs/heads/topic2 -# - refs/heads/topic3 -# + refs/heads/topic1 -# +## Call hook: reference-transaction aborted ## -# + refs/heads/topic2 -# +## Call hook: reference-transaction prepared ## -# + refs/heads/topic2 -# +## Call hook: reference-transaction committed ## -# + refs/heads/topic2 +# refs/heads/topic2 # +## Call hook: reference-transaction aborted ## # + refs/heads/topic3 # +## Call hook: reference-transaction prepared ## -# + refs/heads/topic3 +# + refs/heads/topic3 # +## Call hook: reference-transaction committed ## -# + refs/heads/topic3 +# 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 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" ' # + refs/tags/v3 # +## Call hook: reference-transaction aborted ## # + refs/tags/v1 +# +## Call hook: reference-transaction prepared ## +# + refs/tags/v1 +# +## Call hook: reference-transaction committed ## +# + refs/tags/v1 +# +## Call hook: reference-transaction aborted ## +# + refs/tags/v2 # ## Call hook: reference-transaction prepared ## # - refs/tags/v1 -# - refs/tags/v2 +# refs/tags/v2 # - refs/tags/v3 -# + refs/tags/v1 # ## Call hook: reference-transaction committed ## # - refs/tags/v1 -# - refs/tags/v2 -# - refs/tags/v3 -# + refs/tags/v1 -# +## Call hook: reference-transaction aborted ## -# + refs/tags/v2 -# +## Call hook: reference-transaction prepared ## -# + refs/tags/v2 -# +## Call hook: reference-transaction committed ## -# + refs/tags/v2 +# refs/tags/v2 # +## Call hook: reference-transaction aborted ## # + refs/tags/v3 # +## Call hook: reference-transaction prepared ## -# + refs/tags/v3 +# + refs/tags/v3 # +## Call hook: reference-transaction committed ## -# + refs/tags/v3 +# 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 . -# # * 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 ## # HEAD # refs/heads/topic3 # +## Call hook: reference-transaction aborted ## # + CHERRY_PICK_HEAD # ## Call hook: reference-transaction prepared ## -# - CHERRY_PICK_HEAD -# + CHERRY_PICK_HEAD +# CHERRY_PICK_HEAD # ## Call hook: reference-transaction committed ## -# - CHERRY_PICK_HEAD -# + 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 . -# # * 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 ## # REBASE_HEAD # ## Call hook: reference-transaction committed ## -# @@ -18,10 +20,12 @@ +# @@ -18,6 +20,8 @@ # HEAD # ## Call hook: reference-transaction committed ## # HEAD # +## Call hook: reference-transaction aborted ## # + CHERRY_PICK_HEAD # ## Call hook: reference-transaction prepared ## -# - CHERRY_PICK_HEAD -# + CHERRY_PICK_HEAD -# ## Call hook: reference-transaction committed ## -# - CHERRY_PICK_HEAD -# + CHERRY_PICK_HEAD -# ## Call hook: reference-transaction prepared ## -# refs/heads/topic4 +# CHERRY_PICK_HEAD # ## Call hook: reference-transaction committed ## test_expect_failure "worktree: topic4: rebase" ' test_when_finished "rm -f $HOOK_OUTPUT" && From patchwork Fri Aug 19 03:21:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948258 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2550C00140 for ; Fri, 19 Aug 2022 03:22:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241054AbiHSDWE (ORCPT ); Thu, 18 Aug 2022 23:22:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34106 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238548AbiHSDV4 (ORCPT ); Thu, 18 Aug 2022 23:21:56 -0400 Received: from mail-pg1-x533.google.com (mail-pg1-x533.google.com [IPv6:2607:f8b0:4864:20::533]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 67CA8C7FB0 for ; Thu, 18 Aug 2022 20:21:55 -0700 (PDT) Received: by mail-pg1-x533.google.com with SMTP id r69so2768497pgr.2 for ; Thu, 18 Aug 2022 20:21:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=zUJxeVBSLQhBv1hy0UZ1JJHVb3nKuQodY61E0cnhUOg=; b=prnmAvc2ODuJP1vPz/w+Nsc63ZVD65ZCd2QxRdbP0IhRF9TYrAcpsikE3AjH0kdWBt TDNfnc567zNpEBJ33zhUNz7XwthQ2K9Bcimkpb4BNnxjuja91DiTZaz88b5hjV64j3v/ aTIaGc0agOtFjbF9G/3lsAUCxPevEcPxhFCWdn3FQUeq+kKfWF2w0/sjvca9t68JrV+u bACQ1/0NL+rfWAkpoYsF+orqeSg0iUmQ+6pTOGG05YWKiBtod8jUDjJXBPb4T9EyXTKJ gHR4Rne3WXF8wTj5rw6B7U4OZMp2B0kuIYXKPGM4TWtuChC2eEDzhcESZhQeI25c3cKm HUpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=zUJxeVBSLQhBv1hy0UZ1JJHVb3nKuQodY61E0cnhUOg=; b=4crgjy5VAxAcBZa6aIHR3KL0ev1KBlCMlSVaLfJAHlhKYnqAzRAqCJcv8PBjWhyNWJ 195gJrekfjCADMJwXJ4XQOPRqrcgYMS/OX8WFwLJm4XjmzQpMfIXnBkj+1NBDEe6OyUU xmQDbEViUh6yKzBBvilWJXulsZxcDlDAk3xwNobBVzpRCQd7TSgkrn7Sz5yeTrkw6O8z hN49xbz8GHCITJxY6oXf7cvLZ651gA0XZKvkM+Zs+4b0x65TNGRUtuDZaf0Y8Rf4UQSg p6LvOgzf9Ke6769RyJQLJDmiYB6pbRFZk7dcTFHXaaWDUUZcT1UYpZ5JGIMBA9UdN9bT cg2A== X-Gm-Message-State: ACgBeo1XVbx8i6VC923142Xkw13o7oJ6ob1AiSuftD9Geo/Q6g9HfFmR R4Xz3gOXIRQtnbUmANkaiHFD2K7PZns= X-Google-Smtp-Source: AA6agR7AQf1Immh4lixSZzB8pxCzCYwbHEVHXtPC97XCma59POgXbxIR46cQJaMO/whgaNPUX8uDlQ== X-Received: by 2002:a65:5847:0:b0:42a:df5:1035 with SMTP id s7-20020a655847000000b0042a0df51035mr3755227pgr.248.1660879314873; Thu, 18 Aug 2022 20:21:54 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.54 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:21:54 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin Subject: [PATCH v2 3/9] refs: add new field in transaction for running transaction hook Date: Fri, 19 Aug 2022 11:21:41 +0800 Message-Id: <20220819032147.28841-4-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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 --- 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; }; From patchwork Fri Aug 19 03:21:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948260 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0A3E7C3F6B0 for ; Fri, 19 Aug 2022 03:22:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241311AbiHSDWL (ORCPT ); Thu, 18 Aug 2022 23:22:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34110 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238802AbiHSDV5 (ORCPT ); Thu, 18 Aug 2022 23:21:57 -0400 Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4D598D91F6 for ; Thu, 18 Aug 2022 20:21:56 -0700 (PDT) Received: by mail-pj1-x102f.google.com with SMTP id ds12-20020a17090b08cc00b001fae6343d9fso238078pjb.0 for ; Thu, 18 Aug 2022 20:21:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=+7Qt1Vbtb4K4qoLiXFWPZtfwYiT1v2vtwDp2473VM3g=; b=WE6ZIJ0XzF6mZdbEcX5A5xnRQkIdY1g8U2v6kQNvJqmwFl7ISWvNxkGi99WQe8f7e7 jGodcoYcI8hpqltfmqVfxaQy1Bzzt32q5mbIhaRHOHnJnao2R4o6l9/ugJUiamBOGHKU EDhQvh4/Twc53w8Y/gRxODQ0gI5kHwH6zvq2a6yEExjcd8ypyVx/coC3z0JL13KTI9tx jzsC3XUPskoYhNEMtvhcZkRJtMJLQy3w2O9ky+IfIFu3wuGrch3XE0x27DmEMgne5Vh2 TwL1zF9BW7IBmYhT3Wxfghkg1f++fKRQZP+NdR/7rEH2LcwZfq6a1ujq9bDddgJFQRY/ gqZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=+7Qt1Vbtb4K4qoLiXFWPZtfwYiT1v2vtwDp2473VM3g=; b=Tk+owt4gqlSxW/t7TQWNbTdU4Jej476dsMfetWBJpFSavNRffYRwDaGZCZtA8MCzqg in3Ayzgx6i4Gen8B2L+0foHnA4Okml242wr1ixTdV7zYa7K7Hz7ZKuNAc9e5nXlCsiyT /+ZHoV8D1r+XHn9teY9dBioInkiC+nE8jq4I1r2o8lk0OtAZN50OqOyI0XtgsFQuKUC9 HDvcpsZYiNS5CyVBeK9z6+lGmlpzKEP64yhW4ya9njT1UDsKqwDHjtMM+5TKyofvMCDa MDlDdsx1D+6NzlhgToTZsNO1OnHuxd9ToUWC9amviwKji5ISjPlCJANr6YvLQNWSlHtv fWcg== X-Gm-Message-State: ACgBeo0CeKb2wXqdNTpMVXaQxsmu4gSeLbB0vW+WdCWZB2Q0Ig9zPWR8 BOlnVEv7WD5yuXDmLbuCpAI= X-Google-Smtp-Source: AA6agR7bUo0c8b0R/Cb12vXZq8obDCZ9kOYLr4BHXAmoRyGG5Ys6FSyE9pr/8Fk2V1k9dZ1fJeEhKA== X-Received: by 2002:a17:90b:3b85:b0:1f4:f76a:d5f6 with SMTP id pc5-20020a17090b3b8500b001f4f76ad5f6mr11918784pjb.156.1660879315806; Thu, 18 Aug 2022 20:21:55 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.55 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:21:55 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin Subject: [PATCH v2 4/9] refs: do not run transaction hook for git-pack-refs Date: Fri, 19 Aug 2022 11:21:42 +0800 Message-Id: <20220819032147.28841-5-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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 --- 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 && From patchwork Fri Aug 19 03:21:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948261 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AC27FC00140 for ; Fri, 19 Aug 2022 03:22:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241306AbiHSDWO (ORCPT ); Thu, 18 Aug 2022 23:22:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34116 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239515AbiHSDV6 (ORCPT ); Thu, 18 Aug 2022 23:21:58 -0400 Received: from mail-pf1-x433.google.com (mail-pf1-x433.google.com [IPv6:2607:f8b0:4864:20::433]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4EDA1DA3DE for ; Thu, 18 Aug 2022 20:21:57 -0700 (PDT) Received: by mail-pf1-x433.google.com with SMTP id p9so2158421pfq.13 for ; Thu, 18 Aug 2022 20:21:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=YxPlYWFTi8MYRDgdciM3M46fXhqc5pJROhBnPQ5CFUo=; b=ZXSsD6zb+Pj9rv7IgCNVf0jKP8ygFAnwkHiNX/aVe59L5MKxaDVD59AALp48C9TdLZ QZbwo8eZ9NudUPbqEVzQs8XptGpTCLLAZSsFbez+0bi6+4/Br19mV+iWrz3akyfvg+Um K2BgcKUjUK+6luoLchkav/Ib9P+ubxXsGoGHRZWzwJw/iuiyOKpRjxTS4+bZIVy8zkrJ k27YrQVUDd3P9+Et0zQYHmU+jrvTMpcBk7kNfh1Dr2RvBSZlf0qV8F43Y5tr89/Bmk2M IBZZm49C03KCr4ofrow24xOaNheUedfCEzlqo4shpK5E2m6BkXLM3Gz4YRqXNvSHjQml tmRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=YxPlYWFTi8MYRDgdciM3M46fXhqc5pJROhBnPQ5CFUo=; b=MUOb+gbS0Qdpjk3cCBbPdhSWuP1nzIaUF7ChpMNM6r9DHML7CDdpubbDQ4dPPHjfVu 9hGDUG9xY5KYQy6A5YID6wsAEqhf9R0TV1ok0FQp/YL/zj9Xkw8HnMiA9wpCWpTkZyzk 9L+iky3uh9D+Rn/IRGglFXrNENNNsw6UixMvi9TvCorgSic8HWx0rrJP8Kjjqyr/42zV vLuzm+WUpT3biYf8va9xo8RAcUMPr2JQ2Nney+w4cKroNlP1npJxgNu4ZfUzCfkSZWkX bRljOD0baufSyWX4ulO2RDdwEstPMtiBNUq3ngW2HWqG2/hUc4xeuDaD95v58nLQ1pSv gKjQ== X-Gm-Message-State: ACgBeo3dTmJmD22u3EWR+FyaJwWda2nvpqXwPTbRdRS6LBaub0qHDgZV GJ7k1Garp5eqCEdMBJGzqEM= X-Google-Smtp-Source: AA6agR4vYHmW5NZv62kpuFuFE3knwfmNLVMBryyCWP2uPHpjstE7U/Zu8FJvO4LMV17j/FnNNu2wcg== X-Received: by 2002:a05:6a00:16c1:b0:520:6ede:24fb with SMTP id l1-20020a056a0016c100b005206ede24fbmr5904593pfc.7.1660879316770; Thu, 18 Aug 2022 20:21:56 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.55 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:21:56 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin Subject: [PATCH v2 5/9] refs: avoid duplicate running of the reference-transaction hook Date: Fri, 19 Aug 2022 11:21:43 +0800 Message-Id: <20220819032147.28841-6-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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 * git update-ref --stdin # delete refs Signed-off-by: Jiang Xin --- 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 @@ -# refs/heads/topic1 -# HEAD -# ## Call hook: reference-transaction committed ## -# + refs/heads/topic1 -# +## Call hook: reference-transaction committed ## -# refs/heads/topic1 -# HEAD -# ## Call hook: reference-transaction prepared ## -# @@ -11,13 +13,19 @@ -# ## Call hook: reference-transaction prepared ## -# refs/heads/topic2 -# ## Call hook: reference-transaction committed ## -# + refs/heads/topic2 -# +## Call hook: reference-transaction committed ## -# refs/heads/topic2 -# ## Call hook: reference-transaction prepared ## -# refs/heads/topic3 -# ## Call hook: reference-transaction prepared ## -# refs/heads/topic3 -# ## Call hook: reference-transaction committed ## -# + refs/heads/topic3 -# +## Call hook: reference-transaction committed ## -# refs/heads/topic3 -# +## Call hook: reference-transaction aborted ## -# + refs/heads/topic4 -# ## Call hook: reference-transaction prepared ## -# 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 @@ -# refs/heads/topic4 -# HEAD -# ## Call hook: reference-transaction committed ## -# + refs/heads/topic1 -# + refs/heads/topic2 -# + refs/heads/topic3 -# + refs/heads/topic4 -# +## Call hook: reference-transaction committed ## -# refs/heads/topic1 -# refs/heads/topic2 -# 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 ## -# + refs/heads/topic4 -# ## Call hook: reference-transaction prepared ## -# refs/heads/topic4 +# @@ -3,14 +3,6 @@ # ## Call hook: reference-transaction committed ## # refs/heads/topic4 -# -## Call hook: reference-transaction prepared ## +# ## Call hook: reference-transaction prepared ## # - refs/heads/topic6 # -## Call hook: reference-transaction committed ## # - refs/heads/topic6 -# +## Call hook: reference-transaction aborted ## -# + refs/heads/topic5 -# ## Call hook: reference-transaction prepared ## +# -## Call hook: reference-transaction prepared ## # refs/heads/topic5 # ## Call hook: reference-transaction committed ## # 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 @@ # refs/heads/topic1 # refs/heads/topic2 # refs/heads/topic3 @@ -898,14 +830,10 @@ test_expect_failure "branch: rename branches" ' # + refs/heads/topic1 # + refs/heads/topic2 # + refs/heads/topic3 -# +## Call hook: reference-transaction aborted ## -# + refs/heads/topic1 # +## Call hook: reference-transaction prepared ## # + refs/heads/topic1 # +## Call hook: reference-transaction committed ## # + refs/heads/topic1 -# +## Call hook: reference-transaction aborted ## -# + refs/heads/topic2 # ## Call hook: reference-transaction prepared ## # - refs/heads/topic1 # refs/heads/topic2 @@ -913,8 +841,6 @@ test_expect_failure "branch: rename branches" ' # ## Call hook: reference-transaction committed ## # - refs/heads/topic1 # refs/heads/topic2 -# +## Call hook: reference-transaction aborted ## -# + refs/heads/topic3 # +## Call hook: reference-transaction prepared ## # + 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 @@ # refs/tags/v1 # refs/tags/v2 # refs/tags/v3 @@ -1067,14 +991,10 @@ test_expect_success "tag: update refs to create loose refs" ' # + refs/tags/v1 # + refs/tags/v2 # + refs/tags/v3 -# +## Call hook: reference-transaction aborted ## -# + refs/tags/v1 # +## Call hook: reference-transaction prepared ## # + refs/tags/v1 # +## Call hook: reference-transaction committed ## # + refs/tags/v1 -# +## Call hook: reference-transaction aborted ## -# + refs/tags/v2 # ## Call hook: reference-transaction prepared ## # - refs/tags/v1 # refs/tags/v2 @@ -1082,8 +1002,6 @@ test_expect_success "tag: update refs to create loose refs" ' # ## Call hook: reference-transaction committed ## # - refs/tags/v1 # refs/tags/v2 -# +## Call hook: reference-transaction aborted ## -# + refs/tags/v3 # +## Call hook: reference-transaction prepared ## # + 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 ## -# HEAD -# refs/heads/topic3 -# +## Call hook: reference-transaction aborted ## -# + CHERRY_PICK_HEAD -# ## Call hook: reference-transaction prepared ## -# 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 @@ -# HEAD -# ## Call hook: reference-transaction committed ## -# HEAD -# +## Call hook: reference-transaction aborted ## -# + REBASE_HEAD -# ## Call hook: reference-transaction prepared ## -# REBASE_HEAD -# ## Call hook: reference-transaction committed ## -# @@ -18,6 +20,8 @@ -# HEAD -# ## Call hook: reference-transaction committed ## -# HEAD -# +## Call hook: reference-transaction aborted ## -# + CHERRY_PICK_HEAD -# ## Call hook: reference-transaction prepared ## -# 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 ## -# HEAD -# refs/heads/topic5 -# +## Call hook: reference-transaction aborted ## -# + CHERRY_PICK_HEAD -# ## Call hook: reference-transaction prepared ## -# 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 && From patchwork Fri Aug 19 03:21:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948262 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 66F63C28B2B for ; Fri, 19 Aug 2022 03:22:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244343AbiHSDWR (ORCPT ); Thu, 18 Aug 2022 23:22:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34126 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240290AbiHSDV7 (ORCPT ); Thu, 18 Aug 2022 23:21:59 -0400 Received: from mail-pf1-x42f.google.com (mail-pf1-x42f.google.com [IPv6:2607:f8b0:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6977AC7FB0 for ; Thu, 18 Aug 2022 20:21:58 -0700 (PDT) Received: by mail-pf1-x42f.google.com with SMTP id k14so3275683pfh.0 for ; Thu, 18 Aug 2022 20:21:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=4bcnfL3Gr8/Z7eVqnBVN3xJGSA3Pb5fF3zzYypK61V4=; b=UGWgYp8vT9pNuS/rUUaCnDMs0z3RKClz6w9uWwfiIJTZZXdwpxTgAqv+eW4UaKKmE5 Z6DmW9vTvSUSjUzncbBDU3tzTAjcQ02gnAT3vSqSfjAz1aB54RigmX4mNHTPbxA2ZUX6 isKVy0TQj4pWcv+ImS/JCY/6Tp24HGqWyT3995oiQimhOaEaIBbw3OBtHDHsfZQD+SMN Ne/bWEujh561U4S5cJc732MmIO/zSeiPTnOusdfllzl34q6+0ZK7ebHkM+KTK/DFM363 4IcZexq4a7slXptmDHRGDPNiYYRMOJlZVOnXnKziU0IkplJ1qg9iNnvaBcnFfB5RKJ3B pyaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=4bcnfL3Gr8/Z7eVqnBVN3xJGSA3Pb5fF3zzYypK61V4=; b=7dtg89vFMutYqpn7n6O5EvMrSXGFYuCp0KIkiPjEa7vMazeKDB3NpxRF9SsHY5kL/B xT8/08t+1oF7EoIl7w/OnQ5KNAC//aSC1GhaPdHwNQS/tkGN6qRrDjruspz+kdlGetIT lrom0GkTV+1QQAv+5iR5aZFkqI7sH+cFBLbfTY2V/Dm8D5QE5Mle2mdCIHx3U3jt0OlE FWo4SkpzIIpM4zDRkDHWs7ZM8HrOnFjx3AKjn6cQN0sc4KfYJD06xQwC4L1F4eErE4a0 Q5Nk7OEXGKG8dRYu/J4wG4Kxkl0iN0jaWBTeu088zScLRDI6p1Yj+Zkn0ImPl0bw6HCC y5DA== X-Gm-Message-State: ACgBeo3bRkU4xH7q/sQtdeCpwfUQ+gDO4PKXPrSdsdDQVieffvVaOpCH Gxbg6kZ7N/hkwhNxEomiS3U= X-Google-Smtp-Source: AA6agR48VgCNs2/Cfg/cCZqYrZ4y4HLLOdkJ1sddLFXwKbVCopfnHiCZI80+Nw/MsbFDx4PhksY1jg== X-Received: by 2002:a05:6a00:1688:b0:52b:cf1f:807a with SMTP id k8-20020a056a00168800b0052bcf1f807amr5736130pfc.21.1660879317849; Thu, 18 Aug 2022 20:21:57 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.56 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:21:57 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin Subject: [PATCH v2 6/9] refs: add reflog_info to hold more fields for reflog entry Date: Fri, 19 Aug 2022 11:21:44 +0800 Message-Id: <20220819032147.28841-7-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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 --- 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. From patchwork Fri Aug 19 03:21:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948264 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 094E4C00140 for ; Fri, 19 Aug 2022 03:22:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238802AbiHSDWW (ORCPT ); Thu, 18 Aug 2022 23:22:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34144 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240757AbiHSDWB (ORCPT ); Thu, 18 Aug 2022 23:22:01 -0400 Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 67D95D7D3C for ; Thu, 18 Aug 2022 20:21:59 -0700 (PDT) Received: by mail-pl1-x630.google.com with SMTP id x19so3095616plc.5 for ; Thu, 18 Aug 2022 20:21:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=WeHK+Igph7MQZ6Kmhv2hKyoc7Y8PG/v6/Ri7NEyfNY8=; b=JKF+rk24dg27UzQaNPMKUOSw/Ii54i44+zAPUqOTTFd7H+30nXArDrlYcSWPd7brn3 K4W91M6UmW9WXNJ3XoR7sk8oqj2SSf74qzT+p5Lc2FpGVHVejgD7sd9A/4Hln+b0ysA9 BheTlwH50fFCWi0ZKmanJgSso0wg8zO9nzhAb+6OdU0+PSng0Pr7sg25+ZFXzSr5BOkC 4qYS9wSaGi71M56C8sfaWZJWqShdrb0KS1Nm9AdJZiHRHAKrCM+mCtvyfSoOy+w4fZBr 6GE85EoS5rW1GEiaMd0j2AzuPdP6jqu222UDm1Gw8W7pkJa+KTuhrVJy7IZbtJWPi/AW fU0w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=WeHK+Igph7MQZ6Kmhv2hKyoc7Y8PG/v6/Ri7NEyfNY8=; b=Pvpb3taVLUS3mFitvo2ZzjYRDJl6kwr+W2Iep7NQntuv47RIf0Imf24Ve63wM0Q7oN Lgcf8yPR3ANzzxopaK6tA3FQKXgyBk3Vo/cJqe9oFEjmJXLlVVkDZ8dirCf4Z2tT6pju 5rUrJzcTSZ5VKBSCHXHrR1ubNe80Qcox+IILFWL6c3Qn8Dmuzw6Jsijm5tE8zZAQN+mY p1KMp6LELlkxxOJS919VkJloG9rXn7krULtOI1QDwghQYLbDAOa1rSlZcAStsRJ/MPzr jC401FJDkGBYxwbu1VB/SBrqAcdG9TRsKZbUKKSuriPZXONEvCMqTUs6ZQXS6XOnqj4b WtOg== X-Gm-Message-State: ACgBeo1JRzr2IC2CMPkU2CtKxQG3LQCUTLeNtR8g5rWEyrqA8WCTIfAC 6ZIKSNxG01PaxlC2Xo4rMizo6r+cPV4= X-Google-Smtp-Source: AA6agR7AshKc9+6sN4ZGO2gshCQ9uJAXYyG43LVVFrkIOwO18WTGGlYJdvk+Zf8eb/aAP4BEKsOZcQ== X-Received: by 2002:a17:90b:4f8e:b0:1f4:ed30:d286 with SMTP id qe14-20020a17090b4f8e00b001f4ed30d286mr6349745pjb.66.1660879318982; Thu, 18 Aug 2022 20:21:58 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.58 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:21:58 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin Subject: [PATCH v2 7/9] refs: get error message via refs_update_ref_extended() Date: Fri, 19 Aug 2022 11:21:45 +0800 Message-Id: <20220819032147.28841-8-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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 --- 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); From patchwork Fri Aug 19 03:21:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948265 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AC6C8C00140 for ; Fri, 19 Aug 2022 03:22:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244548AbiHSDW1 (ORCPT ); Thu, 18 Aug 2022 23:22:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34116 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241940AbiHSDWC (ORCPT ); Thu, 18 Aug 2022 23:22:02 -0400 Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 61B27DA3E8 for ; Thu, 18 Aug 2022 20:22:00 -0700 (PDT) Received: by mail-pj1-x102a.google.com with SMTP id g18so3491171pju.0 for ; Thu, 18 Aug 2022 20:22:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=Akql0iNkYSozHtEjoAQSaojCWdGc3+btUviECy1lkiU=; b=RlJuUaZa4y9hO9ChxuVS12SazMIJhf/FYl4Cg7QesWfEuppP/xuSoU9bfrI7qW3xjR ZCBpODjRsW1J6mlkpnqQjsigJV5b9NYscVj4CRL2najS0u7drobfm7/ugWlX6gY+5NXn 3K8+UoEPtv8r6yy3s5Y4w679AaDcdSbO+b9Nm8V+HZpF4NFGVC7Odm39hutwB+UhfIof OD+9jSaVV+WD5tGDg70KEWoXsNVeOaMzwe0WaCHOTvRc2MSHWvQ7obCfxhfxG64JZhJ6 Q1o2SDxObwb8EDubIerQ8Zm3ZQKgl7uoGA4HAjrg4XAXCKVXk8Si0a+67nwfjD752Wz3 kKMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=Akql0iNkYSozHtEjoAQSaojCWdGc3+btUviECy1lkiU=; b=xnuz7gC3x+a+4wDN8+aOOEBEIbcls2E5K1VJiwZvF5UW3P/OY7oouZCxQj1DqdHP0q 9/sqPP5+6B5sgyn5ibug41lxkrUwImDNm8G12owRTWZueogfKAM2bL+hzLx2aF6Rkft+ HRTPwEgI1g6SK2niInJOFIkse62nAai2mSoYFlRH4/8O8By4+QrsGsyJKY+KXJoJYU0y 5TqGGhfnJuvfBkBcu9idZqjx+2d8N0opvTGiCbyrxl58OOYL/FDwCCHPitE2f/u4+6rc zA/OVYV0IrEvkiFK5YSzuRQRdQDAo/gJtFMm89WkwFeF1vu7xhz+71AGsFNYRwtVEEAE RU9A== X-Gm-Message-State: ACgBeo1S6CXzy3wgVVPXY8oMlMmfM5MtS93Pw0iPE4ImtUobbIXCZpp8 mDhIfnxZnbzl78CPrCzK0F4= X-Google-Smtp-Source: AA6agR6//OC10UGWZ06OWGhhVvYy9D6fE+ROmSCQPbnj0xYHNs9+5W3weqPQiicBvp/7Aos6YRJ3mA== X-Received: by 2002:a17:902:d2d1:b0:172:aaf5:861a with SMTP id n17-20020a170902d2d100b00172aaf5861amr5354090plc.114.1660879319829; Thu, 18 Aug 2022 20:21:59 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.59 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:21:59 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin Subject: [PATCH v2 8/9] refs: reimplement files_copy_or_rename_ref() to run refs-txn hook Date: Fri, 19 Aug 2022 11:21:46 +0800 Message-Id: <20220819032147.28841-9-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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 # copy branch * git branch -m # rename branch Signed-off-by: Jiang Xin --- 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 ## -# refs/heads/topic4 -# ## Call hook: reference-transaction prepared ## -# - refs/heads/topic6 -# -## Call hook: reference-transaction committed ## -# - refs/heads/topic6 -# -## Call hook: reference-transaction prepared ## -# refs/heads/topic5 -# ## Call hook: reference-transaction committed ## -# refs/heads/topic5 -# -## Call hook: reference-transaction prepared ## -# - refs/heads/topic7 -# -## Call hook: reference-transaction committed ## -# - 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 && From patchwork Fri Aug 19 03:21:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Xin X-Patchwork-Id: 12948263 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7708FC00140 for ; Fri, 19 Aug 2022 03:22:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244358AbiHSDWU (ORCPT ); Thu, 18 Aug 2022 23:22:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34126 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242219AbiHSDWC (ORCPT ); Thu, 18 Aug 2022 23:22:02 -0400 Received: from mail-pl1-x634.google.com (mail-pl1-x634.google.com [IPv6:2607:f8b0:4864:20::634]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4AC11DC5F2 for ; Thu, 18 Aug 2022 20:22:01 -0700 (PDT) Received: by mail-pl1-x634.google.com with SMTP id u22so3078347plq.12 for ; Thu, 18 Aug 2022 20:22:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc; bh=uXKCWVWtQjc1NdGCr3YIv96Jsg/UJxXiRI9uw1i1pOU=; b=m3Pwh2M2EmY7P364ZYpdM50/+fiTtHvC62GlyhVxlWvYoaDN66GHnKDHRiBZxJ/06O bcFdPTL2jHQmalrZ12CXCWbAjV3Bar7dS/9ekbr2WpsoJMwTPCLWMjzKKDwo1SfYfLjl /Vr9X3MVynM7qzvN0teZLPZ6BDrahCBazPiQNvS3YIWcLiTm7vkVYPRDyOhkXq6Pc+j4 oodbGQEOO4HfZQ9fGDn4VPaB/2TuGIq72e1Ne6fT3LgtXgDyDkNBuho71/4ZOsbYFlR6 SEnUmNgRzAmnCc41SPJlZeTLPT2RPY3zFjn4Jpc+v7zWMZv7P0P2ajdh9zZyinLuV+S0 mAzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=uXKCWVWtQjc1NdGCr3YIv96Jsg/UJxXiRI9uw1i1pOU=; b=qb8pDYxC4EQgD7+tr+nO0l1XNxDxU6nJtExltPHL8ed8YdG0Wp825jLTebXl8vK/bs JbSXW5NI+u8QX1/W0Xhqzr8Fov3Tl/nLGhqmOnftWlF7b2kqwJShJEKQLxo5//ihU4tC wZC7qaRazSc/tXG7ZR/wnvnsAW9DsIhiNRnK1QrI0fBA4WhtNIU1DyXsquzMHCxbqPjL H6cq27/D29cq/LnZDbzVTLuszof9BNlZqNtSQgVjP0vcXcBq+2Tzwnj5cYEvkFQH8Djk dguBTjXHsA+AhVT4akcNrAYQABQv7k18UFJeh6+2t0GiE9yHTercu1QjrpadL3sxN/iZ kbyQ== X-Gm-Message-State: ACgBeo2u78KFqovYAFPohGi2X87gEpryZ3b9mQDKTFupzh4RxjhVxbjM CzrGLddgHU9uV3BlyHiq6d5VO1L2BwE= X-Google-Smtp-Source: AA6agR5VZG5mrySQHu9XbRMOwgRtRgNpxSHuy31GneczUBeYGIYztx6z3yVRuwh9mibDXuxluNxBnQ== X-Received: by 2002:a17:903:31c9:b0:16c:3024:69c4 with SMTP id v9-20020a17090331c900b0016c302469c4mr5356443ple.81.1660879320748; Thu, 18 Aug 2022 20:22:00 -0700 (PDT) Received: from tigtog-proxy.localdomain.localdomain (144.34.163.219.16clouds.com. [144.34.163.219]) by smtp.gmail.com with ESMTPSA id t3-20020a170902d14300b0016f1319d2a7sm2036995plt.297.2022.08.18.20.21.59 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Aug 2022 20:22:00 -0700 (PDT) From: Jiang Xin To: Junio C Hamano , Patrick Steinhardt , Michael Heemskerk , Git List Cc: Jiang Xin , Jiang Xin Subject: [PATCH v2 9/9] refs: reimplement refs_delete_refs() and run hook once Date: Fri, 19 Aug 2022 11:21:47 +0800 Message-Id: <20220819032147.28841-10-worldhello.net@gmail.com> X-Mailer: git-send-email 2.32.0.rc3 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jiang Xin 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 * git tag -d 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 --- 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 @@ -# refs/heads/topic1 -# refs/heads/topic2 -# refs/heads/topic3 -# +## Call hook: reference-transaction committed ## -# + refs/heads/topic1 -# + refs/heads/topic2 -# + refs/heads/topic3 -# +## Call hook: reference-transaction prepared ## -# + refs/heads/topic1 -# +## Call hook: reference-transaction committed ## -# + refs/heads/topic1 -# ## Call hook: reference-transaction prepared ## -# - refs/heads/topic1 -# refs/heads/topic2 -# - refs/heads/topic3 -# ## Call hook: reference-transaction committed ## -# - refs/heads/topic1 -# refs/heads/topic2 -# +## Call hook: reference-transaction prepared ## -# + refs/heads/topic3 -# +## Call hook: reference-transaction committed ## -# 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 @@ -# refs/tags/v1 -# refs/tags/v2 -# refs/tags/v3 -# +## Call hook: reference-transaction committed ## -# + refs/tags/v1 -# + refs/tags/v2 -# + refs/tags/v3 -# +## Call hook: reference-transaction prepared ## -# + refs/tags/v1 -# +## Call hook: reference-transaction committed ## -# + refs/tags/v1 -# ## Call hook: reference-transaction prepared ## -# - refs/tags/v1 -# refs/tags/v2 -# - refs/tags/v3 -# ## Call hook: reference-transaction committed ## -# - refs/tags/v1 -# refs/tags/v2 -# +## Call hook: reference-transaction prepared ## -# + refs/tags/v3 -# +## Call hook: reference-transaction committed ## -# 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" &&