Merge pull request #2692 from ethomson/bug2515

checkout_index: handle other stages

src/checkout.c

@@ -37,9 +37,10 @@ enum {
 	CHECKOUT_ACTION__UPDATE_BLOB = 2,
 	CHECKOUT_ACTION__UPDATE_SUBMODULE = 4,
 	CHECKOUT_ACTION__CONFLICT = 8,
-	CHECKOUT_ACTION__UPDATE_CONFLICT = 16,
-	CHECKOUT_ACTION__MAX = 16,
-	CHECKOUT_ACTION__DEFER_REMOVE = 32,
+	CHECKOUT_ACTION__REMOVE_CONFLICT = 16,
+	CHECKOUT_ACTION__UPDATE_CONFLICT = 32,
+	CHECKOUT_ACTION__MAX = 32,
+	CHECKOUT_ACTION__DEFER_REMOVE = 64,
 	CHECKOUT_ACTION__REMOVE_AND_UPDATE =
 		(CHECKOUT_ACTION__UPDATE_BLOB | CHECKOUT_ACTION__REMOVE),
 };
@@ -54,9 +55,10 @@ typedef struct {
 	git_index *index;
 	git_pool pool;
 	git_vector removes;
-	git_vector conflicts;
-	git_vector *reuc;
-	git_vector *names;
+	git_vector remove_conflicts;
+	git_vector update_conflicts;
+	git_vector *update_reuc;
+	git_vector *update_names;
 	git_buf path;
 	size_t workdir_len;
 	git_buf tmp;
@@ -793,49 +795,60 @@ done:
 	return error;
 }
 
-static int checkout_conflicts_load(checkout_data *data, git_iterator *workdir, git_vector *pathspec)
+static int checkout_conflict_append_update(
+	const git_index_entry *ancestor,
+	const git_index_entry *ours,
+	const git_index_entry *theirs,
+	void *payload)
+{
+	checkout_data *data = payload;
+	checkout_conflictdata *conflict;
+	int error;
+
+	conflict = git__calloc(1, sizeof(checkout_conflictdata));
+	GITERR_CHECK_ALLOC(conflict);
+
+	conflict->ancestor = ancestor;
+	conflict->ours = ours;
+	conflict->theirs = theirs;
+
+	if ((error = checkout_conflict_detect_submodule(conflict)) < 0 ||
+		(error = checkout_conflict_detect_binary(data->repo, conflict)) < 0)
+	{
+		git__free(conflict);
+		return error;
+	}
+
+	if (git_vector_insert(&data->update_conflicts, conflict))
+		return -1;
+
+	return 0;
+}
+
+static int checkout_conflicts_foreach(
+	checkout_data *data,
+	git_index *index,
+	git_iterator *workdir,
+	git_vector *pathspec,
+	int (*cb)(const git_index_entry *, const git_index_entry *, const git_index_entry *, void *),
+	void *payload)
 {
 	git_index_conflict_iterator *iterator = NULL;
-	git_index *index;
 	const git_index_entry *ancestor, *ours, *theirs;
-	checkout_conflictdata *conflict;
 	int error = 0;
 
-	/* Only write conficts from sources that have them: indexes. */
-	if ((index = git_iterator_get_index(data->target)) == NULL)
-		return 0;
-
 	if ((error = git_index_conflict_iterator_new(&iterator, index)) < 0)
 		goto done;
 
-	data->conflicts._cmp = checkout_conflictdata_cmp;
-
 	/* Collect the conflicts */
 	while ((error = git_index_conflict_next(&ancestor, &ours, &theirs, iterator)) == 0) {
 		if (!conflict_pathspec_match(data, workdir, pathspec, ancestor, ours, theirs))
 			continue;
 
-		conflict = git__calloc(1, sizeof(checkout_conflictdata));
-		GITERR_CHECK_ALLOC(conflict);
-
-		conflict->ancestor = ancestor;
-		conflict->ours = ours;
-		conflict->theirs = theirs;
-
-		if ((error = checkout_conflict_detect_submodule(conflict)) < 0 ||
-		    (error = checkout_conflict_detect_binary(data->repo, conflict)) < 0)
-		{
-			git__free(conflict);
+		if ((error = cb(ancestor, ours, theirs, payload)) < 0)
 			goto done;
-		}
-
-		git_vector_insert(&data->conflicts, conflict);
 	}
 
-	/* Collect the REUC and NAME entries */
-	data->reuc = &index->reuc;
-	data->names = &index->names;
-
 	if (error == GIT_ITEROVER)
 		error = 0;
 
@@ -845,6 +858,26 @@ done:
 	return error;
 }
 
