From patchwork Mon Jan 17 08:12:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12714957 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 2FC92C433F5 for ; Mon, 17 Jan 2022 08:12:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237949AbiAQIMf (ORCPT ); Mon, 17 Jan 2022 03:12:35 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:55251 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230177AbiAQIMe (ORCPT ); Mon, 17 Jan 2022 03:12:34 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 5202F5C012F; Mon, 17 Jan 2022 03:12:34 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 17 Jan 2022 03:12:34 -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=JOcWTm0r7D/whALFjlcpQVap6tG /EDNmROIwTOlhCFY=; b=O9hVULzvxqt/xFKSFIvKGYRQDZiACzqGg4CqNAieMfm EbUsU8qG0ENlDOWBUl55Wk8uPfoqjoOevKgxvGKn39R7MAy6czChFpCba+QfCsP5 JxE+21eeC1UOjmY+WM6r2mnTIS7d9Tg66Dmc7rbZtSyfp+P7aoGokXNC9XJ0uT/6 I/foHTyTEIf2PwoYHP8iaTrqypkjVcBIUQWpM2ayhsfBLrDq3uk4cnk+NcScHr8p 2876hXzscIykSaH4PUBNRFWh35k6VxfQdjmhmb1icCwjBHseGg+yGEYdSJitiElU /JYwl4rgIwoHfjPP/YQs05vlX0XlX3MFvlA6R8tnb9Q== 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=JOcWTm 0r7D/whALFjlcpQVap6tG/EDNmROIwTOlhCFY=; b=l8S+YR2V5XRkZJFdmLtsAM CKjmorZ5s4jJ/qd51bQVaWZvyuTsd5fJ0NxHrNtVIn9Nii1vgesGm7eDQchdnk3/ sDckn7MPczZxZZS9tgIMma7zK7TNjBfmVAsHs0/GpUGwoCGmgxoTajVHTfpmcv3D 2IH90CzWNRnfwTgaBytokk468IJPfopci2WsE+Kr7rQCBTSXnR2ChgGdKD6T/Dqn p8xAMULsaSfvhwmVXHTsaVw2cdvuGASztHAkWt7BA0UvFkyIVX79/uUbJsypRw/R DyKyANGqeu9WVFdpTmQE+rT2edMoh852qz9jKvcn7x51QbsGFlemo/kqdVndRJOA == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddruddtgdduudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 17 Jan 2022 03:12:32 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 71306157 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 17 Jan 2022 08:12:32 +0000 (UTC) Date: Mon, 17 Jan 2022 09:12:31 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Bryan Turner , Waleed Khan , =?iso-8859-1?q?=C6var_Arnfj=F6r=F0?= Bjarmason , Han-Wen Nienhuys Subject: [PATCH v4 1/6] refs: extract packed_refs_delete_refs() to allow control of transaction Message-ID: <14775046e1fb73a65b2198e72e72428b16d3e65c.1642406989.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 loose refs, then we also have to delete the refs in the packed backend. This is done by calling `refs_delete_refs()`, which then uses the packed-backend's logic to delete refs. This doesn't allow us to exercise any control over the reference transaction which is being created in the packed backend, which is required in a subsequent commit. Extract a new function `packed_refs_delete_refs()`, which hosts most of the logic to delete refs except for creating the transaction itself. Like this, we can easily create the transaction in the files backend and thus exert more control over it. Signed-off-by: Patrick Steinhardt --- refs/files-backend.c | 13 ++++++++++--- refs/packed-backend.c | 26 ++++++++++++++++++++------ refs/packed-backend.h | 7 +++++++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 90b671025a..459f18dbc1 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1249,6 +1249,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg, { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_WRITE, "delete_refs"); + struct ref_transaction *transaction = NULL; struct strbuf err = STRBUF_INIT; int i, result = 0; @@ -1258,10 +1259,14 @@ 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; - if (refs_delete_refs(refs->packed_ref_store, msg, refnames, flags)) { - packed_refs_unlock(refs->packed_ref_store); + transaction = ref_store_transaction_begin(refs->packed_ref_store, &err); + if (!transaction) + goto error; + + result = packed_refs_delete_refs(refs->packed_ref_store, + transaction, msg, refnames, flags); + if (result) goto error; - } packed_refs_unlock(refs->packed_ref_store); @@ -1272,6 +1277,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg, result |= error(_("could not remove reference %s"), refname); } + ref_transaction_free(transaction); strbuf_release(&err); return result; @@ -1288,6 +1294,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 67152c664e..c964dd1617 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1522,15 +1522,10 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store, 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; @@ -1544,6 +1539,26 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg, if (!transaction) return -1; + ret = packed_refs_delete_refs(ref_store, transaction, + msg, refnames, flags); + + ref_transaction_free(transaction); + return ret; +} + +int packed_refs_delete_refs(struct ref_store *ref_store, + struct ref_transaction *transaction, + const char *msg, + struct string_list *refnames, + unsigned int flags) +{ + struct strbuf err = STRBUF_INIT; + struct string_list_item *item; + int ret; + + /* Assert that the ref store refers to a packed backend. */ + packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs"); + for_each_string_list_item(item, refnames) { if (ref_transaction_delete(transaction, item->string, NULL, flags, msg, &err)) { @@ -1563,7 +1578,6 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg, error(_("could not delete references: %s"), err.buf); } - ref_transaction_free(transaction); strbuf_release(&err); return ret; } diff --git a/refs/packed-backend.h b/refs/packed-backend.h index f61a73ec25..a2cca5d9a3 100644 --- a/refs/packed-backend.h +++ b/refs/packed-backend.h @@ -3,6 +3,7 @@ struct repository; struct ref_transaction; +struct string_list; /* * Support for storing references in a `packed-refs` file. @@ -27,6 +28,12 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err) void packed_refs_unlock(struct ref_store *ref_store); int packed_refs_is_locked(struct ref_store *ref_store); +int packed_refs_delete_refs(struct ref_store *ref_store, + struct ref_transaction *transaction, + const char *msg, + struct string_list *refnames, + unsigned int flags); + /* * Return true if `transaction` really needs to be carried out against * the specified packed_ref_store, or false if it can be skipped From patchwork Mon Jan 17 08:12:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12714958 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 E9CCAC433F5 for ; Mon, 17 Jan 2022 08:12:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237936AbiAQIMk (ORCPT ); Mon, 17 Jan 2022 03:12:40 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:35877 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230177AbiAQIMj (ORCPT ); Mon, 17 Jan 2022 03:12:39 -0500 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id B595A5C00C4; Mon, 17 Jan 2022 03:12:38 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Mon, 17 Jan 2022 03:12:38 -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=jaQeOC/BM7/i7Uns5br8pxMJkP7 Aq3qnxFd6LP+EOF8=; b=BgYjEdviIsZohLntQYcqvAz8YFDZXTVX0bepLTWCUrj wPfu2FRJUCXylGB5QtvhiX7SPIhlsAT5sAJkjigkZrbIMKhWHl+TiJdAR9XB9nyz eQLHa8SSAV6ZQek14xwA3ahSeAZg05CAOAVk2dJbgmx2jLzQuxAnQsC4pYvCg67i ih+Dj3HHvPrquVtdp4d1fQLNwu4f+p9H98+d26DdoKWQMAB1HKhMGORt1ESSXnkb zaaUbePhefwSUAmG8N0cpsYRSsB3M4SX0IBH36zPfYgE9MrpOW1aiE/sPW9Ea6fy V215AvuoO8BYNLPoEzilR0XCcDwDPwiMHTo07mP2rag== 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=jaQeOC /BM7/i7Uns5br8pxMJkP7Aq3qnxFd6LP+EOF8=; b=lOM0Z3+mYAj9kruN5BZolB vKdKvbgDKSs7ACgh3RkDUonJlVdXXEL6SyaU3snMLfrwPuAzqfaqWRdnRKQOyhF+ FYwgsfKrR2VZX/HVDwKzS5riuQpZEVSpACbfikaE59hnvXmuE35wW0tgfMC6dWAY rQXIFQRnGcBDDxSu8YpSRXrae0QpTmpQo/cGug+iiV0UBqcDpMjecfAIkhLSgBIE uMPx9llFQKRqT/Q5noSe+dUsi7ptGFHtOm7BuuBUsJMAIFTkKYz4XtdXE174tmK3 OPM1dO9tnhhimMwy7srCLF9SPAp76DxS1bggkGs4mI28izMFZZgPavuhYmdJUJNA == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddruddtgdduudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 17 Jan 2022 03:12:37 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 76c96d95 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 17 Jan 2022 08:12:36 +0000 (UTC) Date: Mon, 17 Jan 2022 09:12:35 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Bryan Turner , Waleed Khan , =?iso-8859-1?q?=C6var_Arnfj=F6r=F0?= Bjarmason , Han-Wen Nienhuys Subject: [PATCH v4 2/6] refs: allow passing flags when beginning transactions Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org We do not currently have any flags when creating reference transactions, but we'll add one to disable execution of the reference transaction hook in some cases. Allow passing flags to `ref_store_transaction_begin()` to prepare for this change. Signed-off-by: Patrick Steinhardt --- refs.c | 8 +++++--- refs.h | 3 ++- refs/files-backend.c | 10 +++++----- refs/packed-backend.c | 2 +- refs/refs-internal.h | 1 + sequencer.c | 2 +- 6 files changed, 15 insertions(+), 11 deletions(-) diff --git a/refs.c b/refs.c index 4da4996c4d..7415864b62 100644 --- a/refs.c +++ b/refs.c @@ -800,7 +800,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg, struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; - transaction = ref_store_transaction_begin(refs, &err); + transaction = ref_store_transaction_begin(refs, 0, &err); if (!transaction || ref_transaction_delete(transaction, refname, old_oid, flags, msg, &err) || @@ -1005,6 +1005,7 @@ int read_ref_at(struct ref_store *refs, const char *refname, } struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs, + unsigned int flags, struct strbuf *err) { struct ref_transaction *tr; @@ -1012,12 +1013,13 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs, CALLOC_ARRAY(tr, 1); tr->ref_store = refs; + tr->flags = flags; return tr; } struct ref_transaction *ref_transaction_begin(struct strbuf *err) { - return ref_store_transaction_begin(get_main_ref_store(the_repository), err); + return ref_store_transaction_begin(get_main_ref_store(the_repository), 0, err); } void ref_transaction_free(struct ref_transaction *transaction) @@ -1156,7 +1158,7 @@ int refs_update_ref(struct ref_store *refs, const char *msg, struct strbuf err = STRBUF_INIT; int ret = 0; - t = ref_store_transaction_begin(refs, &err); + t = ref_store_transaction_begin(refs, 0, &err); if (!t || ref_transaction_update(t, refname, new_oid, old_oid, flags, msg, &err) || diff --git a/refs.h b/refs.h index 92360e55a2..31f7bf9642 100644 --- a/refs.h +++ b/refs.h @@ -231,7 +231,7 @@ char *repo_default_branch_name(struct repository *r, int quiet); * struct strbuf err = STRBUF_INIT; * int ret = 0; * - * transaction = ref_store_transaction_begin(refs, &err); + * transaction = ref_store_transaction_begin(refs, 0, &err); * if (!transaction || * ref_transaction_update(...) || * ref_transaction_create(...) || @@ -573,6 +573,7 @@ enum action_on_err { * be freed by calling ref_transaction_free(). */ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs, + unsigned int flags, struct strbuf *err); struct ref_transaction *ref_transaction_begin(struct strbuf *err); diff --git a/refs/files-backend.c b/refs/files-backend.c index 459f18dbc1..4d4f0c2099 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1121,7 +1121,7 @@ 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); + transaction = ref_store_transaction_begin(&refs->base, 0, &err); if (!transaction) goto cleanup; ref_transaction_add_update( @@ -1192,7 +1192,7 @@ 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); + transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, &err); if (!transaction) return -1; @@ -1259,7 +1259,7 @@ 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, &err); + transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, &err); if (!transaction) goto error; @@ -2774,7 +2774,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, */ if (!packed_transaction) { packed_transaction = ref_store_transaction_begin( - refs->packed_ref_store, err); + refs->packed_ref_store, 0, err); if (!packed_transaction) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; @@ -3045,7 +3045,7 @@ 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, err); + packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, err); if (!packed_transaction) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index c964dd1617..0b45598e18 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1535,7 +1535,7 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg, * updates into a single transaction. */ - transaction = ref_store_transaction_begin(ref_store, &err); + transaction = ref_store_transaction_begin(ref_store, 0, &err); if (!transaction) return -1; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 46a839539e..a0af63f162 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -213,6 +213,7 @@ struct ref_transaction { size_t nr; enum ref_transaction_state state; void *backend_data; + unsigned int flags; }; /* diff --git a/sequencer.c b/sequencer.c index 6abd72160c..61edd39d7a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3588,7 +3588,7 @@ static int do_label(struct repository *r, const char *name, int len) strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name); strbuf_addf(&msg, "rebase (label) '%.*s'", len, name); - transaction = ref_store_transaction_begin(refs, &err); + transaction = ref_store_transaction_begin(refs, 0, &err); if (!transaction) { error("%s", err.buf); ret = -1; From patchwork Mon Jan 17 08:12:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12714959 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 57A97C433F5 for ; Mon, 17 Jan 2022 08:12:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237953AbiAQIMo (ORCPT ); Mon, 17 Jan 2022 03:12:44 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:38237 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230177AbiAQIMn (ORCPT ); Mon, 17 Jan 2022 03:12:43 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 1E8445C00C5; Mon, 17 Jan 2022 03:12:43 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 17 Jan 2022 03:12:43 -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=I2zRV07sfwxW3btYSVMwHIY//nR f9QC4BAaDAkvXvME=; b=oGgMUbTqQ2xhMSPaz0SGx2DGnkMeDFlV104Hqz86adA Z3KRe+uWzzP39O7Fw2ox5L7f1pLbONnY2zSigr2QVMFoUPEdZTknR1tiLNx6654C Guygh73po01ctAaOGepwaGOt6sHpbJVxSiPvbTulXNqmDbjd6TxEtd33UESJPE9z hmARUdSi8jH6+NUWL8l315n5i5ctt7noU1RGLCg0Ju3nyOcLqbltpmwWGQEtwADl fYI78jiPzKV2SlsC0lwrc0ROM/OP7zJoH8x83hrFsyc0eSByO1Y8buSRWcbnJ2DW 3IccqGkTt1GOXsNIqlNsGVMvDOnmjSM14mIivn+oM2w== 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=I2zRV0 7sfwxW3btYSVMwHIY//nRf9QC4BAaDAkvXvME=; b=IIupYzEZW/2MlJhHbAC/cc Hh8x17G3c7DW/SlBX9g7/ABihsb+LxXAwUYWTiFpzTBD13eLSBVXWd3FyEbnZNUQ PwPX+hx4Q9BkzvI8IIL4YtD82k1gJwiio0nEaVV623K9RNdNmrvowb9YbLcq66vb Lkm/vhrLa0CSiLYwQq/7+1VSnOzrNB9X+lCnbVB2SPkBbchqI9QgwjzdkwiREb6C vG9/goSF+ZSBxWB+E+JD/t+CXFcCyXNEjFR8eUkkgg0KIAvRzhCXpo34rVkEDOdN LCpgjR8qnuzzawWywfpyKNcFUD136+DZZ0FnSDyZL+vZLoI36VksIoKiqwZONOHg == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddruddtgdduudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 17 Jan 2022 03:12:41 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 99ed193f (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 17 Jan 2022 08:12:40 +0000 (UTC) Date: Mon, 17 Jan 2022 09:12:39 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Bryan Turner , Waleed Khan , =?iso-8859-1?q?=C6var_Arnfj=F6r=F0?= Bjarmason , Han-Wen Nienhuys Subject: [PATCH v4 3/6] refs: allow skipping the reference-transaction hook Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The reference-transaction hook is executing whenever we prepare, commit or abort a reference transaction. While this is mostly intentional, in case of the files backend we're leaking the implementation detail that the store is in fact a composite store with one loose and one packed backend to the caller. So while we want to execute the hook for all logical updates, executing it for such implementation details is unexpected. Prepare for a fix by adding a new flag which allows to skip execution of the hook. Signed-off-by: Patrick Steinhardt --- refs.c | 3 +++ refs.h | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/refs.c b/refs.c index 7415864b62..526bf5ed97 100644 --- a/refs.c +++ b/refs.c @@ -2084,6 +2084,9 @@ static int run_transaction_hook(struct ref_transaction *transaction, const char *hook; int ret = 0, i; + if (transaction->flags & REF_TRANSACTION_SKIP_HOOK) + return 0; + hook = find_hook("reference-transaction"); if (!hook) return ret; diff --git a/refs.h b/refs.h index 31f7bf9642..d4056f9fe2 100644 --- a/refs.h +++ b/refs.h @@ -568,6 +568,11 @@ enum action_on_err { UPDATE_REFS_QUIET_ON_ERR }; +/* + * Skip executing the reference-transaction hook. + */ +#define REF_TRANSACTION_SKIP_HOOK (1 << 0) + /* * Begin a reference transaction. The reference transaction must * be freed by calling ref_transaction_free(). From patchwork Mon Jan 17 08:12:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12714960 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 D9CC8C433F5 for ; Mon, 17 Jan 2022 08:12:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237947AbiAQIMt (ORCPT ); Mon, 17 Jan 2022 03:12:49 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:56063 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237951AbiAQIMq (ORCPT ); Mon, 17 Jan 2022 03:12:46 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 821455C00C5; Mon, 17 Jan 2022 03:12:46 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 17 Jan 2022 03:12:46 -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=63dCw0k3urB/vN7FoEkXXuzEdUC J+D7Q45z6b/rF/go=; b=H9HFrRSVfYz2DZ116Pz1B6igQosuGYD4h6/QODD4omW QxLZBXonc1c4mkEOyKZznMoXtgMeUG7SIvy7I1mHyxwroUabUWxekAhD7lO1yFns 9T8OWAPKFRlqaDJ4vxpI/PZXT51POt4u3Yhxglb9QMWpMPiPWQsBdZwssd7RbxaR D+7sXM4LotclEYvfTvHt6CKyrfrTqz6A3Hhw5HholaNNEwMc8RjL+Yby6b2rSjxQ 35z4Of/YWLqHEE82LcPlehzI3bAZpHpMIKwFMQyjctqtkcLaw7Ljr0cYMvacCnPY JIKJDOJwL4ssygMo3HcU7DA2BmKGk/+3SlaLg+eDlEg== 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=63dCw0 k3urB/vN7FoEkXXuzEdUCJ+D7Q45z6b/rF/go=; b=e9V5pCLHGWeVciuH9j+pIW rv/XBvkoVA53nEf0gLd2TxuZLRvUxa3X72SL6ObGNb7e+EWbPZRLKgGqNh6ODi+Z KLOUmuxo3U8LDEOflQ6jWKdUYC1t/ntCDw8HlnoG0MkKmgJhPX+uE1IC7VyqhdeJ DKE4wEI8/FJC9jCXVxLEvLuAMjsALG+FXolSbh15YdHJc5DpO9/SAP9aoqRa8PZy 0VPcCq5FGNkwKAXTKOlcCq0mJtlNv/BJ72NjgPk2vME35het0IZ+3yoVSqSsljtM jkKjIVwT+rnKiEHwaRb5dDa+hnnZNxl1blAsI/MmD2exzszHlyGYsV33gVZvTupg == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddruddtgdduudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 17 Jan 2022 03:12:45 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id d0e13a53 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 17 Jan 2022 08:12:45 +0000 (UTC) Date: Mon, 17 Jan 2022 09:12:44 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Bryan Turner , Waleed Khan , =?iso-8859-1?q?=C6var_Arnfj=F6r=F0?= Bjarmason , Han-Wen Nienhuys Subject: [PATCH v4 4/6] refs: demonstrate excessive execution of the reference-transaction hook Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add tests which demonstate that we're executing the reference-transaction hook too often in some cases, which thus leaks implementation details about the reference store's implementation itself. Behaviour will be fixed in follow-up commits. Signed-off-by: Patrick Steinhardt --- t/t1416-ref-transaction-hooks.sh | 64 ++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index 6c941027a8..0567fbdf0b 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -136,4 +136,68 @@ test_expect_success 'interleaving hook calls succeed' ' test_cmp expect target-repo.git/actual ' +test_expect_success 'hook does not get called on packing refs' ' + # Pack references first such that we are in a known state. + git pack-refs --all && + + write_script .git/hooks/reference-transaction <<-\EOF && + echo "$@" >>actual + cat >>actual + EOF + rm -f actual && + + git update-ref refs/heads/unpacked-ref $POST_OID && + git pack-refs --all && + + # We only expect a single hook invocation, which is the call to + # git-update-ref(1). But currently, packing refs will also trigger the + # hook. + cat >expect <<-EOF && + prepared + $ZERO_OID $POST_OID refs/heads/unpacked-ref + committed + $ZERO_OID $POST_OID refs/heads/unpacked-ref + prepared + $ZERO_OID $POST_OID refs/heads/unpacked-ref + committed + $ZERO_OID $POST_OID refs/heads/unpacked-ref + prepared + $POST_OID $ZERO_OID refs/heads/unpacked-ref + committed + $POST_OID $ZERO_OID refs/heads/unpacked-ref + EOF + + test_cmp expect actual +' + +test_expect_success 'deleting packed ref calls hook once' ' + # Create a reference and pack it. + git update-ref refs/heads/to-be-deleted $POST_OID && + git pack-refs --all && + + write_script .git/hooks/reference-transaction <<-\EOF && + echo "$@" >>actual + cat >>actual + EOF + rm -f actual && + + 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. + 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 + + test_cmp expect actual +' + test_done From patchwork Mon Jan 17 08:12:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12714961 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 7B606C433EF for ; Mon, 17 Jan 2022 08:12:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237951AbiAQIMx (ORCPT ); Mon, 17 Jan 2022 03:12:53 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:54393 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237957AbiAQIMw (ORCPT ); Mon, 17 Jan 2022 03:12:52 -0500 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 30CC55C00C5; Mon, 17 Jan 2022 03:12:51 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Mon, 17 Jan 2022 03:12:51 -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=lcle+/jERTgceEHA6Sc9ld1ZNgN n7xqFnJkM5ZyQbKY=; b=HO+tCjGxMlPtpIgUMQy5hpt99S3rAsAjBsY02O/KkkR qCEEZMIvXXNWUn3ZU77bWGDTXILOMi9Yu1iJNleVx3tIoQVp/zTRkWf+rqUmpU45 XJC4ju3Plgoi2wDpJ+VGc9vmwvdPliEII0swRM8+UYkp6NRX3WNf4V8NoRMjtrFt a/RmeG64XAAKlb6JQL95h6OzIolPtVJig2CQ35OPp0cKJH9Q96QLxuJ/dVfRHOkt iVwmGvxH8OIvobAm/SLDzbA68nXl3hG3NiCiR3uYLYIIrXZeIg/pY/riICHM9kt9 tgZABKKsyuYVWYC1B/0nQSAmq5RKeaSA9maMGEBiNUg== 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=lcle+/ jERTgceEHA6Sc9ld1ZNgNn7xqFnJkM5ZyQbKY=; b=EmATkszqNo4G7dry+AFjjk 82Pi8OMpnY18gZBfHi5XAk6wLQ3cTI/AdSzEgt1Yx1zprNL9cw3GwgzWqvcVXugM Y6VYr5X8WpjKpgNzEs2XN5iLJrYwrlqlLIMk4g/p3w00IMoM+pfG6I+mvch/SBVb PIT9rtOuWpNsK64Dq3XTsXzPlCuDxz+8oAITSGyroKFtnqpsFEa9a7I+mYRCuNfD +1NxERikT154Q1YcemqP7nsoqesLvuyVf60MlsqdA2qPAwZyWOhAmWlEBClywpnI UWHaGFTSyga1TLnlcIG6sCQ81OJtqu6lVtvzXUlL7zdOOt6Yncq6G88zA43hlTgw == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddruddtgdduudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 17 Jan 2022 03:12:49 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 746ac70c (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 17 Jan 2022 08:12:49 +0000 (UTC) Date: Mon, 17 Jan 2022 09:12:48 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Bryan Turner , Waleed Khan , =?iso-8859-1?q?=C6var_Arnfj=F6r=F0?= Bjarmason , Han-Wen Nienhuys Subject: [PATCH v4 5/6] refs: do not execute reference-transaction hook on packing refs Message-ID: <23c344854e499d25721502405763d185ac2ae2ea.1642406989.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 The reference-transaction hook is supposed to track logical changes to references, but it currently also gets executed when packing refs in a repository. This is unexpected and ultimately not all that useful: packing refs is not supposed to result in any user-visible change to the refs' state, and it ultimately is an implementation detail of how refs stores work. Fix this excessive execution of the hook when packing refs. Reported-by: Waleed Khan Signed-off-by: Patrick Steinhardt --- refs/files-backend.c | 6 ++++-- t/t1416-ref-transaction-hooks.sh | 11 +---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 4d4f0c2099..565929210a 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1121,7 +1121,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, 0, &err); + transaction = ref_store_transaction_begin(&refs->base, + REF_TRANSACTION_SKIP_HOOK, &err); if (!transaction) goto cleanup; ref_transaction_add_update( @@ -1192,7 +1193,8 @@ 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, 0, &err); + transaction = ref_store_transaction_begin(refs->packed_ref_store, + REF_TRANSACTION_SKIP_HOOK, &err); if (!transaction) return -1; diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index 0567fbdf0b..f9d3d5213f 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -150,21 +150,12 @@ test_expect_success 'hook does not get called on packing refs' ' git pack-refs --all && # We only expect a single hook invocation, which is the call to - # git-update-ref(1). But currently, packing refs will also trigger the - # hook. + # git-update-ref(1). cat >expect <<-EOF && prepared $ZERO_OID $POST_OID refs/heads/unpacked-ref committed $ZERO_OID $POST_OID refs/heads/unpacked-ref - prepared - $ZERO_OID $POST_OID refs/heads/unpacked-ref - committed - $ZERO_OID $POST_OID refs/heads/unpacked-ref - prepared - $POST_OID $ZERO_OID refs/heads/unpacked-ref - committed - $POST_OID $ZERO_OID refs/heads/unpacked-ref EOF test_cmp expect actual From patchwork Mon Jan 17 08:12:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12714962 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 7CA88C433F5 for ; Mon, 17 Jan 2022 08:12:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237961AbiAQIM4 (ORCPT ); Mon, 17 Jan 2022 03:12:56 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:40621 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237970AbiAQIM4 (ORCPT ); Mon, 17 Jan 2022 03:12:56 -0500 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 9EB355C012F; Mon, 17 Jan 2022 03:12:55 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Mon, 17 Jan 2022 03:12:55 -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=5L59ZEYboVBPWID//vXwT1PEYG4 VnruvwNAyldgUBaM=; b=Uxg4OGZdVDkkTXJ0tAiDtHbPo3WGvGTpFK/6SU/YwTE 5d4eehkxdYVm3axrscJAIrlCSrbf2iyWe8KA6DHpxgcIiCTCJ4JkjbfHrzV0I7gC wYKS8C6Xrx5+fBeBRK6F5fsN/9xD9rnCcDdhTVXpLOoqyjEbggHgMJw9NMPkvZuU uK+x42K8kAQtVzlg8HZysNBQmtvTblX6h9nLnH3O6AWiHwEIvCEVXdMKqvbDvIBv DejT9OheOqSPdYVT+++YAIjobIEBDS1eU8HYEdM6zrsFjMUoSwtlOnLgU7iNTYDZ r30p6dZOpqp/MLP+a8m6Ob0Qx4IhnW5qCSpN17n1mqg== 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=5L59ZE YboVBPWID//vXwT1PEYG4VnruvwNAyldgUBaM=; b=cOU4AvJEpb6PKFKEfTQwKO FMs1z1vTmOHCAIxRjYWG1RPPSmTa/+q83vQoYpRIZmSZ7KIajED8phlYCYRJ+IkQ /J3TfiL2G/KNXmS78bTjL5UhYzzgEfde0C6IPi4i1K7H9XugDFNaHowmm5faP741 Tu6hbXrxMBhPIz8Zy74h4rlyYSCYCr9i9qWr41JoWxqnDnFjQ/3OQOLOA6EpvcxZ HpfWjyELqw/NpJG2SRYm9LC6ljnYA8MNbzrF+qs201o0itTwAySHs4Aj3de+IcsC SQL1nBgwKH9vqk0IESIwCfmXRcbYAZP4/xZH0XuW1sMVYHktn+k5gLT3V7t4cU0Q == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddruddtgdduudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 17 Jan 2022 03:12:54 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id bb0e01c6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 17 Jan 2022 08:12:54 +0000 (UTC) Date: Mon, 17 Jan 2022 09:12:53 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Bryan Turner , Waleed Khan , =?iso-8859-1?q?=C6var_Arnfj=F6r=F0?= Bjarmason , Han-Wen Nienhuys Subject: [PATCH v4 6/6] refs: skip hooks when deleting uncovered packed refs Message-ID: 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. This works as expected even in case the refs to be deleted only exist in the packed-refs backend because the loose-backend always queues refs in its own transaction even if they don't exist such that they can be locked for concurrent creation. And it also does the right thing in case neither of the backends has the ref because that would cause the transaction to fail completely. 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 565929210a..844918cbd8 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; @@ -2776,7 +2777,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; @@ -3047,7 +3049,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