Merge pull request #3489 from libgit2/vmg/reuc-insert

Better REUC generation when merging

3 files changed, 66 insertions, 49 deletions

diff --git a/include/git2/merge.h b/include/git2/merge.h
index ced9e51ff..b7da63e0e 100644
--- a/include/git2/merge.h
+++ b/include/git2/merge.h
@@ -79,6 +79,11 @@ typedef enum {
 	 * GIT_EMERGECONFLICT and no index will be returned.
 	 */
 	GIT_MERGE_TREE_FAIL_ON_CONFLICT = (1 << 1),
+
+	/**
+	 * Do not write the REUC extension on the generated index
+	 */
+	GIT_MERGE_TREE_SKIP_REUC = (1 << 2),
 } git_merge_tree_flag_t;
 
 /**
diff --git a/src/index.c b/src/index.c
index 334a13135..d9e713899 100644
--- a/src/index.c
+++ b/src/index.c
@@ -2000,27 +2000,24 @@ size_t git_index_reuc_entrycount(git_index *index)
 	return index->reuc.length;
 }
 
+static int index_reuc_on_dup(void **old, void *new)
+{
+	index_entry_reuc_free(*old);
+	*old = new;
+	return GIT_EEXISTS;
+}
+
 static int index_reuc_insert(
 	git_index *index,
-	git_index_reuc_entry *reuc,
-	int replace)
+	git_index_reuc_entry *reuc)
 {
-	git_index_reuc_entry **existing = NULL;
-	size_t position;
+	int res;
 
 	assert(index && reuc && reuc->path != NULL);
+	assert(git_vector_is_sorted(&index->reuc));
 
-	if (!git_index_reuc_find(&position, index, reuc->path))
-		existing = (git_index_reuc_entry **)&index->reuc.contents[position];
-
-	if (!replace || !existing)
-		return git_vector_insert(&index->reuc, reuc);
-
-	/* exists, replace it */
-	git__free(*existing);
-	*existing = reuc;
-
-	return 0;
+	res = git_vector_insert_sorted(&index->reuc, reuc, &index_reuc_on_dup);
+	return res == GIT_EEXISTS ? 0 : res;
 }
 
 int git_index_reuc_add(git_index *index, const char *path,
@@ -2035,7 +2032,7 @@ int git_index_reuc_add(git_index *index, const char *path,
 
 	if ((error = index_entry_reuc_init(&reuc, path, ancestor_mode,
 			ancestor_oid, our_mode, our_oid, their_mode, their_oid)) < 0 ||
-		(error = index_reuc_insert(index, reuc, 1)) < 0)
+		(error = index_reuc_insert(index, reuc)) < 0)
 		index_entry_reuc_free(reuc);
 
 	return error;
@@ -2055,7 +2052,7 @@ const git_index_reuc_entry *git_index_reuc_get_bypath(
 	if (!index->reuc.length)
 		return NULL;
 
-	git_vector_sort(&index->reuc);
+	assert(git_vector_is_sorted(&index->reuc));
 
 	if (git_index_reuc_find(&pos, index, path) < 0)
 		return NULL;
@@ -2067,8 +2064,8 @@ const git_index_reuc_entry *git_index_reuc_get_byindex(
 	git_index *index, size_t n)
 {
 	assert(index);
+	assert(git_vector_is_sorted(&index->reuc));
 
-	git_vector_sort(&index->reuc);
 	return git_vector_get(&index->reuc, n);
 }
 
@@ -2077,7 +2074,7 @@ int git_index_reuc_remove(git_index *index, size_t position)
 	int error;
 	git_index_reuc_entry *reuc;
 
-	git_vector_sort(&index->reuc);
+	assert(git_vector_is_sorted(&index->reuc));
 
 	reuc = git_vector_get(&index->reuc, position);
 	error = git_vector_remove(&index->reuc, position);
diff --git a/src/merge.c b/src/merge.c
index 3bed0fd3b..186c77037 100644
--- a/src/merge.c
+++ b/src/merge.c
@@ -1541,7 +1541,45 @@ static int merge_index_insert_reuc(
 		mode[0], oid[0], mode[1], oid[1], mode[2], oid[2]);
 }
 
-int index_from_diff_list(git_index **out, git_merge_diff_list *diff_list)
+static int index_update_reuc(git_index *index, git_merge_diff_list *diff_list)
+{
+	int error;
+	size_t i;
+	git_merge_diff *conflict;
+
+	/* Add each entry in the resolved conflict to the REUC independently, since
+	 * the paths may differ due to renames. */
+	git_vector_foreach(&diff_list->resolved, i, conflict) {
+		const git_index_entry *ancestor =
+			GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->ancestor_entry) ?
+			&conflict->ancestor_entry : NULL;
+
+		const git_index_entry *ours =
+			GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->our_entry) ?
+			&conflict->our_entry : NULL;
+
+		const git_index_entry *theirs =
+			GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->their_entry) ?
+			&conflict->their_entry : NULL;
+
+		if (ancestor != NULL &&
+			(error = merge_index_insert_reuc(index, TREE_IDX_ANCESTOR, ancestor)) < 0)
+			return error;
+
+		if (ours != NULL &&
+			(error = merge_index_insert_reuc(index, TREE_IDX_OURS, ours)) < 0)
+			return error;
+
+		if (theirs != NULL &&
+			(error = merge_index_insert_reuc(index, TREE_IDX_THEIRS, theirs)) < 0)
+			return error;
+	}
+
+	return 0;
+}
+
+static int index_from_diff_list(git_index **out,
+	git_merge_diff_list *diff_list, bool skip_reuc)
 {
 	git_index *index;
 	size_t i;
@@ -1600,31 +1638,8 @@ int index_from_diff_list(git_index **out, git_merge_diff_list *diff_list)
 		}
 	}
 
