From ce9636d645714974e5f7e1e9abc65fa57186b147 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Mar 2023 13:35:20 +0100 Subject: fetch: move reference width calculation into `display_state` In order to print references in proper columns we need to calculate the width of the reference column before starting to print the references. This is done with the help of a global variable `refcol_width`. Refactor the code to instead use a new structure `display_state` that contains the computed width and plumb it through the stack as required. This is only the first step towards de-globalizing the state required to print references. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fetch.c | 111 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 65 insertions(+), 46 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index a09606b472..391959ad3d 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -47,6 +47,10 @@ enum { TAGS_SET = 2 }; +struct display_state { + int refcol_width; +}; + static int fetch_prune_config = -1; /* unspecified */ static int fetch_show_forced_updates = 1; static uint64_t forced_updates_ms = 0; @@ -741,16 +745,15 @@ out: return ret; } -static int refcol_width = 10; static int compact_format; -static void adjust_refcol_width(const struct ref *ref) +static int refcol_width(const struct ref *ref) { int max, rlen, llen, len; /* uptodate lines are only shown on high verbosity level */ if (verbosity <= 0 && oideq(&ref->peer_ref->old_oid, &ref->old_oid)) - return; + return 0; max = term_columns(); rlen = utf8_strwidth(prettify_refname(ref->name)); @@ -769,22 +772,18 @@ static void adjust_refcol_width(const struct ref *ref) } len = 21 /* flag and summary */ + rlen + 4 /* -> */ + llen; if (len >= max) - return; + return 0; - /* - * Not precise calculation for compact mode because '*' can - * appear on the left hand side of '->' and shrink the column - * back. - */ - if (refcol_width < rlen) - refcol_width = rlen; + return rlen; } -static void prepare_format_display(struct ref *ref_map) +static void display_state_init(struct display_state *display_state, struct ref *ref_map) { struct ref *rm; const char *format = "full"; + memset(display_state, 0, sizeof(*display_state)); + if (verbosity < 0) return; @@ -797,20 +796,32 @@ static void prepare_format_display(struct ref *ref_map) die(_("invalid value for '%s': '%s'"), "fetch.output", format); + display_state->refcol_width = 10; for (rm = ref_map; rm; rm = rm->next) { + int width; + if (rm->status == REF_STATUS_REJECT_SHALLOW || !rm->peer_ref || !strcmp(rm->name, "HEAD")) continue; - adjust_refcol_width(rm); + width = refcol_width(rm); + + /* + * Not precise calculation for compact mode because '*' can + * appear on the left hand side of '->' and shrink the column + * back. + */ + if (display_state->refcol_width < width) + display_state->refcol_width = width; } } -static void print_remote_to_local(struct strbuf *display, +static void print_remote_to_local(struct display_state *display_state, + struct strbuf *display_buffer, const char *remote, const char *local) { - strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local); + strbuf_addf(display_buffer, "%-*s -> %s", display_state->refcol_width, remote, local); } static int find_and_replace(struct strbuf *haystack, @@ -840,14 +851,14 @@ static int find_and_replace(struct strbuf *haystack, return 1; } -static void print_compact(struct strbuf *display, +static void print_compact(struct display_state *display_state, struct strbuf *display_buffer, const char *remote, const char *local) { struct strbuf r = STRBUF_INIT; struct strbuf l = STRBUF_INIT; if (!strcmp(remote, local)) { - strbuf_addf(display, "%-*s -> *", refcol_width, remote); + strbuf_addf(display_buffer, "%-*s -> *", display_state->refcol_width, remote); return; } @@ -856,13 +867,14 @@ static void print_compact(struct strbuf *display, if (!find_and_replace(&r, local, "*")) find_and_replace(&l, remote, "*"); - print_remote_to_local(display, r.buf, l.buf); + print_remote_to_local(display_state, display_buffer, r.buf, l.buf); strbuf_release(&r); strbuf_release(&l); } -static void format_display(struct strbuf *display, char code, +static void format_display(struct display_state *display_state, + struct strbuf *display_buffer, char code, const char *summary, const char *error, const char *remote, const char *local, int summary_width) @@ -874,17 +886,18 @@ static void format_display(struct strbuf *display, char code, width = (summary_width + strlen(summary) - gettext_width(summary)); - strbuf_addf(display, "%c %-*s ", code, width, summary); + strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); if (!compact_format) - print_remote_to_local(display, remote, local); + print_remote_to_local(display_state, display_buffer, remote, local); else - print_compact(display, remote, local); + print_compact(display_state, display_buffer, remote, local); if (error) - strbuf_addf(display, " (%s)", error); + strbuf_addf(display_buffer, " (%s)", error); } static int update_local_ref(struct ref *ref, struct ref_transaction *transaction, + struct display_state *display_state, const char *remote, const struct ref *remote_ref, struct strbuf *display, int summary_width) { @@ -897,7 +910,7 @@ static int update_local_ref(struct ref *ref, if (oideq(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) - format_display(display, '=', _("[up to date]"), NULL, + format_display(display_state, display, '=', _("[up to date]"), NULL, remote, pretty_ref, summary_width); return 0; } @@ -909,7 +922,7 @@ static int update_local_ref(struct ref *ref, * If this is the head, and it's not okay to update * the head, and the old value of the head isn't empty... */ - format_display(display, '!', _("[rejected]"), + format_display(display_state, display, '!', _("[rejected]"), _("can't fetch into checked-out branch"), remote, pretty_ref, summary_width); return 1; @@ -920,12 +933,13 @@ static int update_local_ref(struct ref *ref, if (force || ref->force) { int r; r = s_update_ref("updating tag", ref, transaction, 0); - format_display(display, r ? '!' : 't', _("[tag update]"), + format_display(display_state, display, r ? '!' : 't', _("[tag update]"), r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); return r; } else { - format_display(display, '!', _("[rejected]"), _("would clobber existing tag"), + format_display(display_state, display, '!', _("[rejected]"), + _("would clobber existing tag"), remote, pretty_ref, summary_width); return 1; } @@ -957,7 +971,7 @@ static int update_local_ref(struct ref *ref, } r = s_update_ref(msg, ref, transaction, 0); - format_display(display, r ? '!' : '*', what, + format_display(display_state, display, r ? '!' : '*', what, r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); return r; @@ -979,7 +993,7 @@ static int update_local_ref(struct ref *ref, strbuf_addstr(&quickref, ".."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); r = s_update_ref("fast-forward", ref, transaction, 1); - format_display(display, r ? '!' : ' ', quickref.buf, + format_display(display_state, display, r ? '!' : ' ', quickref.buf, r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); strbuf_release(&quickref); @@ -991,13 +1005,13 @@ static int update_local_ref(struct ref *ref, strbuf_addstr(&quickref, "..."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); r = s_update_ref("forced-update", ref, transaction, 1); - format_display(display, r ? '!' : '+', quickref.buf, + format_display(display_state, display, r ? '!' : '+', quickref.buf, r ? _("unable to update local ref") : _("forced update"), remote, pretty_ref, summary_width); strbuf_release(&quickref); return r; } else { - format_display(display, '!', _("[rejected]"), _("non-fast-forward"), + format_display(display_state, display, '!', _("[rejected]"), _("non-fast-forward"), remote, pretty_ref, summary_width); return 1; } @@ -1108,7 +1122,8 @@ N_("it took %.2f seconds to check forced updates; you can use\n" "'--no-show-forced-updates' or run 'git config fetch.showForcedUpdates false'\n" "to avoid this check\n"); -static int store_updated_refs(const char *raw_url, const char *remote_name, +static int store_updated_refs(struct display_state *display_state, + const char *raw_url, const char *remote_name, int connectivity_checked, struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head) @@ -1139,8 +1154,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, } } - prepare_format_display(ref_map); - /* * We do a pass for each fetch_head_status type in their enum order, so * merged entries are written before not-for-merge. That lets readers @@ -1240,7 +1253,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, strbuf_reset(¬e); if (ref) { - rc |= update_local_ref(ref, transaction, what, + rc |= update_local_ref(ref, transaction, display_state, what, rm, ¬e, summary_width); free(ref); } else if (write_fetch_head || dry_run) { @@ -1249,7 +1262,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, * would be written to FETCH_HEAD, if --dry-run * is set). */ - format_display(¬e, '*', + format_display(display_state, ¬e, '*', *kind ? kind : "branch", NULL, *what ? what : "HEAD", "FETCH_HEAD", summary_width); @@ -1328,7 +1341,8 @@ static int check_exist_and_connected(struct ref *ref_map) return check_connected(iterate_ref_map, &rm, &opt); } -static int fetch_and_consume_refs(struct transport *transport, +static int fetch_and_consume_refs(struct display_state *display_state, + struct transport *transport, struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head) @@ -1352,7 +1366,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, + ret = store_updated_refs(display_state, transport->url, transport->remote->name, connectivity_checked, transaction, ref_map, fetch_head); trace2_region_leave("fetch", "consume_refs", the_repository); @@ -1362,7 +1376,8 @@ out: return ret; } -static int prune_refs(struct refspec *rs, +static int prune_refs(struct display_state *display_state, + struct refspec *rs, struct ref_transaction *transaction, struct ref *ref_map, const char *raw_url) @@ -1416,7 +1431,7 @@ static int prune_refs(struct refspec *rs, fprintf(stderr, _("From %.*s\n"), url_len, url); shown_url = 1; } - format_display(&sb, '-', _("[deleted]"), NULL, + format_display(display_state, &sb, '-', _("[deleted]"), NULL, _("(none)"), prettify_refname(ref->name), summary_width); fprintf(stderr, " %s\n",sb.buf); @@ -1542,7 +1557,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen) return transport; } -static int backfill_tags(struct transport *transport, +static int backfill_tags(struct display_state *display_state, + struct transport *transport, struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head) @@ -1566,7 +1582,7 @@ static int backfill_tags(struct transport *transport, 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); - retcode = fetch_and_consume_refs(transport, transaction, ref_map, fetch_head); + retcode = fetch_and_consume_refs(display_state, transport, transaction, ref_map, fetch_head); if (gsecondary) { transport_disconnect(gsecondary); @@ -1581,6 +1597,7 @@ static int do_fetch(struct transport *transport, { struct ref_transaction *transaction = NULL; struct ref *ref_map = NULL; + struct display_state display_state; int autotags = (transport->remote->fetch_tags == 1); int retcode = 0; const struct ref *remote_refs; @@ -1662,6 +1679,8 @@ static int do_fetch(struct transport *transport, if (retcode) goto cleanup; + display_state_init(&display_state, ref_map); + if (atomic_fetch) { transaction = ref_transaction_begin(&err); if (!transaction) { @@ -1679,9 +1698,9 @@ static int do_fetch(struct transport *transport, * don't care whether --tags was specified. */ if (rs->nr) { - retcode = prune_refs(rs, transaction, ref_map, transport->url); + retcode = prune_refs(&display_state, rs, transaction, ref_map, transport->url); } else { - retcode = prune_refs(&transport->remote->fetch, + retcode = prune_refs(&display_state, &transport->remote->fetch, transaction, ref_map, transport->url); } @@ -1689,7 +1708,7 @@ static int do_fetch(struct transport *transport, retcode = 1; } - if (fetch_and_consume_refs(transport, transaction, ref_map, &fetch_head)) { + if (fetch_and_consume_refs(&display_state, transport, transaction, ref_map, &fetch_head)) { retcode = 1; goto cleanup; } @@ -1711,7 +1730,7 @@ static int do_fetch(struct transport *transport, * when `--atomic` is passed: in that case we'll abort * the transaction and don't commit anything. */ - if (backfill_tags(transport, transaction, tags_ref_map, + if (backfill_tags(&display_state, transport, transaction, tags_ref_map, &fetch_head)) retcode = 1; } -- cgit v1.2.3 From 5cab51ff7156edca8cb9eba30205efa11900e49c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Mar 2023 13:35:24 +0100 Subject: fetch: move output format into `display_state` The git-fetch(1) command supports printing references either in "full" or "compact" format depending on the `fetch.ouput` config key. The format that is to be used is tracked in a global variable. De-globalize the variable by moving it into the `display_state` structure. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fetch.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 391959ad3d..6d6146b0f0 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -49,6 +49,7 @@ enum { struct display_state { int refcol_width; + int compact_format; }; static int fetch_prune_config = -1; /* unspecified */ @@ -745,9 +746,7 @@ out: return ret; } -static int compact_format; - -static int refcol_width(const struct ref *ref) +static int refcol_width(const struct ref *ref, int compact_format) { int max, rlen, llen, len; @@ -789,9 +788,9 @@ static void display_state_init(struct display_state *display_state, struct ref * git_config_get_string_tmp("fetch.output", &format); if (!strcasecmp(format, "full")) - compact_format = 0; + display_state->compact_format = 0; else if (!strcasecmp(format, "compact")) - compact_format = 1; + display_state->compact_format = 1; else die(_("invalid value for '%s': '%s'"), "fetch.output", format); @@ -805,7 +804,7 @@ static void display_state_init(struct display_state *display_state, struct ref * !strcmp(rm->name, "HEAD")) continue; - width = refcol_width(rm); + width = refcol_width(rm, display_state->compact_format); /* * Not precise calculation for compact mode because '*' can @@ -887,7 +886,7 @@ static void format_display(struct display_state *display_state, width = (summary_width + strlen(summary) - gettext_width(summary)); strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); - if (!compact_format) + if (!display_state->compact_format) print_remote_to_local(display_state, display_buffer, remote, local); else print_compact(display_state, display_buffer, remote, local); -- cgit v1.2.3 From 7c978db889f2a0eddf0799268f57b9457ef9ab3d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Mar 2023 13:35:28 +0100 Subject: fetch: pass the full local reference name to `format_display` Before printing the name of the local references that would be updated by a fetch we first prettify the reference name. This is done at the calling side so that `format_display()` never sees the full name of the local reference. This restricts our ability to introduce new output formats that might want to print the full reference name. Right now, all callsites except one are prettifying the reference name anyway. And the only callsite that doesn't passes `FETCH_HEAD` as the hardcoded reference name to `format_display()`, which would never be changed by a call to `prettify_refname()` anyway. So let's refactor the code to pass in the full local reference name and then prettify it in the formatting code. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fetch.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 6d6146b0f0..81ba3900cb 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -887,9 +887,9 @@ static void format_display(struct display_state *display_state, strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); if (!display_state->compact_format) - print_remote_to_local(display_state, display_buffer, remote, local); + print_remote_to_local(display_state, display_buffer, remote, prettify_refname(local)); else - print_compact(display_state, display_buffer, remote, local); + print_compact(display_state, display_buffer, remote, prettify_refname(local)); if (error) strbuf_addf(display_buffer, " (%s)", error); } @@ -901,7 +901,6 @@ static int update_local_ref(struct ref *ref, struct strbuf *display, int summary_width) { struct commit *current = NULL, *updated; - const char *pretty_ref = prettify_refname(ref->name); int fast_forward = 0; if (!repo_has_object_file(the_repository, &ref->new_oid)) @@ -910,7 +909,7 @@ static int update_local_ref(struct ref *ref, if (oideq(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) format_display(display_state, display, '=', _("[up to date]"), NULL, - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return 0; } @@ -923,7 +922,7 @@ static int update_local_ref(struct ref *ref, */ format_display(display_state, display, '!', _("[rejected]"), _("can't fetch into checked-out branch"), - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return 1; } @@ -934,12 +933,12 @@ static int update_local_ref(struct ref *ref, r = s_update_ref("updating tag", ref, transaction, 0); format_display(display_state, display, r ? '!' : 't', _("[tag update]"), r ? _("unable to update local ref") : NULL, - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return r; } else { format_display(display_state, display, '!', _("[rejected]"), _("would clobber existing tag"), - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return 1; } } @@ -972,7 +971,7 @@ static int update_local_ref(struct ref *ref, r = s_update_ref(msg, ref, transaction, 0); format_display(display_state, display, r ? '!' : '*', what, r ? _("unable to update local ref") : NULL, - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return r; } @@ -994,7 +993,7 @@ static int update_local_ref(struct ref *ref, r = s_update_ref("fast-forward", ref, transaction, 1); format_display(display_state, display, r ? '!' : ' ', quickref.buf, r ? _("unable to update local ref") : NULL, - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); strbuf_release(&quickref); return r; } else if (force || ref->force) { @@ -1006,12 +1005,12 @@ static int update_local_ref(struct ref *ref, r = s_update_ref("forced-update", ref, transaction, 1); format_display(display_state, display, r ? '!' : '+', quickref.buf, r ? _("unable to update local ref") : _("forced update"), - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); strbuf_release(&quickref); return r; } else { format_display(display_state, display, '!', _("[rejected]"), _("non-fast-forward"), - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return 1; } } @@ -1431,7 +1430,7 @@ static int prune_refs(struct display_state *display_state, shown_url = 1; } format_display(display_state, &sb, '-', _("[deleted]"), NULL, - _("(none)"), prettify_refname(ref->name), + _("(none)"), ref->name, summary_width); fprintf(stderr, " %s\n",sb.buf); strbuf_release(&sb); -- cgit v1.2.3 From 331b7d29f05b62fc73f1218e0e6db6969481a3cd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Mar 2023 13:35:32 +0100 Subject: fetch: centralize handling of per-reference format The function `format_display()` is used to print a single reference update to a buffer which will then ultimately be printed by the caller. This architecture causes us to duplicate some logic across the different callsites of this function. This makes it hard to follow the code as some parts of the logic are located in one place, while other parts of the logic are located in a different place. Furthermore, by having the logic scattered around it becomes quite hard to implement a new output format for the reference updates. We can make the logic a whole lot easier to understand by making the `format_display()` function self-contained so that it handles formatting and printing of the references. This will eventually allow us to easily implement a completely different output format, but also opens the door to conditionally print to either stdout or stderr depending on the output format. As a first step towards that goal we move the formatting directive used by both callers to print a single reference update into this function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fetch.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 81ba3900cb..a66428dfd8 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -885,13 +885,14 @@ static void format_display(struct display_state *display_state, width = (summary_width + strlen(summary) - gettext_width(summary)); - strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); + strbuf_addf(display_buffer, " %c %-*s ", code, width, summary); if (!display_state->compact_format) print_remote_to_local(display_state, display_buffer, remote, prettify_refname(local)); else print_compact(display_state, display_buffer, remote, prettify_refname(local)); if (error) strbuf_addf(display_buffer, " (%s)", error); + strbuf_addch(display_buffer, '\n'); } static int update_local_ref(struct ref *ref, @@ -1271,7 +1272,7 @@ static int store_updated_refs(struct display_state *display_state, url_len, url); shown_url = 1; } - fprintf(stderr, " %s\n", note.buf); + fputs(note.buf, stderr); } } } @@ -1432,7 +1433,7 @@ static int prune_refs(struct display_state *display_state, format_display(display_state, &sb, '-', _("[deleted]"), NULL, _("(none)"), ref->name, summary_width); - fprintf(stderr, " %s\n",sb.buf); + fputs(sb.buf, stderr); strbuf_release(&sb); warn_dangling_symref(stderr, dangling_msg, ref->name); } -- cgit v1.2.3 From c4ef5edbc952302097d6a55ac490bad7726bf840 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Mar 2023 13:35:36 +0100 Subject: 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 Signed-off-by: Junio C Hamano --- 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(¬e); 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(¬e); 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(¬e); - 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 @@ out: 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 @@ cleanup: error("%s", err.buf); } + display_state_release(&display_state); close_fetch_head(&fetch_head); strbuf_release(&err); free_refs(ref_map); -- cgit v1.2.3 From d6606e02aaff963f51fe9cbda048613ff0cd5870 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Mar 2023 13:35:40 +0100 Subject: fetch: centralize printing of reference updates In order to print updated references during a fetch, the two different call sites that do this will first call `format_display()` followed by a call to `fputs()`. This is needlessly roundabout now that we have the `display_state` structure that encapsulates all of the printing logic for references. Move displaying the reference updates into `format_display()` and rename it to `display_ref_update()` to better match its new purpose, which finalizes the conversion to make both the formatting and printing logic of reference updates self-contained. This will make it easier to add new output formats and printing to a different file descriptor than stderr. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fetch.c | 108 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 1e3599cb74..c202c18fb4 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -48,6 +48,8 @@ enum { }; struct display_state { + struct strbuf buf; + int refcol_width; int compact_format; @@ -787,6 +789,8 @@ static void display_state_init(struct display_state *display_state, struct ref * memset(display_state, 0, sizeof(*display_state)); + strbuf_init(&display_state->buf, 0); + if (raw_url) display_state->url = transport_anonymize_url(raw_url); else @@ -834,14 +838,15 @@ static void display_state_init(struct display_state *display_state, struct ref * static void display_state_release(struct display_state *display_state) { + strbuf_release(&display_state->buf); 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) { - strbuf_addf(display_buffer, "%-*s -> %s", display_state->refcol_width, remote, local); + strbuf_addf(&display_state->buf, "%-*s -> %s", + display_state->refcol_width, remote, local); } static int find_and_replace(struct strbuf *haystack, @@ -871,14 +876,14 @@ static int find_and_replace(struct strbuf *haystack, return 1; } -static void print_compact(struct display_state *display_state, struct strbuf *display_buffer, +static void print_compact(struct display_state *display_state, const char *remote, const char *local) { struct strbuf r = STRBUF_INIT; struct strbuf l = STRBUF_INIT; if (!strcmp(remote, local)) { - strbuf_addf(display_buffer, "%-*s -> *", display_state->refcol_width, remote); + strbuf_addf(&display_state->buf, "%-*s -> *", display_state->refcol_width, remote); return; } @@ -887,46 +892,49 @@ static void print_compact(struct display_state *display_state, struct strbuf *di if (!find_and_replace(&r, local, "*")) find_and_replace(&l, remote, "*"); - print_remote_to_local(display_state, display_buffer, r.buf, l.buf); + print_remote_to_local(display_state, r.buf, l.buf); strbuf_release(&r); strbuf_release(&l); } -static void format_display(struct display_state *display_state, - struct strbuf *display_buffer, char code, - const char *summary, const char *error, - const char *remote, const char *local, - int summary_width) +static void display_ref_update(struct display_state *display_state, char code, + const char *summary, const char *error, + const char *remote, const char *local, + int summary_width) { int width; if (verbosity < 0) return; + strbuf_reset(&display_state->buf); + if (!display_state->shown_url) { - strbuf_addf(display_buffer, _("From %.*s\n"), + strbuf_addf(&display_state->buf, _("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); + strbuf_addf(&display_state->buf, " %c %-*s ", code, width, summary); if (!display_state->compact_format) - print_remote_to_local(display_state, display_buffer, remote, prettify_refname(local)); + print_remote_to_local(display_state, remote, prettify_refname(local)); else - print_compact(display_state, display_buffer, remote, prettify_refname(local)); + print_compact(display_state, remote, prettify_refname(local)); if (error) - strbuf_addf(display_buffer, " (%s)", error); - strbuf_addch(display_buffer, '\n'); + strbuf_addf(&display_state->buf, " (%s)", error); + strbuf_addch(&display_state->buf, '\n'); + + fputs(display_state->buf.buf, stderr); } static int update_local_ref(struct ref *ref, struct ref_transaction *transaction, struct display_state *display_state, const char *remote, const struct ref *remote_ref, - struct strbuf *display, int summary_width) + int summary_width) { struct commit *current = NULL, *updated; int fast_forward = 0; @@ -936,8 +944,8 @@ static int update_local_ref(struct ref *ref, if (oideq(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) - format_display(display_state, display, '=', _("[up to date]"), NULL, - remote, ref->name, summary_width); + display_ref_update(display_state, '=', _("[up to date]"), NULL, + remote, ref->name, summary_width); return 0; } @@ -948,9 +956,9 @@ static int update_local_ref(struct ref *ref, * If this is the head, and it's not okay to update * the head, and the old value of the head isn't empty... */ - format_display(display_state, display, '!', _("[rejected]"), - _("can't fetch into checked-out branch"), - remote, ref->name, summary_width); + display_ref_update(display_state, '!', _("[rejected]"), + _("can't fetch into checked-out branch"), + remote, ref->name, summary_width); return 1; } @@ -959,14 +967,14 @@ static int update_local_ref(struct ref *ref, if (force || ref->force) { int r; r = s_update_ref("updating tag", ref, transaction, 0); - format_display(display_state, display, r ? '!' : 't', _("[tag update]"), - r ? _("unable to update local ref") : NULL, - remote, ref->name, summary_width); + display_ref_update(display_state, r ? '!' : 't', _("[tag update]"), + r ? _("unable to update local ref") : NULL, + remote, ref->name, summary_width); return r; } else { - format_display(display_state, display, '!', _("[rejected]"), - _("would clobber existing tag"), - remote, ref->name, summary_width); + display_ref_update(display_state, '!', _("[rejected]"), + _("would clobber existing tag"), + remote, ref->name, summary_width); return 1; } } @@ -997,9 +1005,9 @@ static int update_local_ref(struct ref *ref, } r = s_update_ref(msg, ref, transaction, 0); - format_display(display_state, display, r ? '!' : '*', what, - r ? _("unable to update local ref") : NULL, - remote, ref->name, summary_width); + display_ref_update(display_state, r ? '!' : '*', what, + r ? _("unable to update local ref") : NULL, + remote, ref->name, summary_width); return r; } @@ -1019,9 +1027,9 @@ static int update_local_ref(struct ref *ref, strbuf_addstr(&quickref, ".."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); r = s_update_ref("fast-forward", ref, transaction, 1); - format_display(display_state, display, r ? '!' : ' ', quickref.buf, - r ? _("unable to update local ref") : NULL, - remote, ref->name, summary_width); + display_ref_update(display_state, r ? '!' : ' ', quickref.buf, + r ? _("unable to update local ref") : NULL, + remote, ref->name, summary_width); strbuf_release(&quickref); return r; } else if (force || ref->force) { @@ -1031,14 +1039,14 @@ static int update_local_ref(struct ref *ref, strbuf_addstr(&quickref, "..."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); r = s_update_ref("forced-update", ref, transaction, 1); - format_display(display_state, display, r ? '!' : '+', quickref.buf, - r ? _("unable to update local ref") : _("forced update"), - remote, ref->name, summary_width); + display_ref_update(display_state, r ? '!' : '+', quickref.buf, + r ? _("unable to update local ref") : _("forced update"), + remote, ref->name, summary_width); strbuf_release(&quickref); return r; } else { - format_display(display_state, display, '!', _("[rejected]"), _("non-fast-forward"), - remote, ref->name, summary_width); + display_ref_update(display_state, '!', _("[rejected]"), _("non-fast-forward"), + remote, ref->name, summary_width); return 1; } } @@ -1266,10 +1274,9 @@ static int store_updated_refs(struct display_state *display_state, note.buf, display_state->url, display_state->url_len); - strbuf_reset(¬e); if (ref) { rc |= update_local_ref(ref, transaction, display_state, what, - rm, ¬e, summary_width); + rm, summary_width); free(ref); } else if (write_fetch_head || dry_run) { /* @@ -1277,13 +1284,11 @@ static int store_updated_refs(struct display_state *display_state, * would be written to FETCH_HEAD, if --dry-run * is set). */ - format_display(display_state, ¬e, '*', - *kind ? kind : "branch", NULL, - *what ? what : "HEAD", - "FETCH_HEAD", summary_width); + display_ref_update(display_state, '*', + *kind ? kind : "branch", NULL, + *what ? what : "HEAD", + "FETCH_HEAD", summary_width); } - if (note.len) - fputs(note.buf, stderr); } } @@ -1419,12 +1424,9 @@ static int prune_refs(struct display_state *display_state, int summary_width = transport_summary_width(stale_refs); for (ref = stale_refs; ref; ref = ref->next) { - struct strbuf sb = STRBUF_INIT; - format_display(display_state, &sb, '-', _("[deleted]"), NULL, - _("(none)"), ref->name, - summary_width); - fputs(sb.buf, stderr); - strbuf_release(&sb); + display_ref_update(display_state, '-', _("[deleted]"), NULL, + _("(none)"), ref->name, + summary_width); warn_dangling_symref(stderr, dangling_msg, ref->name); } } -- cgit v1.2.3