+static int checkout_conflicts_load(checkout_data *data, git_iterator *workdir, git_vector *pathspec)
+{
+	git_index *index;
+
+	/* Only write conficts from sources that have them: indexes. */
+	if ((index = git_iterator_get_index(data->target)) == NULL)
+		return 0;
+
+	data->update_conflicts._cmp = checkout_conflictdata_cmp;
+
+	if (checkout_conflicts_foreach(data, index, workdir, pathspec, checkout_conflict_append_update, data) < 0)
+		return -1;
+
+	/* Collect the REUC and NAME entries */
+	data->update_reuc = &index->reuc;
+	data->update_names = &index->names;
+
+	return 0;
+}
+
 GIT_INLINE(int) checkout_conflicts_cmp_entry(
 	const char *path,
 	const git_index_entry *entry)
@@ -869,10 +902,10 @@ static checkout_conflictdata *checkout_conflicts_search_ancestor(
 {
 	size_t pos;
 
-	if (git_vector_bsearch2(&pos, &data->conflicts, checkout_conflicts_cmp_ancestor, path) < 0)
+	if (git_vector_bsearch2(&pos, &data->update_conflicts, checkout_conflicts_cmp_ancestor, path) < 0)
 		return NULL;
 
-	return git_vector_get(&data->conflicts, pos);
+	return git_vector_get(&data->update_conflicts, pos);
 }
 
 static checkout_conflictdata *checkout_conflicts_search_branch(
@@ -882,7 +915,7 @@ static checkout_conflictdata *checkout_conflicts_search_branch(
 	checkout_conflictdata *conflict;
 	size_t i;
 
-	git_vector_foreach(&data->conflicts, i, conflict) {
+	git_vector_foreach(&data->update_conflicts, i, conflict) {
 		int cmp = -1;
 
 		if (conflict->ancestor)
@@ -1022,7 +1055,7 @@ static int checkout_conflicts_coalesce_renames(
 	}
 
 	git_vector_remove_matching(
-		&data->conflicts, checkout_conflictdata_empty, NULL);
+		&data->update_conflicts, checkout_conflictdata_empty, NULL);
 
 done:
 	return error;
@@ -1044,7 +1077,7 @@ static int checkout_conflicts_mark_directoryfile(
 	len = git_index_entrycount(index);
 
 	/* Find d/f conflicts */
-	git_vector_foreach(&data->conflicts, i, conflict) {
+	git_vector_foreach(&data->update_conflicts, i, conflict) {
 		if ((conflict->ours && conflict->theirs) ||
 			(!conflict->ours && !conflict->theirs))
 			continue;
@@ -1084,7 +1117,7 @@ done:
 	return error;
 }
 
-static int checkout_get_conflicts(
+static int checkout_get_update_conflicts(
 	checkout_data *data,
 	git_iterator *workdir,
 	git_vector *pathspec)
@@ -1103,6 +1136,38 @@ done:
 	return error;
 }
 
+static int checkout_conflict_append_remove(
+	const git_index_entry *ancestor,
+	const git_index_entry *ours,
+	const git_index_entry *theirs,
+	void *payload)
+{
+	checkout_data *data = payload;
+	const char *name;
+
+	if (ancestor)
+		name = git__strdup(ancestor->path);
+	else if (ours)
+		name = git__strdup(ours->path);
+	else if (theirs)
+		name = git__strdup(theirs->path);
+
+	GITERR_CHECK_ALLOC(name);
+
+	return git_vector_insert(&data->remove_conflicts, (char *)name);
+}
+
+static int checkout_get_remove_conflicts(
+	checkout_data *data,
+	git_iterator *workdir,
+	git_vector *pathspec)
+{
+	if ((data->strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) != 0)
+		return 0;
+
+	return checkout_conflicts_foreach(data, data->index, workdir, pathspec, checkout_conflict_append_remove, data);
+}
+
 static int checkout_get_actions(
 	uint32_t **actions_ptr,
 	size_t **counts_ptr,
@@ -1170,10 +1235,12 @@ static int checkout_get_actions(
 	}
 
 
-	if ((error = checkout_get_conflicts(data, workdir, &pathspec)) < 0)
+	if ((error = checkout_get_remove_conflicts(data, workdir, &pathspec)) < 0 ||
+		(error = checkout_get_update_conflicts(data, workdir, &pathspec)) < 0)
 		goto fail;
 
-	counts[CHECKOUT_ACTION__UPDATE_CONFLICT] = git_vector_length(&data->conflicts);
+	counts[CHECKOUT_ACTION__REMOVE_CONFLICT] = git_vector_length(&data->remove_conflicts);
+	counts[CHECKOUT_ACTION__UPDATE_CONFLICT] = git_vector_length(&data->update_conflicts);
 
 	git_pathspec__vfree(&pathspec);
 	git_pool_clear(&pathpool);
@@ -1845,6 +1912,20 @@ done:
 	return error;
 }
 
+static int checkout_conflict_add(
+	checkout_data *data,
+	const git_index_entry *conflict)
+{
+	int error = git_index_remove(data->index, conflict->path, 0);
+
+	if (error == GIT_ENOTFOUND)
+		giterr_clear();
+	else if (error < 0)
+		return error;
+
+	return git_index_add(data->index, conflict);
+}
+
 static int checkout_conflict_update_index(
 	checkout_data *data,
 	checkout_conflictdata *conflict)
@@ -1852,13 +1933,13 @@ static int checkout_conflict_update_index(
 	int error = 0;
 
 	if (conflict->ancestor)
-		error = git_index_add(data->index, conflict->ancestor);
+		error = checkout_conflict_add(data, conflict->ancestor);
 
 	if (!error && conflict->ours)
-		error = git_index_add(data->index, conflict->ours);
+		error = checkout_conflict_add(data, conflict->ours);
 
 	if (!error && conflict->theirs)
-		error = git_index_add(data->index, conflict->theirs);
+		error = checkout_conflict_add(data, conflict->theirs);
 
 	return error;
 }
@@ -1869,7 +1950,7 @@ static int checkout_create_conflicts(checkout_data *data)
 	size_t i;
 	int error = 0;
 
-	git_vector_foreach(&data->conflicts, i, conflict) {
+	git_vector_foreach(&data->update_conflicts, i, conflict) {
 
 		/* Both deleted: nothing to do */
 		if (conflict->ours == NULL && conflict->theirs == NULL)
@@ -1943,6 +2024,21 @@ static int checkout_create_conflicts(checkout_data *data)
 	return error;
 }
 
+static int checkout_remove_conflicts(checkout_data *data)
+{
+	const char *conflict;
+	size_t i;
+
+	git_vector_foreach(&data->remove_conflicts, i, conflict) {
+		if (git_index_conflict_remove(data->index, conflict) < 0)
+			return -1;
+
+		data->completed_steps++;
+	}
+
+	return 0;
+}
+
 static int checkout_extensions_update_index(checkout_data *data)
 {
 	const git_index_reuc_entry *reuc_entry;
@@ -1953,8 +2049,8 @@ static int checkout_extensions_update_index(checkout_data *data)
 	if ((data->strategy & GIT_CHECKOUT_UPDATE_ONLY) != 0)
 		return 0;
 
-	if (data->reuc) {
-		git_vector_foreach(data->reuc, i, reuc_entry) {
+	if (data->update_reuc) {
+		git_vector_foreach(data->update_reuc, i, reuc_entry) {
 			if ((error = git_index_reuc_add(data->index, reuc_entry->path,
 				reuc_entry->mode[0], &reuc_entry->oid[0],
 				reuc_entry->mode[1], &reuc_entry->oid[1],
@@ -1963,8 +2059,8 @@ static int checkout_extensions_update_index(checkout_data *data)
 		}
 	}
 
-	if (data->names) {
-		git_vector_foreach(data->names, i, name_entry) {
+	if (data->update_names) {
+		git_vector_foreach(data->update_names, i, name_entry) {
 			if ((error = git_index_name_add(data->index, name_entry->ancestor,
 				name_entry->ours, name_entry->theirs)) < 0)
 				goto done;
@@ -1985,7 +2081,8 @@ static void checkout_data_clear(checkout_data *data)
 	git_vector_free(&data->removes);
 	git_pool_clear(&data->pool);
 
-	git_vector_free_deep(&data->conflicts);
+	git_vector_free_deep(&data->remove_conflicts);
+	git_vector_free_deep(&data->update_conflicts);
 
 	git__free(data->pfx);
 	data->pfx = NULL;
@@ -2127,7 +2224,8 @@ static int checkout_data_init(
 	}
 
 	if ((error = git_vector_init(&data->removes, 0, git__strcmp_cb)) < 0 ||
-		(error = git_vector_init(&data->conflicts, 0, NULL)) < 0 ||
+		(error = git_vector_init(&data->remove_conflicts, 0, NULL)) < 0 ||
+		(error = git_vector_init(&data->update_conflicts, 0, NULL)) < 0 ||
 		(error = git_pool_init(&data->pool, 1, 0)) < 0 ||
 		(error = git_buf_puts(&data->path, data->opts.target_directory)) < 0 ||
 		(error = git_path_to_dir(&data->path)) < 0)
@@ -2206,6 +2304,7 @@ int git_checkout_iterator(
 		goto cleanup;
 
 	data.total_steps = counts[CHECKOUT_ACTION__REMOVE] +
+		counts[CHECKOUT_ACTION__REMOVE_CONFLICT] +
 		counts[CHECKOUT_ACTION__UPDATE_BLOB] +
 		counts[CHECKOUT_ACTION__UPDATE_SUBMODULE] +
 		counts[CHECKOUT_ACTION__UPDATE_CONFLICT];
@@ -2219,6 +2318,10 @@ int git_checkout_iterator(
 		(error = checkout_remove_the_old(actions, &data)) < 0)
 		goto cleanup;
 
+	if (counts[CHECKOUT_ACTION__REMOVE_CONFLICT] > 0 &&
+		(error = checkout_remove_conflicts(&data)) < 0)
+		goto cleanup;
+
 	if (counts[CHECKOUT_ACTION__UPDATE_BLOB] > 0 &&
 		(error = checkout_create_the_new(actions, &data)) < 0)
 		goto cleanup;

tests/checkout/index.c

@@ -619,17 +619,14 @@ void test_checkout_index__can_get_repo_from_index(void)
 	git_index_free(index);
 }
 
-static void add_conflict(void)
+static void add_conflict(git_index *index, const char *path)
 {
-	git_index *index;
 	git_index_entry entry;
 
 	memset(&entry, 0, sizeof(git_index_entry));
 
-	cl_git_pass(git_repository_index(&index, g_repo));
-
 	entry.mode = 0100644;
-	entry.path = "conflicting.txt";
+	entry.path = path;
 
 	git_oid_fromstr(&entry.id, "d427e0b2e138501a3d15cc376077a3631e15bd46");
 	entry.flags = (1 << GIT_IDXENTRY_STAGESHIFT);
@@ -642,17 +639,19 @@ static void add_conflict(void)
 	git_oid_fromstr(&entry.id, "2bd0a343aeef7a2cf0d158478966a6e587ff3863");
 	entry.flags = (3 << GIT_IDXENTRY_STAGESHIFT);
 	cl_git_pass(git_index_add(index, &entry));
-
-	cl_git_pass(git_index_write(index));
-	git_index_free(index);
 }
 
 void test_checkout_index__writes_conflict_file(void)
 {
+	git_index *index;
 	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
 	git_buf conflicting_buf = GIT_BUF_INIT;
 
-	add_conflict();
+	cl_git_pass(git_repository_index(&index, g_repo));
+
+	add_conflict(index, "conflicting.txt");
+	cl_git_pass(git_index_write(index));
+
 	cl_git_pass(git_checkout_index(g_repo, NULL, &opts));
 
 	cl_git_pass(git_futils_readbuffer(&conflicting_buf, "testrepo/conflicting.txt"));
@@ -663,18 +662,46 @@ void test_checkout_index__writes_conflict_file(void)
 		"this file is changed in branch and master\n"
 		">>>>>>> theirs\n") == 0);
 	git_buf_free(&conflicting_buf);
+
+	git_index_free(index);
+}
+
+void test_checkout_index__adding_conflict_removes_stage_0(void)
+{
+	git_index *new_index, *index;
+	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
+
+	cl_git_pass(git_index_new(&new_index));
+
+	add_conflict(new_index, "new.txt");
+	cl_git_pass(git_checkout_index(g_repo, new_index, &opts));
+
+	cl_git_pass(git_repository_index(&index, g_repo));
+
+	cl_assert(git_index_get_bypath(index, "new.txt", 0) == NULL);
+	cl_assert(git_index_get_bypath(index, "new.txt", 1) != NULL);
+	cl_assert(git_index_get_bypath(index, "new.txt", 2) != NULL);
+	cl_assert(git_index_get_bypath(index, "new.txt", 3) != NULL);
+
+	git_index_free(index);
+	git_index_free(new_index);
 }
 
 void test_checkout_index__conflicts_honor_coreautocrlf(void)
 {
 #ifdef GIT_WIN32
+	git_index *index;
 	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
 	git_buf conflicting_buf = GIT_BUF_INIT;
 
 	cl_git_pass(p_unlink("./testrepo/.gitattributes"));
 	cl_repo_set_bool(g_repo, "core.autocrlf", true);
 
-	add_conflict();
+	cl_git_pass(git_repository_index(&index, g_repo));
+
+	add_conflict(index, "conflicting.txt");
+	cl_git_pass(git_index_write(index));
+
 	cl_git_pass(git_checkout_index(g_repo, NULL, &opts));
 
 	cl_git_pass(git_futils_readbuffer(&conflicting_buf, "testrepo/conflicting.txt"));
@@ -685,5 +712,7 @@ void test_checkout_index__conflicts_honor_coreautocrlf(void)
 		"this file is changed in branch and master\r\n"
 		">>>>>>> theirs\r\n") == 0);
 	git_buf_free(&conflicting_buf);
+
+	git_index_free(index);
 #endif
 }

tests/checkout/tree.c

@@ -882,7 +882,7 @@ void test_checkout_tree__extremely_long_file_name(void)
 	cl_assert(!git_path_exists(path));
 }
 
-static void create_conflict(void)
+static void create_conflict(const char *path)
 {
 	git_index *index;
 	git_index_entry entry;
@@ -893,7 +893,7 @@ static void create_conflict(void)
 	entry.mode = 0100644;
 	entry.flags = 1 << GIT_IDXENTRY_STAGESHIFT;
 	git_oid_fromstr(&entry.id, "d427e0b2e138501a3d15cc376077a3631e15bd46");
-	entry.path = "conflicts.txt";
+	entry.path = path;
 	cl_git_pass(git_index_add(index, &entry));
 
 	entry.flags = 2 << GIT_IDXENTRY_STAGESHIFT;
@@ -919,7 +919,7 @@ void test_checkout_tree__fails_when_conflicts_exist_in_index(void)
 	cl_git_pass(git_reference_name_to_id(&oid, g_repo, "HEAD"));
 	cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY));
 
-	create_conflict();
+	create_conflict("conflicts.txt");
 
 	cl_git_fail(git_checkout_tree(g_repo, obj, &opts));
 
@@ -948,3 +948,87 @@ void test_checkout_tree__filemode_preserved_in_index(void)
 	git_commit_free(commit);
 	git_index_free(index);
 }
+
+void test_checkout_tree__removes_conflicts(void)
+{
+	git_oid commit_id;
+	git_commit *commit;
+	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
+	git_index *index;
+	
+	cl_git_pass(git_oid_fromstr(&commit_id, "afe4393b2b2a965f06acf2ca9658eaa01e0cd6b6"));
+	cl_git_pass(git_commit_lookup(&commit, g_repo, &commit_id));
+
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE;
+
+	cl_git_pass(git_checkout_tree(g_repo, (const git_object *)commit, &opts));
+
+	cl_git_pass(git_repository_index(&index, g_repo));
+	cl_git_pass(git_index_remove(index, "executable.txt", 0));
+
+	create_conflict("executable.txt");
+	cl_git_mkfile("testrepo/executable.txt", "This is the conflict file.\n");
+
+	create_conflict("other.txt");
+	cl_git_mkfile("testrepo/other.txt", "This is another conflict file.\n");
+
+	git_index_write(index);
+
+	cl_git_pass(git_checkout_tree(g_repo, (const git_object *)commit, &opts));
+
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 1));
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 2));
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 3));
+
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "other.txt", 1));
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "other.txt", 2));
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "other.txt", 3));
+
+	cl_assert(!git_path_exists("testrepo/other.txt"));
+
+	git_index_free(index);
+}
+
+
+void test_checkout_tree__removes_conflicts_only_by_pathscope(void)
+{
+	git_oid commit_id;
+	git_commit *commit;
+	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
+	git_index *index;
+	const char *path = "executable.txt";
+	
+	cl_git_pass(git_oid_fromstr(&commit_id, "afe4393b2b2a965f06acf2ca9658eaa01e0cd6b6"));
+	cl_git_pass(git_commit_lookup(&commit, g_repo, &commit_id));
+
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE;
+	opts.paths.count = 1;
+	opts.paths.strings = (char **)&path;
+
+	cl_git_pass(git_checkout_tree(g_repo, (const git_object *)commit, &opts));
+
+	cl_git_pass(git_repository_index(&index, g_repo));
+	cl_git_pass(git_index_remove(index, "executable.txt", 0));
+
+	create_conflict("executable.txt");
+	cl_git_mkfile("testrepo/executable.txt", "This is the conflict file.\n");
+
+	create_conflict("other.txt");
+	cl_git_mkfile("testrepo/other.txt", "This is another conflict file.\n");
+
+	git_index_write(index);
+
+	cl_git_pass(git_checkout_tree(g_repo, (const git_object *)commit, &opts));
+
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 1));
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 2));
+	cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 3));
+
+	cl_assert(git_index_get_bypath(index, "other.txt", 1) != NULL);
+	cl_assert(git_index_get_bypath(index, "other.txt", 2) != NULL);
+	cl_assert(git_index_get_bypath(index, "other.txt", 3) != NULL);
+
+	cl_assert(git_path_exists("testrepo/other.txt"));
+
+	git_index_free(index);
+}