1 files changed, 244 insertions, 198 deletions

diff --git a/merge-recursive.c b/merge-recursive.c
index 42be7c9960..3355d50e8a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -61,11 +61,6 @@ static int path_hashmap_cmp(const void *cmp_data,
 		return strcmp(a->path, key ? key : b->path);
 }
 
-static unsigned int path_hash(const char *path)
-{
-	return ignore_case ? strihash(path) : strhash(path);
-}
-
 /*
  * For dir_rename_entry, directory names are stored as a full path from the
  * toplevel of the repository and do not include a trailing '/'.  Also:
@@ -121,7 +116,7 @@ static void dir_rename_entry_init(struct dir_rename_entry *entry,
 	entry->dir = directory;
 	entry->non_unique_new_dir = 0;
 	strbuf_init(&entry->new_dir, 0);
-	string_list_init(&entry->possible_new_dirs, 0);
+	string_list_init_nodup(&entry->possible_new_dirs);
 }
 
 struct collision_entry {
@@ -167,6 +162,7 @@ static void flush_output(struct merge_options *opt)
 	}
 }
 
+__attribute__((format (printf, 2, 3)))
 static int err(struct merge_options *opt, const char *err, ...)
 {
 	va_list params;
@@ -224,17 +220,6 @@ static struct commit *make_virtual_commit(struct repository *repo,
 	return commit;
 }
 
-/*
- * Since we use get_tree_entry(), which does not put the read object into
- * the object pool, we cannot rely on a == b.
- */
-static int oid_eq(const struct object_id *a, const struct object_id *b)
-{
-	if (!a && !b)
-		return 2;
-	return a && b && oideq(a, b);
-}
-
 enum rename_type {
 	RENAME_NORMAL = 0,
 	RENAME_VIA_DIR,
@@ -314,7 +299,7 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type,
 		return;
 	}
 
-	ci = xcalloc(1, sizeof(struct rename_conflict_info));
+	CALLOC_ARRAY(ci, 1);
 	ci->rename_type = rename_type;
 	ci->ren1 = ren1;
 	ci->ren2 = ren2;
