index: commit the changes to the index properly

Now that the index has a "dirty" state, where it has changes that have
not yet been committed or rolled back, our tests need to be adapted to
actually commit or rollback the changes instead of assuming that the
index can be operated on in its indeterminate state.

Author: ethomson

src/checkout.c

@@ -2412,7 +2412,7 @@ static int checkout_data_init(
 		 * and those should not be overwritten.)
 		 */
 		if (data->index != git_iterator_index(target)) {
-			if ((error = git_index_read(data->index, true)) < 0)
+			if ((error = git_index_read_safely(data->index)) < 0)
 				goto cleanup;
 
 			/* cannot checkout if unresolved conflicts exist */

src/index.c

@@ -680,6 +680,17 @@ int git_index_read(git_index *index, int force)
 	return error;
 }
 
+int git_index_read_safely(git_index *index)
+{
+	if (index->dirty) {
+		giterr_set(GITERR_INDEX,
+			"the index has unsaved changes that would be overwritten by this operation");
+		return -1;
+	}
+
+	return git_index_read(index, false);
+}
+
 int git_index__changed_relative_to(
 	git_index *index, const git_oid *checksum)
 {

src/index.h

@@ -149,6 +149,8 @@ GIT_INLINE(int) git_index_is_dirty(git_index *index)
 	return index->dirty;
 }
 
+extern int git_index_read_safely(git_index *index);
+
 typedef struct {
 	git_index *index;
 	git_filebuf file;

src/status.c

@@ -294,7 +294,7 @@ int git_status_list_new(
 
 	/* refresh index from disk unless prevented */
 	if ((flags & GIT_STATUS_OPT_NO_REFRESH) == 0 &&
-		git_index_read(index, false) < 0)
+		git_index_read_safely(index) < 0)
 		giterr_clear();
 
 	status = git_status_list_alloc(index);

tests/checkout/tree.c

@@ -543,6 +543,7 @@ void assert_conflict(
 	 */
 	cl_git_pass(git_object_peel(&hack_tree, g_object, GIT_OBJ_TREE));
 	cl_git_pass(git_index_read_tree(index, (git_tree *)hack_tree));
+	cl_git_pass(git_index_write(index));
 	git_object_free(hack_tree);
 	git_object_free(g_object);
 	g_object = NULL;
@@ -671,9 +672,12 @@ void test_checkout_tree__can_cancel_checkout_from_notify(void)
 	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
 	git_oid oid;
 	git_object *obj = NULL;
+	git_index *index = NULL;
 
 	assert_on_branch(g_repo, "master");
 
+	cl_git_pass(git_repository_index(&index, g_repo));
+
 	cl_git_pass(git_reference_name_to_id(&oid, g_repo, "refs/heads/dir"));
 	cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY));
 
@@ -700,6 +704,8 @@ void test_checkout_tree__can_cancel_checkout_from_notify(void)
 	else
 		cl_assert_equal_i(4, ca.count);
 
+	cl_git_pass(git_index_read(index, 1));
+
 	/* and again with a different stopping point and return code */
 	ca.filename = "README";
 	ca.error = 123;
@@ -715,6 +721,7 @@ void test_checkout_tree__can_cancel_checkout_from_notify(void)
 		cl_assert_equal_i(1, ca.count);
 
 	git_object_free(obj);
+	git_index_free(index);
 }
 
 void test_checkout_tree__can_checkout_with_last_workdir_item_missing(void)
@@ -739,7 +746,9 @@ void test_checkout_tree__can_checkout_with_last_workdir_item_missing(void)
 	cl_git_mkfile("./testrepo/this-is-dir/contained_file", "content\n");
 
 	cl_git_pass(git_index_add_bypath(index, "this-is-dir/contained_file"));
-	git_index_write_tree(&tree_id, index);
+	cl_git_pass(git_index_write(index));
+
+	cl_git_pass(git_index_write_tree(&tree_id, index));
 	cl_git_pass(git_tree_lookup(&tree, g_repo, &tree_id));
 
 	cl_git_pass(p_unlink("./testrepo/this-is-dir/contained_file"));
