Commit 21515f228b739a3ecd2237bafbba50e8d219d8dd

Edward Thomson 2015-09-29T15:49:16

index: also try conflict mode when inserting When we do not trust the on-disk mode, we use the mode of an existing index entry. This allows us to preserve executable bits on platforms that do not honor them on the filesystem. If there is no stage 0 index entry, also look at conflicts to attempt to answer this question: prefer the data from the 'ours' side, then the 'theirs' side before falling back to the common ancestor.

diff --git a/include/git2/index.h b/include/git2/index.h
index 176ba30..466765b 100644
--- a/include/git2/index.h
+++ b/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
  *
diff --git a/src/index.c b/src/index.c
index 2e89347..c0be5b9 100644
--- a/src/index.c
+++ b/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)
diff --git a/tests/index/bypath.c b/tests/index/bypath.c
index d26273f..0c10cfe 100644
--- a/tests/index/bypath.c
+++ b/tests/index/bypath.c
@@ -242,7 +242,8 @@ void test_index_bypath__add_honors_existing_case_4(void)
 
 void test_index_bypath__add_honors_mode(void)
 {
-	git_index_entry *entry, new_entry;
+	const git_index_entry *entry;
+	git_index_entry new_entry;
 
 	cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL);
 
@@ -263,3 +264,67 @@ void test_index_bypath__add_honors_mode(void)
 	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);
+}