git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/6] fetch: control lifecycle of FETCH_HEAD in a single place

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 11, 2022, 07:46 UTC
Message-ID
<4059d5034bd9137ffca4929ed5bd8b7ce75ea09c.1644565025.git.ps@pks.im>
In-Reply-To
<cover.1644565025.git.ps@pks.im>

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 <ps@pks.im>
---
 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(&note, "'%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;
-- 
2.35.1
Previous: Christian CouderNext: Christian Couder
Message 7 of 27 in “fetch: improve atomicity of `--atomic` flag”
  1. 0/6 fetch: improve atomicity of `--atomic` flagPatrick Steinhardt, Feb 11, 2022
  2. 1/6 fetch: increase test coverage of fetchesPatrick Steinhardt, Feb 11, 2022
  3. Christian CouderFeb 15, 2022
  4. Patrick SteinhardtFeb 17, 2022
  5. 2/6 fetch: backfill tags before setting upstreamPatrick Steinhardt, Feb 11, 2022
  6. Christian CouderFeb 15, 2022
  7. 3/6 fetch: control lifecycle of FETCH_HEAD in a single placePatrick Steinhardt, Feb 11, 2022
  8. Christian CouderFeb 15, 2022
  9. Patrick SteinhardtFeb 17, 2022
  10. 4/6 fetch: report errors when backfilling tags failsPatrick Steinhardt, Feb 11, 2022
  11. Christian CouderFeb 15, 2022
  12. Patrick SteinhardtFeb 17, 2022
  13. Patrick SteinhardtFeb 17, 2022
  14. Jonathan TanFeb 16, 2022
  15. Elijah NewrenFeb 17, 2022
  16. 5/6 fetch: make `--atomic` flag cover backfilling of tagsPatrick Steinhardt, Feb 11, 2022
  17. Christian CouderFeb 15, 2022
  18. Jonathan TanFeb 16, 2022
  19. Patrick SteinhardtFeb 17, 2022
  20. Elijah NewrenFeb 17, 2022
  21. Patrick SteinhardtFeb 17, 2022
  22. 6/6 fetch: make `--atomic` flag cover pruning of refsPatrick Steinhardt, Feb 11, 2022
  23. Christian CouderFeb 15, 2022
  24. Patrick SteinhardtFeb 17, 2022
  25. Jonathan TanFeb 16, 2022
  26. Elijah NewrenFeb 17, 2022
  27. Patrick SteinhardtFeb 17, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.