-	/* Add each entry in the resolved conflict to the REUC independently, since
-	 * the paths may differ due to renames. */
-	git_vector_foreach(&diff_list->resolved, i, conflict) {
-		const git_index_entry *ancestor =
-			GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->ancestor_entry) ?
-			&conflict->ancestor_entry : NULL;
-
-		const git_index_entry *ours =
-			GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->our_entry) ?
-			&conflict->our_entry : NULL;
-
-		const git_index_entry *theirs =
-			GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->their_entry) ?
-			&conflict->their_entry : NULL;
-
-		if (ancestor != NULL &&
-			(error = merge_index_insert_reuc(index, TREE_IDX_ANCESTOR, ancestor)) < 0)
-			goto on_error;
-
-		if (ours != NULL &&
-			(error = merge_index_insert_reuc(index, TREE_IDX_OURS, ours)) < 0)
-			goto on_error;
-
-		if (theirs != NULL &&
-			(error = merge_index_insert_reuc(index, TREE_IDX_THEIRS, theirs)) < 0)
+	if (!skip_reuc) {
+		if ((error = index_update_reuc(index, diff_list)) < 0)
 			goto on_error;
 	}
 
@@ -1633,7 +1648,6 @@ int index_from_diff_list(git_index **out, git_merge_diff_list *diff_list)
 
 on_error:
 	git_index_free(index);
-
 	return error;
 }
 
@@ -1712,12 +1726,13 @@ int git_merge__iterators(
 		}
 	}
 
+	error = index_from_diff_list(out, diff_list,
+		(opts.tree_flags & GIT_MERGE_TREE_SKIP_REUC));
+
+done:
 	if (!given_opts || !given_opts->metric)
 		git__free(opts.metric);
 
-	error = index_from_diff_list(out, diff_list);
-
-done:
 	git_merge_diff_list__free(diff_list);
 	git_iterator_free(empty_ancestor);
 	git_iterator_free(empty_ours);