Merge branch 'js/am-3-merge-recursive-direct' into jch

The merge_recursive_generic() function has been made a bit safer to
call from inside a process.  "git am -3" was taught to make a direct
call to the function when falling back to three-way merge.

Being able to make a direct call would be good in general, but as a
performance thing, the change needs to be backed up by numbers.

I haven't gone through the "gently" change with fine toothed comb;
I can see that the change avoids calling die(), but I haven't made
sure that the program states (e.g. what's in the in-core index) are
adjusted sensibly when it returns to the caller instead of dying,
or the codepaths that used to die() are free of resource leaks.
The original code certainly did not care the program states at the
point of dying exactly because it knew it is going to exit, but now
they have to care, and they need to be audited.

* js/am-3-merge-recursive-direct:
  am: make a direct call to merge_recursive
  merge_recursive_options: introduce the "gently" flag

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Author: gitster

builtin/am.c

@@ -1588,25 +1588,25 @@ static int run_fallback_merge_recursive(const struct am_state *state,
 					unsigned char *our_tree,
 					unsigned char *his_tree)
 {
-	struct child_process cp = CHILD_PROCESS_INIT;
+	const unsigned char *bases[1] = {orig_tree};
+	struct merge_options o;
+	struct commit *result;
+	char *his_tree_name;
 	int status;
 
-	cp.git_cmd = 1;
+	init_merge_options(&o);
+
+	o.gently = 1;
+	o.branch1 = "HEAD";
+	his_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
+	o.branch2 = his_tree_name;
 
-	argv_array_pushf(&cp.env_array, "GITHEAD_%s=%.*s",
-			 sha1_to_hex(his_tree), linelen(state->msg), state->msg);
 	if (state->quiet)
-		argv_array_push(&cp.env_array, "GIT_MERGE_VERBOSITY=0");
+		o.verbosity = 0;
 
-	argv_array_push(&cp.args, "merge-recursive");
-	argv_array_push(&cp.args, sha1_to_hex(orig_tree));
-	argv_array_push(&cp.args, "--");
-	argv_array_push(&cp.args, sha1_to_hex(our_tree));
-	argv_array_push(&cp.args, sha1_to_hex(his_tree));
+	status = merge_recursive_generic(&o, our_tree, his_tree, 1, bases, &result);
+	free(his_tree_name);
 
-	status = run_command(&cp) ? (-1) : 0;
-	discard_cache();
-	read_cache();
 	return status;
 }
 

merge-recursive.c

@@ -266,8 +266,12 @@ struct tree *write_tree_from_memory(struct merge_options *o)
 		active_cache_tree = cache_tree();
 
 	if (!cache_tree_fully_valid(active_cache_tree) &&
-	    cache_tree_update(&the_index, 0) < 0)
-		die(_("error building trees"));
+	    cache_tree_update(&the_index, 0) < 0) {
+		if (!o->gently)
+			die(_("error building trees"));
+		error(_("error building trees"));
+		return NULL;
+	}
 
 	result = lookup_tree(active_cache_tree->sha1);
 
@@ -714,6 +718,8 @@ static int make_room_for_path(struct merge_options *o, const char *path)
 			error(msg, path, _(": perhaps a D/F conflict?"));
 			return -1;
 		}
+		if (o->gently)
+			return error(msg, path, "");
 		die(msg, path, "");
 	}
 
@@ -1342,8 +1348,11 @@ static int process_renames(struct merge_options *o,
 			const char *ren2_src = ren2->pair->one->path;
 			const char *ren2_dst = ren2->pair->two->path;
 			enum rename_type rename_type;
-			if (strcmp(ren1_src, ren2_src) != 0)
+			if (strcmp(ren1_src, ren2_src) != 0) {
+				if (o->gently)
+					return error("ren1_src != ren2_src");
 				die("ren1_src != ren2_src");
+			}
 			ren2->dst_entry->processed = 1;
 			ren2->processed = 1;
 			if (strcmp(ren1_dst, ren2_dst) != 0) {
@@ -1376,8 +1385,11 @@ static int process_renames(struct merge_options *o,
 			char *ren2_dst;
 			ren2 = lookup->util;
 			ren2_dst = ren2->pair->two->path;
-			if (strcmp(ren1_dst, ren2_dst) != 0)
+			if (strcmp(ren1_dst, ren2_dst) != 0) {
+				if (o->gently)
+					return error("ren1_dst != ren2_dst");
 				die("ren1_dst != ren2_dst");
+			}
 
 			clean_merge = 0;
 			ren2->processed = 1;
@@ -1824,6 +1836,13 @@ int merge_trees(struct merge_options *o,
 	code = git_merge_trees(o->call_depth, common, head, merge);
 
 	if (code != 0) {
+		if (o->gently) {
+			error(_("merging of trees %s and %s failed"),
+			    oid_to_hex(&head->object.oid),
+			    oid_to_hex(&merge->object.oid));
+			return -128;
+		}
+
 		if (show(o, 4) || o->call_depth)
 			die(_("merging of trees %s and %s failed"),
 			    oid_to_hex(&head->object.oid),
@@ -1870,8 +1889,8 @@ int merge_trees(struct merge_options *o,
 	else
 		clean = 1;
 
-	if (o->call_depth)
-		*result = write_tree_from_memory(o);
+	if (o->call_depth && !(*result = write_tree_from_memory(o)))
+		return -1;
 
 	return clean;
 }
@@ -1946,14 +1965,18 @@ int merge_recursive(struct merge_options *o,
 		saved_b2 = o->branch2;
 		o->branch1 = "Temporary merge branch 1";
 		o->branch2 = "Temporary merge branch 2";
-		merge_recursive(o, merged_common_ancestors, iter->item,
-				NULL, &merged_common_ancestors);
+		if (merge_recursive(o, merged_common_ancestors, iter->item,
+				NULL, &merged_common_ancestors) < 0)
+			return -1;
 		o->branch1 = saved_b1;
 		o->branch2 = saved_b2;
 		o->call_depth--;
 
-		if (!merged_common_ancestors)
+		if (!merged_common_ancestors) {
+			if (o->gently)
+				return error(_("merge returned no commit"));
 			die(_("merge returned no commit"));
+		}
 	}
 
 	discard_cache();
@@ -1963,6 +1986,8 @@ int merge_recursive(struct merge_options *o,
 	o->ancestor = "merged common ancestors";
 	clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree,
 			    &mrtree);
+	if (clean < 0)
+		return clean;
 
 	if (o->call_depth) {
 		*result = make_virtual_commit(mrtree, "merged tree");
@@ -2019,6 +2044,9 @@ int merge_recursive_generic(struct merge_options *o,
 	hold_locked_index(lock, 1);
 	clean = merge_recursive(o, head_commit, next_commit, ca,
 			result);
+	if (clean < 0)
+		return clean;
+
 	if (active_cache_changed &&
 	    write_locked_index(&the_index, lock, COMMIT_LOCK))
 		return error(_("Unable to write index."));

merge-recursive.h

@@ -15,6 +15,7 @@ struct merge_options {
 	const char *subtree_shift;
 	unsigned buffer_output : 1;
 	unsigned renormalize : 1;
+	unsigned gently : 1;
 	long xdl_opts;
 	int verbosity;
 	int detect_rename;