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

[PATCH v2 5/6] fetch: centralize logic to print remote URL

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 20, 2023, 12:35 UTC
Message-ID
<98b799af713b2ff22cc79531d5057fda64ddfb2a.1679315383.git.ps@pks.im>
In-Reply-To
<cover.1679315383.git.ps@pks.im>

When fetching from a remote, we not only print the actual references that have changed, but will also print the URL from which we have fetched them to standard output. The logic to handle this is duplicated across two different callsites with some non-trivial logic to compute the anonymized URL. Furthermore, we're using global state to track whether we have already shown the URL to the user or not.

Refactor the code by moving it into `format_display()`. Like this, we can convert the global variable into a member of `display_state`. And second, we can deduplicate the logic to compute the anonymized URL.

This also works as expected when fetching from multiple remotes, for example via a group of remotes, as we do this by forking a standalone git-fetch(1) process per remote that is to be fetched.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fetch.c | 99 ++++++++++++++++++++++---------------------------
 1 file changed, 44 insertions(+), 55 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a66428dfd8..1e3599cb74 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -50,6 +50,9 @@ enum {
 struct display_state {
 	int refcol_width;
 	int compact_format;
+
+	char *url;
+	int url_len, shown_url;
 };
 
 static int fetch_prune_config = -1; /* unspecified */
@@ -84,7 +87,6 @@ static const char *submodule_prefix = "";
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
 static int recurse_submodules_default = RECURSE_SUBMODULES_ON_DEMAND;
-static int shown_url = 0;
 static struct refspec refmap = REFSPEC_INIT_FETCH;
 static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
 static struct string_list server_options = STRING_LIST_INIT_DUP;
@@ -776,13 +778,27 @@ static int refcol_width(const struct ref *ref, int compact_format)
 	return rlen;
 }
 
-static void display_state_init(struct display_state *display_state, struct ref *ref_map)
+static void display_state_init(struct display_state *display_state, struct ref *ref_map,
+			       const char *raw_url)
 {
 	struct ref *rm;
 	const char *format = "full";
+	int i;
 
 	memset(display_state, 0, sizeof(*display_state));
 
+	if (raw_url)
+		display_state->url = transport_anonymize_url(raw_url);
+	else
+		display_state->url = xstrdup("foreign");
+
+	display_state->url_len = strlen(display_state->url);
+	for (i = display_state->url_len - 1; display_state->url[i] == '/' && 0 <= i; i--)
+		;
+	display_state->url_len = i + 1;
+	if (4 < i && !strncmp(".git", display_state->url + i - 3, 4))
+		display_state->url_len = i - 3;
+
 	if (verbosity < 0)
 		return;
 
@@ -816,6 +832,11 @@ static void display_state_init(struct display_state *display_state, struct ref *
 	}
 }
 
+static void display_state_release(struct display_state *display_state)
+{
+	free(display_state->url);
+}
+
 static void print_remote_to_local(struct display_state *display_state,
 				  struct strbuf *display_buffer,
 				  const char *remote, const char *local)
