From patchwork Tue Nov 14 08:58:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13454970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5AA2B154B1 for ; Tue, 14 Nov 2023 08:58:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="T4MKqS+8"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="zTTU6ecz" Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 01CF0FA for ; Tue, 14 Nov 2023 00:58:42 -0800 (PST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 74D275C02A2; Tue, 14 Nov 2023 03:58:41 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Tue, 14 Nov 2023 03:58:41 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1699952321; x=1700038721; bh=hQ cst9/Eujhka2vrOSDghx4rNUm/LFaa7AyjX/NTz9s=; b=T4MKqS+82RrhhflDuP DdKOZfulj5f9SkegR2qFkQ9d03V47+vmukCARGGmKkQIxjuqk5cOMelwGY+vGEi3 1l1fYSLt1aPXdIqzTejqEO2oikbuRSXIQM02oUyfibSGbsjPHv8r02H3IQ/BW0aw S6CtkONTqdptxOJ9eYhagcEkPZ9iOzjSsdfC6ssflTEWR4gtJuZAG6BhgwOG9kro uozLFhFJuXs9r/JxmQcZ+H+3V8kPooXXweGhzPMlr8SdGhqxTv0AaAIwez7OEF6I uVhSbc2kVvr6jElhEDpNyBhf/IdhB7q9tCr6ObbuXrZ0h91oxM+R0ZaqpBl6swS8 0IBg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1699952321; x=1700038721; bh=hQcst9/Eujhka 2vrOSDghx4rNUm/LFaa7AyjX/NTz9s=; b=zTTU6eczqahPK/48IIKmF1IHfEODv YxpIxIiEZJMsTrw9PtOGjH0wNYJA41FIcDFDoxJUczho7vTkzjZvCzgkx5+PK3In HMP5tGVDne/2/btCcCOFYyoPk4rBKu0WGqciS4NPiCfOmNhicpKIT2wRv9DI6p9Z boCSWzFmYkpfK76ayo444BES5sWnonAQ8eX/PuZUerc5jNJ0qoudEii9tbqQgYuV 7tL/4uEoGE+4vZIV8U8XN8nXNQh0xmH3zQv4pbpUsYGqkzf2ZpW/SYXhW5ynNvc+ Nke++QGj2mrJo03bI21l7u/qwaLIA79PTphL739e1RqVqVQUijvV2x/mg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudefuddguddviecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 14 Nov 2023 03:58:40 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 34b0fea3 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 14 Nov 2023 08:58:02 +0000 (UTC) Date: Tue, 14 Nov 2023 09:58:38 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: David Turner , Han-Wen Nienhuys Subject: [PATCH 1/4] t5510: ensure that the packed-refs file needs locking Message-ID: <4be411fa413fda675314b8507d41f4141a1f97ac.1699951815.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: One of the tests in t5510 asserts that a `git fetch --prune` detects failures to prune branches. This is done by locking the packed-refs file, which would then later lead to a locking issue when Git tries to rewrite the file to prune the branches from it. Interestingly though, we do not pack the about-to-be-pruned branch into the packed-refs file, so it never even contained that branch in the first place. While this is good enough right now because the pruning will always lock the file regardless of whether it contains the branch or not, this is a mere implementation detail. In fact, we're about to rewrite branch deletions to make use of the ref transaction interface, which knows to skip rewrites of the packed-refs file in the case where it does not contain the branches in the first place, and this will break the test. Prepare the test for that change by packing the refs before trying to prune them. Signed-off-by: Patrick Steinhardt --- t/t5510-fetch.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index dcadd56d3a..79592a3b0a 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -169,6 +169,7 @@ test_expect_success REFFILES 'fetch --prune fails to delete branches' ' git clone . prune-fail && cd prune-fail && git update-ref refs/remotes/origin/extrabranch main && + git pack-refs --all && : this will prevent --prune from locking packed-refs for deleting refs, but adding loose refs still succeeds && >.git/packed-refs.new && From patchwork Tue Nov 14 08:58:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13454971 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B8B0E15ACE for ; Tue, 14 Nov 2023 08:58:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="XAYvf+Ul"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="YNdyU6jr" Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 469C9A4 for ; Tue, 14 Nov 2023 00:58:46 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.nyi.internal (Postfix) with ESMTP id A2BA55C0117; Tue, 14 Nov 2023 03:58:45 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 14 Nov 2023 03:58:45 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1699952325; x=1700038725; bh=VL O1zHZx036srSc7DVRFwaPRXgE5IgC/eKk1JRsFr/0=; b=XAYvf+UlHiXnaccY2r YFKF7iZ2YjuCFVTf/aMwmQalxwJ47jcI7oaYexH3xB39m8FWOp62fbxqKDeLxXed N/IX89w/fSZTEjEi18QwO3Y0wz+lXT7DeJfDcbgGkz20tKQD3+yqb4dnmJT+Pxet +JdL3eqaJOcO+V17/WUgsX5t8vWQOgniHuR6xccWziOgidJf3aJuq1Fpq1NK8PE1 rbidJ6K0oJ6kEXfXOY6zEmRUDdtOH8MuzJCLfEknyXvy7rpYj1nSQchdFiUNzKUa 6Ig0a0t3ajlPSh7ZoI13hQqTjomWC92q2F5ycjsSxmlEtwHlvdcPFHCDlK09Y5Nu TUAw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1699952325; x=1700038725; bh=VLO1zHZx036sr Sc7DVRFwaPRXgE5IgC/eKk1JRsFr/0=; b=YNdyU6jrnRU+nAVkpdshUM/JEbbiT yI+PRW6SKtbQai+zCoUS3PcWpJinbeAdZsZ+XADSRdN/rNq8ciTnuC9eWfwpwWJ7 cafK7sG5PkcUQXRTM3Pqzd3Ag3Yp7wodPuIfIcqZ+bLLKkMJuCGlj5tx/v1/yHA0 zpo4fYtqgDGXZexuWLpjQHyYYZgh4+Rd/Df6hkQnrMG/qGen7Jt+Je4xcJvukDOQ RF6g29mro9Qao0Pw8ehIZf8NlMpK7YVLAccbKL3Rhpz0D/zQsynpqEBER6f8QwKi QEdy0lBrgWtyR1cYeErH1DsNzmbrhRoikTKKLIE3jEmJVOQvKUseHotnQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudefuddguddviecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttdejnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeetueevhffhudefvdegieeuieelgedthfegfedtueevjeejtdfgjeehudejuedt udenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 14 Nov 2023 03:58:44 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id cc10b4ed (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 14 Nov 2023 08:58:06 +0000 (UTC) Date: Tue, 14 Nov 2023 09:58:42 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: David Turner , Han-Wen Nienhuys Subject: [PATCH 2/4] refs/files: use transactions to delete references Message-ID: <2cfa0245fb089ed9d85cd44dad38c7f261e2200a.1699951815.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In the `files_delete_refs()` callback function of the files backend we implement deletion of references. This is done in two steps: 1. We lock the packed-refs file and delete all references from it in a single transaction. 2. We delete all loose references via separate calls to `refs_delete_ref()`. These steps essentially duplicate the logic around locking and deletion order that we already have in the transactional interfaces, where we do know to lock and evict references from the packed-refs file. Despite the fact that we duplicate the logic, it's also less efficient than if we used a single generic transaction: - The transactional interface knows to skip locking of the packed refs in case they don't contain any of the refs which are about to be deleted. - We end up creating N+1 separate reference transactions, one for the packed-refs file and N for the individual loose references. Refactor the code to instead delete references via a single transaction. As we don't assert the expected old object ID this is equivalent to the previous behaviour, and we already do the same in the packed-refs backend. Despite the fact that the result is simpler to reason about, this change also results in improved performance. The following benchmarks have been executed in linux.git: ``` $ hyperfine -n '{rev}, packed={packed} refcount={refcount}' \ -L packed true,false -L refcount 1,1000 -L rev master,pks-ref-store-generic-delete-refs \ --setup 'git -C /home/pks/Development/git switch --detach {rev} && make -C /home/pks/Development/git -j17' \ --prepare 'printf "create refs/heads/new-branch-%d HEAD\n" $(seq {refcount}) | git -C /home/pks/Reproduction/linux.git update-ref --stdin && if test {packed} = true; then git pack-refs --all; fi' \ --warmup=10 \ '/home/pks/Development/git/bin-wrappers/git -C /home/pks/Reproduction/linux.git branch -d new-branch-{1..{refcount}}' Benchmark 1: master packed=true refcount=1 Time (mean ± σ): 7.8 ms ± 1.6 ms [User: 3.4 ms, System: 4.4 ms] Range (min … max): 5.5 ms … 11.0 ms 120 runs Benchmark 2: master packed=false refcount=1 Time (mean ± σ): 7.0 ms ± 1.1 ms [User: 3.2 ms, System: 3.8 ms] Range (min … max): 5.7 ms … 9.8 ms 180 runs Benchmark 3: master packed=true refcount=1000 Time (mean ± σ): 283.8 ms ± 5.2 ms [User: 45.7 ms, System: 231.5 ms] Range (min … max): 276.7 ms … 291.6 ms 10 runs Benchmark 4: master packed=false refcount=1000 Time (mean ± σ): 284.4 ms ± 5.3 ms [User: 44.2 ms, System: 233.6 ms] Range (min … max): 277.1 ms … 293.3 ms 10 runs Benchmark 5: generic-delete-refs packed=true refcount=1 Time (mean ± σ): 6.2 ms ± 1.8 ms [User: 2.3 ms, System: 3.9 ms] Range (min … max): 4.1 ms … 12.2 ms 142 runs Benchmark 6: generic-delete-refs packed=false refcount=1 Time (mean ± σ): 7.1 ms ± 1.4 ms [User: 2.8 ms, System: 4.3 ms] Range (min … max): 4.2 ms … 10.8 ms 157 runs Benchmark 7: generic-delete-refs packed=true refcount=1000 Time (mean ± σ): 198.9 ms ± 1.7 ms [User: 29.5 ms, System: 165.7 ms] Range (min … max): 196.1 ms … 201.4 ms 10 runs Benchmark 8: generic-delete-refs packed=false refcount=1000 Time (mean ± σ): 199.7 ms ± 7.8 ms [User: 32.2 ms, System: 163.2 ms] Range (min … max): 193.8 ms … 220.7 ms 10 runs Summary generic-delete-refs packed=true refcount=1 ran 1.14 ± 0.37 times faster than master packed=false refcount=1 1.15 ± 0.40 times faster than generic-delete-refs packed=false refcount=1 1.26 ± 0.44 times faster than master packed=true refcount=1 32.24 ± 9.17 times faster than generic-delete-refs packed=true refcount=1000 32.36 ± 9.29 times faster than generic-delete-refs packed=false refcount=1000 46.00 ± 13.10 times faster than master packed=true refcount=1000 46.10 ± 13.13 times faster than master packed=false refcount=1000 ``` Especially in the case where we have many references we can see a clear performance speedup of nearly 30%. This is in contrast to the stated objecive in a27dcf89b68 (refs: make delete_refs() virtual, 2016-09-04), where the virtual `delete_refs()` function was introduced with the intent to speed things up rather than making things slower. So it seems like we have outlived the need for a virtual function. Signed-off-by: Patrick Steinhardt --- refs/files-backend.c | 66 +++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index db5c0c7a72..778d3a96ba 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1268,49 +1268,51 @@ static int files_pack_refs(struct ref_store *ref_store, 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; + struct string_list_item *item; + int ret = 0, failures = 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); - goto error; + /* + * 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) { + ret = error("%s", err.buf); + goto out; } - 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)) - result |= error(_("could not remove reference %s"), refname); + for_each_string_list_item(item, refnames) { + ret = ref_transaction_delete(transaction, item->string, + NULL, flags, msg, &err); + if (ret) { + warning(_("could not delete reference %s: %s"), + item->string, err.buf); + strbuf_reset(&err); + failures = 1; + } } - strbuf_release(&err); - return result; - -error: - /* - * If we failed to rewrite the packed-refs file, then it is - * unsafe to try to remove loose refs, because doing so might - * expose an obsolete packed value for a reference that might - * even point at an object that has been garbage collected. - */ - 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); + 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); + } +out: + if (!ret && failures) + ret = -1; + ref_transaction_free(transaction); strbuf_release(&err); - return -1; + return ret; } /* From patchwork Tue Nov 14 08:58:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13454972 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0D87016414 for ; Tue, 14 Nov 2023 08:58:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="Uh5WLVyn"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="BO8W5zAt" Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 74BDCA4 for ; Tue, 14 Nov 2023 00:58:50 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id E345A5C0117; Tue, 14 Nov 2023 03:58:49 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 14 Nov 2023 03:58:49 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1699952329; x=1700038729; bh=Iu BiEhzXwNi4SZqlBHKdppXpLLO9BAJWCDEc3r6ODs4=; b=Uh5WLVynVlkDY2BdHq VStlYcZHWFO7rVfcKrAPkEiqncfPWL/FCZ22yi4Y4OuopS1FY/qA1GoYijx7e12N Mnn6Igjby7qMZ6jUICV2a/q2QUqrrif1MVnqukAzDsjpG/WhOVjpbJ07WE6aXQ5j TI/gnStLYbtrmFzwfcsQCGsmLdMzn40LsgFHUOhH6LQHCy+Vfns2M3Xxs/KQdsw3 9FacoWICzJmASRsA/Y6tjtMmybDWtLxqdO7XxXNsfGvKLItPBuohTfiagd6/j/OK dGPrr/V/x2AhdIy+VfYF8/ssWzu1FFTSfmAdSGDegcm0CyXpENNRylrbdBX8Jdtc 2x2g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1699952329; x=1700038729; bh=IuBiEhzXwNi4S ZqlBHKdppXpLLO9BAJWCDEc3r6ODs4=; b=BO8W5zAtlwQkSVuLqzm436MYjOR8y gIu+Fl02TL1UgiVd8lUMaYu+n7M+cLXTBp3mrk3tkCMGDKA6cYsL4efLgOCI/5JG OdUfIiy/dSFCqP4QkAaow5gSYZ5tdi1vaLrnh/sRXWaCGee1nH2fQ3e/JlcZg1X5 9/hCHmaGRx+AWjETusZuyzPaPxKcR+k80oAG94BcxxXnCRWCkR+tCBwks0d+husF ic1fDlWxU/oxyOfdQafbxZyZJu6d+mw8E8Q6Ymu3sjQTP3O+V8j+4pZVsoyKUzQr f078y9yd0ySkzvUEuRNPa+180ptzh+0Kh35b85xqJqymShEztzztTvznA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudefuddguddviecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 14 Nov 2023 03:58:48 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id b2a66564 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 14 Nov 2023 08:58:10 +0000 (UTC) Date: Tue, 14 Nov 2023 09:58:46 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: David Turner , Han-Wen Nienhuys Subject: [PATCH 3/4] refs: deduplicate code to delete references Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Both the files and the packed-refs reference backends now use the same generic transactions-based code to delete references. Let's pull these implementations up into `refs_delete_refs()` to deduplicate the code. Signed-off-by: Patrick Steinhardt --- refs.c | 48 ++++++++++++++++++++++++++++++++++++++++--- refs/files-backend.c | 46 +---------------------------------------- refs/packed-backend.c | 45 +--------------------------------------- 3 files changed, 47 insertions(+), 92 deletions(-) diff --git a/refs.c b/refs.c index fcae5dddc6..16bfa21df7 100644 --- a/refs.c +++ b/refs.c @@ -2599,13 +2599,55 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, int refs_delete_refs(struct ref_store *refs, const char *logmsg, struct string_list *refnames, unsigned int flags) { + struct ref_transaction *transaction; + struct strbuf err = STRBUF_INIT; + struct string_list_item *item; + int ret = 0, failures = 0; char *msg; - int retval; + + if (!refnames->nr) + return 0; msg = normalize_reflog_message(logmsg); - retval = refs->be->delete_refs(refs, msg, refnames, flags); + + /* + * 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(refs, &err); + if (!transaction) { + ret = error("%s", err.buf); + goto out; + } + + for_each_string_list_item(item, refnames) { + ret = ref_transaction_delete(transaction, item->string, + NULL, flags, msg, &err); + if (ret) { + warning(_("could not delete reference %s: %s"), + item->string, err.buf); + strbuf_reset(&err); + failures = 1; + } + } + + 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); + } + +out: + if (!ret && failures) + ret = -1; + ref_transaction_free(transaction); + strbuf_release(&err); free(msg); - return retval; + return ret; } int delete_refs(const char *msg, struct string_list *refnames, diff --git a/refs/files-backend.c b/refs/files-backend.c index 778d3a96ba..8d28810e67 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1268,51 +1268,7 @@ static int files_pack_refs(struct ref_store *ref_store, static int files_delete_refs(struct ref_store *ref_store, const char *msg, struct string_list *refnames, unsigned int flags) { - struct ref_transaction *transaction; - struct strbuf err = STRBUF_INIT; - struct string_list_item *item; - int ret = 0, failures = 0; - - 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) { - ret = error("%s", err.buf); - goto out; - } - - for_each_string_list_item(item, refnames) { - ret = ref_transaction_delete(transaction, item->string, - NULL, flags, msg, &err); - if (ret) { - warning(_("could not delete reference %s: %s"), - item->string, err.buf); - strbuf_reset(&err); - failures = 1; - } - } - - 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); - } - -out: - if (!ret && failures) - ret = -1; - ref_transaction_free(transaction); - strbuf_release(&err); - return ret; + return refs_delete_refs(ref_store, msg, refnames, flags); } /* diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 59c78d7941..1589577005 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1691,50 +1691,7 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store UNUSED, 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; + return refs_delete_refs(ref_store, msg, refnames, flags); } static int packed_pack_refs(struct ref_store *ref_store UNUSED, From patchwork Tue Nov 14 08:58:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13454973 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 04D71156C8 for ; Tue, 14 Nov 2023 08:58:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="WfpZgftf"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="h68cEoE0" Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9DADAA4 for ; Tue, 14 Nov 2023 00:58:54 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 189055C02CD; Tue, 14 Nov 2023 03:58:54 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 14 Nov 2023 03:58:54 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1699952334; x=1700038734; bh=XS 5T7Q19tnCREkTM3xevNpdFo16EPMp+TJZHqxDkdk8=; b=WfpZgftfqIlLW9sCtR qSaktpAbzZdYs1anDJgyEykOvUB52D7yevXx+lg5RBIRFANDziZu0kJR0Pb9Ucr0 2EBYdZF/4r/557UMHQlnNkhUNQ9vyzv3Gv7nwnmbFUfm3t7r6yDosOIesof/jmfD G7sJhdiZKRkxd5q8ZdXfcacJ6Y8ixszrRc1xBP2K72hdREfmn9NwN7kWaXoljn2V 13oLlcJF63qQg+MD0DZxyM57fMYDdXMgdx3FWdrKariIS39kO16C9W99+dpGl9Bk 7V+8Lxp0xgf3WXHrf6xKKrqEPJb7Voqe3DPPfcfwBEZ/Zwv8lLggWi20KKo2WbTi 3giQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1699952334; x=1700038734; bh=XS5T7Q19tnCRE kTM3xevNpdFo16EPMp+TJZHqxDkdk8=; b=h68cEoE0VgeHqZXzCJVa5/dfdcqsk jiB8/mzB/DtlU+E0pUTjvh2SfKajK+Ka25I2QPjUkFml22oi7KI5UmLAizxBlUP+ pkqFo3l2SUanagUaLBkDZtxCM/LcDF+dPn6ReQhggObsqgJ4qNt8Tf/tnYYeFiaq XLLyV1mMdwUbXxpZrjVEIgNuIFU2fhJu73Qj1yEAL99gOgynXaWlyHUmvvCI9ZDC llD3Q9R2CCcoqnbyeysYjGGkRW0gomUFaQYE5u+ixZuxqemAMXcxgfjGWoVFbwqA ZRe7VnJw8bPqYdjfN691br9AUxjMirlrs5tcvRAqPeI4szrOQt3Rp1qnQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudefuddguddviecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 14 Nov 2023 03:58:53 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id aa269565 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 14 Nov 2023 08:58:15 +0000 (UTC) Date: Tue, 14 Nov 2023 09:58:50 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: David Turner , Han-Wen Nienhuys Subject: [PATCH 4/4] refs: remove `delete_refs` callback from backends Message-ID: <0e61c42a92f5fbaba28d7326645a6f1903a5ae66.1699951815.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Now that `refs_delete_refs` is implemented in a generic way via the ref transaction interfaces there are no callers left that invoke the `delete_refs` callback anymore. Remove it from all of our backends. Signed-off-by: Patrick Steinhardt --- refs/debug.c | 15 --------------- refs/files-backend.c | 7 ------- refs/packed-backend.c | 7 ------- refs/refs-internal.h | 3 --- 4 files changed, 32 deletions(-) diff --git a/refs/debug.c b/refs/debug.c index b7ffc4ce67..83b7a0ba65 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -143,20 +143,6 @@ static int debug_create_symref(struct ref_store *ref_store, return res; } -static int debug_delete_refs(struct ref_store *ref_store, const char *msg, - struct string_list *refnames, unsigned int flags) -{ - struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; - int res = - drefs->refs->be->delete_refs(drefs->refs, msg, refnames, flags); - int i; - trace_printf_key(&trace_refs, "delete_refs {\n"); - for (i = 0; i < refnames->nr; i++) - trace_printf_key(&trace_refs, "%s\n", refnames->items[i].string); - trace_printf_key(&trace_refs, "}: %d\n", res); - return res; -} - static int debug_rename_ref(struct ref_store *ref_store, const char *oldref, const char *newref, const char *logmsg) { @@ -458,7 +444,6 @@ struct ref_storage_be refs_be_debug = { .pack_refs = debug_pack_refs, .create_symref = debug_create_symref, - .delete_refs = debug_delete_refs, .rename_ref = debug_rename_ref, .copy_ref = debug_copy_ref, diff --git a/refs/files-backend.c b/refs/files-backend.c index 8d28810e67..ad8b1d143f 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1265,12 +1265,6 @@ static int files_pack_refs(struct ref_store *ref_store, return 0; } -static int files_delete_refs(struct ref_store *ref_store, const char *msg, - struct string_list *refnames, unsigned int flags) -{ - return refs_delete_refs(ref_store, msg, refnames, flags); -} - /* * People using contrib's git-new-workdir have .git/logs/refs -> * /some/other/path/.git/logs/refs, and that may live on another device. @@ -3258,7 +3252,6 @@ struct ref_storage_be refs_be_files = { .pack_refs = files_pack_refs, .create_symref = files_create_symref, - .delete_refs = files_delete_refs, .rename_ref = files_rename_ref, .copy_ref = files_copy_ref, diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 1589577005..b9fa097a29 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1688,12 +1688,6 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store UNUSED, 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) -{ - return refs_delete_refs(ref_store, msg, refnames, flags); -} - static int packed_pack_refs(struct ref_store *ref_store UNUSED, struct pack_refs_opts *pack_opts UNUSED) { @@ -1722,7 +1716,6 @@ struct ref_storage_be refs_be_packed = { .pack_refs = packed_pack_refs, .create_symref = NULL, - .delete_refs = packed_delete_refs, .rename_ref = NULL, .copy_ref = NULL, diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 9db8aec4da..4af83bf9a5 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -553,8 +553,6 @@ typedef int create_symref_fn(struct ref_store *ref_store, const char *ref_target, const char *refs_heads_master, const char *logmsg); -typedef int delete_refs_fn(struct ref_store *ref_store, const char *msg, - struct string_list *refnames, unsigned int flags); typedef int rename_ref_fn(struct ref_store *ref_store, const char *oldref, const char *newref, const char *logmsg); @@ -677,7 +675,6 @@ struct ref_storage_be { pack_refs_fn *pack_refs; create_symref_fn *create_symref; - delete_refs_fn *delete_refs; rename_ref_fn *rename_ref; copy_ref_fn *copy_ref;