From 6debb7527b0e2f791256a216394503899b0488ee Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 18 Jun 2022 00:20:54 +0000 Subject: merge-ort: store messages in a list, not in a single strbuf To prepare for using the `merge-ort` machinery in server operations, we cannot simply produce a free-form string that combines a variable-length list of messages. Instead, we need to list them one by one. The natural fit for this is a `string_list`. We will subsequently add even more information in the `util` attribute of the string list items. Based-on-a-patch-by: Elijah Newren Signed-off-by: Johannes Schindelin Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-ort.c | 123 +++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 80 insertions(+), 43 deletions(-) (limited to 'merge-ort.c') diff --git a/merge-ort.c b/merge-ort.c index 3432efcacd..9e9f2fc674 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -349,13 +349,15 @@ struct merge_options_internal { struct mem_pool pool; /* - * output: special messages and conflict notices for various paths + * conflicts: logical conflicts and messages stored by _primary_ path * * This is a map of pathnames (a subset of the keys in "paths" above) - * to strbufs. It gathers various warning/conflict/notice messages - * for later processing. + * to struct string_list, with each item's `util` containing a + * `struct logical_conflict_info`. Note, though, that for each path, + * it only stores the logical conflicts for which that path is the + * primary path; the path might be part of additional conflicts. */ - struct strmap output; + struct strmap conflicts; /* * renames: various data relating to rename detection @@ -567,20 +569,20 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti, struct strmap_entry *e; /* Release and free each strbuf found in output */ - strmap_for_each_entry(&opti->output, &iter, e) { - struct strbuf *sb = e->value; - strbuf_release(sb); + strmap_for_each_entry(&opti->conflicts, &iter, e) { + struct string_list *list = e->value; /* - * While strictly speaking we don't need to free(sb) - * here because we could pass free_values=1 when - * calling strmap_clear() on opti->output, that would - * require strmap_clear to do another - * strmap_for_each_entry() loop, so we just free it - * while we're iterating anyway. + * While strictly speaking we don't need to + * free(conflicts) here because we could pass + * free_values=1 when calling strmap_clear() on + * opti->conflicts, that would require strmap_clear + * to do another strmap_for_each_entry() loop, so we + * just free it while we're iterating anyway. */ - free(sb); + string_list_clear(list, 1); + free(list); } - strmap_clear(&opti->output, 0); + strmap_clear(&opti->conflicts, 0); } mem_pool_discard(&opti->pool, 0); @@ -634,7 +636,9 @@ static void path_msg(struct merge_options *opt, const char *fmt, ...) { va_list ap; - struct strbuf *sb, *dest; + struct string_list *path_conflicts; + struct strbuf buf = STRBUF_INIT; + struct strbuf *dest; struct strbuf tmp = STRBUF_INIT; if (opt->record_conflict_msgs_as_headers && omittable_hint) @@ -642,14 +646,16 @@ static void path_msg(struct merge_options *opt, if (opt->priv->call_depth && opt->verbosity < 5) return; /* Ignore messages from inner merges */ - sb = strmap_get(&opt->priv->output, path); - if (!sb) { - sb = xmalloc(sizeof(*sb)); - strbuf_init(sb, 0); - strmap_put(&opt->priv->output, path, sb); + /* Ensure path_conflicts (ptr to array of logical_conflict) allocated */ + path_conflicts = strmap_get(&opt->priv->conflicts, path); + if (!path_conflicts) { + path_conflicts = xmalloc(sizeof(*path_conflicts)); + string_list_init_dup(path_conflicts); + strmap_put(&opt->priv->conflicts, path, path_conflicts); } - dest = (opt->record_conflict_msgs_as_headers ? &tmp : sb); + /* Handle message and its format, in normal case */ + dest = (opt->record_conflict_msgs_as_headers ? &tmp : &buf); va_start(ap, fmt); if (opt->priv->call_depth) { @@ -660,32 +666,31 @@ static void path_msg(struct merge_options *opt, strbuf_vaddf(dest, fmt, ap); va_end(ap); + /* Handle specialized formatting of message under --remerge-diff */ if (opt->record_conflict_msgs_as_headers) { int i_sb = 0, i_tmp = 0; /* Start with the specified prefix */ if (opt->msg_header_prefix) - strbuf_addf(sb, "%s ", opt->msg_header_prefix); + strbuf_addf(&buf, "%s ", opt->msg_header_prefix); /* Copy tmp to sb, adding spaces after newlines */ - strbuf_grow(sb, sb->len + 2*tmp.len); /* more than sufficient */ + strbuf_grow(&buf, buf.len + 2*tmp.len); /* more than sufficient */ for (; i_tmp < tmp.len; i_tmp++, i_sb++) { /* Copy next character from tmp to sb */ - sb->buf[sb->len + i_sb] = tmp.buf[i_tmp]; + buf.buf[buf.len + i_sb] = tmp.buf[i_tmp]; /* If we copied a newline, add a space */ if (tmp.buf[i_tmp] == '\n') - sb->buf[++i_sb] = ' '; + buf.buf[++i_sb] = ' '; } /* Update length and ensure it's NUL-terminated */ - sb->len += i_sb; - sb->buf[sb->len] = '\0'; + buf.len += i_sb; + buf.buf[buf.len] = '\0'; strbuf_release(&tmp); } - - /* Add final newline character to sb */ - strbuf_addch(sb, '\n'); + string_list_append_nodup(path_conflicts, strbuf_detach(&buf, NULL)); } static struct diff_filespec *pool_alloc_filespec(struct mem_pool *pool, @@ -4257,7 +4262,6 @@ void merge_display_update_messages(struct merge_options *opt, struct hashmap_iter iter; struct strmap_entry *e; struct string_list olist = STRING_LIST_INIT_NODUP; - int i; if (opt->record_conflict_msgs_as_headers) BUG("Either display conflict messages or record them as headers, not both"); @@ -4265,20 +4269,20 @@ void merge_display_update_messages(struct merge_options *opt, trace2_region_enter("merge", "display messages", opt->repo); /* Hack to pre-allocate olist to the desired size */ - ALLOC_GROW(olist.items, strmap_get_size(&opti->output), + ALLOC_GROW(olist.items, strmap_get_size(&opti->conflicts), olist.alloc); /* Put every entry from output into olist, then sort */ - strmap_for_each_entry(&opti->output, &iter, e) { + strmap_for_each_entry(&opti->conflicts, &iter, e) { string_list_append(&olist, e->key)->util = e->value; } string_list_sort(&olist); /* Iterate over the items, printing them */ - for (i = 0; i < olist.nr; ++i) { - struct strbuf *sb = olist.items[i].util; - - printf("%s", sb->buf); + for (int path_nr = 0; path_nr < olist.nr; ++path_nr) { + struct string_list *conflicts = olist.items[path_nr].util; + for (int i = 0; i < conflicts->nr; i++) + puts(conflicts->items[i].string); } string_list_clear(&olist, 0); @@ -4367,6 +4371,8 @@ void merge_finalize(struct merge_options *opt, struct merge_result *result) { struct merge_options_internal *opti = result->priv; + struct hashmap_iter iter; + struct strmap_entry *e; if (opt->renormalize) git_attr_set_direction(GIT_ATTR_CHECKIN); @@ -4374,6 +4380,15 @@ void merge_finalize(struct merge_options *opt, clear_or_reinit_internal_opts(opti, 0); FREE_AND_NULL(opti); + + /* Release and free each strbuf found in path_messages */ + strmap_for_each_entry(result->path_messages, &iter, e) { + struct strbuf *buf = e->value; + + strbuf_release(buf); + } + strmap_clear(result->path_messages, 1); + FREE_AND_NULL(result->path_messages); } /*** Function Grouping: helper functions for merge_incore_*() ***/ @@ -4532,11 +4547,11 @@ static void merge_start(struct merge_options *opt, struct merge_result *result) strmap_init_with_options(&opt->priv->conflicted, pool, 0); /* - * keys & strbufs in output will sometimes need to outlive "paths", - * so it will have a copy of relevant keys. It's probably a small - * subset of the overall paths that have special output. + * keys & string_lists in conflicts will sometimes need to outlive + * "paths", so it will have a copy of relevant keys. It's probably + * a small subset of the overall paths that have special output. */ - strmap_init(&opt->priv->output); + strmap_init(&opt->priv->conflicts); trace2_region_leave("merge", "allocate/init", opt->repo); } @@ -4597,6 +4612,8 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt, struct merge_result *result) { struct object_id working_tree_oid; + struct hashmap_iter iter; + struct strmap_entry *e; if (opt->subtree_shift) { side2 = shift_tree_object(opt->repo, side1, side2, @@ -4637,7 +4654,27 @@ redo: trace2_region_leave("merge", "process_entries", opt->repo); /* Set return values */ - result->path_messages = &opt->priv->output; + result->path_messages = xcalloc(1, sizeof(*result->path_messages)); + strmap_init_with_options(result->path_messages, NULL, 0); + strmap_for_each_entry(&opt->priv->conflicts, &iter, e) { + const char *path = e->key; + struct strbuf *buf = strmap_get(result->path_messages, path); + struct string_list *conflicts = e->value; + + if (!buf) { + buf = xcalloc(1, sizeof(*buf)); + strbuf_init(buf, 0); + strmap_put(result->path_messages, path, buf); + } + + for (int i = 0; i < conflicts->nr; i++) { + if (buf->len) + strbuf_addch(buf, '\n'); + strbuf_addstr(buf, conflicts->items[i].string); + strbuf_trim_trailing_newline(buf); + } + } + result->tree = parse_tree_indirect(&working_tree_oid); /* existence of conflicted entries implies unclean */ result->clean &= strmap_empty(&opt->priv->conflicted); -- cgit v1.2.3