@@ -883,6 +904,12 @@ static void format_display(struct display_state *display_state,
 	if (verbosity < 0)
 		return;
 
+	if (!display_state->shown_url) {
+		strbuf_addf(display_buffer, _("From %.*s\n"),
+			    display_state->url_len, display_state->url);
+		display_state->shown_url = 1;
+	}
+
 	width = (summary_width + strlen(summary) - gettext_width(summary));
 
 	strbuf_addf(display_buffer, " %c %-*s ", code, width, summary);
@@ -1122,33 +1149,28 @@ N_("it took %.2f seconds to check forced updates; you can use\n"
    "to avoid this check\n");
 
 static int store_updated_refs(struct display_state *display_state,
-			      const char *raw_url, const char *remote_name,
+			      const char *remote_name,
 			      int connectivity_checked,
 			      struct ref_transaction *transaction, struct ref *ref_map,
 			      struct fetch_head *fetch_head)
 {
-	int url_len, i, rc = 0;
+	int rc = 0;
 	struct strbuf note = STRBUF_INIT;
 	const char *what, *kind;
 	struct ref *rm;
-	char *url;
 	int want_status;
 	int summary_width = 0;
 
 	if (verbosity >= 0)
 		summary_width = transport_summary_width(ref_map);
 
-	if (raw_url)
-		url = transport_anonymize_url(raw_url);
-	else
-		url = xstrdup("foreign");
-
 	if (!connectivity_checked) {
 		struct check_connected_options opt = CHECK_CONNECTED_INIT;
 
 		rm = ref_map;
 		if (check_connected(iterate_ref_map, &rm, &opt)) {
-			rc = error(_("%s did not send all necessary objects\n"), url);
+			rc = error(_("%s did not send all necessary objects\n"),
+				   display_state->url);
 			goto abort;
 		}
 	}
@@ -1232,13 +1254,6 @@ static int store_updated_refs(struct display_state *display_state,
 				what = rm->name;
 			}
 
-			url_len = strlen(url);
-			for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
-				;
-			url_len = i + 1;
-			if (4 < i && !strncmp(".git", url + i - 3, 4))
-				url_len = i - 3;
-
 			strbuf_reset(&note);
 			if (*what) {
 				if (*kind)
@@ -1248,7 +1263,8 @@ static int store_updated_refs(struct display_state *display_state,
 
 			append_fetch_head(fetch_head, &rm->old_oid,
 					  rm->fetch_head_status,
-					  note.buf, url, url_len);
+					  note.buf, display_state->url,
+					  display_state->url_len);
 
 			strbuf_reset(&note);
 			if (ref) {
@@ -1266,14 +1282,8 @@ static int store_updated_refs(struct display_state *display_state,
 					       *what ? what : "HEAD",
 					       "FETCH_HEAD", summary_width);
 			}
-			if (note.len) {
-				if (!shown_url) {
-					fprintf(stderr, _("From %.*s\n"),
-							url_len, url);
-					shown_url = 1;
-				}
+			if (note.len)
 				fputs(note.buf, stderr);
-			}
 		}
 	}
 
@@ -1293,7 +1303,6 @@ static int store_updated_refs(struct display_state *display_state,
 
  abort:
 	strbuf_release(&note);
-	free(url);
 	return rc;
 }
 
@@ -1365,7 +1374,7 @@ static int fetch_and_consume_refs(struct display_state *display_state,
 	}
 
 	trace2_region_enter("fetch", "consume_refs", the_repository);
-	ret = store_updated_refs(display_state, transport->url, transport->remote->name,
+	ret = store_updated_refs(display_state, transport->remote->name,
 				 connectivity_checked, transaction, ref_map,
 				 fetch_head);
 	trace2_region_leave("fetch", "consume_refs", the_repository);
@@ -1378,30 +1387,15 @@ static int fetch_and_consume_refs(struct display_state *display_state,
 static int prune_refs(struct display_state *display_state,
 		      struct refspec *rs,
 		      struct ref_transaction *transaction,
-		      struct ref *ref_map,
-		      const char *raw_url)
+		      struct ref *ref_map)
 {
-	int url_len, i, result = 0;
+	int result = 0;
 	struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);
 	struct strbuf err = STRBUF_INIT;
-	char *url;
 	const char *dangling_msg = dry_run
 		? _("   (%s will become dangling)")
 		: _("   (%s has become dangling)");
 
-	if (raw_url)
-		url = transport_anonymize_url(raw_url);
-	else
-		url = xstrdup("foreign");
-
-	url_len = strlen(url);
-	for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
-		;
-
-	url_len = i + 1;
-	if (4 < i && !strncmp(".git", url + i - 3, 4))
-		url_len = i - 3;
-
 	if (!dry_run) {
 		if (transaction) {
 			for (ref = stale_refs; ref; ref = ref->next) {
@@ -1426,10 +1420,6 @@ static int prune_refs(struct display_state *display_state,
 
 		for (ref = stale_refs; ref; ref = ref->next) {
 			struct strbuf sb = STRBUF_INIT;
-			if (!shown_url) {
-				fprintf(stderr, _("From %.*s\n"), url_len, url);
-				shown_url = 1;
-			}
 			format_display(display_state, &sb, '-', _("[deleted]"), NULL,
 				       _("(none)"), ref->name,
 				       summary_width);
@@ -1441,7 +1431,6 @@ static int prune_refs(struct display_state *display_state,
 
 cleanup:
 	strbuf_release(&err);
-	free(url);
 	free_refs(stale_refs);
 	return result;
 }
@@ -1596,7 +1585,7 @@ static int do_fetch(struct transport *transport,
 {
 	struct ref_transaction *transaction = NULL;
 	struct ref *ref_map = NULL;
-	struct display_state display_state;
+	struct display_state display_state = { 0 };
 	int autotags = (transport->remote->fetch_tags == 1);
 	int retcode = 0;
 	const struct ref *remote_refs;
@@ -1678,7 +1667,7 @@ static int do_fetch(struct transport *transport,
 	if (retcode)
 		goto cleanup;
 
-	display_state_init(&display_state, ref_map);
+	display_state_init(&display_state, ref_map, transport->url);
 
 	if (atomic_fetch) {
 		transaction = ref_transaction_begin(&err);
@@ -1697,11 +1686,10 @@ static int do_fetch(struct transport *transport,
 		 * don't care whether --tags was specified.
 		 */
 		if (rs->nr) {
-			retcode = prune_refs(&display_state, rs, transaction, ref_map, transport->url);
+			retcode = prune_refs(&display_state, rs, transaction, ref_map);
 		} else {
 			retcode = prune_refs(&display_state, &transport->remote->fetch,
-					     transaction, ref_map,
-					     transport->url);
+					     transaction, ref_map);
 		}
 		if (retcode != 0)
 			retcode = 1;
@@ -1812,6 +1800,7 @@ static int do_fetch(struct transport *transport,
 		error("%s", err.buf);
 	}
 
+	display_state_release(&display_state);
 	close_fetch_head(&fetch_head);
 	strbuf_release(&err);
 	free_refs(ref_map);

-- 
2.40.0
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 35 of 39 in “fetch: refactor code that prints reference updates”
  1. 0/8 fetch: refactor code that prints reference updatesPatrick Steinhardt, Mar 15, 2023
  2. 1/8 fetch: rename `display` buffer to avoid name conflictPatrick Steinhardt, Mar 15, 2023
  3. 2/8 fetch: move reference width calculation into `display_state`Patrick Steinhardt, Mar 15, 2023
  4. Junio C HamanoMar 15, 2023
  5. Patrick SteinhardtMar 16, 2023
  6. Junio C HamanoMar 16, 2023
  7. Patrick SteinhardtMar 17, 2023
  8. Junio C HamanoMar 16, 2023
  9. 3/8 fetch: move output format into `display_state`Patrick Steinhardt, Mar 15, 2023
  10. 4/8 fetch: pass the full local reference name to `format_display`Patrick Steinhardt, Mar 15, 2023
  11. Junio C HamanoMar 15, 2023
  12. 5/8 fetch: deduplicate handling of per-reference formatPatrick Steinhardt, Mar 15, 2023
  13. Junio C HamanoMar 15, 2023
  14. Patrick SteinhardtMar 16, 2023
  15. Junio C HamanoMar 16, 2023
  16. Patrick SteinhardtMar 17, 2023
  17. Junio C HamanoMar 17, 2023
  18. 7/8 fetch: fix inconsistent summary width for pruned and updated refsPatrick Steinhardt, Mar 15, 2023
  19. Junio C HamanoMar 15, 2023
  20. Patrick SteinhardtMar 16, 2023
  21. Junio C HamanoMar 16, 2023
  22. Patrick SteinhardtMar 17, 2023
  23. 6/8 fetch: deduplicate logic to print remote URLPatrick Steinhardt, Mar 15, 2023
  24. Junio C HamanoMar 15, 2023
  25. Patrick SteinhardtMar 16, 2023
  26. 8/8 fetch: centralize printing of reference updatesPatrick Steinhardt, Mar 15, 2023
  27. Jonathan TanMar 17, 2023
  28. Patrick SteinhardtMar 20, 2023
  29. Patrick SteinhardtMar 20, 2023
  30. 0/6 fetch: refactor code that prints reference updatesPatrick Steinhardt, Mar 20, 2023
  31. 1/6 fetch: move reference width calculation into `display_state`Patrick Steinhardt, Mar 20, 2023
  32. 2/6 fetch: move output format into `display_state`Patrick Steinhardt, Mar 20, 2023
  33. 3/6 fetch: pass the full local reference name to `format_display`Patrick Steinhardt, Mar 20, 2023
  34. 4/6 fetch: centralize handling of per-reference formatPatrick Steinhardt, Mar 20, 2023
  35. 5/6 fetch: centralize logic to print remote URLPatrick Steinhardt, Mar 20, 2023
  36. 6/6 fetch: centralize printing of reference updatesPatrick Steinhardt, Mar 20, 2023
  37. Jonathan TanMar 20, 2023
  38. Patrick SteinhardtMar 22, 2023
  39. Junio C HamanoMar 29, 2023

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.