@@ -464,7 +449,7 @@ static void unpack_trees_finish(struct merge_options *opt)
 
 static int save_files_dirs(const struct object_id *oid,
 			   struct strbuf *base, const char *path,
-			   unsigned int mode, int stage, void *context)
+			   unsigned int mode, void *context)
 {
 	struct path_hashmap_entry *entry;
 	int baselen = base->len;
@@ -473,7 +458,7 @@ static int save_files_dirs(const struct object_id *oid,
 	strbuf_addstr(base, path);
 
 	FLEX_ALLOC_MEM(entry, path, base->buf, base->len);
-	hashmap_entry_init(&entry->e, path_hash(entry->path));
+	hashmap_entry_init(&entry->e, fspathhash(entry->path));
 	hashmap_add(&opt->priv->current_file_dir_set, &entry->e);
 
 	strbuf_setlen(base, baselen);
@@ -484,8 +469,8 @@ static void get_files_dirs(struct merge_options *opt, struct tree *tree)
 {
 	struct pathspec match_all;
 	memset(&match_all, 0, sizeof(match_all));
-	read_tree_recursive(opt->repo, tree, "", 0, 0,
-			    &match_all, save_files_dirs, opt);
+	read_tree(opt->repo, tree,
+		  &match_all, save_files_dirs, opt);
 }
 
 static int get_tree_entry_if_blob(struct repository *r,
@@ -497,7 +482,7 @@ static int get_tree_entry_if_blob(struct repository *r,
 
 	ret = get_tree_entry(r, tree, path, &dfs->oid, &dfs->mode);
 	if (S_ISDIR(dfs->mode)) {
-		oidcpy(&dfs->oid, &null_oid);
+		oidcpy(&dfs->oid, null_oid());
 		dfs->mode = 0;
 	}
 	return ret;
@@ -533,6 +518,8 @@ static struct string_list *get_unmerged(struct index_state *istate)
 
 	unmerged->strdup_strings = 1;
 
+	/* TODO: audit for interaction with sparse-index. */
+	ensure_full_index(istate);
 	for (i = 0; i < istate->cache_nr; i++) {
 		struct string_list_item *item;
 		struct stage_data *e;
@@ -745,14 +732,14 @@ static char *unique_path(struct merge_options *opt,
 
 	base_len = newpath.len;
 	while (hashmap_get_from_hash(&opt->priv->current_file_dir_set,
-				     path_hash(newpath.buf), newpath.buf) ||
+				     fspathhash(newpath.buf), newpath.buf) ||
 	       (!opt->priv->call_depth && file_exists(newpath.buf))) {
 		strbuf_setlen(&newpath, base_len);
 		strbuf_addf(&newpath, "_%d", suffix++);
 	}
 
 	FLEX_ALLOC_MEM(entry, path, newpath.buf, newpath.len);
-	hashmap_entry_init(&entry->e, path_hash(entry->path));
+	hashmap_entry_init(&entry->e, fspathhash(entry->path));
 	hashmap_add(&opt->priv->current_file_dir_set, &entry->e);
 	return strbuf_detach(&newpath, NULL);
 }
@@ -805,7 +792,7 @@ static int was_tracked_and_matches(struct merge_options *opt, const char *path,
 
 	/* See if the file we were tracking before matches */
 	ce = opt->priv->orig_index.cache[pos];
-	return (oid_eq(&ce->oid, &blob->oid) && ce->ce_mode == blob->mode);
+	return (oideq(&ce->oid, &blob->oid) && ce->ce_mode == blob->mode);
 }
 
 /*
@@ -969,7 +956,7 @@ static int update_file_flags(struct merge_options *opt,
 		if (S_ISREG(contents->mode)) {
 			struct strbuf strbuf = STRBUF_INIT;
 			if (convert_to_working_tree(opt->repo->index,
-						    path, buf, size, &strbuf)) {
+						    path, buf, size, &strbuf, NULL)) {
 				free(buf);
 				size = strbuf.len;
 				buf = strbuf_detach(&strbuf, NULL);
@@ -1009,10 +996,13 @@ static int update_file_flags(struct merge_options *opt,
 		free(buf);
 	}
 update_index:
-	if (!ret && update_cache)
-		if (add_cacheinfo(opt, contents, path, 0, update_wd,
+	if (!ret && update_cache) {
+		int refresh = (!opt->priv->call_depth &&
+			       contents->mode != S_IFGITLINK);
+		if (add_cacheinfo(opt, contents, path, 0, refresh,
 				  ADD_CACHE_OK_TO_ADD))
 			return -1;
+	}
 	return ret;
 }
 
@@ -1083,6 +1073,11 @@ static int merge_3way(struct merge_options *opt,
 	read_mmblob(&src1, &a->oid);
 	read_mmblob(&src2, &b->oid);
 
+	/*
+	 * FIXME: Using a->path for normalization rules in ll_merge could be
+	 * wrong if we renamed from a->path to b->path.  We should use the
+	 * target path for where the file will be written.
+	 */
 	merge_status = ll_merge(result_buf, a->path, &orig, base,
 				&src1, name1, &src2, name2,
 				opt->repo->index, &ll_opts);
@@ -1162,6 +1157,8 @@ static void print_commit(struct commit *commit)
 	struct strbuf sb = STRBUF_INIT;
 	struct pretty_print_context ctx = {0};
 	ctx.date_mode.type = DATE_NORMAL;
+	/* FIXME: Merge this with output_commit_title() */
+	assert(!merge_remote_util(commit));
 	format_commit_message(commit, " %h: %m %s", &sb, &ctx);
 	fprintf(stderr, "%s\n", sb.buf);
 	strbuf_release(&sb);
@@ -1185,6 +1182,11 @@ static int merge_submodule(struct merge_options *opt,
 	int search = !opt->priv->call_depth;
 
 	/* store a in result in case we fail */
+	/* FIXME: This is the WRONG resolution for the recursive case when
+	 * we need to be careful to avoid accidentally matching either side.
+	 * Should probably use o instead there, much like we do for merging
+	 * binaries.
+	 */
 	oidcpy(result, a);
 
 	/* we can not handle deletion conflicts */
@@ -1309,6 +1311,13 @@ static int merge_mode_and_contents(struct merge_options *opt,
 
 	if ((S_IFMT & a->mode) != (S_IFMT & b->mode)) {
 		result->clean = 0;
+		/*
+		 * FIXME: This is a bad resolution for recursive case; for
+		 * the recursive case we want something that is unlikely to
+		 * accidentally match either side.  Also, while it makes
+		 * sense to prefer regular files over symlinks, it doesn't
+		 * make sense to prefer regular files over submodules.
+		 */
 		if (S_ISREG(a->mode)) {
 			result->blob.mode = a->mode;
 			oidcpy(&result->blob.oid, &a->oid);
@@ -1317,7 +1326,7 @@ static int merge_mode_and_contents(struct merge_options *opt,
 			oidcpy(&result->blob.oid, &b->oid);
 		}
 	} else {
-		if (!oid_eq(&a->oid, &o->oid) && !oid_eq(&b->oid, &o->oid))
+		if (!oideq(&a->oid, &o->oid) && !oideq(&b->oid, &o->oid))
 			result->merge = 1;
 
 		/*
@@ -1333,9 +1342,9 @@ static int merge_mode_and_contents(struct merge_options *opt,
 			}
 		}
 
-		if (oid_eq(&a->oid, &b->oid) || oid_eq(&a->oid, &o->oid))
+		if (oideq(&a->oid, &b->oid) || oideq(&a->oid, &o->oid))
 			oidcpy(&result->blob.oid, &b->oid);
-		else if (oid_eq(&b->oid, &o->oid))
+		else if (oideq(&b->oid, &o->oid))
 			oidcpy(&result->blob.oid, &a->oid);
 		else if (S_ISREG(a->mode)) {
 			mmbuffer_t result_buf;
@@ -1357,6 +1366,7 @@ static int merge_mode_and_contents(struct merge_options *opt,
 			free(result_buf.ptr);
 			if (ret)
 				return ret;
+			/* FIXME: bug, what if modes didn't match? */
 			result->clean = (merge_status == 0);
 		} else if (S_ISGITLINK(a->mode)) {
 			result->clean = merge_submodule(opt, &result->blob.oid,
@@ -1368,7 +1378,7 @@ static int merge_mode_and_contents(struct merge_options *opt,
 			switch (opt->recursive_variant) {
 			case MERGE_VARIANT_NORMAL:
 				oidcpy(&result->blob.oid, &a->oid);
-				if (!oid_eq(&a->oid, &b->oid))
+				if (!oideq(&a->oid, &b->oid))
 					result->clean = 0;
 				break;
 			case MERGE_VARIANT_OURS:
@@ -1568,35 +1578,6 @@ static int handle_file_collision(struct merge_options *opt,
 					     b, a);
 	}
 
-	/*
-	 * In the recursive case, we just opt to undo renames
-	 */
-	if (opt->priv->call_depth && (prev_path1 || prev_path2)) {
-		/* Put first file (a->oid, a->mode) in its original spot */
-		if (prev_path1) {
-			if (update_file(opt, 1, a, prev_path1))
-				return -1;
-		} else {
-			if (update_file(opt, 1, a, collide_path))
-				return -1;
-		}
-
-		/* Put second file (b->oid, b->mode) in its original spot */
-		if (prev_path2) {
-			if (update_file(opt, 1, b, prev_path2))
-				return -1;
-		} else {
-			if (update_file(opt, 1, b, collide_path))
-				return -1;
-		}
-
-		/* Don't leave something at collision path if unrenaming both */
-		if (prev_path1 && prev_path2)
-			remove_file(opt, 1, collide_path, 0);
-
-		return 0;
-	}
-
 	/* Remove rename sources if rename/add or rename/rename(2to1) */
 	if (prev_path1)
 		remove_file(opt, 1, prev_path1,
@@ -1641,7 +1622,7 @@ static int handle_file_collision(struct merge_options *opt,
 
 	/* Store things in diff_filespecs for functions that need it */
 	null.path = (char *)collide_path;
-	oidcpy(&null.oid, &null_oid);
+	oidcpy(&null.oid, null_oid());
 	null.mode = 0;
 
 	if (merge_mode_and_contents(opt, &null, a, b, collide_path,
@@ -1723,6 +1704,14 @@ static char *find_path_for_conflict(struct merge_options *opt,
 	return new_path;
 }
 
+/*
+ * Toggle the stage number between "ours" and "theirs" (2 and 3).
+ */
+static inline int flip_stage(int stage)
+{
+	return (2 + 3) - stage;
+}
+
 static int handle_rename_rename_1to2(struct merge_options *opt,
 				     struct rename_conflict_info *ci)
 {
@@ -1749,85 +1738,68 @@ static int handle_rename_rename_1to2(struct merge_options *opt,
 		return -1;
 	free(path_desc);
 
-	if (opt->priv->call_depth) {
-		/*
-		 * FIXME: For rename/add-source conflicts (if we could detect
-		 * such), this is wrong.  We should instead find a unique
-		 * pathname and then either rename the add-source file to that
-		 * unique path, or use that unique path instead of src here.
-		 */
-		if (update_file(opt, 0, &mfi.blob, o->path))
+	if (opt->priv->call_depth)
+		remove_file_from_index(opt->repo->index, o->path);
+
+	/*
+	 * For each destination path, we need to see if there is a
+	 * rename/add collision.  If not, we can write the file out
+	 * to the specified location.
+	 */
+	add = &ci->ren1->dst_entry->stages[flip_stage(2)];
+	if (is_valid(add)) {
+		add->path = mfi.blob.path = a->path;
+		if (handle_file_collision(opt, a->path,
+					  NULL, NULL,
+					  ci->ren1->branch,
+					  ci->ren2->branch,
+					  &mfi.blob, add) < 0)
 			return -1;
+	} else {
+		char *new_path = find_path_for_conflict(opt, a->path,
+							ci->ren1->branch,
+							ci->ren2->branch);
+		if (update_file(opt, 0, &mfi.blob,
+				new_path ? new_path : a->path))
+			return -1;
+		free(new_path);
+		if (!opt->priv->call_depth &&
+		    update_stages(opt, a->path, NULL, a, NULL))
+			return -1;
+	}
 
+	if (!mfi.clean && mfi.blob.mode == a->mode &&
+	    oideq(&mfi.blob.oid, &a->oid)) {
 		/*
-		 * Above, we put the merged content at the merge-base's
-		 * path.  Now we usually need to delete both a->path and
-		 * b->path.  However, the rename on each side of the merge
-		 * could also be involved in a rename/add conflict.  In
-		 * such cases, we should keep the added file around,
-		 * resolving the conflict at that path in its favor.
+		 * Getting here means we were attempting to merge a binary
+		 * blob.  Since we can't merge binaries, the merge algorithm
+		 * just takes one side.  But we don't want to copy the
+		 * contents of one side to both paths; we'd rather use the
+		 * original content at the given path for each path.
 		 */
-		add = &ci->ren1->dst_entry->stages[2 ^ 1];
-		if (is_valid(add)) {
-			if (update_file(opt, 0, add, a->path))
-				return -1;
-		}
-		else
-			remove_file_from_index(opt->repo->index, a->path);
-		add = &ci->ren2->dst_entry->stages[3 ^ 1];
-		if (is_valid(add)) {
-			if (update_file(opt, 0, add, b->path))
-				return -1;
-		}
-		else
-			remove_file_from_index(opt->repo->index, b->path);
+		oidcpy(&mfi.blob.oid, &b->oid);
+		mfi.blob.mode = b->mode;
+	}
+	add = &ci->ren2->dst_entry->stages[flip_stage(3)];
+	if (is_valid(add)) {
+		add->path = mfi.blob.path = b->path;
+		if (handle_file_collision(opt, b->path,
+					  NULL, NULL,
+					  ci->ren1->branch,
+					  ci->ren2->branch,
+					  add, &mfi.blob) < 0)
+			return -1;
 	} else {
-		/*
-		 * For each destination path, we need to see if there is a
-		 * rename/add collision.  If not, we can write the file out
-		 * to the specified location.
-		 */
-		add = &ci->ren1->dst_entry->stages[2 ^ 1];
-		if (is_valid(add)) {
-			add->path = mfi.blob.path = a->path;
-			if (handle_file_collision(opt, a->path,
-						  NULL, NULL,
-						  ci->ren1->branch,
-						  ci->ren2->branch,
-						  &mfi.blob, add) < 0)
-				return -1;
-		} else {
-			char *new_path = find_path_for_conflict(opt, a->path,
-								ci->ren1->branch,
-								ci->ren2->branch);
-			if (update_file(opt, 0, &mfi.blob,
-					new_path ? new_path : a->path))
-				return -1;
-			free(new_path);
-			if (update_stages(opt, a->path, NULL, a, NULL))
-				return -1;
-		}
-
-		add = &ci->ren2->dst_entry->stages[3 ^ 1];
-		if (is_valid(add)) {
-			add->path = mfi.blob.path = b->path;
-			if (handle_file_collision(opt, b->path,
-						  NULL, NULL,
-						  ci->ren1->branch,
-						  ci->ren2->branch,
-						  add, &mfi.blob) < 0)
-				return -1;
-		} else {
-			char *new_path = find_path_for_conflict(opt, b->path,
-								ci->ren2->branch,
-								ci->ren1->branch);
-			if (update_file(opt, 0, &mfi.blob,
-					new_path ? new_path : b->path))
-				return -1;
-			free(new_path);
-			if (update_stages(opt, b->path, NULL, NULL, b))
-				return -1;
-		}
+		char *new_path = find_path_for_conflict(opt, b->path,
+							ci->ren2->branch,
+							ci->ren1->branch);
+		if (update_file(opt, 0, &mfi.blob,
+				new_path ? new_path : b->path))
+			return -1;
+		free(new_path);
+		if (!opt->priv->call_depth &&
+		    update_stages(opt, b->path, NULL, NULL, b))
+			return -1;
 	}
 
 	return 0;
@@ -1857,7 +1829,7 @@ static int handle_rename_rename_2to1(struct merge_options *opt,
 	path_side_1_desc = xstrfmt("version of %s from %s", path, a->path);
 	path_side_2_desc = xstrfmt("version of %s from %s", path, b->path);
 	ostage1 = ci->ren1->branch == opt->branch1 ? 3 : 2;
-	ostage2 = ostage1 ^ 1;
+	ostage2 = flip_stage(ostage1);
 	ci->ren1->src_entry->stages[ostage1].path = a->path;
 	ci->ren2->src_entry->stages[ostage2].path = b->path;
 	if (merge_mode_and_contents(opt, a, c1,
@@ -1903,7 +1875,7 @@ static struct diff_queue_struct *get_diffpairs(struct merge_options *opt,
 	 */
 	if (opts.detect_rename > DIFF_DETECT_RENAME)
 		opts.detect_rename = DIFF_DETECT_RENAME;
-	opts.rename_limit = (opt->rename_limit >= 0) ? opt->rename_limit : 1000;
+	opts.rename_limit = (opt->rename_limit >= 0) ? opt->rename_limit : 7000;
 	opts.rename_score = opt->rename_score;
 	opts.show_rename_progress = opt->show_rename_progress;
 	opts.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -1951,6 +1923,16 @@ static char *apply_dir_rename(struct dir_rename_entry *entry,
 		return NULL;
 
 	oldlen = strlen(entry->dir);
+	if (entry->new_dir.len == 0)
+		/*
+		 * If someone renamed/merged a subdirectory into the root
+		 * directory (e.g. 'some/subdir' -> ''), then we want to
+		 * avoid returning
+		 *     '' + '/filename'
+		 * as the rename; we need to make old_path + oldlen advance
+		 * past the '/' character.
+		 */
+		oldlen++;
 	newlen = entry->new_dir.len + (strlen(old_path) - oldlen) + 1;
 	strbuf_grow(&new_path, newlen);
 	strbuf_addbuf(&new_path, &entry->new_dir);
@@ -1963,8 +1945,8 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,
 				    char **old_dir, char **new_dir)
 {
 	char *end_of_old, *end_of_new;
-	int old_len, new_len;
 
+	/* Default return values: NULL, meaning no rename */
 	*old_dir = NULL;
 	*new_dir = NULL;
 
@@ -1975,43 +1957,91 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,
 	 *    "a/b/c/d" was renamed to "a/b/some/thing/else"
 	 * so, for this example, this function returns "a/b/c/d" in
 	 * *old_dir and "a/b/some/thing/else" in *new_dir.
-	 *
-	 * Also, if the basename of the file changed, we don't care.  We
-	 * want to know which portion of the directory, if any, changed.
+	 */
+
+	/*
+	 * If the basename of the file changed, we don't care.  We want
+	 * to know which portion of the directory, if any, changed.
 	 */
 	end_of_old = strrchr(old_path, '/');
 	end_of_new = strrchr(new_path, '/');
 
-	if (end_of_old == NULL || end_of_new == NULL)
+	/*
+	 * If end_of_old is NULL, old_path wasn't in a directory, so there
+	 * could not be a directory rename (our rule elsewhere that a
+	 * directory which still exists is not considered to have been
+	 * renamed means the root directory can never be renamed -- because
+	 * the root directory always exists).
+	 */
+	if (end_of_old == NULL)
+		return; /* Note: *old_dir and *new_dir are still NULL */
+
+	/*
+	 * If new_path contains no directory (end_of_new is NULL), then we
+	 * have a rename of old_path's directory to the root directory.
+	 */
+	if (end_of_new == NULL) {
+		*old_dir = xstrndup(old_path, end_of_old - old_path);
+		*new_dir = xstrdup("");
 		return;
+	}
+
+	/* Find the first non-matching character traversing backwards */
 	while (*--end_of_new == *--end_of_old &&
 	       end_of_old != old_path &&
 	       end_of_new != new_path)
 		; /* Do nothing; all in the while loop */
+
 	/*
-	 * We've found the first non-matching character in the directory
-	 * paths.  That means the current directory we were comparing
-	 * represents the rename.  Move end_of_old and end_of_new back
-	 * to the full directory name.
+	 * If both got back to the beginning of their strings, then the
+	 * directory didn't change at all, only the basename did.
 	 */
-	if (*end_of_old == '/')
-		end_of_old++;
-	if (*end_of_old != '/')
-		end_of_new++;
-	end_of_old = strchr(end_of_old, '/');
-	end_of_new = strchr(end_of_new, '/');
+	if (end_of_old == old_path && end_of_new == new_path &&
+	    *end_of_old == *end_of_new)
+		return; /* Note: *old_dir and *new_dir are still NULL */
 
 	/*
-	 * It may have been the case that old_path and new_path were the same
-	 * directory all along.  Don't claim a rename if they're the same.
+	 * If end_of_new got back to the beginning of its string, and
+	 * end_of_old got back to the beginning of some subdirectory, then
+	 * we have a rename/merge of a subdirectory into the root, which
+	 * needs slightly special handling.
+	 *
+	 * Note: There is no need to consider the opposite case, with a
+	 * rename/merge of the root directory into some subdirectory
+	 * because as noted above the root directory always exists so it
+	 * cannot be considered to be renamed.
 	 */
-	old_len = end_of_old - old_path;
-	new_len = end_of_new - new_path;
-
-	if (old_len != new_len || strncmp(old_path, new_path, old_len)) {
-		*old_dir = xstrndup(old_path, old_len);
-		*new_dir = xstrndup(new_path, new_len);
+	if (end_of_new == new_path &&
+	    end_of_old != old_path && end_of_old[-1] == '/') {
+		*old_dir = xstrndup(old_path, --end_of_old - old_path);
+		*new_dir = xstrdup("");
+		return;
 	}
+
+	/*
+	 * We've found the first non-matching character in the directory
+	 * paths.  That means the current characters we were looking at
+	 * were part of the first non-matching subdir name going back from
+	 * the end of the strings.  Get the whole name by advancing both
+	 * end_of_old and end_of_new to the NEXT '/' character.  That will
+	 * represent the entire directory rename.
+	 *
+	 * The reason for the increment is cases like
+	 *    a/b/star/foo/whatever.c -> a/b/tar/foo/random.c
+	 * After dropping the basename and going back to the first
+	 * non-matching character, we're now comparing:
+	 *    a/b/s          and         a/b/
+	 * and we want to be comparing:
+	 *    a/b/star/      and         a/b/tar/
+	 * but without the pre-increment, the one on the right would stay
+	 * a/b/.
+	 */
+	end_of_old = strchr(++end_of_old, '/');
+	end_of_new = strchr(++end_of_new, '/');
+
+	/* Copy the old and new directories into *old_dir and *new_dir. */
+	*old_dir = xstrndup(old_path, end_of_old - old_path);
+	*new_dir = xstrndup(new_path, end_of_new - new_path);
 }
 
 static void remove_hashmap_entries(struct hashmap *dir_renames,
@@ -2118,7 +2148,7 @@ static char *handle_path_level_conflicts(struct merge_options *opt,
  *      implicit renaming of files that should be left in place.  (See
  *      testcase 6b in t6043 for details.)
  *   2. Prune directory renames if there are still files left in the
- *      the original directory.  These represent a partial directory rename,
+ *      original directory.  These represent a partial directory rename,
  *      i.e. a rename where only some of the files within the directory
  *      were renamed elsewhere.  (Technically, this could be done earlier
  *      in get_directory_renames(), except that would prevent us from
@@ -2377,8 +2407,7 @@ static void compute_collisions(struct hashmap *collisions,
 			continue;
 		collision_ent = collision_find_entry(collisions, new_path);
 		if (!collision_ent) {
-			collision_ent = xcalloc(1,
-						sizeof(struct collision_entry));
+			CALLOC_ARRAY(collision_ent, 1);
 			hashmap_entry_init(&collision_ent->ent,
 						strhash(new_path));
 			hashmap_put(collisions, &collision_ent->ent);
@@ -2582,7 +2611,7 @@ static struct string_list *get_renames(struct merge_options *opt,
 	struct string_list *renames;
 
 	compute_collisions(&collisions, dir_renames, pairs);
-	renames = xcalloc(1, sizeof(struct string_list));
+	CALLOC_ARRAY(renames, 1);
 
 	for (i = 0; i < pairs->nr; ++i) {
 		struct string_list_item *item;
@@ -2639,7 +2668,7 @@ static struct string_list *get_renames(struct merge_options *opt,
 		free(e->target_file);
 		string_list_clear(&e->source_files, 0);
 	}
-	hashmap_free_entries(&collisions, struct collision_entry, ent);
+	hashmap_clear_and_free(&collisions, struct collision_entry, ent);
 	return renames;
 }
 
@@ -2652,6 +2681,14 @@ static int process_renames(struct merge_options *opt,
 	struct string_list b_by_dst = STRING_LIST_INIT_NODUP;
 	const struct rename *sre;
 
+	/*
+	 * FIXME: As string-list.h notes, it's O(n^2) to build a sorted
+	 * string_list one-by-one, but O(n log n) to build it unsorted and
+	 * then sort it.  Note that as we build the list, we do not need to
+	 * check if the existing destination path is already in the list,
+	 * because the structure of diffcore_rename guarantees we won't
+	 * have duplicates.
+	 */
 	for (i = 0; i < a_renames->nr; i++) {
 		sre = a_renames->items[i].util;
 		string_list_insert(&a_by_dst, sre->pair->two->path)->util
@@ -2763,12 +2800,19 @@ static int process_renames(struct merge_options *opt,
 			int renamed_stage = a_renames == renames1 ? 2 : 3;
 			int other_stage =   a_renames == renames1 ? 3 : 2;
 
+			/*
+			 * Directory renames have a funny corner case...
+			 */
+			int renamed_to_self = !strcmp(ren1_src, ren1_dst);
+
 			/* BUG: We should only remove ren1_src in the base
 			 * stage and in other_stage (think of rename +
 			 * add-source case).
 			 */
-			remove_file(opt, 1, ren1_src,
-				    renamed_stage == 2 || !was_tracked(opt, ren1_src));
+			if (!renamed_to_self)
+				remove_file(opt, 1, ren1_src,
+					    renamed_stage == 2 ||
+					    !was_tracked(opt, ren1_src));
 
 			oidcpy(&src_other.oid,
 			       &ren1->src_entry->stages[other_stage].oid);
@@ -2778,15 +2822,18 @@ static int process_renames(struct merge_options *opt,
 			dst_other.mode = ren1->dst_entry->stages[other_stage].mode;
 			try_merge = 0;
 
-			if (oid_eq(&src_other.oid, &null_oid) &&
+			if (oideq(&src_other.oid, null_oid()) &&
 			    ren1->dir_rename_original_type == 'A') {
 				setup_rename_conflict_info(RENAME_VIA_DIR,
 							   opt, ren1, NULL);
-			} else if (oid_eq(&src_other.oid, &null_oid)) {
+			} else if (renamed_to_self) {
+				setup_rename_conflict_info(RENAME_NORMAL,
+							   opt, ren1, NULL);
+			} else if (oideq(&src_other.oid, null_oid())) {
 				setup_rename_conflict_info(RENAME_DELETE,
 							   opt, ren1, NULL);
 			} else if ((dst_other.mode == ren1->pair->two->mode) &&
-				   oid_eq(&dst_other.oid, &ren1->pair->two->oid)) {
+				   oideq(&dst_other.oid, &ren1->pair->two->oid)) {
 				/*
 				 * Added file on the other side identical to
 				 * the file being renamed: clean merge.
@@ -2801,7 +2848,7 @@ static int process_renames(struct merge_options *opt,
 						      1, /* update_cache */
 						      0  /* update_wd    */))
 					clean_merge = -1;
-			} else if (!oid_eq(&dst_other.oid, &null_oid)) {
+			} else if (!oideq(&dst_other.oid, null_oid())) {
 				/*
 				 * Probably not a clean merge, but it's
 				 * premature to set clean_merge to 0 here,
@@ -2858,7 +2905,7 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs,
 		strbuf_release(&e->new_dir);
 		/* possible_new_dirs already cleared in get_directory_renames */
 	}
-	hashmap_free_entries(dir_renames, struct dir_rename_entry, ent);
+	hashmap_clear_and_free(dir_renames, struct dir_rename_entry, ent);
 	free(dir_renames);
 
 	free(pairs->queue);
@@ -2975,11 +3022,11 @@ static int blob_unchanged(struct merge_options *opt,
 	struct strbuf obuf = STRBUF_INIT;
 	struct strbuf abuf = STRBUF_INIT;
 	int ret = 0; /* assume changed for safety */
-	const struct index_state *idx = opt->repo->index;
+	struct index_state *idx = opt->repo->index;
 
 	if (a->mode != o->mode)
 		return 0;
-	if (oid_eq(&o->oid, &a->oid))
+	if (oideq(&o->oid, &a->oid))
 		return 1;
 	if (!renormalize)
 		return 0;
@@ -3139,7 +3186,6 @@ static int handle_rename_normal(struct merge_options *opt,
 	struct rename *ren = ci->ren1;
 	struct merge_file_info mfi;
 	int clean;
-	int side = (ren->branch == opt->branch1 ? 2 : 3);
 
 	/* Merge the content and write it out */
 	clean = handle_content_merge(&mfi, opt, path, was_dirty(opt, path),
@@ -3149,9 +3195,7 @@ static int handle_rename_normal(struct merge_options *opt,
 	    opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_CONFLICT &&
 	    ren->dir_rename_original_dest) {
 		if (update_stages(opt, path,
-				  NULL,
-				  side == 2 ? &mfi.blob : NULL,
-				  side == 2 ? NULL : &mfi.blob))
+				  &mfi.blob, &mfi.blob, &mfi.blob))
 			return -1;
 		clean = 0; /* not clean, but conflicted */
 	}
@@ -3420,8 +3464,8 @@ static int merge_trees_internal(struct merge_options *opt,
 					       opt->subtree_shift);
 	}
 
-	if (oid_eq(&merge_base->object.oid, &merge->object.oid)) {
-		output(opt, 0, _("Already up to date!"));
+	if (oideq(&merge_base->object.oid, &merge->object.oid)) {
+		output(opt, 0, _("Already up to date."));
 		*result = head;
 		return 1;
 	}
@@ -3485,7 +3529,7 @@ static int merge_trees_internal(struct merge_options *opt,
 		string_list_clear(entries, 1);
 		free(entries);
 
-		hashmap_free_entries(&opt->priv->current_file_dir_set,
+		hashmap_clear_and_free(&opt->priv->current_file_dir_set,
 					struct path_hashmap_entry, e);
 
 		if (clean < 0) {
@@ -3505,20 +3549,10 @@ static int merge_trees_internal(struct merge_options *opt,
 	return clean;
 }
 
-static struct commit_list *reverse_commit_list(struct commit_list *list)
-{
-	struct commit_list *next = NULL, *current, *backup;
-	for (current = list; current; current = backup) {
-		backup = current->next;
-		current->next = next;
-		next = current;
-	}
-	return next;
-}
-
 /*
- * Merge the commits h1 and h2, return the resulting virtual
- * commit object and a flag indicating the cleanness of the merge.
+ * Merge the commits h1 and h2, returning a flag (int) indicating the
+ * cleanness of the merge.  Also, if opt->priv->call_depth, create a
+ * virtual commit and write its location to *result.
  */
 static int merge_recursive_internal(struct merge_options *opt,
 				    struct commit *h1,
@@ -3600,6 +3634,15 @@ static int merge_recursive_internal(struct merge_options *opt,
 			return err(opt, _("merge returned no commit"));
 	}
 
+	/*
+	 * FIXME: Since merge_recursive_internal() is only ever called by
+	 * places that ensure the index is loaded first
+	 * (e.g. builtin/merge.c, rebase/sequencer, etc.), in the common
+	 * case where the merge base was unique that means when we get here
+	 * we immediately discard the index and re-read it, which is a
+	 * complete waste of time.  We should only be discarding and
+	 * re-reading if we were forced to recurse.
+	 */
 	discard_index(opt->repo->index);
 	if (!opt->priv->call_depth)
 		repo_read_index(opt->repo);
@@ -3662,8 +3705,8 @@ static int merge_start(struct merge_options *opt, struct tree *head)
 		return -1;
 	}
 
-	opt->priv = xcalloc(1, sizeof(*opt->priv));
-	string_list_init(&opt->priv->df_conflict_file_set, 1);
+	CALLOC_ARRAY(opt->priv, 1);
+	string_list_init_dup(&opt->priv->df_conflict_file_set);
 	return 0;
 }
 
@@ -3779,9 +3822,12 @@ int merge_recursive_generic(struct merge_options *opt,
 static void merge_recursive_config(struct merge_options *opt)
 {
 	char *value = NULL;
+	int renormalize = 0;
 	git_config_get_int("merge.verbosity", &opt->verbosity);
 	git_config_get_int("diff.renamelimit", &opt->rename_limit);
 	git_config_get_int("merge.renamelimit", &opt->rename_limit);
+	git_config_get_bool("merge.renormalize", &renormalize);
+	opt->renormalize = renormalize;
 	if (!git_config_get_string("diff.renames", &value)) {
 		opt->detect_renames = git_config_rename("diff.renames", value);
 		free(value);