Merge pull request #3444 from ethomson/add_preserves_conflict_mode

Preserve modes from a conflict in `git_index_insert`

include/git2/index.h

@@ -154,13 +154,27 @@ typedef enum {
 	GIT_INDEX_ADD_CHECK_PATHSPEC = (1u << 2),
 } git_index_add_option_t;
 
-/**
- * Match any index stage.
- *
- * Some index APIs take a stage to match; pass this value to match
- * any entry matching the path regardless of stage.
- */
-#define GIT_INDEX_STAGE_ANY -1
+typedef enum {
+	/**
+	 * Match any index stage.
+	 *
+	 * Some index APIs take a stage to match; pass this value to match
+	 * any entry matching the path regardless of stage.
+	 */
+	GIT_INDEX_STAGE_ANY = -1,
+
+	/** A normal staged file in the index. */
+	GIT_INDEX_STAGE_NORMAL = 0,
+
+	/** The ancestor side of a conflict. */
+	GIT_INDEX_STAGE_ANCESTOR = 1,
+
+	/** The "ours" side of a conflict. */
+	GIT_INDEX_STAGE_OURS = 2,
+
+	/** The "theirs" side of a conflict. */
+	GIT_INDEX_STAGE_THEIRS = 3,
+} git_index_stage_t;
 
 /** @name Index File Functions
  *

src/index.c

@@ -1114,7 +1114,9 @@ static int check_file_directory_collision(git_index *index,
 }
 
 static int canonicalize_directory_path(
-	git_index *index, git_index_entry *entry)
+	git_index *index,
+	git_index_entry *entry,
+	git_index_entry *existing)
 {
 	const git_index_entry *match, *best = NULL;
 	char *search, *sep;
@@ -1124,8 +1126,8 @@ static int canonicalize_directory_path(
 		return 0;
 
 	/* item already exists in the index, simply re-use the existing case */
-	if ((match = git_index_get_bypath(index, entry->path, 0)) != NULL) {
-		memcpy((char *)entry->path, match->path, strlen(entry->path));
+	if (existing) {
+		memcpy((char *)entry->path, existing->path, strlen(existing->path));
 		return 0;
 	}
 
@@ -1190,6 +1192,52 @@ static int index_no_dups(void **old, void *new)
 	return GIT_EEXISTS;
 }
 
+static void index_existing_and_best(
+	const git_index_entry **existing,
+	size_t *existing_position,
+	const git_index_entry **best,
+	git_index *index,
+	const git_index_entry *entry)
+{
+	const git_index_entry *e;
+	size_t pos;
+	int error;
+
+	error = index_find(&pos,
+		index, entry->path, 0, GIT_IDXENTRY_STAGE(entry), false);
+
+	if (error == 0) {
+		*existing = index->entries.contents[pos];
+		*existing_position = pos;
+		*best = index->entries.contents[pos];
+		return;
+	}
+
+	*existing = NULL;
+	*existing_position = 0;
+	*best = NULL;
+
+	if (GIT_IDXENTRY_STAGE(entry) == 0) {
+		for (; pos < index->entries.length; pos++) {
+			int (*strcomp)(const char *a, const char *b) =
+				index->ignore_case ? git__strcasecmp : git__strcmp;
+
+			e = index->entries.contents[pos];
+
+			if (strcomp(entry->path, e->path) != 0)
+				break;
+
+			if (GIT_IDXENTRY_STAGE(e) == GIT_INDEX_STAGE_ANCESTOR) {
+				*best = e;
+				continue;
+			} else {
+				*best = e;
+				break;
+			}
+		}
+	}
+}
+
 /* index_insert takes ownership of the new entry - if it can't insert
  * it, then it will return an error **and also free the entry**.  When
  * it replaces an existing entry, it will update the entry_ptr with the
@@ -1208,7 +1256,7 @@ static int index_insert(
 {
 	int error = 0;
 	size_t path_length, position;
-	git_index_entry *existing = NULL, *entry;
+	git_index_entry *existing, *best, *entry;
 
 	assert(index && entry_ptr);
 
@@ -1231,20 +1279,19 @@ static int index_insert(
 
 	git_vector_sort(&index->entries);
 
-	/* look if an entry with this path already exists */
-	if (!index_find(
-			&position, index, entry->path, 0, GIT_IDXENTRY_STAGE(entry), false)) {
-		existing = index->entries.contents[position];
-		/* update filemode to existing values if stat is not trusted */
-		if (trust_mode)
-			entry->mode = git_index__create_mode(entry->mode);
-		else
-			entry->mode = index_merge_mode(index, existing, entry->mode);
-	}
+	/* look if an entry with this path already exists, either staged, or (if
+	 * this entry is a regular staged item) as the "ours" side of a conflict.
+	 */
+	index_existing_and_best(&existing, &position, &best, index, entry);
+
+	/* update the file mode */
+	entry->mode = trust_mode ?
+		git_index__create_mode(entry->mode) :
+		index_merge_mode(index, best, entry->mode);
 
 	/* canonicalize the directory name */
 	if (!trust_path)
