From patchwork Mon Feb 21 08:02:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12753222 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 3D3D8C433EF for ; Mon, 21 Feb 2022 08:02:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346747AbiBUICi (ORCPT ); Mon, 21 Feb 2022 03:02:38 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:56044 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346745AbiBUICh (ORCPT ); Mon, 21 Feb 2022 03:02:37 -0500 Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 11E9710B2 for ; Mon, 21 Feb 2022 00:02:13 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 50BF75C01BE; Mon, 21 Feb 2022 03:02:13 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Mon, 21 Feb 2022 03:02:13 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :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=fm3; bh=0uSrj3awEo+Z54YxS2JISWEwlLxTjnaLK5l5em X/H0o=; b=tX8MKmrm5CMg8gDThd7XZliCcRHh9m4U0emNtYvJbRlZC5GpeGHvHO UHZQw4ppXn0UIG54lvuxxtwQxF8Rn9IvDZOyP5CxfF+yTR9+FaIfr8TkXfjDFYL2 n5ujePmSIzieuh0cDo4KNfZCGm44W6HeKcA1ZJtJJHOfss/Hsynq4qC0oULCk9T6 9bj37LVm4JcpU/0maLnufcz71EddpiNZLyzZ69cT5O9HFbLxbc5+GiyxCHMWABLL Jz/Zr0sukGQ26Zef3Zf/P2XYcW1UlK93OBlha9DTcMHA/mapNC6Rs5rLPkGJUBOd aktP9+bAl6iyGs0MvrOpo/czGm0IFbSg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date: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=fm2; bh=0uSrj3awEo+Z54YxS 2JISWEwlLxTjnaLK5l5emX/H0o=; b=mxJsfK232fWAhPSpqPzoPnefVl26E+wRX CTRc0njUr8BYg2syWwwGRLw1fENK1mF5o4zyThcQdstUjqGz3GXUpE93zwFdfjkH T+K1ZxqVOS/kD/V0Bgstz99orysygpY9dmM4gLJ855GdnYcXWJlBm0fpwEk6CAE8 gti7VzzJ6onJ8pAFccyoLVmRhniJO86XGzOnx7s0+4SNXcv2W5CXjK7ZW8arkkjh 5qVoO1BAHwMge2+C54vwBov8RSaWccVSNFPg1qPzPtLjLoCxUaz6K/48x5ElLHrA 1qrKsDVTjUEA7nCWJPVeOV+LPSWv+/6MgoKRP8FmQm4KvvSfoT8/w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrkeehgdduuddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 21 Feb 2022 03:02:11 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 9f672f38 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 21 Feb 2022 08:02:11 +0000 (UTC) Date: Mon, 21 Feb 2022 09:02:10 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Christian Couder , Jonathan Tan , Elijah Newren , Junio C Hamano Subject: [PATCH v3 1/7] fetch: increase test coverage of fetches Message-ID: <081174b7a00cf094a7dacd8aba89fb137ea40f66.1645430423.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 using git-fetch(1) with the `--atomic` flag the expectation is that either all of the references are updated, or alternatively none are in case the fetch fails. While we already have tests for this, we do not have any tests which exercise atomicity either when pruning deleted refs or when backfilling tags. This gap in test coverage hides that we indeed don't handle atomicity correctly for both of these cases. Add test cases which cover these testing gaps to demonstrate the broken behaviour. Note that tests are not marked as `test_expect_failure`: this is done to explicitly demonstrate the current known-wrong behaviour, and they will be fixed up as soon as we fix the underlying bugs. While at it this commit also adds another test case which demonstrates that backfilling of tags does not return an error code in case the backfill fails. This bug will also be fixed by a subsequent commit. Signed-off-by: Patrick Steinhardt --- t/t5503-tagfollow.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++ t/t5510-fetch.sh | 33 ++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh index 195fc64dd4..7dadaafd4b 100755 --- a/t/t5503-tagfollow.sh +++ b/t/t5503-tagfollow.sh @@ -160,4 +160,85 @@ test_expect_success 'new clone fetch main and tags' ' test_cmp expect actual ' +test_expect_success 'atomic fetch with failing backfill' ' + git init clone3 && + + # We want to test whether a failure in `backfill_tags()` correctly + # aborts the complete transaction when `--atomic` is passed: we should + # neither create the branch nor should we create the tag when either + # one of both fails to update correctly. + # + # To trigger failure we simply abort when backfilling a tag. + write_script clone3/.git/hooks/reference-transaction <<-\EOF && + while read oldrev newrev reference + do + if test "$reference" = refs/tags/tag1 + then + exit 1 + fi + done + EOF + + test_must_fail git -C clone3 fetch --atomic .. $B:refs/heads/something && + + # Creation of the tag has failed, so ideally refs/heads/something + # should not exist. The fact that it does demonstrates that there is + # a bug in the `--atomic` flag. + test $B = "$(git -C clone3 rev-parse --verify refs/heads/something)" +' + +test_expect_success 'atomic fetch with backfill should use single transaction' ' + git init clone4 && + + # Fetching with the `--atomic` flag should update all references in a + # single transaction, including backfilled tags. We thus expect to see + # a single reference transaction for the created branch and tags. + cat >expected <<-EOF && + prepared + $ZERO_OID $B refs/heads/something + $ZERO_OID $S refs/tags/tag2 + committed + $ZERO_OID $B refs/heads/something + $ZERO_OID $S refs/tags/tag2 + prepared + $ZERO_OID $T refs/tags/tag1 + committed + $ZERO_OID $T refs/tags/tag1 + EOF + + write_script clone4/.git/hooks/reference-transaction <<-\EOF && + ( echo "$*" && cat ) >>actual + EOF + + git -C clone4 fetch --atomic .. $B:refs/heads/something && + test_cmp expected clone4/actual +' + +test_expect_success 'backfill failure causes command to fail' ' + git init clone5 && + + write_script clone5/.git/hooks/reference-transaction <<-EOF && + while read oldrev newrev reference + do + if test "\$reference" = refs/tags/tag1 + then + # Create a nested tag below the actual tag we + # wanted to write, which causes a D/F conflict + # later when we want to commit refs/tags/tag1. + # We cannot just `exit 1` here given that this + # would cause us to die immediately. + git update-ref refs/tags/tag1/nested $B + exit \$! + fi + done + EOF + + # Even though we fail to create refs/tags/tag1 the below command + # unexpectedly succeeds. + git -C clone5 fetch .. $B:refs/heads/something && + test $B = $(git -C clone5 rev-parse --verify refs/heads/something) && + test $S = $(git -C clone5 rev-parse --verify tag2) && + test_must_fail git -C clone5 rev-parse --verify tag1 +' + test_done diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ef0da0a63b..70d51f343b 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -343,6 +343,39 @@ test_expect_success 'fetch --atomic --append appends to FETCH_HEAD' ' test_cmp expected atomic/.git/FETCH_HEAD ' +test_expect_success 'fetch --atomic --prune executes a single reference transaction only' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git branch scheduled-for-deletion && + git clone . atomic && + git branch -D scheduled-for-deletion && + git branch new-branch && + head_oid=$(git rev-parse HEAD) && + + # Fetching with the `--atomic` flag should update all references in a + # single transaction. It is currently missing coverage of pruned + # references though, and as a result those may be committed to disk + # even if updating references fails later. + cat >expected <<-EOF && + prepared + $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion + committed + $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion + prepared + $ZERO_OID $head_oid refs/remotes/origin/new-branch + committed + $ZERO_OID $head_oid refs/remotes/origin/new-branch + EOF + + write_script atomic/.git/hooks/reference-transaction <<-\EOF && + ( echo "$*" && cat ) >>actual + EOF + + git -C atomic fetch --atomic --prune origin && + test_cmp expected atomic/actual +' + test_expect_success '--refmap="" ignores configured refspec' ' cd "$TRASH_DIRECTORY" && git clone "$D" remote-refs && From patchwork Mon Feb 21 08:02:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12753223 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 D6F80C433FE for ; Mon, 21 Feb 2022 08:02:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346763AbiBUICw (ORCPT ); Mon, 21 Feb 2022 03:02:52 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:56734 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346757AbiBUICm (ORCPT ); Mon, 21 Feb 2022 03:02:42 -0500 Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 267C1640F for ; Mon, 21 Feb 2022 00:02:18 -0800 (PST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id BF76F5C01C5; Mon, 21 Feb 2022 03:02:17 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 21 Feb 2022 03:02:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :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=fm3; bh=DDmoBpZCGGsURk2U4IHK66lrUTM95rRaioMUfT iBqaI=; b=N9Hxu0p+/I9jqxNoYg8vW4wNcytjwvCdoCVkjDEoNnEv7/+yrwCGDN 3lXIrk+j0pJXZ6a72Lv2n7/fT0zraaWHGmJP2i1n+0HHVoEvaTIuzXFyk30RqYln 6tHROw0+D0UoMInFq6SJDzuOIzVWjdS5XxJYvqPuBZ0VmVrc76g31tfUPh/cuhKN Ji7FKXyl+NnyTznZFruO1P9CY6EqdSnFdaDwcSZRU5DkHDQauhRQ9aDrD1Zkc2D2 cf1inWg9PUSaYH0DJuX0hF7fuFdLU3oj0/fMlFdd7bU/rn5FdsYSuAf8p8gyMyN8 s+wSaCC5NVhlhrQPptJqXFICBy4tqerQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date: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=fm2; bh=DDmoBpZCGGsURk2U4 IHK66lrUTM95rRaioMUfTiBqaI=; b=T8XQCo3WkAKhxXL9hP46NslirW6x5aGQN +43EDBXc54uNFdO2mhvfh0wId0ljHsTFKJLbXK65lA10BUIjI2nBeMN6Dl30QTN4 ucZTsA4qCREpQ4w6Vh90PZwFU5BN+2e3w8mnJCbmL+PXrPasbUALmx4a/nJUMH8t ZE9O155VQ4dbT2Gb2wNXPFqGiCKg+oN5BWlnz/Z7Uf3sipec9KO0LmLrRMjuHZRr YntxCwvwhRFcYPLFdK8auonVdUYBmiEBj/z6k8FfxEmO0pqHfzuyVQx3Mj7V17r6 q7JDcFUd+/OdU6YME//3R2Qvfs56wDhoVaVwc5URDZvDpy7Fq0udA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrkeehgdduuddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 21 Feb 2022 03:02:16 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 60f8c235 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 21 Feb 2022 08:02:15 +0000 (UTC) Date: Mon, 21 Feb 2022 09:02:14 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Christian Couder , Jonathan Tan , Elijah Newren , Junio C Hamano Subject: [PATCH v3 2/7] fetch: backfill tags before setting upstream Message-ID: <9867e76ac70c51cc6ccaf3ad7ef64250dad24920.1645430423.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 fetch code flow is a bit hard to understand right now: 1. We optionally prune all references which have vanished on the remote side. 2. We fetch and update all other references locally. 3. We update the upstream branch in the gitconfig. 4. We backfill tags pointing into the history we have just fetched. It is quite confusing that we fetch objects and update references in both (2) and (4), which is further stressed by the point that we use a `skip` goto label to jump from (3) to (4) in case we fail to update the gitconfig as expected. Reorder the code to first update all local references, and only after we have done so update the upstream branch information. This improves the code flow and furthermore makes it easier to refactor the way we update references together. Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 6f5e157863..904ca9f1ca 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1536,7 +1536,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map, static int do_fetch(struct transport *transport, struct refspec *rs) { - struct ref *ref_map; + struct ref *ref_map = NULL; int autotags = (transport->remote->fetch_tags == 1); int retcode = 0; const struct ref *remote_refs; @@ -1620,11 +1620,24 @@ static int do_fetch(struct transport *transport, retcode = 1; } if (fetch_and_consume_refs(transport, ref_map, worktrees)) { - free_refs(ref_map); retcode = 1; goto cleanup; } + /* + * If neither --no-tags nor --tags was specified, do automated tag + * following. + */ + if (tags == TAGS_DEFAULT && autotags) { + struct ref *tags_ref_map = NULL, **tail = &tags_ref_map; + + find_non_local_tags(remote_refs, &tags_ref_map, &tail); + if (tags_ref_map) + backfill_tags(transport, tags_ref_map, worktrees); + + free_refs(tags_ref_map); + } + if (set_upstream) { struct branch *branch = branch_get("HEAD"); struct ref *rm; @@ -1644,7 +1657,7 @@ static int do_fetch(struct transport *transport, if (!rm->peer_ref) { if (source_ref) { warning(_("multiple branches detected, incompatible with --set-upstream")); - goto skip; + goto cleanup; } else { source_ref = rm; } @@ -1658,7 +1671,7 @@ static int do_fetch(struct transport *transport, warning(_("could not set upstream of HEAD to '%s' from '%s' when " "it does not point to any branch."), shortname, transport->remote->name); - goto skip; + goto cleanup; } if (!strcmp(source_ref->name, "HEAD") || @@ -1678,21 +1691,9 @@ static int do_fetch(struct transport *transport, "you need to specify exactly one branch with the --set-upstream option")); } } -skip: - free_refs(ref_map); - - /* if neither --no-tags nor --tags was specified, do automated tag - * following ... */ - if (tags == TAGS_DEFAULT && autotags) { - struct ref **tail = &ref_map; - ref_map = NULL; - find_non_local_tags(remote_refs, &ref_map, &tail); - if (ref_map) - backfill_tags(transport, ref_map, worktrees); - free_refs(ref_map); - } cleanup: + free_refs(ref_map); free_worktrees(worktrees); return retcode; } From patchwork Mon Feb 21 08:02:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12753224 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 A1665C433EF for ; Mon, 21 Feb 2022 08:02:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346771AbiBUICy (ORCPT ); Mon, 21 Feb 2022 03:02:54 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:57112 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346769AbiBUICt (ORCPT ); Mon, 21 Feb 2022 03:02:49 -0500 Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 518466548 for ; Mon, 21 Feb 2022 00:02:22 -0800 (PST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 198635C01B2; Mon, 21 Feb 2022 03:02:22 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Mon, 21 Feb 2022 03:02:22 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :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=fm3; bh=FW/C05LVjEyRJS4pN+Zduw5lTxDkjGDwjlSKt9 4cHgg=; b=wCA2Qy1lowEJdl6/epaDPj5Udm8HJnz1kXCkrU97eyvHeHHiA73j0h vRoRyrVLgMhwlSdMDATOZ8I/hc9RaOyUW5u4boHoY6DaBdQETOebbaKy8Nfbbny6 VDrXXOcRKAxE+JRjFmU/WaoZxgUamcrUbgbsjXzwXhukVI7DqIH3FRMPtk8tIlyl QpdFxWqGp/FhnVvEs5Mg9trwvDLwCFEbhBzgfhhYSEZ9Ubg7XfvLdXbVaIVF7HXi UGYu35ohmHC+8g/U+EZFxX+s+FtGraBjMHJtBW+r9hxBPALbNiESeCNJkl8jobCj 3aA5BUa2Ume38Ilu9xChKr3u0BJ7Pfag== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date: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=fm2; bh=FW/C05LVjEyRJS4pN +Zduw5lTxDkjGDwjlSKt94cHgg=; b=OVUY+RvLPVIjpw+NaL8H/uGCYLtoVpUbK RSAnf2erdraYYvYU/4mEgAZtKXw7CrtgCZQEmGb2zUhXfE+SX0XWtxMve/ynipUu Nn/4lQ8RbzB6RPKssLLHV42pV2MgaW9+S4erwjHpWFxnH3w3KO+UWmGfxGjBgIfh LXBTWJft3LmQzjIpmrcpGwVV4q7gubbJa1vrac771gbH1G0pU1X86QCeexbdSsSt F0+ew82HGtNjvLOX/ov0AwnAGpLcflVV3BNNjU06xApW603c6AAAuzTYglw+ePyu 5zhFULENipS3k/rtOh1yEjOpyDXUDCnxaJpA0AFkCVrdiJsm+XJGA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrkeehgdduuddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 21 Feb 2022 03:02:20 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 17d9f865 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 21 Feb 2022 08:02:19 +0000 (UTC) Date: Mon, 21 Feb 2022 09:02:18 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Christian Couder , Jonathan Tan , Elijah Newren , Junio C Hamano Subject: [PATCH v3 3/7] fetch: control lifecycle of FETCH_HEAD in a single place Message-ID: <7c36ecbcf4daf1580c7deb9b7536b413dec8618b.1645430423.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 There are two different locations where we're appending to FETCH_HEAD: first when storing updated references, and second when backfilling tags. Both times we open the file, append to it and then commit it into place, which is essentially duplicate work. Improve the lifecycle of updating FETCH_HEAD by opening and committing it once in `do_fetch()`, where we pass the structure down to the code which wants to append to it. Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 904ca9f1ca..f8adb40b45 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1084,9 +1084,8 @@ N_("it took %.2f seconds to check forced updates; you can use\n" static int store_updated_refs(const char *raw_url, const char *remote_name, int connectivity_checked, struct ref *ref_map, - struct worktree **worktrees) + struct fetch_head *fetch_head, struct worktree **worktrees) { - struct fetch_head fetch_head; int url_len, i, rc = 0; struct strbuf note = STRBUF_INIT, err = STRBUF_INIT; struct ref_transaction *transaction = NULL; @@ -1096,10 +1095,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, int want_status; int summary_width = transport_summary_width(ref_map); - rc = open_fetch_head(&fetch_head); - if (rc) - return -1; - if (raw_url) url = transport_anonymize_url(raw_url); else @@ -1206,7 +1201,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, strbuf_addf(¬e, "'%s' of ", what); } - append_fetch_head(&fetch_head, &rm->old_oid, + append_fetch_head(fetch_head, &rm->old_oid, rm->fetch_head_status, note.buf, url, url_len); @@ -1246,9 +1241,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, } } - if (!rc) - commit_fetch_head(&fetch_head); - if (rc & STORE_REF_ERROR_DF_CONFLICT) error(_("some local refs could not be updated; try running\n" " 'git remote prune %s' to remove any old, conflicting " @@ -1268,7 +1260,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, strbuf_release(&err); ref_transaction_free(transaction); free(url); - close_fetch_head(&fetch_head); return rc; } @@ -1309,6 +1300,7 @@ static int check_exist_and_connected(struct ref *ref_map) static int fetch_and_consume_refs(struct transport *transport, struct ref *ref_map, + struct fetch_head *fetch_head, struct worktree **worktrees) { int connectivity_checked = 1; @@ -1331,7 +1323,7 @@ static int fetch_and_consume_refs(struct transport *transport, trace2_region_enter("fetch", "consume_refs", the_repository); ret = store_updated_refs(transport->url, transport->remote->name, - connectivity_checked, ref_map, worktrees); + connectivity_checked, ref_map, fetch_head, worktrees); trace2_region_leave("fetch", "consume_refs", the_repository); out: @@ -1503,7 +1495,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen) return transport; } -static void backfill_tags(struct transport *transport, struct ref *ref_map, +static void backfill_tags(struct transport *transport, + struct ref *ref_map, + struct fetch_head *fetch_head, struct worktree **worktrees) { int cannot_reuse; @@ -1525,7 +1519,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map, transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); - fetch_and_consume_refs(transport, ref_map, worktrees); + fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); if (gsecondary) { transport_disconnect(gsecondary); @@ -1544,6 +1538,7 @@ static int do_fetch(struct transport *transport, TRANSPORT_LS_REFS_OPTIONS_INIT; int must_list_refs = 1; struct worktree **worktrees = get_worktrees(); + struct fetch_head fetch_head = { 0 }; if (tags == TAGS_DEFAULT) { if (transport->remote->fetch_tags == 2) @@ -1601,6 +1596,10 @@ static int do_fetch(struct transport *transport, if (!update_head_ok) check_not_current_branch(ref_map, worktrees); + retcode = open_fetch_head(&fetch_head); + if (retcode) + goto cleanup; + if (tags == TAGS_DEFAULT && autotags) transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1"); if (prune) { @@ -1619,7 +1618,8 @@ static int do_fetch(struct transport *transport, if (retcode != 0) retcode = 1; } - if (fetch_and_consume_refs(transport, ref_map, worktrees)) { + + if (fetch_and_consume_refs(transport, ref_map, &fetch_head, worktrees)) { retcode = 1; goto cleanup; } @@ -1633,11 +1633,13 @@ static int do_fetch(struct transport *transport, find_non_local_tags(remote_refs, &tags_ref_map, &tail); if (tags_ref_map) - backfill_tags(transport, tags_ref_map, worktrees); + backfill_tags(transport, tags_ref_map, &fetch_head, worktrees); free_refs(tags_ref_map); } + commit_fetch_head(&fetch_head); + if (set_upstream) { struct branch *branch = branch_get("HEAD"); struct ref *rm; @@ -1693,6 +1695,7 @@ static int do_fetch(struct transport *transport, } cleanup: + close_fetch_head(&fetch_head); free_refs(ref_map); free_worktrees(worktrees); return retcode; From patchwork Mon Feb 21 08:02:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12753225 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 8C1B1C433EF for ; Mon, 21 Feb 2022 08:02:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346785AbiBUIDA (ORCPT ); Mon, 21 Feb 2022 03:03:00 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:57188 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346775AbiBUICu (ORCPT ); Mon, 21 Feb 2022 03:02:50 -0500 Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 30785658C for ; Mon, 21 Feb 2022 00:02:26 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 7A7855C01C5; Mon, 21 Feb 2022 03:02:25 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Mon, 21 Feb 2022 03:02:25 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :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=fm3; bh=guQrSiCNXZHeCa15jC0foN9AFRnZp2nmW4AG4f +xO/0=; b=Yxn1BQpXcH1mCRbp4k4T9yhpIXSeS0KiDxGZOtmBc1mttq3b+1BJTl ZIQJ+G2H0d/8m7TKC41denpzbbV9gD/b+atkJAuqEzkpDoXhjJ1aY3jqiVzyUe00 ZY+cnP7lQNZcPcdYrZN0Kc4gF/UXz/pCRfa+KOUH5oKUu/ZZ0ClkzAQg6yowDMhb HSBubTtI0RZHvP3wbjnYC1LqXguzkdArhZlbS6Dr81XGFnRCiJQZYx4mqzeiAeaq fmYK2Qlrx7glfJz4SDgAa4WzgzumPmZVSGPbqAeWCMSFdg7X0sKb71Jx7rWK9Ltd Ol0k/BeA8Zt7nIK5nOt69yQnjaU8of3g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date: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=fm2; bh=guQrSiCNXZHeCa15j C0foN9AFRnZp2nmW4AG4f+xO/0=; b=C+rs6n/WLrF05JNLPcoXgmTp7GPdddKZd k8WATQg9pbtZmIpROJ1iXEJekxk7QqKQG9x8asN8vJWIt1KeZ12AkHhBw0mVXvrY 3uxjMpwyvy7NO/e9S1+wMULk0tPMdbXqmMBiLoD9oaJX46KcmXktHE2KWLI7X1vb v8vDAPVIuS6XFNFyvvgd2UAE5OCsuhnlrmP3IBrn/smA3hBzXa9Hf/J+MiFmA3rA xCAJ0CJSTnnXRrXfWzY5IZjddDmP3ClIneqVlXgnl2q0Uc4bXORdeaW5wkVkSOzI lGw0fWlqFNXlVZG2bx28X4aNQBB4w5fZNJOP9SYptfcYZIcaHxLOw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrkeehgdduuddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 21 Feb 2022 03:02:24 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 19cb84a9 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 21 Feb 2022 08:02:23 +0000 (UTC) Date: Mon, 21 Feb 2022 09:02:22 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Christian Couder , Jonathan Tan , Elijah Newren , Junio C Hamano Subject: [PATCH v3 4/7] fetch: report errors when backfilling tags fails 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 the backfilling of tags fails we do not report this error to the caller, but only report it implicitly at a later point when reporting updated references. This leaves callers unable to act upon the information of whether the backfilling succeeded or not. Refactor the function to return an error code and pass it up the callstack. This causes us to correctly propagate the error back to the user of git-fetch(1). Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 26 ++++++++++++++++++-------- t/t5503-tagfollow.sh | 4 +--- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index f8adb40b45..d304314f16 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1495,12 +1495,12 @@ static struct transport *prepare_transport(struct remote *remote, int deepen) return transport; } -static void backfill_tags(struct transport *transport, - struct ref *ref_map, - struct fetch_head *fetch_head, - struct worktree **worktrees) +static int backfill_tags(struct transport *transport, + struct ref *ref_map, + struct fetch_head *fetch_head, + struct worktree **worktrees) { - int cannot_reuse; + int retcode, cannot_reuse; /* * Once we have set TRANS_OPT_DEEPEN_SINCE, we can't unset it @@ -1519,12 +1519,14 @@ static void backfill_tags(struct transport *transport, transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); - fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); + retcode = fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); if (gsecondary) { transport_disconnect(gsecondary); gsecondary = NULL; } + + return retcode; } static int do_fetch(struct transport *transport, @@ -1632,8 +1634,16 @@ static int do_fetch(struct transport *transport, struct ref *tags_ref_map = NULL, **tail = &tags_ref_map; find_non_local_tags(remote_refs, &tags_ref_map, &tail); - if (tags_ref_map) - backfill_tags(transport, tags_ref_map, &fetch_head, worktrees); + if (tags_ref_map) { + /* + * If backfilling of tags fails then we want to tell + * the user so, but we have to continue regardless to + * populate upstream information of the references we + * have already fetched above. + */ + if (backfill_tags(transport, tags_ref_map, &fetch_head, worktrees)) + retcode = 1; + } free_refs(tags_ref_map); } diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh index 7dadaafd4b..27a1dfa3c0 100755 --- a/t/t5503-tagfollow.sh +++ b/t/t5503-tagfollow.sh @@ -233,9 +233,7 @@ test_expect_success 'backfill failure causes command to fail' ' done EOF - # Even though we fail to create refs/tags/tag1 the below command - # unexpectedly succeeds. - git -C clone5 fetch .. $B:refs/heads/something && + test_must_fail git -C clone5 fetch .. $B:refs/heads/something && test $B = $(git -C clone5 rev-parse --verify refs/heads/something) && test $S = $(git -C clone5 rev-parse --verify tag2) && test_must_fail git -C clone5 rev-parse --verify tag1 From patchwork Mon Feb 21 08:02:26 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12753226 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 CA25BC433EF for ; Mon, 21 Feb 2022 08:02:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346753AbiBUIDD (ORCPT ); Mon, 21 Feb 2022 03:03:03 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:57182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346770AbiBUICx (ORCPT ); Mon, 21 Feb 2022 03:02:53 -0500 Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6AF75B7DD for ; Mon, 21 Feb 2022 00:02:30 -0800 (PST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id C94CA5C01BB; Mon, 21 Feb 2022 03:02:29 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Mon, 21 Feb 2022 03:02:29 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :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=fm3; bh=V4tjAGBbHvpLW7pnlVDcdjMx04NRXmr1D98T/F ietXU=; b=CoEYWlmUHEjQdcMqdWB1GDiTE1j63lpiTREychvSlVp/FW+WngcX5+ 8Kn0QqrwsCwFjELoP3tRto+74XXu9G6xj38ps0PapBRo7JVNzVUi5nYX94p/6Ygo VugcfLr5edi7+5S3Jgdro7E4QcvYkHmohe+lDfuKpN/avT3xxdW/apYIq1Bo+9Pf vZlo4lrZHerFrnyDdRUDoPuIvlFyA7ul6WqmRJ/pag36LKs1JE8++tU4dlQTKyno kNCPppsVXG4vLBPKaKSuQHwiIsVDX9zuYE/RkwMqOZSB2GaBc3nXb4ulDe/uCzZJ K8njUg4zW/EM0SHG8iUSRu7e+XFYPldw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date: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=fm2; bh=V4tjAGBbHvpLW7pnl VDcdjMx04NRXmr1D98T/FietXU=; b=PqRoGxeWQmqT8WyNCabJ0InNUhVDfwbI6 ZNVueSdRtjmNBw2VBy7hWX/2/ebDx9ztNkbkw7qiwWOdLyTemB7ShXeMBM+pTaeN DniIWPHUP30R4Ebg34Nm+Dr5zJ7LS/z6DId2J2e/yY6ug9oo6PaVqk6ltFNjyNPg S+34lHmvzaJDsqRlPJACkbzuLbQXNp0phJX4/B6sy0DrVrQggnTqmgEqEkoaL01/ mbDp9pEV05tXSwWHvAiNMaZ0vGWNPu7rmN0Dwh8BYqUU1TfENlvv7fqLd4l4gBGi yNNLsOr8hKKx+5KfZIWttXNXdSoH63ASzzgaVbiNWcTVOvfsH3xBA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrkeehgdduuddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 21 Feb 2022 03:02:28 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id a12ce5fc (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 21 Feb 2022 08:02:27 +0000 (UTC) Date: Mon, 21 Feb 2022 09:02:26 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Christian Couder , Jonathan Tan , Elijah Newren , Junio C Hamano Subject: [PATCH v3 5/7] refs: add interface to iterate over queued transactional updates Message-ID: <0316d770e9590641a942370c84e4d8a42df2ffcd.1645430423.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 There is no way for a caller to see whether a reference update has already been queued up for a given reference transaction. There are multiple alternatives to provide this functionality: - We may add a function that simply tells us whether a specific reference has already been queued. If implemented naively then this would potentially be quadratic in runtime behaviour if this question is asked repeatedly because we have to iterate over all references every time. The alternative would be to add a hashmap of all queued reference updates to speed up the lookup, but this adds overhead to all callers. - We may add a flag to `ref_transaction_add_update()` that causes it to skip duplicates, but this has the same runtime concerns as the first alternative. - We may add an interface which lets callers collect all updates which have already been queued such that he can avoid re-adding them. This is the most flexible approach and puts the burden on the caller, but also allows us to not impact any of the existing callsites which don't need this information. This commit implements the last approach: it allows us to compute the map of already-queued updates once up front such that we can then skip all subsequent references which are already part of this map. Signed-off-by: Patrick Steinhardt --- refs.c | 16 ++++++++++++++++ refs.h | 14 ++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/refs.c b/refs.c index d680de3bc0..35d4e69687 100644 --- a/refs.c +++ b/refs.c @@ -2417,6 +2417,22 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction, return refs->be->initial_transaction_commit(refs, transaction, err); } +void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, + ref_transaction_for_each_queued_update_fn cb, + void *cb_data) +{ + int i; + + for (i = 0; i < transaction->nr; i++) { + struct ref_update *update = transaction->updates[i]; + + cb(update->refname, + (update->flags & REF_HAVE_OLD) ? &update->old_oid : NULL, + (update->flags & REF_HAVE_NEW) ? &update->new_oid : NULL, + cb_data); + } +} + int refs_delete_refs(struct ref_store *refs, const char *logmsg, struct string_list *refnames, unsigned int flags) { diff --git a/refs.h b/refs.h index ff859d5951..1ae12c410a 100644 --- a/refs.h +++ b/refs.h @@ -776,6 +776,20 @@ int ref_transaction_abort(struct ref_transaction *transaction, int initial_ref_transaction_commit(struct ref_transaction *transaction, struct strbuf *err); +/* + * Execute the given callback function for each of the reference updates which + * have been queued in the given transaction. `old_oid` and `new_oid` may be + * `NULL` pointers depending on whether the update has these object IDs set or + * not. + */ +typedef void ref_transaction_for_each_queued_update_fn(const char *refname, + const struct object_id *old_oid, + const struct object_id *new_oid, + void *cb_data); +void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, + ref_transaction_for_each_queued_update_fn cb, + void *cb_data); + /* * Free `*transaction` and all associated data. */ From patchwork Mon Feb 21 08:02:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12753227 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 3FD81C433F5 for ; Mon, 21 Feb 2022 08:03:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346752AbiBUIDZ (ORCPT ); Mon, 21 Feb 2022 03:03:25 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:57664 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346776AbiBUIC6 (ORCPT ); Mon, 21 Feb 2022 03:02:58 -0500 Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3B846BF68 for ; Mon, 21 Feb 2022 00:02:34 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 39B5E5C01D3; Mon, 21 Feb 2022 03:02:34 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Mon, 21 Feb 2022 03:02:34 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :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=fm3; bh=ltrQOEAjHBnuzB5KSZycMd3b1CIDLJokhztcNt 6z1Do=; b=NNzmuTaMn1HxWlvNvM2UjgR5Koyrd99X+lYtvE3JMC2v3iNl7O0S/F y+SPDppVKNWC60/vCdhOAQwXNizVLEyRs5t/427KiIJRBiIVGsemslHCcWJsbDrJ J3Jupb04YbW9lwDnW6IObkY7IKi76xgu6ZbgbVTq5hDQVRwKKeIP/v151I3E4VzQ 2wnYGu3IdUSRBAASKhcMZxvLsyl7Dh4LPyCWssWJfeyk4wqK+METgLEPpPyLrhX5 IBeQEuh2Pj7uODq2ISOltwPuKdmh/Z71ZjW6mxXzNGhwXrenHiDnHUzG/WLh/t61 AyzM4lQIsG0SrpXB0JEG6pICPP/UN40g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date: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=fm2; bh=ltrQOEAjHBnuzB5KS ZycMd3b1CIDLJokhztcNt6z1Do=; b=DeGL9y6//INH/So2pBlNj8jCSxQPVqTVQ TvqASJ/gkOyC4zu+xfpbIwV4wK4F7WfDUlBKOll6kU570OC0zBFT4jftegtJX4pz OGK+Pk7/8Goo437l/RBZlVPkBZMKeylVRkOHL5rq2mlrkBLm+Ht2+xg5/vF9ml5j 4z6Kt/F7S6wSBpBAVhXcemYD9T9B1YSQ6VHwyUHHlHa860f+/bWo8g+x1xrGepra tlekyitNA/B/X8JtIWlMrkDZGQzdH4TDmJWI2WcL2t0Z5jW80VrmQMYGfgzcVNvc pQKeqWiySDmZUDoySpXOBNHECDyfA/nst9btuoITAaIHpZuOMTiGw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrkeehgdduuddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 21 Feb 2022 03:02:32 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 28d82717 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 21 Feb 2022 08:02:32 +0000 (UTC) Date: Mon, 21 Feb 2022 09:02:30 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Christian Couder , Jonathan Tan , Elijah Newren , Junio C Hamano Subject: [PATCH v3 6/7] fetch: make `--atomic` flag cover backfilling of tags Message-ID: <2f98e34c8461f09f7c56c5ec106a2c0be3f396ab.1645430423.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 fetching references from a remote we by default also fetch all tags which point into the history we have fetched. This is a separate step performed after updating local references because it requires us to walk over the history on the client-side to determine whether the remote has announced any tags which point to one of the fetched commits. This backfilling of tags isn't covered by the `--atomic` flag: right now, it only applies to the step where we update our local references. This is an oversight at the time the flag was introduced: its purpose is to either update all references or none, but right now we happily update local references even in the case where backfilling failed. Fix this by pulling up creation of the reference transaction such that we can pass the same transaction to both the code which updates local references and to the code which backfills tags. This allows us to only commit the transaction in case both actions succeed. Note that we also have to start passing the transaction into `find_non_local_tags()`: this function is responsible for finding all tags which we need to backfill. Right now, it will happily return tags which have already been updated with our local references. But when we use a single transaction for both local references and backfilling then it may happen that we try to queue the same reference update twice to the transaction, which consequently triggers a bug. We thus have to skip over any tags which have already been queued. Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 92 +++++++++++++++++++++++++++++++------------- t/t5503-tagfollow.sh | 11 ++---- 2 files changed, 69 insertions(+), 34 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index d304314f16..67af842091 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -349,7 +349,19 @@ static void clear_item(struct refname_hash_entry *item) item->ignore = 1; } + +static void add_already_queued_tags(const char *refname, + const struct object_id *old_oid, + const struct object_id *new_oid, + void *cb_data) +{ + struct hashmap *queued_tags = cb_data; + if (starts_with(refname, "refs/tags/") && new_oid) + (void) refname_hash_add(queued_tags, refname, new_oid); +} + static void find_non_local_tags(const struct ref *refs, + struct ref_transaction *transaction, struct ref **head, struct ref ***tail) { @@ -367,6 +379,16 @@ static void find_non_local_tags(const struct ref *refs, create_fetch_oidset(head, &fetch_oids); for_each_ref(add_one_refname, &existing_refs); + + /* + * If we already have a transaction, then we need to filter out all + * tags which have already been queued up. + */ + if (transaction) + ref_transaction_for_each_queued_update(transaction, + add_already_queued_tags, + &existing_refs); + for (ref = refs; ref; ref = ref->next) { if (!starts_with(ref->name, "refs/tags/")) continue; @@ -600,7 +622,7 @@ static struct ref *get_ref_map(struct remote *remote, /* also fetch all tags */ get_fetch_map(remote_refs, tag_refspec, &tail, 0); else if (tags == TAGS_DEFAULT && *autotags) - find_non_local_tags(remote_refs, &ref_map, &tail); + find_non_local_tags(remote_refs, NULL, &ref_map, &tail); /* Now append any refs to be updated opportunistically: */ *tail = orefs; @@ -1083,12 +1105,12 @@ N_("it took %.2f seconds to check forced updates; you can use\n" "to avoid this check\n"); static int store_updated_refs(const char *raw_url, const char *remote_name, - int connectivity_checked, struct ref *ref_map, + int connectivity_checked, + struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head, struct worktree **worktrees) { int url_len, i, rc = 0; struct strbuf note = STRBUF_INIT, err = STRBUF_INIT; - struct ref_transaction *transaction = NULL; const char *what, *kind; struct ref *rm; char *url; @@ -1110,14 +1132,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, } } - if (atomic_fetch) { - transaction = ref_transaction_begin(&err); - if (!transaction) { - error("%s", err.buf); - goto abort; - } - } - prepare_format_display(ref_map); /* @@ -1233,14 +1247,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, } } - if (!rc && transaction) { - rc = ref_transaction_commit(transaction, &err); - if (rc) { - error("%s", err.buf); - goto abort; - } - } - if (rc & STORE_REF_ERROR_DF_CONFLICT) error(_("some local refs could not be updated; try running\n" " 'git remote prune %s' to remove any old, conflicting " @@ -1258,7 +1264,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, abort: strbuf_release(¬e); strbuf_release(&err); - ref_transaction_free(transaction); free(url); return rc; } @@ -1299,6 +1304,7 @@ static int check_exist_and_connected(struct ref *ref_map) } static int fetch_and_consume_refs(struct transport *transport, + struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head, struct worktree **worktrees) @@ -1323,7 +1329,8 @@ static int fetch_and_consume_refs(struct transport *transport, trace2_region_enter("fetch", "consume_refs", the_repository); ret = store_updated_refs(transport->url, transport->remote->name, - connectivity_checked, ref_map, fetch_head, worktrees); + connectivity_checked, transaction, ref_map, + fetch_head, worktrees); trace2_region_leave("fetch", "consume_refs", the_repository); out: @@ -1496,6 +1503,7 @@ static struct transport *prepare_transport(struct remote *remote, int deepen) } static int backfill_tags(struct transport *transport, + struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head, struct worktree **worktrees) @@ -1519,7 +1527,7 @@ static int backfill_tags(struct transport *transport, transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); - retcode = fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); + retcode = fetch_and_consume_refs(transport, transaction, ref_map, fetch_head, worktrees); if (gsecondary) { transport_disconnect(gsecondary); @@ -1532,6 +1540,7 @@ static int backfill_tags(struct transport *transport, static int do_fetch(struct transport *transport, struct refspec *rs) { + struct ref_transaction *transaction = NULL; struct ref *ref_map = NULL; int autotags = (transport->remote->fetch_tags == 1); int retcode = 0; @@ -1541,6 +1550,7 @@ static int do_fetch(struct transport *transport, int must_list_refs = 1; struct worktree **worktrees = get_worktrees(); struct fetch_head fetch_head = { 0 }; + struct strbuf err = STRBUF_INIT; if (tags == TAGS_DEFAULT) { if (transport->remote->fetch_tags == 2) @@ -1602,6 +1612,14 @@ static int do_fetch(struct transport *transport, if (retcode) goto cleanup; + if (atomic_fetch) { + transaction = ref_transaction_begin(&err); + if (!transaction) { + retcode = error("%s", err.buf); + goto cleanup; + } + } + if (tags == TAGS_DEFAULT && autotags) transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1"); if (prune) { @@ -1621,7 +1639,7 @@ static int do_fetch(struct transport *transport, retcode = 1; } - if (fetch_and_consume_refs(transport, ref_map, &fetch_head, worktrees)) { + if (fetch_and_consume_refs(transport, transaction, ref_map, &fetch_head, worktrees)) { retcode = 1; goto cleanup; } @@ -1633,21 +1651,37 @@ static int do_fetch(struct transport *transport, if (tags == TAGS_DEFAULT && autotags) { struct ref *tags_ref_map = NULL, **tail = &tags_ref_map; - find_non_local_tags(remote_refs, &tags_ref_map, &tail); + find_non_local_tags(remote_refs, transaction, &tags_ref_map, &tail); if (tags_ref_map) { /* * If backfilling of tags fails then we want to tell * the user so, but we have to continue regardless to * populate upstream information of the references we - * have already fetched above. + * have already fetched above. The exception though is + * when `--atomic` is passed: in that case we'll abort + * the transaction and don't commit anything. */ - if (backfill_tags(transport, tags_ref_map, &fetch_head, worktrees)) + if (backfill_tags(transport, transaction, tags_ref_map, + &fetch_head, worktrees)) retcode = 1; } free_refs(tags_ref_map); } + if (transaction) { + if (retcode) + goto cleanup; + + retcode = ref_transaction_commit(transaction, &err); + if (retcode) { + error("%s", err.buf); + ref_transaction_free(transaction); + transaction = NULL; + goto cleanup; + } + } + commit_fetch_head(&fetch_head); if (set_upstream) { @@ -1705,7 +1739,13 @@ static int do_fetch(struct transport *transport, } cleanup: + if (retcode && transaction) { + ref_transaction_abort(transaction, &err); + error("%s", err.buf); + } + close_fetch_head(&fetch_head); + strbuf_release(&err); free_refs(ref_map); free_worktrees(worktrees); return retcode; diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh index 27a1dfa3c0..fe23cad4ce 100755 --- a/t/t5503-tagfollow.sh +++ b/t/t5503-tagfollow.sh @@ -180,11 +180,8 @@ test_expect_success 'atomic fetch with failing backfill' ' EOF test_must_fail git -C clone3 fetch --atomic .. $B:refs/heads/something && - - # Creation of the tag has failed, so ideally refs/heads/something - # should not exist. The fact that it does demonstrates that there is - # a bug in the `--atomic` flag. - test $B = "$(git -C clone3 rev-parse --verify refs/heads/something)" + test_must_fail git -C clone3 rev-parse --verify refs/heads/something && + test_must_fail git -C clone3 rev-parse --verify refs/tags/tag2 ' test_expect_success 'atomic fetch with backfill should use single transaction' ' @@ -197,12 +194,10 @@ test_expect_success 'atomic fetch with backfill should use single transaction' ' prepared $ZERO_OID $B refs/heads/something $ZERO_OID $S refs/tags/tag2 + $ZERO_OID $T refs/tags/tag1 committed $ZERO_OID $B refs/heads/something $ZERO_OID $S refs/tags/tag2 - prepared - $ZERO_OID $T refs/tags/tag1 - committed $ZERO_OID $T refs/tags/tag1 EOF From patchwork Mon Feb 21 08:02:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12753228 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 06971C433EF for ; Mon, 21 Feb 2022 08:03:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346800AbiBUIDc (ORCPT ); Mon, 21 Feb 2022 03:03:32 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:57182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346759AbiBUIDC (ORCPT ); Mon, 21 Feb 2022 03:03:02 -0500 Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9F6D9DF60 for ; Mon, 21 Feb 2022 00:02:38 -0800 (PST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 88D245C01D4; Mon, 21 Feb 2022 03:02:37 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Mon, 21 Feb 2022 03:02:37 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :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=fm3; bh=vIdFLBXkjRHlNdEuueg7QRVf25qE15sZNoNmEu btm24=; b=JprSvqSS71V92/03CimIwq6jzvfAsjRlEoRITvtOOfBIvzZVaOId4T RhL8jq3mcodBrY+2lCwTW0s74mh6gywJthsysJlfQhT/a5lCqfHr6nFAsRmwcmKP uDCSmrO/bsoYHkog8HbDr4M86gOPLbQJA7YU4cK9iXhTF4aRDE+ZWC+WB6I1YXVi /uuR2mibq6XrDyFq7fbflnOlV+99/RWzeUIhbaWgZpR4TsBvmYdRJ/ditRNylpPk 2Rob9nULBCYxLx1z7gDHtG1QUI9v9Ew4PYXUGJysQeW+es9oC7E/pzpQYDcLKvzb iE77UxyjKitnZkD/YlAYmW0YAqcGGorA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date: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=fm2; bh=vIdFLBXkjRHlNdEuu eg7QRVf25qE15sZNoNmEubtm24=; b=evE35QmKhKQlhV2OnWrp6hm2YLNjc/T4c 64RSFUIgQPVCOkg0wQiYfP9pRpKqI604HTpK+ngCE4emeyi3s9l+8TxbHGapTAkf ndHtHR0DoxxE29BueXFDaQcvQQ6dESs2HQZhFG7b2LmLSnmX+AImTc/lqNvGCgWo x/HkfdBYCTJTU0Suxd/5XOK/LY3Fl6/5IiQ+Zd9qLkHrmfC6rgCvuXuKJGG1Re5l 8Vh7ScoCQKkWhbncjLHrE3es2PxqyUSfCxeP49ngCbDKc9qzpWFmn1nD8GiHzwWC /T5Q19K/MGiX/zKEP8ISR4QH1HniXst5f5Yy8FEiv+uH7WkcakBfw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrkeehgdduuddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttdejnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgfejueevjeetudehgffffeffvdejfeejiedvkeffgfekuefgheevteeufeelkeen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 21 Feb 2022 03:02:36 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id a22c868a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 21 Feb 2022 08:02:36 +0000 (UTC) Date: Mon, 21 Feb 2022 09:02:35 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Christian Couder , Jonathan Tan , Elijah Newren , Junio C Hamano Subject: [PATCH v3 7/7] fetch: make `--atomic` flag cover pruning of refs Message-ID: <7292de826bd871bc12d7541f56c9a10b782c705d.1645430423.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 fetching with the `--prune` flag we will delete any local references matching the fetch refspec which have disappeared on the remote. This step is not currently covered by the `--atomic` flag: we delete branches even though updating of local references has failed, which means that the fetch is not an all-or-nothing operation. Fix this bug by passing in the global transaction into `prune_refs()`: if one is given, then we'll only queue up deletions and not commit them right away. This change also improves performance when pruning many branches in a repository with a big packed-refs file: every references is pruned in its own transaction, which means that we potentially have to rewrite the packed-refs files for every single reference we're about to prune. The following benchmark demonstrates this: it performs a pruning fetch from a repository with a single reference into a repository with 100k references, which causes us to prune all but one reference. This is of course a very artificial setup, but serves to demonstrate the impact of only having to write the packed-refs file once: Benchmark 1: git fetch --prune --atomic +refs/*:refs/* (HEAD~) Time (mean ± σ): 2.366 s ± 0.021 s [User: 0.858 s, System: 1.508 s] Range (min … max): 2.328 s … 2.407 s 10 runs Benchmark 2: git fetch --prune --atomic +refs/*:refs/* (HEAD) Time (mean ± σ): 1.369 s ± 0.017 s [User: 0.715 s, System: 0.641 s] Range (min … max): 1.346 s … 1.400 s 10 runs Summary 'git fetch --prune --atomic +refs/*:refs/* (HEAD)' ran 1.73 ± 0.03 times faster than 'git fetch --prune --atomic +refs/*:refs/* (HEAD~)' Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 30 ++++++++++++++++++++++-------- t/t5510-fetch.sh | 8 ++------ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 67af842091..9a2b5c03a4 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1338,11 +1338,14 @@ static int fetch_and_consume_refs(struct transport *transport, return ret; } -static int prune_refs(struct refspec *rs, struct ref *ref_map, +static int prune_refs(struct refspec *rs, + struct ref_transaction *transaction, + struct ref *ref_map, const char *raw_url) { int url_len, i, result = 0; struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); + struct strbuf err = STRBUF_INIT; char *url; int summary_width = transport_summary_width(stale_refs); const char *dangling_msg = dry_run @@ -1363,13 +1366,22 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, url_len = i - 3; if (!dry_run) { - struct string_list refnames = STRING_LIST_INIT_NODUP; + if (transaction) { + for (ref = stale_refs; ref; ref = ref->next) { + result = ref_transaction_delete(transaction, ref->name, NULL, 0, + "fetch: prune", &err); + if (result) + goto cleanup; + } + } else { + struct string_list refnames = STRING_LIST_INIT_NODUP; - for (ref = stale_refs; ref; ref = ref->next) - string_list_append(&refnames, ref->name); + for (ref = stale_refs; ref; ref = ref->next) + string_list_append(&refnames, ref->name); - result = delete_refs("fetch: prune", &refnames, 0); - string_list_clear(&refnames, 0); + result = delete_refs("fetch: prune", &refnames, 0); + string_list_clear(&refnames, 0); + } } if (verbosity >= 0) { @@ -1388,6 +1400,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, } } +cleanup: + strbuf_release(&err); free(url); free_refs(stale_refs); return result; @@ -1629,10 +1643,10 @@ static int do_fetch(struct transport *transport, * don't care whether --tags was specified. */ if (rs->nr) { - retcode = prune_refs(rs, ref_map, transport->url); + retcode = prune_refs(rs, transaction, ref_map, transport->url); } else { retcode = prune_refs(&transport->remote->fetch, - ref_map, + transaction, ref_map, transport->url); } if (retcode != 0) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 70d51f343b..48e14e2dab 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -354,17 +354,13 @@ test_expect_success 'fetch --atomic --prune executes a single reference transact head_oid=$(git rev-parse HEAD) && # Fetching with the `--atomic` flag should update all references in a - # single transaction. It is currently missing coverage of pruned - # references though, and as a result those may be committed to disk - # even if updating references fails later. + # single transaction. cat >expected <<-EOF && prepared $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion - committed - $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion - prepared $ZERO_OID $head_oid refs/remotes/origin/new-branch committed + $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion $ZERO_OID $head_oid refs/remotes/origin/new-branch EOF