From patchwork Thu Jan 7 13:51:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12004227 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 56263C433DB for ; Thu, 7 Jan 2021 13:52:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 258DC22B45 for ; Thu, 7 Jan 2021 13:52:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728551AbhAGNwe (ORCPT ); Thu, 7 Jan 2021 08:52:34 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:38075 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728537AbhAGNwd (ORCPT ); Thu, 7 Jan 2021 08:52:33 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id B37575C0170 for ; Thu, 7 Jan 2021 08:51:41 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 07 Jan 2021 08:51:41 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=date :from:to:subject:message-id:references:mime-version:content-type :in-reply-to; s=fm3; bh=dNLt0OW6KpyDsY6e8Tf2jmr5Zy1BhGqtQpoAyJN6 0hU=; b=YD9Guk3+x7E0pqkKBLa16c+Zw5DbrgcfEhP+MZ1E16OXRmm6nTmK+aaj 5X2Gw6sQOaA2I+qSxC1OQu5DKHj3DGBCUfqkgAxx4MugD+XuUf2VAw6TH86URddl LgGGOS8iOZmWtZsg/R7OtGVeq0qcf+u8J/IIuUivxh5iGnKpkOC9TH6UbQskjYMg +EpYOAoEgV2zut6wYI4bnR2leoOOrT4D4SPW73Bv05WXQY7gB3tHynMw/oGGVZxx FjU/xCxUfeaY1bymGfbEtYaFyU9tpsXh039RwqhpuyxhgOPJUjOWI9/AzdO5CLhs jMaRm6Gh2UB0Zf023+kpO0tBGhICTw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=dNLt0O W6KpyDsY6e8Tf2jmr5Zy1BhGqtQpoAyJN60hU=; b=LkG6ImHHpIAl2IX9y37BG8 GoHEld9gS57nsFid6fU0ZyoM/FaPeitJXuD4vEO455UDvB1WO4LWY5ZGU1y44bF5 TelaHMwwI4aJz+NboZMCBwk+9cE/tmnrdaFKanMCT/kPfZUGIF7TNg9IPKiucQRy VsLfC3IKyvBDRIascuJMDQxA2AC4UiCQP51RC+koCVqd2f3npKy+m8CcRclsSvBM pFiGY10RLCFLnw0//4e7dSDxlZ/BwvziSsSAajrAhaBrK3Hntr9vtgn9NmLqCSjF xA6pQ1uLFrsCmsthHDJnY/QbauHp7R91AA9osy2SKB4e1g36zSLtkydMzMS8s7Qg == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdegvddgheejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhepheeghfdtfeeuffehkefgffduleffjedthf dvjeektdfhhedvlefgtefgvdettdfhnecukfhppeejjedrudefrdefgedrvdefvdenucev lhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhksh drihhm X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-077-013-034-232.77.13.pool.telefonica.de [77.13.34.232]) by mail.messagingengine.com (Postfix) with ESMTPA id 329A0108005C for ; Thu, 7 Jan 2021 08:51:41 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id d5e6043f (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Thu, 7 Jan 2021 13:51:40 +0000 (UTC) Date: Thu, 7 Jan 2021 14:51:39 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 1/2] fetch: allow passing a transaction to `s_update_ref()` 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 handling of ref updates is completely handled by `s_update_ref()`, which will manage the complete lifecycle of the reference transaction. This is fine right now given that git-fetch(1) does not support atomic fetches, so each reference gets its own transaction. It is quite inflexible though, as `s_update_ref()` only knows about a single reference update at a time, so it doesn't allow us to alter the strategy. This commit prepares `s_update_ref()` and its only caller `update_local_ref()` to allow passing an external transaction. If none is given, then the existing behaviour is triggered which creates a new transaction and directly commits it. Otherwise, if the caller provides a transaction, then we only queue the update but don't commit it. This optionally allows the caller to manage when a transaction will be committed. Given that `update_local_ref()` is always called with a `NULL` transaction for now, no change in behaviour is expected from this change. Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 48 +++++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ecf8537605..020a977bc7 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -583,13 +583,14 @@ static struct ref *get_ref_map(struct remote *remote, static int s_update_ref(const char *action, struct ref *ref, + struct ref_transaction *transaction, int check_old) { char *msg; char *rla = getenv("GIT_REFLOG_ACTION"); - struct ref_transaction *transaction; + struct ref_transaction *transaction_to_free = NULL; struct strbuf err = STRBUF_INIT; - int ret, df_conflict = 0; + int ret, df_conflict = 0, commit = 0; if (dry_run) return 0; @@ -597,26 +598,38 @@ static int s_update_ref(const char *action, rla = default_rla.buf; msg = xstrfmt("%s: %s", rla, action); - transaction = ref_transaction_begin(&err); - if (!transaction || - ref_transaction_update(transaction, ref->name, + /* + * If no transaction was passed to us, we manage the transaction + * ourselves. Otherwise, we trust the caller to handle the transaction + * lifecycle. + */ + if (!transaction) { + transaction = transaction_to_free = ref_transaction_begin(&err); + if (!transaction) + goto fail; + commit = 1; + } + + if (ref_transaction_update(transaction, ref->name, &ref->new_oid, check_old ? &ref->old_oid : NULL, 0, msg, &err)) goto fail; - ret = ref_transaction_commit(transaction, &err); - if (ret) { - df_conflict = (ret == TRANSACTION_NAME_CONFLICT); - goto fail; + if (commit) { + ret = ref_transaction_commit(transaction, &err); + if (ret) { + df_conflict = (ret == TRANSACTION_NAME_CONFLICT); + goto fail; + } } - ref_transaction_free(transaction); + ref_transaction_free(transaction_to_free); strbuf_release(&err); free(msg); return 0; fail: - ref_transaction_free(transaction); + ref_transaction_free(transaction_to_free); error("%s", err.buf); strbuf_release(&err); free(msg); @@ -759,6 +772,7 @@ static void format_display(struct strbuf *display, char code, } static int update_local_ref(struct ref *ref, + struct ref_transaction *transaction, const char *remote, const struct ref *remote_ref, struct strbuf *display, @@ -799,7 +813,7 @@ static int update_local_ref(struct ref *ref, starts_with(ref->name, "refs/tags/")) { if (force || ref->force) { int r; - r = s_update_ref("updating tag", ref, 0); + r = s_update_ref("updating tag", ref, transaction, 0); format_display(display, r ? '!' : 't', _("[tag update]"), r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); @@ -836,7 +850,7 @@ static int update_local_ref(struct ref *ref, what = _("[new ref]"); } - r = s_update_ref(msg, ref, 0); + r = s_update_ref(msg, ref, transaction, 0); format_display(display, r ? '!' : '*', what, r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); @@ -858,7 +872,7 @@ static int update_local_ref(struct ref *ref, strbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV); strbuf_addstr(&quickref, ".."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); - r = s_update_ref("fast-forward", ref, 1); + r = s_update_ref("fast-forward", ref, transaction, 1); format_display(display, r ? '!' : ' ', quickref.buf, r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); @@ -870,7 +884,7 @@ static int update_local_ref(struct ref *ref, strbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV); strbuf_addstr(&quickref, "..."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); - r = s_update_ref("forced-update", ref, 1); + r = s_update_ref("forced-update", ref, transaction, 1); format_display(display, r ? '!' : '+', quickref.buf, r ? _("unable to update local ref") : _("forced update"), remote, pretty_ref, summary_width); @@ -1034,8 +1048,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, strbuf_reset(¬e); if (ref) { - rc |= update_local_ref(ref, what, rm, ¬e, - summary_width); + rc |= update_local_ref(ref, NULL, what, + rm, ¬e, summary_width); free(ref); } else if (write_fetch_head || dry_run) { /* From patchwork Thu Jan 7 13:51:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12004231 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6F160C433E0 for ; Thu, 7 Jan 2021 13:53:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 336492312A for ; Thu, 7 Jan 2021 13:53:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728566AbhAGNwx (ORCPT ); Thu, 7 Jan 2021 08:52:53 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:50737 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728445AbhAGNwx (ORCPT ); Thu, 7 Jan 2021 08:52:53 -0500 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 44E6F5C0064 for ; Thu, 7 Jan 2021 08:51:46 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 07 Jan 2021 08:51:46 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=date :from:to:subject:message-id:references:mime-version:content-type :in-reply-to; s=fm3; bh=YHi/SVROH3BReFet9Rn1++u14YagPfOR0FbZ/huD 2mA=; b=MoMaSPFSY0kf/VX1udIWxzoVDPBt9JMdB/VzbEZahjVMvCzxWjvEgMlj 7ujVKV2eLz+Ad9gro1kq7T6z5leqmk3uXDZuNZTShzfWTxlTk0s2hWd2eF/KhBPb EquyeEoegGZvW9C2SZbKjZHaq99JJa67qC7APFABnhJ+LR5xtPMBTy1G+QMJjgu3 M43vpA/JpKQLQ/v26FYIxbrlbK5ijqETq/YLt8IYgv0ixupKH5gHu3MWQseM0/i4 szq11SjPZzJK6m4EHBnp77xjRoh5qa0UIOR9UKYUuUVVrO9AMjLvydQs353/6Umr 1hRKFgwl5GTqoIW0y3DoqHJxwYRNGw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=YHi/SV ROH3BReFet9Rn1++u14YagPfOR0FbZ/huD2mA=; b=hoata4MIrvSw51yU8TQbdV UmgxuY1SMico6g7CbDYvgqTYpjEZl1LHYMdQuXBenQg64m7H2pl3SPlP0Bezgm9B z7hhKozzX684AS2bV3GrdeEYy4dZYq+CKOpoBIgSWJHjg7iDCwiRCHDW4pItDI0g gowfW1GjYzlkw5CCbyoFfkg6QAmCyE82NKx8taz3iTOjso0FwzavBho2Njbyzxwy ifZylslSAQxNAlam8gPZdjpF/tmiheHdFowUgdhZtoxebKCoxIOJbAmsTchJdBe8 46rTTFgVJueaR7B8OI0twN7AQTu4klvJ2DDZON24ycZwLyYgmGrlmOlsGyXnA21Q == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdegvddgheeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhepheeghfdtfeeuffehkefgffduleffjedthf dvjeektdfhhedvlefgtefgvdettdfhnecukfhppeejjedrudefrdefgedrvdefvdenucev lhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhksh drihhm X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-077-013-034-232.77.13.pool.telefonica.de [77.13.34.232]) by mail.messagingengine.com (Postfix) with ESMTPA id CD72A108005B for ; Thu, 7 Jan 2021 08:51:45 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 6719ab77 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Thu, 7 Jan 2021 13:51:45 +0000 (UTC) Date: Thu, 7 Jan 2021 14:51:44 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 2/2] fetch: implement support for atomic reference updates Message-ID: <4807344e92bedbac37243434850da1f0787ad496.1610027375.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 executing a fetch, then git will currently allocate one reference transaction per reference update and directly commit it. This means that fetches are non-atomic: even if some of the reference updates fail, others may still succeed and modify local references. This is fine in many scenarios, but this strategy has its downsides. - The view of remote references may be inconsistent and may show a bastardized state of the remote repository. - Batching together updates may improve performance in certain scenarios. While the impact probably isn't as pronounced with loose references, the upcoming reftable backend may benefit as it needs to write less files in case the update is batched. - The reference-update hook is currently being executed twice per updated reference. While this doesn't matter when there is no such hook, we have seen severe performance regressions when doing a git-fetch(1) with reference-transaction hook when the remote repository has hundreds of thousands of references. Similar to `git push --atomic`, this commit thus introduces atomic fetches. Instead of allocating one reference transaction per updated reference, it causes us to only allocate a single transaction and commit it as soon as all updates were received. If locking of any reference fails, then we abort the complete transaction and don't update any reference, which gives us an all-or-nothing fetch. Note that this may not completely fix the first of above downsides, as the consistent view also depends on the server-side. If the server doesn't have a consistent view of its own references during the reference negotiation phase, then the client would get the same inconsistent view the server has. This is a separate problem though and, if it actually exists, can be fixed at a later point. Signed-off-by: Patrick Steinhardt --- Documentation/fetch-options.txt | 4 + builtin/fetch.c | 26 +++++- t/t5510-fetch.sh | 139 ++++++++++++++++++++++++++++++++ 3 files changed, 167 insertions(+), 2 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 2bf77b46fd..07783deee3 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -7,6 +7,10 @@ existing contents of `.git/FETCH_HEAD`. Without this option old data in `.git/FETCH_HEAD` will be overwritten. +--atomic:: + Use an atomic transaction to update local refs. Either all refs are + updated, or on error, no refs are updated. + --depth=:: Limit fetching to the specified number of commits from the tip of each remote branch history. If fetching to a 'shallow' repository diff --git a/builtin/fetch.c b/builtin/fetch.c index 020a977bc7..5675ae4ec3 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -63,6 +63,7 @@ static int enable_auto_gc = 1; static int tags = TAGS_DEFAULT, unshallow, update_shallow, deepen; static int max_jobs = -1, submodule_fetch_jobs_config = -1; static int fetch_parallel_config = 1; +static int atomic_fetch; static enum transport_family family; static const char *depth; static const char *deepen_since; @@ -144,6 +145,8 @@ static struct option builtin_fetch_options[] = { N_("set upstream for git pull/fetch")), OPT_BOOL('a', "append", &append, N_("append to .git/FETCH_HEAD instead of overwriting")), + OPT_BOOL(0, "atomic", &atomic_fetch, + N_("use atomic transaction to update references")), OPT_STRING(0, "upload-pack", &upload_pack, N_("path"), N_("path to upload pack on remote end")), OPT__FORCE(&force, N_("force overwrite of local reference"), 0), @@ -926,7 +929,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, FILE *fp; struct commit *commit; int url_len, i, rc = 0; - struct strbuf note = STRBUF_INIT; + struct strbuf note = STRBUF_INIT, err = STRBUF_INIT; + struct ref_transaction *transaction = NULL; const char *what, *kind; struct ref *rm; char *url; @@ -955,6 +959,14 @@ 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); /* @@ -1048,7 +1060,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, strbuf_reset(¬e); if (ref) { - rc |= update_local_ref(ref, NULL, what, + rc |= update_local_ref(ref, transaction, what, rm, ¬e, summary_width); free(ref); } else if (write_fetch_head || dry_run) { @@ -1074,6 +1086,14 @@ 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 " @@ -1090,6 +1110,8 @@ 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); fclose(fp); return rc; diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 2013051a64..57da3ab2b3 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -176,6 +176,145 @@ test_expect_success 'fetch --prune --tags with refspec prunes based on refspec' git rev-parse sometag ' +test_expect_success 'fetch --atomic works with a single branch' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git clone . atomic && + git branch atomic-branch && + git rev-parse atomic-branch >expected && + + git -C atomic fetch --atomic origin && + git -C atomic rev-parse origin/atomic-branch >actual && + test_cmp expected actual +' + +test_expect_success 'fetch --atomic works with multiple branches' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git clone . atomic && + git branch atomic-branch-1 && + git branch atomic-branch-2 && + git branch atomic-branch-3 && + git rev-parse refs/heads/atomic-branch-1 refs/heads/atomic-branch-2 refs/heads/atomic-branch-3 >actual && + + git -C atomic fetch --atomic origin && + git -C atomic rev-parse refs/remotes/origin/atomic-branch-1 refs/remotes/origin/atomic-branch-2 refs/remotes/origin/atomic-branch-3 >expected && + test_cmp expected actual +' + +test_expect_success 'fetch --atomic works with mixed branches and tags' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git clone . atomic && + git branch atomic-mixed-branch && + git tag atomic-mixed-tag && + git rev-parse refs/heads/atomic-mixed-branch refs/tags/atomic-mixed-tag >actual && + + git -C atomic fetch --tags --atomic origin && + git -C atomic rev-parse refs/remotes/origin/atomic-mixed-branch refs/tags/atomic-mixed-tag >expected && + test_cmp expected actual +' + +test_expect_success 'fetch --atomic prunes references' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git branch atomic-prune-delete && + git clone . atomic && + git branch --delete atomic-prune-delete && + git branch atomic-prune-create && + git rev-parse refs/heads/atomic-prune-create >actual && + + git -C atomic fetch --prune --atomic origin && + test_must_fail git -C atomic rev-parse refs/remotes/origin/atomic-prune-delete && + git -C atomic rev-parse refs/remotes/origin/atomic-prune-create >expected && + test_cmp expected actual +' + +test_expect_success 'fetch --atomic aborts with non-fast-forward update' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git branch atomic-non-ff && + git clone . atomic && + git rev-parse HEAD >actual && + + git branch atomic-new-branch && + parent_commit=$(git rev-parse atomic-non-ff~) && + git update-ref refs/heads/atomic-non-ff $parent_commit && + + test_must_fail git -C atomic fetch --atomic origin refs/heads/*:refs/remotes/origin/* && + test_must_fail git -C atomic rev-parse refs/remotes/origin/atomic-new-branch && + git -C atomic rev-parse refs/remotes/origin/atomic-non-ff >expected && + test_cmp expected actual +' + +test_expect_success 'fetch --atomic executes a single reference transaction only' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git clone . atomic && + git branch atomic-hooks-1 && + git branch atomic-hooks-2 && + head_oid=$(git rev-parse HEAD) && + + cat >expected <<-EOF && + prepared + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1 + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2 + committed + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1 + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2 + EOF + + rm -f atomic/actual && + write_script atomic/.git/hooks/reference-transaction <<-\EOF && + ( echo "$*" && cat ) >>actual + EOF + + git -C atomic fetch --atomic origin && + test_cmp expected atomic/actual +' + +test_expect_success 'fetch --atomic aborts all reference updates if hook aborts' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git clone . atomic && + git branch atomic-hooks-abort-1 && + git branch atomic-hooks-abort-2 && + git branch atomic-hooks-abort-3 && + git tag atomic-hooks-abort && + head_oid=$(git rev-parse HEAD) && + + cat >expected <<-EOF && + prepared + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-1 + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-2 + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-3 + $ZERO_OID $head_oid refs/tags/atomic-hooks-abort + aborted + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-1 + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-2 + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-3 + $ZERO_OID $head_oid refs/tags/atomic-hooks-abort + EOF + + rm -f atomic/actual && + write_script atomic/.git/hooks/reference-transaction <<-\EOF && + ( echo "$*" && cat ) >>actual + exit 1 + EOF + + git -C atomic for-each-ref >expected-refs && + test_must_fail git -C atomic fetch --tags --atomic origin && + git -C atomic for-each-ref >actual-refs && + test_cmp expected-refs actual-refs +' + test_expect_success '--refmap="" ignores configured refspec' ' cd "$TRASH_DIRECTORY" && git clone "$D" remote-refs && From patchwork Tue Jan 12 12:27:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12013357 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 13E16C433DB for ; Tue, 12 Jan 2021 12:28:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CEFAF22BE9 for ; Tue, 12 Jan 2021 12:28:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730850AbhALM2h (ORCPT ); Tue, 12 Jan 2021 07:28:37 -0500 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:38257 "EHLO wout4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729974AbhALM2g (ORCPT ); Tue, 12 Jan 2021 07:28:36 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 1EE11188E; Tue, 12 Jan 2021 07:27:47 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Tue, 12 Jan 2021 07:27:47 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm3; bh=PPxRwlRSNFxWOyT5xVTOoHtXfcT rDuxHBXlpu4cNUlk=; b=b5iqmYf43cYLLFH1jdKfEIosbKqQ3m27KzDLVRAxR9O l06fdAmUD7n7TpsjTFXQVreeq22wdZn+j27JxeHw7NkNjk7UWjYwODj8xtM2LJoj ZEJciG/HXiLKo0sdY9/O188W6WvtJWy7gUh2S/7mA3gC8Qt9++aluIANKlOScvbg T6tdaOkHTRwq3hi0IUglXmNBSciVghU05+vOI++T6xJGvqWRsGDDqGaTtMZflnL8 STSyEVtb6/ror8v2eZFm85yuMYOSCIpI2u9dGPT4om2UBx75uTsVegMVK66QmQNP XQfjYO4CLcPlbHMFb/WSLGOUkiXIjc2zto5W7n03HSw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=PPxRwl RSNFxWOyT5xVTOoHtXfcTrDuxHBXlpu4cNUlk=; b=UNl85p6EoSQ1QJq6dJnYqc lz2k53po0bBWSXAkjteEhDlSYcyZx/aIN+tPSCr0lKIl1sBtaxap+s9yGglQCPLw Jioa8xHb0FiQm+QslLwfLe7v7gHra3mb/CeqpVyR7anoxJrkWqo/4LzPTrcmaGIj g+igejNIcNEgFeo+M/F2ASj51yILZMBwLexSj8GnAwXTM7V2a1uLj2HctOjWZxy4 jLJKdtuNB8KR3OmFNiLAQgYr4IlRL6S4+kmFrCkd0g9D//YCqKzPYBV70WYgGmYd Nv7mx6X92M0V2/uslfigMM9G6e4owqHv7RjvDaLmJdPTuUCfWFNzrf5N+9ioXn5g == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedukedrtddtgddtkecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhrihgt khcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvghrnh epheeghfdtfeeuffehkefgffduleffjedthfdvjeektdfhhedvlefgtefgvdettdfhnecu kfhppeejjedrudefrdehvddrleefnecuvehluhhsthgvrhfuihiivgepuddunecurfgrrh grmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-077-013-052-093.77.13.pool.telefonica.de [77.13.52.93]) by mail.messagingengine.com (Postfix) with ESMTPA id 12409240057; Tue, 12 Jan 2021 07:27:45 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 1d16b35f (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 12 Jan 2021 12:27:45 +0000 (UTC) Date: Tue, 12 Jan 2021 13:27:43 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Junio C Hamano , Christian Couder Subject: [PATCH v4 3/5] fetch: refactor `s_update_ref` to use common exit path 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 cleanup code in `s_update_ref()` is currently duplicated for both succesful and erroneous exit paths. This commit refactors the function to have a shared exit path for both cases to remove the duplication. Suggested-by: Christian Couder Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 47 +++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index e317e828cd..b24a9e09a4 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -589,7 +589,7 @@ static int s_update_ref(const char *action, char *rla = getenv("GIT_REFLOG_ACTION"); struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; - int ret, df_conflict = 0; + int ret; if (dry_run) return 0; @@ -598,30 +598,37 @@ static int s_update_ref(const char *action, msg = xstrfmt("%s: %s", rla, action); transaction = ref_transaction_begin(&err); - if (!transaction || - ref_transaction_update(transaction, ref->name, - &ref->new_oid, - check_old ? &ref->old_oid : NULL, - 0, msg, &err)) - goto fail; - - ret = ref_transaction_commit(transaction, &err); - if (ret) { - df_conflict = (ret == TRANSACTION_NAME_CONFLICT); - goto fail; + if (!transaction) { + ret = STORE_REF_ERROR_OTHER; + goto out; } + ret = ref_transaction_update(transaction, ref->name, &ref->new_oid, + check_old ? &ref->old_oid : NULL, + 0, msg, &err); + if (ret) { + ret = STORE_REF_ERROR_OTHER; + goto out; + } + + switch (ref_transaction_commit(transaction, &err)) { + case 0: + break; + case TRANSACTION_NAME_CONFLICT: + ret = STORE_REF_ERROR_DF_CONFLICT; + goto out; + default: + ret = STORE_REF_ERROR_OTHER; + goto out; + } + +out: ref_transaction_free(transaction); + if (ret) + error("%s", err.buf); strbuf_release(&err); free(msg); - return 0; -fail: - ref_transaction_free(transaction); - error("%s", err.buf); - strbuf_release(&err); - free(msg); - return df_conflict ? STORE_REF_ERROR_DF_CONFLICT - : STORE_REF_ERROR_OTHER; + return ret; } static int refcol_width = 10;