-		error = canonicalize_directory_path(index, entry);
+		error = canonicalize_directory_path(index, entry, best);
 
 	/* look for tree / blob name collisions, removing conflicts if requested */
 	if (!error)

tests/index/bypath.c

@@ -240,3 +240,91 @@ void test_index_bypath__add_honors_existing_case_4(void)
 	cl_assert_equal_s("just_a_dir/a/b/Z/y/X/foo.txt", entry->path);
 }
 
+void test_index_bypath__add_honors_mode(void)
+{
+	const git_index_entry *entry;
+	git_index_entry new_entry;
+
+	cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL);
+
+	memcpy(&new_entry, entry, sizeof(git_index_entry));
+	new_entry.path = "README.txt";
+	new_entry.mode = GIT_FILEMODE_BLOB_EXECUTABLE;
+
+	cl_must_pass(p_chmod("submod2/README.txt", GIT_FILEMODE_BLOB_EXECUTABLE));
+
+	cl_git_pass(git_index_add(g_idx, &new_entry));
+	cl_git_pass(git_index_write(g_idx));
+
+	cl_git_rewritefile("submod2/README.txt", "Modified but still executable");
+
+	cl_git_pass(git_index_add_bypath(g_idx, "README.txt"));
+	cl_git_pass(git_index_write(g_idx));
+
+	cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL);
+	cl_assert_equal_i(GIT_FILEMODE_BLOB_EXECUTABLE, entry->mode);
+}
+
+void test_index_bypath__add_honors_conflict_mode(void)
+{
+	const git_index_entry *entry;
+	git_index_entry new_entry;
+	int stage = 0;
+
+	cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL);
+
+	memcpy(&new_entry, entry, sizeof(git_index_entry));
+	new_entry.path = "README.txt";
+	new_entry.mode = GIT_FILEMODE_BLOB_EXECUTABLE;
+
+	cl_must_pass(p_chmod("submod2/README.txt", GIT_FILEMODE_BLOB_EXECUTABLE));
+
+	cl_git_pass(git_index_remove_bypath(g_idx, "README.txt"));
+
+	for (stage = 1; stage <= 3; stage++) {
+		new_entry.flags = stage << GIT_IDXENTRY_STAGESHIFT;
+		cl_git_pass(git_index_add(g_idx, &new_entry));
+	}
+
+	cl_git_pass(git_index_write(g_idx));
+
+	cl_git_rewritefile("submod2/README.txt", "Modified but still executable");
+
+	cl_git_pass(git_index_add_bypath(g_idx, "README.txt"));
+	cl_git_pass(git_index_write(g_idx));
+
+	cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL);
+	cl_assert_equal_i(GIT_FILEMODE_BLOB_EXECUTABLE, entry->mode);
+}
+
+void test_index_bypath__add_honors_conflict_case(void)
+{
+	const git_index_entry *entry;
+	git_index_entry new_entry;
+	int stage = 0;
+
+	cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL);
+
+	memcpy(&new_entry, entry, sizeof(git_index_entry));
+	new_entry.path = "README.txt";
+	new_entry.mode = GIT_FILEMODE_BLOB_EXECUTABLE;
+
+	cl_must_pass(p_chmod("submod2/README.txt", GIT_FILEMODE_BLOB_EXECUTABLE));
+
+	cl_git_pass(git_index_remove_bypath(g_idx, "README.txt"));
+
+	for (stage = 1; stage <= 3; stage++) {
+		new_entry.flags = stage << GIT_IDXENTRY_STAGESHIFT;
+		cl_git_pass(git_index_add(g_idx, &new_entry));
+	}
+
+	cl_git_pass(git_index_write(g_idx));
+
+	cl_git_rewritefile("submod2/README.txt", "Modified but still executable");
+
+	cl_git_pass(git_index_add_bypath(g_idx, "README.txt"));
+	cl_git_pass(git_index_write(g_idx));
+
+	cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL);
+	cl_assert_equal_i(GIT_FILEMODE_BLOB_EXECUTABLE, entry->mode);
+}