From patchwork Fri Feb 11 07:46:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12742981 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 078C7C4332F for ; Fri, 11 Feb 2022 07:46:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347794AbiBKHqw (ORCPT ); Fri, 11 Feb 2022 02:46:52 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:52982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232936AbiBKHqu (ORCPT ); Fri, 11 Feb 2022 02:46:50 -0500 Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 78BC72E9 for ; Thu, 10 Feb 2022 23:46:49 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id 03D8D3200EC0 for ; Fri, 11 Feb 2022 02:46:48 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Fri, 11 Feb 2022 02:46:49 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=3pchqvptU0rdpDMElNWXPwyxzFSh/vIP9EYDrR ucp9w=; b=UNMe3VwWEqTSIY6BUsRAvaYTrMHcx79QT+9QxcYxpNimr0qGliJRRb lyFz/hgSPp+X4gjCVdAaegJsg+m0HHA+IywARtrz/uCOXaTamWw83iq/ai3ZjxX6 6I1xYliIjSsRJ8zLCJNfKkITFmNhm8EwjlRy5+HGFFHYbxIvBjOWZMIr43CGLaqB YhXdb8jep6CpAURE6VLHS8ddFcvk39TVOPl+8r2f9NXlz1RmHjH1A9qORH25M9nc X/uQbH3Y82ZMefZzUyZdxYW6WrI/TAdwZRu8GDHE4XdJmM2AIPS3txL/U3Uv7/Bh WDUP8pIBuy0yE7/+szTxhZxhNf3aMfWw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=3pchqvptU0rdpDMEl NWXPwyxzFSh/vIP9EYDrRucp9w=; b=duclNaRh19nUF4CTQJylh8/cIF7RBIy2l vhOINsnE3qfzMk8yqAyMcek+i9ENAt0zwwEws6sWXlLsKv+FqayUznUM+pcuUsra fQF28qHvCrSi1Qb1LvPwgUis3RQhdavoqhcw/QZzsOYHdp1HGCZDp1bRugmvlkBX hQfGr3BsyKt/DQje8MYHzk/DR3JZBJFUzxBiWj+/Dy1t0w4IJANcvDLtSYyrs2RX A9xVp6t0PlVtqjz6HzVGv7V8VF+rZOBN6IxQNZOAk3nUPvoSKTs+MREzcPqkC+sm T5Lqa1LNY2NWmdFqevchiraE4D/UkCLbL9fL3y4Oqud5yh3LEKQlQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddriedvgddutdeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhepheeghfdtfeeuffehkefgffduleffjedthf dvjeektdfhhedvlefgtefgvdettdfhnecuvehluhhsthgvrhfuihiivgeptdenucfrrghr rghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Fri, 11 Feb 2022 02:46:47 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id c04c585b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Fri, 11 Feb 2022 07:46:47 +0000 (UTC) Date: Fri, 11 Feb 2022 08:46:45 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 1/6] fetch: increase test coverage of fetches Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The `--atomic` flag is missing test coverage for pruning of deleted references and backfilling of tags, and of course both aren't covered correctly by this flag. Furthermore, we don't have tests demonstrating error cases for backfilling tags. Add tests to cover those testing gaps. Signed-off-by: Patrick Steinhardt --- t/t5503-tagfollow.sh | 85 ++++++++++++++++++++++++++++++++++++++++++++ t/t5510-fetch.sh | 33 +++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh index 195fc64dd4..888305ad4d 100755 --- a/t/t5503-tagfollow.sh +++ b/t/t5503-tagfollow.sh @@ -160,4 +160,89 @@ 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 when backfilling 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 && + #!/bin/sh + + 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 is demonstrates that there is + # missing coverage 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 && + #!/bin/sh + + 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 20f7110ec1..93a0db3c68 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -332,6 +332,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 Fri Feb 11 07:46:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12742982 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 D69DAC433EF for ; Fri, 11 Feb 2022 07:46:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347812AbiBKHq5 (ORCPT ); Fri, 11 Feb 2022 02:46:57 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:53026 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347800AbiBKHqy (ORCPT ); Fri, 11 Feb 2022 02:46:54 -0500 Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0F1DE2E9 for ; Thu, 10 Feb 2022 23:46:54 -0800 (PST) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.west.internal (Postfix) with ESMTP id 8954A3200F9C for ; Fri, 11 Feb 2022 02:46:53 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Fri, 11 Feb 2022 02:46:53 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=vUyLctEfWXVyMvN+MP3OM1lxjFFtN8epMFqk9M Z2S/w=; b=DpH2g8ww8cyjrGFrpC/6cCqB2V1hV4wSmvbRFjYeV5rWzttxpkC8bn nc0UZxUcjorRfPEQZpkeFhOpKJ6NDIT10+R2PwklTlrSEy3GrY8lkcj4pvGvAvqP EchmiQgIG0axES7FyG7OkvW1NBiP5hkngikIyly0IRDaKrV38mserbp+tVI7UW4E lCITQqfkCeC0i4Di/5BjtbPBnQ+8parSRDrwlzFxx8Z6Ixl1PK1jztWz/CeqPOyp 2bogtE6M5VbRrwsWo7P87fyoHblw4Cg6zlIUpdI8SVhxHJLUPsRc6qoCHI38YGJi S+mixwc+muvwlhcBJbB13ha/BFsTdaMw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=vUyLctEfWXVyMvN+M P3OM1lxjFFtN8epMFqk9MZ2S/w=; b=RhSqBEFudIix27X0kvfGcI8W7FR+RaIKc mUVdWdzoxm6mo1ePd848oEeApgiUMDWnP0jP6CsDsPjqwNwpmRSSk7PKZXoBqaxv NPrR7O3+SF1NpT/rU+d3h51BHFBbZnnGBnPIgB3kz2QqVfK/zvH+mXxJ3/8aWa3k Jjls8UM6znmkFUUem6OZbx+0WDNCXq0imzCj1muuJSkRyTtaR53TteCbGvRccQLO Knk6NSt7tWac4j6AvpTGbDjI5vjkLR9hmnRAQ7i8jqgZ/iHIhGdwRUt+YJIUqzr2 ds6b1tq+2gVegsIu1cXM2fAdQO+PhFdZsjzv15cX/PuVLAof/bnZA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddriedvgddutdehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhepheeghfdtfeeuffehkefgffduleffjedthf dvjeektdfhhedvlefgtefgvdettdfhnecuvehluhhsthgvrhfuihiivgeptdenucfrrghr rghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Fri, 11 Feb 2022 02:46:52 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 8eea7706 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Fri, 11 Feb 2022 07:46:52 +0000 (UTC) Date: Fri, 11 Feb 2022 08:46:51 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 2/6] fetch: backfill tags before setting upstream Message-ID: <64c94e7a28d83fbaa1b6308b034d07c7be10e767.1644565025.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 require a `skip` label which jumps 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 | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 5b3b18a72f..9c7e4f12cd 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; @@ -1618,11 +1618,22 @@ static int do_fetch(struct transport *transport, } } 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; @@ -1642,7 +1653,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; } @@ -1656,7 +1667,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") || @@ -1676,21 +1687,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 Fri Feb 11 07:46:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12742983 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 831B5C433EF for ; Fri, 11 Feb 2022 07:47:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347809AbiBKHrD (ORCPT ); Fri, 11 Feb 2022 02:47:03 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:53146 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347800AbiBKHq7 (ORCPT ); Fri, 11 Feb 2022 02:46:59 -0500 Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8A5F3391 for ; Thu, 10 Feb 2022 23:46:58 -0800 (PST) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.west.internal (Postfix) with ESMTP id 0C80D3200F9C for ; Fri, 11 Feb 2022 02:46:57 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Fri, 11 Feb 2022 02:46:58 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=EQ+h+gq3to00bmLvIrqCks1dTpPTIaRTpqNxJt rRZcg=; b=q9psSltl5oIt4OFPOSakmU+lNKluLPk65eqit+brOStvLwJ2nPEBAh tmotu5cf/FsvOne8zOLX264NiR3ySb8156TB0GYoe4KgxQ8bds/jeO2l5EJeCWWW XgMQ3mkUSViAC8H1xCC00vMqjBBMyWW2R+Z2tvYpc0oChRu2ElgqgT16TmXM15Xr hF+Y1lOei9aK7KxdaiwtXQUc4B0sTbT3sO+MEbyqlvRF/oE0CIZoWtIJTrQqx7ER 414SWW392EUeH4kbu4Hed7DnszuxZ3RxnrFyTYP3gZ3HgXDonIN2VFyzuZ85ZbLK tUpLiAl5crO6/R9ZpaSmSPZetKKThGjQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=EQ+h+gq3to00bmLvI rqCks1dTpPTIaRTpqNxJtrRZcg=; b=cqUd2ANslNkIu5FtcRdAm3ej2rQTx5Qm3 Fe5QyM70LNuapvtEEUDY/0Jp5beb7XeTG1Q4hfBeHVGYE2PKXlEM/7ODebvl1iZJ 7EHX5Y3RslFUz5s/Fos8QTJS6bJ+U/hNnJG3yLsn4kdQvmAKHS2EPNozsrwco23j djN7l8MZgjpNYPNimzy7K/WjpNcYEl89ZbkNfGW8jsTSCJrbb2hNOcAMbHDqgDMx vpsq/jy2/PZFBA155jc0HZ94nyZV7O7MIkx+U5lgaIRO5Pf9o7+dLnIuxHqHJJri twyw7IlXHOLOoMRIdRtlTctyBXpcvqgiCsRcvrL3EoYR6ipVDztqg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddriedvgddutdehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhepheeghfdtfeeuffehkefgffduleffjedthf dvjeektdfhhedvlefgtefgvdettdfhnecuvehluhhsthgvrhfuihiivgeptdenucfrrghr rghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Fri, 11 Feb 2022 02:46:56 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 72e7ab0a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Fri, 11 Feb 2022 07:46:56 +0000 (UTC) Date: Fri, 11 Feb 2022 08:46:55 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 3/6] fetch: control lifecycle of FETCH_HEAD in a single place Message-ID: <4059d5034bd9137ffca4929ed5bd8b7ce75ea09c.1644565025.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 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 9c7e4f12cd..627847e2f8 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) { @@ -1617,7 +1616,8 @@ static int do_fetch(struct transport *transport, transport->url); } } - if (fetch_and_consume_refs(transport, ref_map, worktrees)) { + + if (fetch_and_consume_refs(transport, ref_map, &fetch_head, worktrees)) { retcode = 1; goto cleanup; } @@ -1629,11 +1629,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; @@ -1689,6 +1691,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 Fri Feb 11 07:46:59 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12742984 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 163A2C433F5 for ; Fri, 11 Feb 2022 07:47:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347814AbiBKHrG (ORCPT ); Fri, 11 Feb 2022 02:47:06 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:53310 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347487AbiBKHrD (ORCPT ); Fri, 11 Feb 2022 02:47:03 -0500 Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 573C9B38 for ; Thu, 10 Feb 2022 23:47:03 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 9D93A3201D9C for ; Fri, 11 Feb 2022 02:47:02 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Fri, 11 Feb 2022 02:47:02 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=BMZ9o2dZnbMjLW+unbLzORyXeZALgaOiIEgHYL rSM0E=; b=VEY8+hzUfA8nzGN/kzqLoBbovgwOsjpsrSeiw+cfrOdDqU9HJCNneN fOoSq31vjMo/MqOwT614cdDQtQu9u3B85OQLwtz7XXvofO65i9lhosneGNmPVfhu fCOizxB6sM6mG0uqSQ6Imc80mngv1p99xBUGtqujnkvquKLWszYUl3kHHEMAyDyW LPbq4cp+DU6V2uBlfhIOSMZ+GceI2OKhRmaupq58AURDhkpLfBR98QOo01tmvzRU 0WZriDFi5La1ujQM+ivqufftG1MZVjesxjD39FLAFd7sUaUxmtpFSNZCLFM4IRA1 yNPhuOjm6/NMvHFKdNDBsfpMzWH0duNA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=BMZ9o2dZnbMjLW+un bLzORyXeZALgaOiIEgHYLrSM0E=; b=McvJPbXiBwwh/9ZjeKbg2RRJZ6qdeVcvz YkV9wsHmUS2DJoIB03RZc3eI+GWUEnKVmyZvnj6b9v1kzQbBkyjwGjJrD2MIu8DB 0+4jULcnPE9jxGBcn7lQNK9lhMIhlPQXQANdYm6FU3OWlAGpTxBfC+1zdB6/9S4l FJA6T8JVQ1GfZz44toUpPexU1T0kJ7V2Hw/VIZvtK6sRE1hiCtZGnYIeskRfiV2W /onT2x7NJDTZcV6KGUdN+8HQkTsjMmyzJOpCKf1V61zL6izMT435333UbW3WbOxw SQfyyS59pmCNUp7eN23ShFm7hhtQX8z0ZPcRKSk2+o+zGRT+PTWog== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddriedvgddutdeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhepheeghfdtfeeuffehkefgffduleffjedthf dvjeektdfhhedvlefgtefgvdettdfhnecuvehluhhsthgvrhfuihiivgeptdenucfrrghr rghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Fri, 11 Feb 2022 02:47:01 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 10438711 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Fri, 11 Feb 2022 07:47:00 +0000 (UTC) Date: Fri, 11 Feb 2022 08:46:59 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 4/6] fetch: report errors when backfilling tags fails Message-ID: <54fdee845bea7f67f46817417f8e5a504bd39665.1644565025.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 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 | 29 +++++++++++++++++++++-------- t/t5503-tagfollow.sh | 4 +--- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 627847e2f8..1eda0b68ff 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, @@ -1628,8 +1630,19 @@ 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 tags succeeds we used to not return + * an error code to the user at all. Instead, we + * silently swallowed that error and updated the local + * state of the repository. We now notify the user of + * any such errors, but we continue to make sure that + * FETCH_HEAD and the upstream branch are configured as + * expected. + */ + 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 888305ad4d..549f908b90 100755 --- a/t/t5503-tagfollow.sh +++ b/t/t5503-tagfollow.sh @@ -237,9 +237,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 Fri Feb 11 07:47:03 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12742985 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 F3EC9C433EF for ; Fri, 11 Feb 2022 07:47:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347806AbiBKHrN (ORCPT ); Fri, 11 Feb 2022 02:47:13 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:53454 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347824AbiBKHrJ (ORCPT ); Fri, 11 Feb 2022 02:47:09 -0500 Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BA71BC66 for ; Thu, 10 Feb 2022 23:47:06 -0800 (PST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id 27E5F3201D51 for ; Fri, 11 Feb 2022 02:47:06 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Fri, 11 Feb 2022 02:47:06 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=mzDk2LKbDo8qWlA48RMH2GinqgKG6ZmmrEO8Ph ANrSg=; b=x1MDlNOMb4zSUesBUVDkBu0s8I0xzyCQ5rEPZTEugj1AVNNtt7AULX 9HlrnFYj3WR6WJaS8x5mbTVqFgk58dRW72W6pa8RMP6onlau/pJmHt5+qFfvrBF2 VB75HROvOgL0oiftAhKrcs5a40VehiR/c5MGDsKJnybLR43N9xNyXM4wJk/WRuAz grNKkDg+y6kjgYxCSHL62rTtLzAECLJxSeIB/MEsYLckP5mcIH6O+94OJ6Hlymal WmUmbEpiGBxhOd62Dah2GA3cgm/4r3lBSjukrRBeP4ssjgMCdupbqNHOHGogFXBf YYSafyN1Fuw+BvYOmSYmtERTERsRd24A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=mzDk2LKbDo8qWlA48 RMH2GinqgKG6ZmmrEO8PhANrSg=; b=jPT7Ve/sOXN5QlqKU5iImjEdbBjZtZ8WN NOFRNX6azDlPva/BbvRohqjh8GXWzxPi4NIvP/33j4p2Qg0hmZgrT3EbvDcNj5i3 5E0gjor+Pmw07J+Mq2lyEL4C1ilN9zAuPxGHLMqpCAMKr84MW2MND7NXq+A14bhv Tk8+KXMR4vrouJ9GqPfhVhYeZEF5zdwqDytDP84QxjaBTG2fmGH3y0OiV3ebFyiF Zq45y3ibcCNT/GhBay8SIpgfm//ouSs0PU214Z2CHstcwy8OpM5JZ3WANtKj3dIB RXoLDTZihEbZD4eth/ODiA4+k1s1eSIpnGkVbQVIcO0/sr2kmlCVg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddriedvgddutdeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhepheeghfdtfeeuffehkefgffduleffjedthf dvjeektdfhhedvlefgtefgvdettdfhnecuvehluhhsthgvrhfuihiivgeptdenucfrrghr rghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Fri, 11 Feb 2022 02:47:05 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 7ff528f8 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Fri, 11 Feb 2022 07:47:04 +0000 (UTC) Date: Fri, 11 Feb 2022 08:47:03 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 5/6] fetch: make `--atomic` flag cover backfilling of tags Message-ID: <55dbe19a1a4d05d84c81356af1a3f04b65f8aa7b.1644565025.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 we 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 consequentially triggers a bug. We thus have to skip over any tags which have already been queued. Unfortunately, this requires us to reach into internals of the reference transaction to access queued updates, but there is no non-internal interface right now which would allow us to access this information. Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 89 ++++++++++++++++++++++++++++++-------------- t/t5503-tagfollow.sh | 11 ++---- 2 files changed, 65 insertions(+), 35 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 1eda0b68ff..348e64cf2c 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -4,7 +4,7 @@ #include "cache.h" #include "config.h" #include "repository.h" -#include "refs.h" +#include "refs/refs-internal.h" #include "refspec.h" #include "object-store.h" #include "oidset.h" @@ -350,6 +350,7 @@ static void clear_item(struct refname_hash_entry *item) } static void find_non_local_tags(const struct ref *refs, + struct ref_transaction *transaction, struct ref **head, struct ref ***tail) { @@ -361,12 +362,28 @@ static void find_non_local_tags(const struct ref *refs, const struct ref *ref; struct refname_hash_entry *item = NULL; const int quick_flags = OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT; + int i; refname_hash_init(&existing_refs); refname_hash_init(&remote_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. + */ + for (i = 0; transaction && i < transaction->nr; i++) { + if (!starts_with(transaction->updates[i]->refname, "refs/tags/") || + !(transaction->updates[i]->flags & REF_HAVE_NEW)) + continue; + (void) refname_hash_add(&existing_refs, + transaction->updates[i]->refname, + &transaction->updates[i]->new_oid); + } + + for (ref = refs; ref; ref = ref->next) { if (!starts_with(ref->name, "refs/tags/")) continue; @@ -600,7 +617,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 +1100,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 +1127,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 +1242,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 +1259,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 +1299,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 +1324,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 +1498,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 +1522,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 +1535,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 +1545,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 +1607,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) { @@ -1619,7 +1632,7 @@ static int do_fetch(struct transport *transport, } } - 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; } @@ -1629,7 +1642,7 @@ 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 tags succeeds we used to not return @@ -1638,15 +1651,31 @@ static int do_fetch(struct transport *transport, * state of the repository. We now notify the user of * any such errors, but we continue to make sure that * FETCH_HEAD and the upstream branch are configured as - * expected. + * expected. 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) { @@ -1704,7 +1733,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 549f908b90..4a8e63aa16 100755 --- a/t/t5503-tagfollow.sh +++ b/t/t5503-tagfollow.sh @@ -182,11 +182,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 is demonstrates that there is - # missing coverage 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' ' @@ -199,12 +196,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 Fri Feb 11 07:47:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12742986 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 4D082C433EF for ; Fri, 11 Feb 2022 07:47:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347828AbiBKHrR (ORCPT ); Fri, 11 Feb 2022 02:47:17 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:53686 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347818AbiBKHrN (ORCPT ); Fri, 11 Feb 2022 02:47:13 -0500 Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1A2AE1030 for ; Thu, 10 Feb 2022 23:47:11 -0800 (PST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 90BBE3201D9C for ; Fri, 11 Feb 2022 02:47:10 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Fri, 11 Feb 2022 02:47:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=fuknSbXO2Wlk9QT2QghdEuK/Gx+r/4ovwXlqPs 29LlQ=; b=jVsfv+XSLPg1/or2jX/05wl610q2OZ3ByTgqgj+T17rKio4Y2vWZnT mfwiXIzp3eBzTiDQEWbMHbYnJhYaWCXvkrKtAcVIS3HIMVfQkydNbWKBhrbsgcuZ 0tlu0U5H64ZjWPT5SrXXSnUsCTivsjg4TXLQyVzZkOt35DQFr9hz/i2SSmhR09tR K4N9NISrfqG15qE+h4SNga6YTS7oXeaDlL1kkoZuoiXvjqeEeNXPeLYEPr1A73gm fkDkq9sFtvyAr9eDxhtLQlRmoMwWiEBupsIERTiuDEOmbuat+mHrNGdB/8H0A+XM kH2VHEfNMOuKTrsd9sWrNkK4Uwdv6lLg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=fuknSbXO2Wlk9QT2Q ghdEuK/Gx+r/4ovwXlqPs29LlQ=; b=B6ZyybBWE6SlBcxF1DegmllYZ3E8hVCeG lRyqO65ifE64qYTheHZ9HmEYtC1btNRhCIa5++IqejCjotSp7nn7zL6OfWfQdrJk 1al5AhQ7nI0SWfWSUVeHbwwNuH8VgR5eNoFUR/WwRgj2ZUS3Rdf/kNzZ6kkpme6N 9xGysPORn9DGp0/AimvETNxeAXDRlcTRntO+i1QyJicbhW/2rt1cZo9SpfNdCZRT cUFmKtX/KToMGznLKwW3cO0q/eCiicZ7jXutOOqLuzj8eAS6LtQY+QnhlhSm7f2u Eibu5j/8yrd/cc5N3lw8fjXvrB8nHRXkXNg0A+URJSrw5m0HNwl8A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddriedvgddutdeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtjeenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhephefgjeeuveejteduhefgffefffdvjeefje eivdekfffgkeeugfehveetueefleeknecuvehluhhsthgvrhfuihiivgeptdenucfrrghr rghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Fri, 11 Feb 2022 02:47:09 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 3b77fc54 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Fri, 11 Feb 2022 07:47:09 +0000 (UTC) Date: Fri, 11 Feb 2022 08:47:07 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 6/6] fetch: make `--atomic` flag cover pruning of refs Message-ID: <682f16117b743bec59c533e15ae5a88d39250222.1644565025.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 | 4 +--- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 348e64cf2c..75e791a4b4 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1333,11 +1333,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 @@ -1358,13 +1361,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) { @@ -1383,6 +1395,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, } } +cleanup: + strbuf_release(&err); free(url); free_refs(stale_refs); return result; @@ -1624,10 +1638,10 @@ static int do_fetch(struct transport *transport, * don't care whether --tags was specified. */ if (rs->nr) { - prune_refs(rs, ref_map, transport->url); + prune_refs(rs, transaction, ref_map, transport->url); } else { prune_refs(&transport->remote->fetch, - ref_map, + transaction, ref_map, transport->url); } } diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 93a0db3c68..afa6bf9f7d 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -349,11 +349,9 @@ test_expect_success 'fetch --atomic --prune executes a single reference transact 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