git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Subject: [PATCH 6/8] fetch: deduplicate logic to print remote URL
Date: Wed, 15 Mar 2023 12:21:28 +0100	[thread overview]
Message-ID: <2ea3a4e308d2fb89f24d0cd2efc9d3867983a4b0.1678878623.git.ps@pks.im> (raw)
In-Reply-To: <cover.1678878623.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 8962 bytes --]

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.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fetch.c | 96 +++++++++++++++++++++----------------------------
 1 file changed, 41 insertions(+), 55 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6fc2fd0d46..4e18c3902d 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, struct ref *ref_map)
+static void display_state_init(struct display_state *display, struct ref *ref_map,
+			       const char *raw_url)
 {
 	struct ref *rm;
 	const char *format = "full";
+	int i;
 
 	memset(display, 0, sizeof(*display));
 
+	if (raw_url)
+		display->url = transport_anonymize_url(raw_url);
+	else
+		display->url = xstrdup("foreign");
+
+	display->url_len = strlen(display->url);
+	for (i = display->url_len - 1; display->url[i] == '/' && 0 <= i; i--)
+		;
+	display->url_len = i + 1;
+	if (4 < i && !strncmp(".git", display->url + i - 3, 4))
+		display->url_len = i - 3;
+
 	if (verbosity < 0)
 		return;
 
@@ -816,6 +832,11 @@ static void display_state_init(struct display_state *display, struct ref *ref_ma
 	}
 }
 
+static void display_state_release(struct display_state *display)
+{
+	free(display->url);
+}
+
 static void print_remote_to_local(struct display_state *display,
 				  struct strbuf *display_buffer,
 				  const char *remote, const char *local)
@@ -883,6 +904,11 @@ static void format_display(struct display_state *display,
 	if (verbosity < 0)
 		return;
 
+	if (!display->shown_url) {
+		strbuf_addf(display_buffer, _("From %.*s\n"), display->url_len, display->url);
+		display->shown_url = 1;
+	}
+
 	width = (summary_width + strlen(summary) - gettext_width(summary));
 
 	strbuf_addf(display_buffer, " %c %-*s ", code, width, summary);
@@ -1122,33 +1148,27 @@ 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,
-			      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->url);
 			goto abort;
 		}
 	}
