From patchwork Tue Dec 7 10:56:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12661593 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 83460C433EF for ; Tue, 7 Dec 2021 10:57:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235482AbhLGLA3 (ORCPT ); Tue, 7 Dec 2021 06:00:29 -0500 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:58283 "EHLO out4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235472AbhLGLA2 (ORCPT ); Tue, 7 Dec 2021 06:00:28 -0500 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 8605C5C019E; Tue, 7 Dec 2021 05:56:58 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Tue, 07 Dec 2021 05:56:58 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm2; bh=okJ2Nn9XAaNmRwqOFXZiX+YiI1E otfryC/lDqgAC8Zc=; b=WZn1tKowUngYxHsJrWU/fWHpoTSbKvQqkMdQ/WowTqr 9wKBxutB23YF1+hHPWISOlfDixVOoXdVp4DAv6pF1FSG68Y/V/1BRgpVH0zwOLvR jlT6e7GlACqBJ7V+GVKK43KmPeGM5RTdmzmGHbyoRD+p1izfkHrvSPJrNSvs/MIL sZmU+ETzljTGJB5brFKYSE1SwHaIHcW+3CTlM4ajKw8SdrRaAHyqrSV0okDW0FXY esF8UxzBHfLl4jIjLPdzXvqTiY5bP2WPx8wgjGxdYVQEkm1pouLMCghpLstOQ+4/ 4d6j44lRDtbIWSpFnYPbIITcMxODydl7Eo/dzpbnDqw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=okJ2Nn 9XAaNmRwqOFXZiX+YiI1EotfryC/lDqgAC8Zc=; b=VkJUy7J6bO3kgHPpclj48Y ahsPdz+0yW6ohyWt3fteiskCF0EItb0Q5hperWvuk4qC2lYQhtqT501OAg3r+Ece TwkkcirX6cAzlizuPs+VfY4ghEs04cxBe8P951YiHNPZoUdWZYblr2VmyqKOphad yV11gDaMsfWCsk8uL8guf8Y7+2GvfsC2PpJG98RiZYE0aU6uwgkEn6rZlnu2Gga5 CZqt+ec/XWStKEjJRn/FekBRsboqkxItczS1nKBsbbgmfDspYXzeLxW/UVbxepud qHLcrUTl/KmLDxo2+WfSUnxPUokWlKA9S/DdkHOIARxfagrULoc0IVJ8iiU/dBKg == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddrjeehgddvfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhrihgt khcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvghrnh epheeghfdtfeeuffehkefgffduleffjedthfdvjeektdfhhedvlefgtefgvdettdfhnecu vehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrghilhhfrhhomhepphhssehpkh hsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 7 Dec 2021 05:56:57 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 4d304524 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 7 Dec 2021 12:26:36 +0000 (UTC) Date: Tue, 7 Dec 2021 11:56:17 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Bryan Turner , Waleed Khan Subject: [PATCH 6/6] refs: skip hooks when deleting uncovered packed refs Message-ID: <04d65ff7991f880920a1fcaab3e4450fad0f15de.1638874287.git.ps@pks.im> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org When deleting refs from the loose-files refs backend, then we need to be careful to also delete the same ref from the packed refs backend, if it exists. If we don't, then deleting the loose ref would "uncover" the packed ref. We thus always have to queue up deletions of refs for both the loose and the packed refs backend. This is done in two separate transactions, where the end result is that the reference-transaction hook is executed twice for the deleted refs. This behaviour is quite misleading: it's exposing implementation details of how the files backend works to the user, in contrast to the logical updates that we'd really want to expose via the hook. Worse yet, whether the hook gets executed once or twice depends on how well-packed the repository is: if the ref only exists as a loose ref, then we execute it once, otherwise if it is also packed then we execute it twice. Fix this behaviour and don't execute the reference-transaction hook at all when refs in the packed-refs backend if it's driven by the files backend. Signed-off-by: Patrick Steinhardt --- refs/files-backend.c | 9 ++++++--- t/t1416-ref-transaction-hooks.sh | 7 +------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index f2bc72f81b..0a2b5ea5da 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1262,7 +1262,8 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg, if (packed_refs_lock(refs->packed_ref_store, 0, &err)) goto error; - transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, &err); + transaction = ref_store_transaction_begin(refs->packed_ref_store, + REF_TRANSACTION_SKIP_HOOK, &err); if (!transaction) goto error; @@ -2769,7 +2770,8 @@ static int files_transaction_prepare(struct ref_store *ref_store, */ if (!packed_transaction) { packed_transaction = ref_store_transaction_begin( - refs->packed_ref_store, 0, err); + refs->packed_ref_store, + REF_TRANSACTION_SKIP_HOOK, err); if (!packed_transaction) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; @@ -3040,7 +3042,8 @@ static int files_initial_transaction_commit(struct ref_store *ref_store, &affected_refnames)) BUG("initial ref transaction called with existing refs"); - packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, err); + packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, + REF_TRANSACTION_SKIP_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 f9d3d5213f..4e1e84a91f 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -175,16 +175,11 @@ test_expect_success 'deleting packed ref calls hook once' ' git update-ref -d refs/heads/to-be-deleted $POST_OID && # We only expect a single hook invocation, which is the logical - # deletion. But currently, we see two interleaving transactions, once - # for deleting the loose refs and once for deleting the packed ref. + # deletion. cat >expect <<-EOF && - prepared - $ZERO_OID $ZERO_OID refs/heads/to-be-deleted prepared $POST_OID $ZERO_OID refs/heads/to-be-deleted committed - $ZERO_OID $ZERO_OID refs/heads/to-be-deleted - committed $POST_OID $ZERO_OID refs/heads/to-be-deleted EOF