From patchwork Fri Jan 7 11:55:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12706577 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 DB442C433F5 for ; Fri, 7 Jan 2022 11:56:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347467AbiAGL4H (ORCPT ); Fri, 7 Jan 2022 06:56:07 -0500 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:55639 "EHLO out4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238255AbiAGL4A (ORCPT ); Fri, 7 Jan 2022 06:56:00 -0500 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id CC13C5C01AE; Fri, 7 Jan 2022 06:55:59 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Fri, 07 Jan 2022 06:55:59 -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=fm3; bh=wo7blteufdf9DqiMliUrzUo3hoE ygOBQvEVIwcbd/T4=; b=kRgK83VoMjlZ8z8c241Z4Bbb943AhqE6VXNvP23q+Zc +D9s8GeyKi/UQNXDM5ftPOeg4QPnGB9h1K+SRQ5JrntSZQFQdna4ah2sGxouHBI3 ZWZawlonJ6YblUKHAUY0RQ+8rlkZdpz2erbtQAkI9M8kDcJOvpX0tvRT+xOLOo1+ ViJR3AnD/17i7klNtBC2/zDzJOSmbG7ESTK91YDaNUWEcX02XXpDu/ZU6qIprshA a2WaouYgDEKaSFZGWd2j8aK0uEbs0tjP2GCgY7B+TdDzLRqwfxiCfyj0PgVaXCLy v2Um0RtPPwUz7bTqb2bwjI06iIBpGuaKcIGldToahQA== 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=wo7blt eufdf9DqiMliUrzUo3hoEygOBQvEVIwcbd/T4=; b=igL6DQ3HmPCpSfFhI3TT9m G2W21J+umKYwDhZB4J3NdDWjCuiwUizsUTW6yevAEFtOusRMxsQy0BytbfeNnY76 aUjieGr7Xr/uK9BKpC4UIszzAmLLOH8Ygt+zJ761Z5CQw/HwIVGiz0qoLp3rbANU fSlagZ8K2iaeOMfVJKmC9oRSsJlvdkBjWOeduE868lm5Ztde5o5CO0nWvK3+YBSw k/KevkX0jHFgpBsN2UA5rqI7MK1y98TKq4unND+7Kczbimh5YeUy4o4+h2TZJuOS aIOjcGNjhHCKLmga3e+T2YYWWTC5ANv77JRx4Rn5zM+ynHBWAR3VuEjDU8mtHPcA == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddrudegvddgtdelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 7 Jan 2022 06:55:58 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id e1c27597 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 7 Jan 2022 11:55:57 +0000 (UTC) Date: Fri, 7 Jan 2022 12:55:34 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Bryan Turner , Waleed Khan Subject: [PATCH v2 6/6] refs: skip hooks when deleting uncovered packed refs Message-ID: <0fbf68dbf434986aa971961e20598675b2194d51.1641556319.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 3c0f3027fe..9a20cb8fa8 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1261,7 +1261,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; @@ -2775,7 +2776,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; @@ -3046,7 +3048,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