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 <newren@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Jun 18, 2022 at 00:20 UTC
6debb7527b0e2f791256a216394503899b0488ee
2 files changed
+81
-44
merge-ort.c
+80
-43
@@ -349,13 +349,15 @@ struct merge_options_internal {
349
struct mem_pool pool;
350
351
/*
352
- * output: special messages and conflict notices for various paths
352
+ * conflicts: logical conflicts and messages stored by _primary_ path
353
*
354
* This is a map of pathnames (a subset of the keys in "paths" above)
355
- * to strbufs. It gathers various warning/conflict/notice messages
356
- * for later processing.
355
+ * to struct string_list, with each item's `util` containing a
356
+ * `struct logical_conflict_info`. Note, though, that for each path,
357
+ * it only stores the logical conflicts for which that path is the
358
+ * primary path; the path might be part of additional conflicts.
359
*/
358
- struct strmap output;
360
+ struct strmap conflicts;
361
362
/*
363
* renames: various data relating to rename detection
@@ -567,20 +569,20 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,
569
struct strmap_entry *e;
570
571
/* Release and free each strbuf found in output */
570
- strmap_for_each_entry(&opti->output, &iter, e) {
571
- struct strbuf *sb = e->value;
572
- strbuf_release(sb);
572
+ strmap_for_each_entry(&opti->conflicts, &iter, e) {
573
+ struct string_list *list = e->value;
574
/*
574
- * While strictly speaking we don't need to free(sb)
575
- * here because we could pass free_values=1 when
576
- * calling strmap_clear() on opti->output, that would
577
- * require strmap_clear to do another
578
- * strmap_for_each_entry() loop, so we just free it
579
- * while we're iterating anyway.
575
+ * While strictly speaking we don't need to
576
+ * free(conflicts) here because we could pass
577
+ * free_values=1 when calling strmap_clear() on
578
+ * opti->conflicts, that would require strmap_clear
579
+ * to do another strmap_for_each_entry() loop, so we
580
+ * just free it while we're iterating anyway.
581
*/
581
- free(sb);
582
+ string_list_clear(list, 1);
583
+ free(list);
584
}
583
- strmap_clear(&opti->output, 0);
585
+ strmap_clear(&opti->conflicts, 0);
586
}
587
588
mem_pool_discard(&opti->pool, 0);
@@ -634,7 +636,9 @@ static void path_msg(struct merge_options *opt,
636
const char *fmt, ...)
637
{
638
va_list ap;
637
- struct strbuf *sb, *dest;
639
+ struct string_list *path_conflicts;
640
+ struct strbuf buf = STRBUF_INIT;
641
+ struct strbuf *dest;
642
struct strbuf tmp = STRBUF_INIT;
643
644
if (opt->record_conflict_msgs_as_headers && omittable_hint)
@@ -642,14 +646,16 @@ static void path_msg(struct merge_options *opt,
646
if (opt->priv->call_depth && opt->verbosity < 5)
647
return; /* Ignore messages from inner merges */
648
645
- sb = strmap_get(&opt->priv->output, path);
646
- if (!sb) {
647
- sb = xmalloc(sizeof(*sb));
648
- strbuf_init(sb, 0);
649
- strmap_put(&opt->priv->output, path, sb);
649
+ /* Ensure path_conflicts (ptr to array of logical_conflict) allocated */
650
+ path_conflicts = strmap_get(&opt->priv->conflicts, path);
651
+ if (!path_conflicts) {
652
+ path_conflicts = xmalloc(sizeof(*path_conflicts));
653
+ string_list_init_dup(path_conflicts);
654
+ strmap_put(&opt->priv->conflicts, path, path_conflicts);
655
}
656
652
- dest = (opt->record_conflict_msgs_as_headers ? &tmp : sb);
657
+ /* Handle message and its format, in normal case */
658
+ dest = (opt->record_conflict_msgs_as_headers ? &tmp : &buf);
659
660
va_start(ap, fmt);
661
if (opt->priv->call_depth) {
@@ -660,32 +666,31 @@ static void path_msg(struct merge_options *opt,
666
strbuf_vaddf(dest, fmt, ap);
667
va_end(ap);
668
669
+ /* Handle specialized formatting of message under --remerge-diff */
670
if (opt->record_conflict_msgs_as_headers) {
671
int i_sb = 0, i_tmp = 0;
672
673
/* Start with the specified prefix */
674
if (opt->msg_header_prefix)
668
- strbuf_addf(sb, "%s ", opt->msg_header_prefix);
675
+ strbuf_addf(&buf, "%s ", opt->msg_header_prefix);
676
677
/* Copy tmp to sb, adding spaces after newlines */
671
- strbuf_grow(sb, sb->len + 2*tmp.len); /* more than sufficient */
678
+ strbuf_grow(&buf, buf.len + 2*tmp.len); /* more than sufficient */
679
for (; i_tmp < tmp.len; i_tmp++, i_sb++) {
680
/* Copy next character from tmp to sb */
674
- sb->buf[sb->len + i_sb] = tmp.buf[i_tmp];
681
+ buf.buf[buf.len + i_sb] = tmp.buf[i_tmp];
682
683
/* If we copied a newline, add a space */
684
if (tmp.buf[i_tmp] == '\n')
678
- sb->buf[++i_sb] = ' ';
685
+ buf.buf[++i_sb] = ' ';
686
}
687
/* Update length and ensure it's NUL-terminated */
681
- sb->len += i_sb;
682
- sb->buf[sb->len] = '\0';
688
+ buf.len += i_sb;
689
+ buf.buf[buf.len] = '\0';
690
691
strbuf_release(&tmp);
692
}
686
-
687
- /* Add final newline character to sb */
688
- strbuf_addch(sb, '\n');
693
+ string_list_append_nodup(path_conflicts, strbuf_detach(&buf, NULL));
694
}
695
696
static struct diff_filespec *pool_alloc_filespec(struct mem_pool *pool,
@@ -4257,7 +4262,6 @@ void merge_display_update_messages(struct merge_options *opt,
4262
struct hashmap_iter iter;
4263
struct strmap_entry *e;
4264
struct string_list olist = STRING_LIST_INIT_NODUP;
4260
- int i;
4265
4266
if (opt->record_conflict_msgs_as_headers)
4267
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,
4269
trace2_region_enter("merge", "display messages", opt->repo);
4270
4271
/* Hack to pre-allocate olist to the desired size */
4268
- ALLOC_GROW(olist.items, strmap_get_size(&opti->output),
4272
+ ALLOC_GROW(olist.items, strmap_get_size(&opti->conflicts),
4273
olist.alloc);
4274
4275
/* Put every entry from output into olist, then sort */
4272
- strmap_for_each_entry(&opti->output, &iter, e) {
4276
+ strmap_for_each_entry(&opti->conflicts, &iter, e) {
4277
string_list_append(&olist, e->key)->util = e->value;
4278
}
4279
string_list_sort(&olist);
4280
4281
/* Iterate over the items, printing them */
4278
- for (i = 0; i < olist.nr; ++i) {
4279
- struct strbuf *sb = olist.items[i].util;
4280
-
4281
- printf("%s", sb->buf);
4282
+ for (int path_nr = 0; path_nr < olist.nr; ++path_nr) {
4283
+ struct string_list *conflicts = olist.items[path_nr].util;
4284
+ for (int i = 0; i < conflicts->nr; i++)
4285
+ puts(conflicts->items[i].string);
4286
}
4287
string_list_clear(&olist, 0);
4288
@@ -4367,6 +4371,8 @@ void merge_finalize(struct merge_options *opt,
4371
struct merge_result *result)
4372
{
4373
struct merge_options_internal *opti = result->priv;
4374
+ struct hashmap_iter iter;
4375
+ struct strmap_entry *e;
4376
4377
if (opt->renormalize)
4378
git_attr_set_direction(GIT_ATTR_CHECKIN);
@@ -4374,6 +4380,15 @@ void merge_finalize(struct merge_options *opt,
4380
4381
clear_or_reinit_internal_opts(opti, 0);
4382
FREE_AND_NULL(opti);
4383
+
4384
+ /* Release and free each strbuf found in path_messages */
4385
+ strmap_for_each_entry(result->path_messages, &iter, e) {
4386
+ struct strbuf *buf = e->value;
4387
+
4388
+ strbuf_release(buf);
4389
+ }
4390
+ strmap_clear(result->path_messages, 1);
4391
+ FREE_AND_NULL(result->path_messages);
4392
}
4393
4394
/*** Function Grouping: helper functions for merge_incore_*() ***/
@@ -4532,11 +4547,11 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
4547
strmap_init_with_options(&opt->priv->conflicted, pool, 0);
4548
4549
/*
4535
- * keys & strbufs in output will sometimes need to outlive "paths",
4536
- * so it will have a copy of relevant keys. It's probably a small
4537
- * subset of the overall paths that have special output.
4550
+ * keys & string_lists in conflicts will sometimes need to outlive
4551
+ * "paths", so it will have a copy of relevant keys. It's probably
4552
+ * a small subset of the overall paths that have special output.
4553
*/
4539
- strmap_init(&opt->priv->output);
4554
+ strmap_init(&opt->priv->conflicts);
4555
4556
trace2_region_leave("merge", "allocate/init", opt->repo);
4557
}
@@ -4597,6 +4612,8 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,
4612
struct merge_result *result)
4613
{
4614
struct object_id working_tree_oid;
4615
+ struct hashmap_iter iter;
4616
+ struct strmap_entry *e;
4617
4618
if (opt->subtree_shift) {
4619
side2 = shift_tree_object(opt->repo, side1, side2,
@@ -4637,7 +4654,27 @@ redo:
4654
trace2_region_leave("merge", "process_entries", opt->repo);
4655
4656
/* Set return values */
4640
- result->path_messages = &opt->priv->output;
4657
+ result->path_messages = xcalloc(1, sizeof(*result->path_messages));
4658
+ strmap_init_with_options(result->path_messages, NULL, 0);
4659
+ strmap_for_each_entry(&opt->priv->conflicts, &iter, e) {
4660
+ const char *path = e->key;
4661
+ struct strbuf *buf = strmap_get(result->path_messages, path);
4662
+ struct string_list *conflicts = e->value;
4663
+
4664
+ if (!buf) {
4665
+ buf = xcalloc(1, sizeof(*buf));
4666
+ strbuf_init(buf, 0);
4667
+ strmap_put(result->path_messages, path, buf);
4668
+ }
4669
+
4670
+ for (int i = 0; i < conflicts->nr; i++) {
4671
+ if (buf->len)
4672
+ strbuf_addch(buf, '\n');
4673
+ strbuf_addstr(buf, conflicts->items[i].string);
4674
+ strbuf_trim_trailing_newline(buf);
4675
+ }
4676
+ }
4677
+
4678
result->tree = parse_tree_indirect(&working_tree_oid);
4679
/* existence of conflicted entries implies unclean */
4680
result->clean &= strmap_empty(&opt->priv->conflicted);
merge-ort.h
+1
-1
@@ -28,7 +28,7 @@ struct merge_result {
28
/*
29
* Special messages and conflict notices for various paths
30
*
31
- * This is a map of pathnames to strbufs. It contains various
31
+ * This is a map of pathnames to strbufs. It contains various
32
* warning/conflict/notice messages (possibly multiple per path)
33
* that callers may want to use.
34
*/