fetch: centralize logic to print remote URL

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 <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: pks-t

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);