@@ -1107,7 +1116,7 @@ void test_checkout_tree__removes_conflicts(void)
 	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));
 
@@ -1150,7 +1159,7 @@ void test_checkout_tree__removes_conflicts_only_by_pathscope(void)
 	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));
 
@@ -1248,7 +1257,7 @@ void test_checkout_tree__case_changing_rename(void)
 
 	cl_git_pass(git_checkout_tree(g_repo, (git_object *)master_commit, &opts));
 	cl_git_pass(git_repository_set_head(g_repo, "refs/heads/master"));
-	
+
 	assert_on_branch(g_repo, "master");
 
 	cl_assert(git_path_isfile("testrepo/README"));
@@ -1493,8 +1502,11 @@ void test_checkout_tree__baseline_is_empty_when_no_index(void)
 	git_reference *head;
 	git_object *obj;
 	size_t conflicts = 0;
+	git_index *index;
 
 	assert_on_branch(g_repo, "master");
+
+	cl_git_pass(git_repository_index(&index, g_repo));
 	cl_git_pass(git_repository_head(&head, g_repo));
 	cl_git_pass(git_reference_peel(&obj, head, GIT_OBJ_COMMIT));
 
@@ -1513,6 +1525,8 @@ void test_checkout_tree__baseline_is_empty_when_no_index(void)
 	cl_git_fail_with(GIT_ECONFLICT, git_checkout_tree(g_repo, obj, &opts));
 	cl_assert_equal_i(4, conflicts);
 
+	cl_git_pass(git_index_read(index, 1));
+
 	/* but force should succeed and update the index */
 	opts.checkout_strategy |= GIT_CHECKOUT_FORCE;
 	cl_git_pass(git_checkout_tree(g_repo, obj, &opts));
@@ -1521,6 +1535,7 @@ void test_checkout_tree__baseline_is_empty_when_no_index(void)
 
 	git_object_free(obj);
 	git_reference_free(head);
+	git_index_free(index);
 }
 
 void test_checkout_tree__mode_change_is_force_updated(void)
@@ -1551,8 +1566,12 @@ void test_checkout_tree__mode_change_is_force_updated(void)
 	cl_must_pass(p_chmod("testrepo/README", 0755));
 	cl_must_pass(git_index_add_bypath(index, "README"));
 
+	cl_git_pass(git_index_write(index));
 	assert_status_entrycount(g_repo, 1);
+
 	cl_git_pass(git_checkout_tree(g_repo, obj, &g_opts));
+	cl_git_pass(git_index_write(index));
+
 	assert_status_entrycount(g_repo, 0);
 
 	git_object_free(obj);

tests/checkout/typechange.c