@@ -1232,13 +1252,6 @@ static int store_updated_refs(struct display_state *display,
 				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 +1261,7 @@ static int store_updated_refs(struct display_state *display,
 
 			append_fetch_head(fetch_head, &rm->old_oid,
 					  rm->fetch_head_status,
-					  note.buf, url, url_len);
+					  note.buf, display->url, display->url_len);
 
 			strbuf_reset(&note);
 			if (ref) {
@@ -1266,14 +1279,8 @@ static int store_updated_refs(struct display_state *display,
 					       *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 +1300,6 @@ static int store_updated_refs(struct display_state *display,
 
  abort:
 	strbuf_release(&note);
-	free(url);
 	return rc;
 }
 
@@ -1365,7 +1371,7 @@ static int fetch_and_consume_refs(struct display_state *display,
 	}
 
 	trace2_region_enter("fetch", "consume_refs", the_repository);
-	ret = store_updated_refs(display, transport->url, transport->remote->name,
+	ret = store_updated_refs(display, transport->remote->name,
 				 connectivity_checked, transaction, ref_map,
 				 fetch_head);
 	trace2_region_leave("fetch", "consume_refs", the_repository);
@@ -1378,30 +1384,15 @@ static int fetch_and_consume_refs(struct display_state *display,
 static int prune_refs(struct display_state *display,
 		      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 +1417,6 @@ static int prune_refs(struct display_state *display,
 
 		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, &sb, '-', _("[deleted]"), NULL,
 				       _("(none)"), ref->name,
 				       summary_width);
@@ -1441,7 +1428,6 @@ static int prune_refs(struct display_state *display,
 
 cleanup:
 	strbuf_release(&err);
-	free(url);
 	free_refs(stale_refs);
 	return result;
 }
@@ -1596,7 +1582,7 @@ static int do_fetch(struct transport *transport,
 {
 	struct ref_transaction *transaction = NULL;
 	struct ref *ref_map = NULL;
-	struct display_state display;
+	struct display_state display = { 0 };
 	int autotags = (transport->remote->fetch_tags == 1);
 	int retcode = 0;
 	const struct ref *remote_refs;
@@ -1678,7 +1664,7 @@ static int do_fetch(struct transport *transport,
 	if (retcode)
 		goto cleanup;
 
-	display_state_init(&display, ref_map);
+	display_state_init(&display, ref_map, transport->url);
 
 	if (atomic_fetch) {
 		transaction = ref_transaction_begin(&err);
@@ -1697,11 +1683,10 @@ static int do_fetch(struct transport *transport,
 		 * don't care whether --tags was specified.
 		 */
 		if (rs->nr) {
-			retcode = prune_refs(&display, rs, transaction, ref_map, transport->url);
+			retcode = prune_refs(&display, rs, transaction, ref_map);
 		} else {
 			retcode = prune_refs(&display, &transport->remote->fetch,
-					     transaction, ref_map,
-					     transport->url);
+					     transaction, ref_map);
 		}
 		if (retcode != 0)
 			retcode = 1;
@@ -1812,6 +1797,7 @@ static int do_fetch(struct transport *transport,
 		error("%s", err.buf);
 	}
 
+	display_state_release(&display);
 	close_fetch_head(&fetch_head);
 	strbuf_release(&err);
 	free_refs(ref_map);
-- 
2.40.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2023-03-15 11:25 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-15 11:21 [PATCH 0/8] fetch: refactor code that prints reference updates Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 1/8] fetch: rename `display` buffer to avoid name conflict Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 2/8] fetch: move reference width calculation into `display_state` Patrick Steinhardt
2023-03-15 20:59   ` Junio C Hamano
2023-03-16 15:05     ` Patrick Steinhardt
2023-03-16 16:18       ` Junio C Hamano
2023-03-17 10:03         ` Patrick Steinhardt
2023-03-16 16:19       ` Junio C Hamano
2023-03-15 11:21 ` [PATCH 3/8] fetch: move output format " Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 4/8] fetch: pass the full local reference name to `format_display` Patrick Steinhardt
2023-03-15 22:18   ` Junio C Hamano
2023-03-15 11:21 ` [PATCH 5/8] fetch: deduplicate handling of per-reference format Patrick Steinhardt
2023-03-15 22:45   ` Junio C Hamano
2023-03-16 15:06     ` Patrick Steinhardt
2023-03-16 16:50       ` Junio C Hamano
2023-03-17  9:51         ` Patrick Steinhardt
2023-03-17 15:41           ` Junio C Hamano
2023-03-15 11:21 ` Patrick Steinhardt [this message]
2023-03-15 23:02   ` [PATCH 6/8] fetch: deduplicate logic to print remote URL Junio C Hamano
2023-03-16 15:06     ` Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 7/8] fetch: fix inconsistent summary width for pruned and updated refs Patrick Steinhardt
2023-03-15 23:12   ` Junio C Hamano
2023-03-16 15:06     ` Patrick Steinhardt
2023-03-16 16:30       ` Junio C Hamano
2023-03-17  9:55         ` Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 8/8] fetch: centralize printing of reference updates Patrick Steinhardt
2023-03-17 20:24 ` [PATCH 0/8] fetch: refactor code that prints " Jonathan Tan
2023-03-20  6:57   ` Patrick Steinhardt
2023-03-20 12:26   ` Patrick Steinhardt
2023-03-20 12:35 ` [PATCH v2 0/6] " Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 1/6] fetch: move reference width calculation into `display_state` Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 2/6] fetch: move output format " Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 3/6] fetch: pass the full local reference name to `format_display` Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 4/6] fetch: centralize handling of per-reference format Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 5/6] fetch: centralize logic to print remote URL Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 6/6] fetch: centralize printing of reference updates Patrick Steinhardt
2023-03-20 22:57     ` Jonathan Tan
2023-03-22  9:04       ` Patrick Steinhardt
2023-03-29 18:45       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2ea3a4e308d2fb89f24d0cd2efc9d3867983a4b0.1678878623.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).