From patchwork Mon Jan 11 11:05:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12010237 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 8B141C433DB for ; Mon, 11 Jan 2021 11:06:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4E91822473 for ; Mon, 11 Jan 2021 11:06:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728383AbhAKLGF (ORCPT ); Mon, 11 Jan 2021 06:06:05 -0500 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:50965 "EHLO wout4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727986AbhAKLGF (ORCPT ); Mon, 11 Jan 2021 06:06:05 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 04E502934; Mon, 11 Jan 2021 06:05:18 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 11 Jan 2021 06:05:19 -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=RZec6KzWWuireTLH14zQdN5lvkF jhb7FuWyueGDafM8=; b=KcbSIjgF3jPBEAmHn2/e/uhSx+zkATu/zvE/KPTpWey JRVkQyKqXIHlIJzK35VKF5v+YnsRfLSDjIdJ9O7yr4hoSYnq4+OO1wV/1XWb/wa/ QhpCRATEkFoPa8FznPirdeJR3b1ZqcEZWuNVYIhZnx5dyBXYTA8I+oQQDNKtsw01 Ycih88sMKSGPQYV1wicZzt7uNZ/+SnfgOJ+vadu2mh5cmElhksfwxc/pgbivFf6Z XpDErqdWpCJ122qhWfzHWppv62EbVmfhO6KRZwE6hMOPbMf/dXs0dcdyfxX6b910 dgfl95/xIg+ewwK+jD6A2k0M3GgY//bzte0ICbMO8pQ== 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=RZec6K zWWuireTLH14zQdN5lvkFjhb7FuWyueGDafM8=; b=PH6TtJcKr48Nno4aaJXbg1 205LjObXLTR6nDcdS7nyAJ0J5othW2nt9yZBJERmmrwAHlMr9SnifXrOL22gmpzx ND2LANyZ7slNcgAehXtzyTO/G5umNw+LOwS/pvr8ryFUmse/YKObKstGeZH0L8dv 2R/iLAlg9sPJnkfVSwbF0wvSSUFBb+H5xPwUpA6hH+pv6qXl3Z+f2of7tTuPXKe2 pcxtQwwGEAcAVstGRLZo+VC/qZjeVJzL5RrPo29yJ1vFux6NJju+37mGmHQJXqHt LF7PmRij38p+SCKmzQ5yQGMmaYa/JZZppX7kk+5YPZrwnd+/QOTrpAPwsYO/sgag == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdehuddgvdehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucfkphepkeelrddugedrgeehrddujedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrg hrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-089-014-045-170.89.14.pool.telefonica.de [89.14.45.170]) by mail.messagingengine.com (Postfix) with ESMTPA id 1701C1080057; Mon, 11 Jan 2021 06:05:17 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id d993f9a7 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Jan 2021 11:05:17 +0000 (UTC) Date: Mon, 11 Jan 2021 12:05:16 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Junio C Hamano , Christian Couder Subject: [PATCH v3 1/5] fetch: extract writing to FETCH_HEAD Message-ID: <61dc19a1cab5b03d07c6635496761108d089eb23.1610362744.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 performing a fetch with the default `--write-fetch-head` option, we write all updated references to FETCH_HEAD while the updates are performed. Given that updates are not performed atomically, it means that we we write to FETCH_HEAD even if some or all of the reference updates fail. Given that we simply update FETCH_HEAD ad-hoc with each reference, the logic is completely contained in `store_update_refs` and thus quite hard to extend. This can already be seen by the way we skip writing to the FETCH_HEAD: instead of having a conditional which simply skips writing, we instead open "/dev/null" and needlessly write all updates there. We are about to extend git-fetch(1) to accept an `--atomic` flag which will make the fetch an all-or-nothing operation with regards to the reference updates. This will also require us to make the updates to FETCH_HEAD an all-or-nothing operation, but as explained doing so is not easy with the current layout. This commit thus refactors the wa we write to FETCH_HEAD and pulls out the logic to open, append to, commit and close the file. While this may seem rather over-the top at first, pulling out this logic will make it a lot easier to update the code in a subsequent commit. It also allows us to easily skip writing completely in case `--no-write-fetch-head` was passed. Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 108 +++++++++++++++++++++++++++++++++++------------- remote.h | 2 +- 2 files changed, 80 insertions(+), 30 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ecf8537605..50f0306a92 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -897,6 +897,73 @@ static int iterate_ref_map(void *cb_data, struct object_id *oid) return 0; } +struct fetch_head { + FILE *fp; +}; + +static int open_fetch_head(struct fetch_head *fetch_head) +{ + const char *filename = git_path_fetch_head(the_repository); + + if (write_fetch_head) { + fetch_head->fp = fopen(filename, "a"); + if (!fetch_head->fp) + return error_errno(_("cannot open %s"), filename); + } else { + fetch_head->fp = NULL; + } + + return 0; +} + +static void append_fetch_head(struct fetch_head *fetch_head, + const struct object_id *old_oid, + enum fetch_head_status fetch_head_status, + const char *note, + const char *url, size_t url_len) +{ + char old_oid_hex[GIT_MAX_HEXSZ + 1]; + const char *merge_status_marker; + size_t i; + + if (!fetch_head->fp) + return; + + switch (fetch_head_status) { + case FETCH_HEAD_NOT_FOR_MERGE: + merge_status_marker = "not-for-merge"; + break; + case FETCH_HEAD_MERGE: + merge_status_marker = ""; + break; + default: + /* do not write anything to FETCH_HEAD */ + return; + } + + fprintf(fetch_head->fp, "%s\t%s\t%s", + oid_to_hex_r(old_oid_hex, old_oid), merge_status_marker, note); + for (i = 0; i < url_len; ++i) + if ('\n' == url[i]) + fputs("\\n", fetch_head->fp); + else + fputc(url[i], fetch_head->fp); + fputc('\n', fetch_head->fp); +} + +static void commit_fetch_head(struct fetch_head *fetch_head) +{ + /* Nothing to commit yet. */ +} + +static void close_fetch_head(struct fetch_head *fetch_head) +{ + if (!fetch_head->fp) + return; + + fclose(fetch_head->fp); +} + static const char warn_show_forced_updates[] = N_("Fetch normally indicates which branches had a forced update,\n" "but that check has been disabled. To re-enable, use '--show-forced-updates'\n" @@ -909,22 +976,19 @@ 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) { - FILE *fp; + struct fetch_head fetch_head; struct commit *commit; int url_len, i, rc = 0; struct strbuf note = STRBUF_INIT; const char *what, *kind; struct ref *rm; char *url; - const char *filename = (!write_fetch_head - ? "/dev/null" - : git_path_fetch_head(the_repository)); int want_status; int summary_width = transport_summary_width(ref_map); - fp = fopen(filename, "a"); - if (!fp) - return error_errno(_("cannot open %s"), filename); + rc = open_fetch_head(&fetch_head); + if (rc) + return -1; if (raw_url) url = transport_anonymize_url(raw_url); @@ -953,7 +1017,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, want_status++) { for (rm = ref_map; rm; rm = rm->next) { struct ref *ref = NULL; - const char *merge_status_marker = ""; if (rm->status == REF_STATUS_REJECT_SHALLOW) { if (want_status == FETCH_HEAD_MERGE) @@ -1011,26 +1074,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, strbuf_addf(¬e, "%s ", kind); strbuf_addf(¬e, "'%s' of ", what); } - switch (rm->fetch_head_status) { - case FETCH_HEAD_NOT_FOR_MERGE: - merge_status_marker = "not-for-merge"; - /* fall-through */ - case FETCH_HEAD_MERGE: - fprintf(fp, "%s\t%s\t%s", - oid_to_hex(&rm->old_oid), - merge_status_marker, - note.buf); - for (i = 0; i < url_len; ++i) - if ('\n' == url[i]) - fputs("\\n", fp); - else - fputc(url[i], fp); - fputc('\n', fp); - break; - default: - /* do not write anything to FETCH_HEAD */ - break; - } + + append_fetch_head(&fetch_head, &rm->old_oid, + rm->fetch_head_status, + note.buf, url, url_len); strbuf_reset(¬e); if (ref) { @@ -1060,6 +1107,9 @@ 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 " @@ -1077,7 +1127,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, abort: strbuf_release(¬e); free(url); - fclose(fp); + close_fetch_head(&fetch_head); return rc; } diff --git a/remote.h b/remote.h index 3211abdf05..aad1a0f080 100644 --- a/remote.h +++ b/remote.h @@ -134,7 +134,7 @@ struct ref { * should be 0, so that xcalloc'd structures get it * by default. */ - enum { + enum fetch_head_status { FETCH_HEAD_MERGE = -1, FETCH_HEAD_NOT_FOR_MERGE = 0, FETCH_HEAD_IGNORE = 1 From patchwork Mon Jan 11 11:05:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12010239 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=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,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 23813C433DB for ; Mon, 11 Jan 2021 11:06:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D524022473 for ; Mon, 11 Jan 2021 11:06:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729424AbhAKLGK (ORCPT ); Mon, 11 Jan 2021 06:06:10 -0500 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:43395 "EHLO wout4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728413AbhAKLGJ (ORCPT ); Mon, 11 Jan 2021 06:06:09 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id D20FD2940; Mon, 11 Jan 2021 06:05:23 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 11 Jan 2021 06:05:24 -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=wornKp3UvFzSX701YFGR0AD7TLm rGRaSkdU6iVvPTnY=; b=nCuXK7Bpotmb/FYU7i8guVFBX7/U2Ef79GMLwZpGsDr FGlbOO++LYQo1fVqTglSGC2S+hRs+Jm4JyPyxZQgVY7N4jMhKxHSPG0i0b3ckA2N 2CTxurndeqAXGfGW/XFRI51LJH5Rf/xBbkiRjKqlYwfGBXjgG/5YB05qdY+LiZKy HfzZNVwcDGX2zWgOmsrCv0qSL05rC2xmLtHMEzuKcxQ1c/ePPj1NjGEE2yiQGLcj YDtG/W97FYGsrzeu8GOOSDCd+PM3En8yuyECSF3bYdxdQa7RVBJH3CRoeKD0q0ge h+3RcOfdAY0bTnvzSOFKwReySbdBaMkjsHFC1vRiSPg== 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=wornKp 3UvFzSX701YFGR0AD7TLmrGRaSkdU6iVvPTnY=; b=XKQJOLMBg1i0sekGwpyWuK qCW909BaH16TwYzy6blhJ3JYGsV8A+F1jD6OiR/v74vNmS3YV0LPnNmE9Xbv0nyC /ulFIZGXugC3RnRPYT0lLw+Qu542tgWbv24WdJCuYiR/USSywv3B2e2v4znYaTAH lO+15GESnxnNd4DrZsdXHUnlccmG8EQm2TbY8fjxq1+CwMIXhkHTDkGg4bsjfYD1 XXMjCDb/dCsdUIs/5cK7GtM7w5y9P1ZQah+c3XTrdnmrwltMPCHgfVgmZiTyrcBU 4zhURm5HLm0VQLFsYZESEmzncE6Xc85l7/hWoAowvJh7a7Vu31OppekhsSKwQemA == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdehuddgvdehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucfkphepkeelrddugedrgeehrddujedtnecuvehluhhsthgvrhfuihiivgepudenucfrrg hrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-089-014-045-170.89.14.pool.telefonica.de [89.14.45.170]) by mail.messagingengine.com (Postfix) with ESMTPA id C4E5C1080057; Mon, 11 Jan 2021 06:05:22 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 6b04185e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Jan 2021 11:05:21 +0000 (UTC) Date: Mon, 11 Jan 2021 12:05:20 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Junio C Hamano , Christian Couder Subject: [PATCH v3 2/5] fetch: use strbuf to format FETCH_HEAD updates Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org This commit refactors `append_fetch_head()` to use a `struct strbuf` for formatting the update which we're about to append to the FETCH_HEAD file. While the refactoring doesn't have much of a benefit right now, it servers as a preparatory step to implement atomic fetches where we need to buffer all updates to FETCH_HEAD and only flush them out if all reference updates succeeded. No change in behaviour is expected from this commit. Signed-off-by: Patrick Steinhardt --- builtin/fetch.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 50f0306a92..1252f37493 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -899,6 +899,7 @@ static int iterate_ref_map(void *cb_data, struct object_id *oid) struct fetch_head { FILE *fp; + struct strbuf buf; }; static int open_fetch_head(struct fetch_head *fetch_head) @@ -909,6 +910,7 @@ static int open_fetch_head(struct fetch_head *fetch_head) fetch_head->fp = fopen(filename, "a"); if (!fetch_head->fp) return error_errno(_("cannot open %s"), filename); + strbuf_init(&fetch_head->buf, 0); } else { fetch_head->fp = NULL; } @@ -941,14 +943,17 @@ static void append_fetch_head(struct fetch_head *fetch_head, return; } - fprintf(fetch_head->fp, "%s\t%s\t%s", - oid_to_hex_r(old_oid_hex, old_oid), merge_status_marker, note); + strbuf_addf(&fetch_head->buf, "%s\t%s\t%s", + oid_to_hex_r(old_oid_hex, old_oid), merge_status_marker, note); for (i = 0; i < url_len; ++i) if ('\n' == url[i]) - fputs("\\n", fetch_head->fp); + strbuf_addstr(&fetch_head->buf, "\\n"); else - fputc(url[i], fetch_head->fp); - fputc('\n', fetch_head->fp); + strbuf_addch(&fetch_head->buf, url[i]); + strbuf_addch(&fetch_head->buf, '\n'); + + strbuf_write(&fetch_head->buf, fetch_head->fp); + strbuf_reset(&fetch_head->buf); } static void commit_fetch_head(struct fetch_head *fetch_head) @@ -962,6 +967,7 @@ static void close_fetch_head(struct fetch_head *fetch_head) return; fclose(fetch_head->fp); + strbuf_release(&fetch_head->buf); } static const char warn_show_forced_updates[] = From patchwork Mon Jan 11 11:05:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12010245 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 BE4F6C433DB for ; Mon, 11 Jan 2021 11:06:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7BEAE224D3 for ; Mon, 11 Jan 2021 11:06:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729634AbhAKLGd (ORCPT ); Mon, 11 Jan 2021 06:06:33 -0500 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:57601 "EHLO wout4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728163AbhAKLGd (ORCPT ); Mon, 11 Jan 2021 06:06:33 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 99E03293C; Mon, 11 Jan 2021 06:05:27 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 11 Jan 2021 06:05:27 -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=g1kSffOjjABCuFSiOFmFnSY9Rrk E1mNRZ0ja7Yxb17Q=; b=IQ+Xox7+lqMtCOLdDPKU21a+X+IZFrXuOH5RbSI9ju1 85z5+a86Mris/H4oqqmrXwNl91EsxG2VQ6bp+wvQUhjQfqnEXH7hE7GpPGo6vS7s VPVuTVxsOTHSjiWP+ahFGGO3SPgkVouBT7Pi8kAujD/p4tgyO2ikBUmzmNcXWLwn pqRd60MHcwYxqazukbPh/Nyu2r2vJSL9rW7M70gELw4sKlpU5RcuaOgis+acy1dJ rWLd4t1Z9Kp6pugPfwk3AlUeFPCzKldQaFex5j7yGIAUkzbE069EnVJLs8jypVUA U4zABccOIWCN/L7Os0AfxXklAxFxcEVAYTQDPlu6c5Q== 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=g1kSff OjjABCuFSiOFmFnSY9RrkE1mNRZ0ja7Yxb17Q=; b=oJhM39z2Z9VduYQJFp1E6T WtSmOOQj1hvLPAD7cVFO1wCz+rVfdMychIB/iQOxgsID2UWd2WkFkv5whpaefi/p PcnuW4/Sj0h1mK9t76VOFoeMObM7DMOXwC6NmJZWMH7RhAVqPFRqK2Ejv2TgpXWU DJqXtYRYWrZJLQrX7fX0XJw8VogfuKHD40pI0mBrY7ZzcSgruGbGx6+zgw2aqNoO ZpDh4ccqzi/GzcQ6igHr4ZoRPGwoAcGluZYVBdzF7TcAaYAquNxG6/AujCw30eCo 6kjswEENhxO6ibNgF99z2k6eZZp0FIJRFXM4ONPSYb617NBroqhqXuYPNSA0VUYQ == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdehuddgvdehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucfkphepkeelrddugedrgeehrddujedtnecuvehluhhsthgvrhfuihiivgepudenucfrrg hrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-089-014-045-170.89.14.pool.telefonica.de [89.14.45.170]) by mail.messagingengine.com (Postfix) with ESMTPA id 8C06E108005B; Mon, 11 Jan 2021 06:05:26 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id df2ba514 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Jan 2021 11:05:26 +0000 (UTC) Date: Mon, 11 Jan 2021 12:05:25 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Junio C Hamano , Christian Couder Subject: [PATCH v3 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 1252f37493..991771f8eb 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; From patchwork Mon Jan 11 11:05:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12010241 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 44ACDC433E0 for ; Mon, 11 Jan 2021 11:06:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 09ADC224D3 for ; Mon, 11 Jan 2021 11:06:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729601AbhAKLGS (ORCPT ); Mon, 11 Jan 2021 06:06:18 -0500 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:49069 "EHLO wout4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728413AbhAKLGS (ORCPT ); Mon, 11 Jan 2021 06:06:18 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 562B128D9; Mon, 11 Jan 2021 06:05:32 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 11 Jan 2021 06:05:32 -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=edlYxcDXIH8o91FSVN6fXcK2GDF QyO7ScrawBjJdph0=; b=skcpAmWLWCOMvOD+xnnngacVFWr2Lmc9a0O2IFbNUsf e3BatomYwXKcjKWTyqf4h4Wouyr6TQy9mW938Jhb/qKHk1ZMfVyM7nFp6/3uuWzW 6wGLKdGSTAlHwdKV241hCZsmeQbgzFRfM/pWUoP5n/GktLnqjkPIczWmmJDIlXWn 3oZOr6FLy1vdpfmGs6I6yHew4egaD33MC5hlDMYr1ppKioegoUARW6yMvZAoiHZl DbgLzTeesp2LUOrASxC6tllthsyXrEKcn0AUCGt7/dztJKZpjPj6JpprfCkXfU/K K+YwxSng6AwU65RuUF3PJEOg3Iw4GTqjf7R4RqpIV3w== 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=edlYxc DXIH8o91FSVN6fXcK2GDFQyO7ScrawBjJdph0=; b=JqBDhbbatU0HN81CPD1/iE TlAz5BBP6fc2UuPXSmSqU9KVHthopx3H0EKGYQyHbmFIBeBbld5pNk0smBZER2BZ WH9ciRUDyDSd+/cBB9JC7/IBh9PLM5Zn7QPfyd2NkmqKGniXowZE/6MWU1B5qFBE od+zlHZls/ArG6w6RdD0rlct5fY9NnD6k0i5OyQp0DzXKRc5kmd+8xwsKdP/i5bh SLgcvrdX8KD7Xh+tqhOpAK56Zcg22taQZkb/7gjDOc/jwWMWqh0NxA0Y64EaGerL o66O+6ECRqorQ7uNJScx3ViyhMnDvbeqQFIrmQrdBW8QThOxHQUAzzNPg/+7YyrQ == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdehuddgvdehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucfkphepkeelrddugedrgeehrddujedtnecuvehluhhsthgvrhfuihiivgepfeenucfrrg hrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-089-014-045-170.89.14.pool.telefonica.de [89.14.45.170]) by mail.messagingengine.com (Postfix) with ESMTPA id 570B81080059; Mon, 11 Jan 2021 06:05:31 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 9cfb5b94 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Jan 2021 11:05:30 +0000 (UTC) Date: Mon, 11 Jan 2021 12:05:29 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Junio C Hamano , Christian Couder Subject: [PATCH v3 4/5] fetch: allow passing a transaction to `s_update_ref()` Message-ID: <865d357ba7436ccd3de3573dc45e2647b09478e0.1610362744.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 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 | 51 ++++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 991771f8eb..654c7a7eed 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -583,11 +583,12 @@ 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 *our_transaction = NULL; struct strbuf err = STRBUF_INIT; int ret; @@ -597,10 +598,17 @@ static int s_update_ref(const char *action, rla = default_rla.buf; msg = xstrfmt("%s: %s", rla, action); - transaction = ref_transaction_begin(&err); + /* + * If no transaction was passed to us, we manage the transaction + * ourselves. Otherwise, we trust the caller to handle the transaction + * lifecycle. + */ if (!transaction) { - ret = STORE_REF_ERROR_OTHER; - goto out; + transaction = our_transaction = ref_transaction_begin(&err); + if (!transaction) { + ret = STORE_REF_ERROR_OTHER; + goto out; + } } ret = ref_transaction_update(transaction, ref->name, &ref->new_oid, @@ -611,19 +619,21 @@ static int s_update_ref(const char *action, 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; + if (our_transaction) { + switch (ref_transaction_commit(our_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); + ref_transaction_free(our_transaction); if (ret) error("%s", err.buf); strbuf_release(&err); @@ -766,6 +776,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, @@ -806,7 +817,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); @@ -843,7 +854,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); @@ -865,7 +876,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); @@ -877,7 +888,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); @@ -1094,8 +1105,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 Mon Jan 11 11:05:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12010247 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 28F0BC433E0 for ; Mon, 11 Jan 2021 11:07:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C995E22473 for ; Mon, 11 Jan 2021 11:07:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728491AbhAKLHM (ORCPT ); Mon, 11 Jan 2021 06:07:12 -0500 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:46689 "EHLO wout4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728251AbhAKLHL (ORCPT ); Mon, 11 Jan 2021 06:07:11 -0500 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id B2C6F28F1; Mon, 11 Jan 2021 06:05:37 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Mon, 11 Jan 2021 06:05:37 -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=Q4mj6L5XP7Ty1ZXrnqQMEcHGhpD zodgpyC+xFckFGP0=; b=heg/yD2rX8D13oRYZBl944hG8/r8uuBRM5dxyZ5fLsj GlFzEotzSPKrkYIqgjaIMDCgkkGrjh/aXe2kpye8BOMO537611ALDOXAm+FiEtmC J4atqjyxeWg7m+2atccA2S42t0x7Akg8fNFNnB+XJRRvG5pbMQdWDMI0oAfsP2/n nu5/0Yzt0ru/N3IXjY0KYSkgOjC2A8qviKkfkPGMDlfhYCJRlOMGnP6IAtbMVxTV feDcUIMAoDOKZY9EM+BNLr9MIaVrhWfBfvS0hP9bnHzETQWJrKiWu9oq6q4TFgAf 6ayetAxKvntwdXd3JnEAIXZwwnS2w2PgsZk64XhWPZA== 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=Q4mj6L 5XP7Ty1ZXrnqQMEcHGhpDzodgpyC+xFckFGP0=; b=Ol+GIrq2oFRa8porlHrUUF 9MK8kpVXG7ONr8Y7jTBo4zBiPLlm583i9azkwKyPu+8qZo/NbSTjSptQeuckCF3U nwOn8xkO/dOFEIHYyreuejW01k3RDFWBdGNepmEtsQaVi9bNZfXfIlpsNw8KcM24 bspdN9ddA/JOApIFAGobg8MzOGy7SyHD084sdKuVLBCQi8aSylh+Uh8C3dHiCTca IM3QyF1NShtOWeIE88eU6wfYD3THVOAV4tfehmB3/XlI8A4Wf3x0C2f4zpPd7Hfu b55x+1/3DGLzsVfAbPpLSilxTYf69M5jcbqGa6Px0ro6CckDKh4UsHBpPZw5zajQ == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdehuddgvdehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucfkphepkeelrddugedrgeehrddujedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrg hrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-089-014-045-170.89.14.pool.telefonica.de [89.14.45.170]) by mail.messagingengine.com (Postfix) with ESMTPA id 15752108005B; Mon, 11 Jan 2021 06:05:35 -0500 (EST) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 24cbc782 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Jan 2021 11:05:34 +0000 (UTC) Date: Mon, 11 Jan 2021 12:05:33 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Junio C Hamano , Christian Couder Subject: [PATCH v3 5/5] fetch: implement support for atomic reference updates Message-ID: <6a79e7adccbe4ee1d20352612a43295b3e7f37a7.1610362744.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. This commit also changes the way we write FETCH_HEAD in case `--atomic` is passed. Instead of writing changes as we go, we need to accumulate all changes first and only commit them at the end when we know that all reference updates succeeded. Ideally, we'd just do so via a temporary file so that we don't need to carry all updates in-memory. This isn't trivially doable though considering the `--append` mode, where we do not truncate the file but simply append to it. And given that we support concurrent processes appending to FETCH_HEAD at the same time without any loss of data, seeding the temporary file with current contents of FETCH_HEAD initially and then doing a rename wouldn't work either. So this commit implements the simple strategy of buffering all changes and appending them to the file on commit. Signed-off-by: Patrick Steinhardt --- Documentation/fetch-options.txt | 4 + builtin/fetch.c | 46 ++++++++- t/t5510-fetch.sh | 168 ++++++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+), 5 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 654c7a7eed..51c9930d06 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), @@ -970,13 +973,23 @@ static void append_fetch_head(struct fetch_head *fetch_head, strbuf_addch(&fetch_head->buf, url[i]); strbuf_addch(&fetch_head->buf, '\n'); - strbuf_write(&fetch_head->buf, fetch_head->fp); - strbuf_reset(&fetch_head->buf); + /* + * When using an atomic fetch, we do not want to update FETCH_HEAD if + * any of the reference updates fails. We thus have to write all + * updates to a buffer first and only commit it as soon as all + * references have been successfully updated. + */ + if (!atomic_fetch) { + strbuf_write(&fetch_head->buf, fetch_head->fp); + strbuf_reset(&fetch_head->buf); + } } static void commit_fetch_head(struct fetch_head *fetch_head) { - /* Nothing to commit yet. */ + if (!fetch_head->fp || !atomic_fetch) + return; + strbuf_write(&fetch_head->buf, fetch_head->fp); } static void close_fetch_head(struct fetch_head *fetch_head) @@ -1003,7 +1016,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, struct fetch_head fetch_head; 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; @@ -1029,6 +1043,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); /* @@ -1105,7 +1127,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) { @@ -1131,6 +1153,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) commit_fetch_head(&fetch_head); @@ -1150,6 +1180,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); close_fetch_head(&fetch_head); return rc; @@ -1961,6 +1993,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) die(_("--filter can only be used with the remote " "configured in extensions.partialclone")); + if (atomic_fetch) + die(_("--atomic can only be used when fetching " + "from one remote")); + if (stdin_refspecs) die(_("--stdin can only be used when fetching " "from one remote")); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 2013051a64..109d15be98 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -176,6 +176,174 @@ 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 && + oid=$(git rev-parse atomic-branch) && + echo "$oid" >expected && + + git -C atomic fetch --atomic origin && + git -C atomic rev-parse origin/atomic-branch >actual && + test_cmp expected actual && + test $oid = "$(git -C atomic rev-parse --verify FETCH_HEAD)" +' + +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_must_be_empty atomic/.git/FETCH_HEAD +' + +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_must_be_empty atomic/.git/FETCH_HEAD +' + +test_expect_success 'fetch --atomic --append appends to FETCH_HEAD' ' + test_when_finished "rm -rf \"$D\"/atomic" && + + cd "$D" && + git clone . atomic && + oid=$(git rev-parse HEAD) && + + git branch atomic-fetch-head-1 && + git -C atomic fetch --atomic origin atomic-fetch-head-1 && + test_line_count = 1 atomic/.git/FETCH_HEAD && + + git branch atomic-fetch-head-2 && + git -C atomic fetch --atomic --append origin atomic-fetch-head-2 && + test_line_count = 2 atomic/.git/FETCH_HEAD && + cp atomic/.git/FETCH_HEAD expected && + + write_script atomic/.git/hooks/reference-transaction <<-\EOF && + exit 1 + EOF + + git branch atomic-fetch-head-3 && + test_must_fail git -C atomic fetch --atomic --append origin atomic-fetch-head-3 && + test_cmp expected atomic/.git/FETCH_HEAD +' + test_expect_success '--refmap="" ignores configured refspec' ' cd "$TRASH_DIRECTORY" && git clone "$D" remote-refs &&