@@ -259,6 +259,7 @@ static int make_submodule_dirty(git_submodule *sm, const char *name, void *paylo
 void test_checkout_typechange__checkout_with_conflicts(void)
 {
 	int i;
+	git_index *index;
 	git_object *obj;
 	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
 	notify_counts cts = {0};
@@ -289,6 +290,10 @@ void test_checkout_typechange__checkout_with_conflicts(void)
 		cl_assert_equal_i(cts.updates, 0);
 		cl_assert_equal_i(cts.ignored, 0);
 
+		cl_git_pass(git_repository_index(&index, g_repo));
+		cl_git_pass(git_index_read(index, 1));
+		git_index_free(index);
+
 		opts.checkout_strategy =
 			GIT_CHECKOUT_FORCE | GIT_CHECKOUT_REMOVE_UNTRACKED;
 		memset(&cts, 0, sizeof(cts));

tests/index/names.c

@@ -86,6 +86,8 @@ void test_index_names__add(void)
 	cl_assert(strcmp(conflict_name->ancestor, "ancestor3") == 0);
 	cl_assert(conflict_name->ours == NULL);
 	cl_assert(strcmp(conflict_name->theirs, "theirs3") == 0);
+
+	cl_git_pass(git_index_write(repo_index));
 }
 
 void test_index_names__roundtrip(void)
@@ -151,12 +153,12 @@ void test_index_names__cleaned_on_checkout_tree(void)
 	git_object *obj;
 	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
 
-	opts.checkout_strategy = GIT_CHECKOUT_SAFE | GIT_CHECKOUT_UPDATE_ONLY;
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE | GIT_CHECKOUT_UPDATE_ONLY;
 
 	test_index_names__add();
-	git_reference_name_to_id(&oid, repo, "refs/heads/master");
-	git_object_lookup(&obj, repo, &oid, GIT_OBJ_ANY);
-	git_checkout_tree(repo, obj, &opts);
+	cl_git_pass(git_reference_name_to_id(&oid, repo, "refs/heads/master"));
+	cl_git_pass(git_object_lookup(&obj, repo, &oid, GIT_OBJ_ANY));
+	cl_git_pass(git_checkout_tree(repo, obj, &opts));
 	cl_assert_equal_sz(0, git_index_name_entrycount(repo_index));
 
 	git_object_free(obj);
@@ -166,20 +168,20 @@ void test_index_names__cleaned_on_checkout_head(void)
 {
 	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
 
-	opts.checkout_strategy = GIT_CHECKOUT_SAFE | GIT_CHECKOUT_UPDATE_ONLY;
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE | GIT_CHECKOUT_UPDATE_ONLY;
 
 	test_index_names__add();
-	git_checkout_head(repo, &opts);
+	cl_git_pass(git_checkout_head(repo, &opts));
 	cl_assert_equal_sz(0, git_index_name_entrycount(repo_index));
 }
 
 void test_index_names__retained_on_checkout_index(void)
 {
 	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
 
-	opts.checkout_strategy = GIT_CHECKOUT_SAFE | GIT_CHECKOUT_UPDATE_ONLY;
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE | GIT_CHECKOUT_UPDATE_ONLY;
 
 	test_index_names__add();
-	git_checkout_index(repo, repo_index, &opts);
+	cl_git_pass(git_checkout_index(repo, repo_index, &opts));
 	cl_assert(git_index_name_entrycount(repo_index) > 0);
 }

tests/index/reuc.c

@@ -56,6 +56,8 @@ void test_index_reuc__add(void)
 	cl_assert_equal_oid(&reuc->oid[0], &ancestor_oid);
 	cl_assert_equal_oid(&reuc->oid[1], &our_oid);
 	cl_assert_equal_oid(&reuc->oid[2], &their_oid);
+
+	cl_git_pass(git_index_write(repo_index));
 }
 
 void test_index_reuc__add_no_ancestor(void)
@@ -81,6 +83,8 @@ void test_index_reuc__add_no_ancestor(void)
 	cl_assert_equal_oid(&reuc->oid[0], &ancestor_oid);
 	cl_assert_equal_oid(&reuc->oid[1], &our_oid);
 	cl_assert_equal_oid(&reuc->oid[2], &their_oid);
+
+	cl_git_pass(git_index_write(repo_index));
 }
 
 void test_index_reuc__read_bypath(void)

tests/rebase/submodule.c

@@ -33,8 +33,9 @@ void test_rebase_submodule__initialize(void)
 	/* We have to commit the rewritten .gitmodules file */
 	cl_git_pass(git_repository_index(&index, repo));
 	cl_git_pass(git_index_add_bypath(index, ".gitmodules"));
-	cl_git_pass(git_index_write_tree(&tree_oid, index));
+	cl_git_pass(git_index_write(index));
 
+	cl_git_pass(git_index_write_tree(&tree_oid, index));
 	cl_git_pass(git_tree_lookup(&tree, repo, &tree_oid));
 
 	cl_git_pass(git_repository_head(&master_ref, repo));