[7/7] merge-ort: add modify/delete handling and delayed output processing

Commit Message

Elijah Newren Dec. 3, 2020, 3:59 p.m. UTC

From: Elijah Newren <[e-mail]>

The focus here is on adding a path_msg() which will queue up
warning/conflict/notice messages about the merge for later processing,
storing these in a pathname -> strbuf map.  It might seem like a big
change, but it really just is:

  * declaration of necessary map with some comments
  * initialization and recording of data
  * a bunch of code to iterate over the map at print/free time
  * at least one caller in order to avoid an error about having an
    unused function (which we provide in the form of implementing
    modify/delete conflict handling).

At this stage, it is probably not clear why I am opting for delayed
output processing.  There are multiple reasons:

  1. Merges are supposed to abort if they would overwrite dirty changes
     in the working tree.  We cannot correctly determine whether changes
     would be overwritten until both rename detection has occurred and
     full processing of entries with the renames has finalized.
     Warning/conflict/notice messages come up at intermediate codepaths
     along the way, so unless we want spurious conflict/warning messages
     being printed when the merge will be aborted anyway, we need to
     save these messages and only print them when relevant.

  2. There can be multiple messages for a single path, and we want all
     messages for a give path to appear together instead of having them
     grouped by conflict/warning type.  This was a problem already with
     merge-recursive.c but became even more important due to the
     splitting apart of conflict types as discussed in the commit
     message for 1f3c9ba707 ("t6425: be more flexible with rename/delete
     conflict messages", 2020-08-10)

  3. Some callers might want to avoid showing the output in certain
     cases, such as if the end result is a clean merge.  Rebases have
     typically done this.

  4. Some callers might not want the output to go to stdout or even
     stderr, but might want to do something else with it entirely.
     For example, a --remerge-diff option to `git show` or `git log
     -p` that remerges on the fly and diffs merge commits against the
     remerged version would benefit from stdout/stderr not being
     written to in the standard form.

Signed-off-by: Elijah Newren <[e-mail]>
---
 merge-ort.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 98 insertions(+), 2 deletions(-)

Patch

diff --git a/merge-ort.c b/merge-ort.c
index e7220cbbb4..64468f0706 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -87,6 +87,15 @@  struct merge_options_internal {
 	 */
 	struct string_list paths_to_free;
 
+	/*
+	 * output: special messages and conflict notices for various paths
+	 *
+	 * 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.
+	 */
+	struct strmap output;
+
 	/*
 	 * current_dir_name: temporary var used in collect_merge_info_callback()
 	 *
@@ -247,6 +256,27 @@  static void clear_internal_opts(struct merge_options_internal *opti,
 	opti->paths_to_free.strdup_strings = 1;
 	string_list_clear(&opti->paths_to_free, 0);
 	opti->paths_to_free.strdup_strings = 0;
+
+	if (!reinitialize) {
+		struct hashmap_iter iter;
+		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);
+			/*
+			 * 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.
+			 */
+			free(sb);
+		}
+		strmap_clear(&opti->output, 0);
+	}
 }
 
 static int err(struct merge_options *opt, const char *err, ...)
@@ -265,6 +295,27 @@  static int err(struct merge_options *opt, const char *err, ...)
 	return -1;
 }
 
+__attribute__((format (printf, 4, 5)))
+static void path_msg(struct merge_options *opt,
+		     const char *path,
+		     int omittable_hint, /* skippable under --remerge-diff */
+		     const char *fmt, ...)
+{
+	va_list ap;
+	struct strbuf *sb = strmap_get(&opt->priv->output, path);
+	if (!sb) {
+		sb = xmalloc(sizeof(*sb));
+		strbuf_init(sb, 0);
+		strmap_put(&opt->priv->output, path, sb);
+	}
+
+	va_start(ap, fmt);
+	strbuf_vaddf(sb, fmt, ap);
+	va_end(ap);
+
+	strbuf_addch(sb, '\n');
+}
+
 /*** Function Grouping: functions related to collect_merge_info() ***/
 
 static void setup_path_info(struct merge_options *opt,
@@ -935,7 +986,23 @@  static void process_entry(struct merge_options *opt,
 		(void)handle_content_merge;
 	} else if (ci->filemask == 3 || ci->filemask == 5) {
 		/* Modify/delete */
-		die("Not yet implemented.");
+		const char *modify_branch, *delete_branch;
+		int side = (ci->filemask == 5) ? 2 : 1;
+		int index = opt->priv->call_depth ? 0 : side;
+
+		ci->merged.result.mode = ci->stages[index].mode;
+		oidcpy(&ci->merged.result.oid, &ci->stages[index].oid);
+		ci->merged.clean = 0;
+
+		modify_branch = (side == 1) ? opt->branch1 : opt->branch2;
+		delete_branch = (side == 1) ? opt->branch2 : opt->branch1;
+
+		path_msg(opt, path, 0,
+			 _("CONFLICT (modify/delete): %s deleted in %s "
+			   "and modified in %s.  Version %s of %s left "
+			   "in tree."),
+			 path, delete_branch, modify_branch,
+			 modify_branch, path);
 	} else if (ci->filemask == 2 || ci->filemask == 4) {
 		/* Added on one side */
 		int side = (ci->filemask == 4) ? 2 : 1;
@@ -1203,7 +1270,29 @@  void merge_switch_to_result(struct merge_options *opt,
 	}
 
 	if (display_update_msgs) {
-		/* TODO: print out CONFLICT and other informational messages. */
+		struct merge_options_internal *opti = result->priv;
+		struct hashmap_iter iter;
+		struct strmap_entry *e;
+		struct string_list olist = STRING_LIST_INIT_NODUP;
+		int i;
+
+		/* Hack to pre-allocate olist to the desired size */
+		ALLOC_GROW(olist.items, strmap_get_size(&opti->output),
+			   olist.alloc);
+
+		/* Put every entry from output into olist, then sort */
+		strmap_for_each_entry(&opti->output, &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);
+		}
+		string_list_clear(&olist, 0);
 	}
 
 	merge_finalize(opt, result);
@@ -1270,6 +1359,13 @@  static void merge_start(struct merge_options *opt, struct merge_result *result)
 	strmap_init_with_options(&opt->priv->paths, NULL, 0);
 	strmap_init_with_options(&opt->priv->conflicted, NULL, 0);
 	string_list_init(&opt->priv->paths_to_free, 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.
+	 */
+	strmap_init(&opt->priv->output);
 }
 
 /*** Function Grouping: merge_incore_*() and their internal variants ***/