Commit 817d625161f212b86c22733f7dde2f2155a65ac5

Russell Belfer 2013-01-03T16:56:27

Fix checkout of index-only dirs and prefixed paths There are a couple of checkout bugs fixed here. One is with untracked working directory entries that are prefixes of tree entries but not in a meaningful way (i.e. "read" is a prefix of "readme.txt" but doesn't interfere in any way). The second bug is actually a redo of 07edfa0fc640f85f95507c3101e77accd7d2bf0d where directory entries in the index that are not in the diff were not being removed correctly. That fix remedied one case but broke another.

diff --git a/src/checkout.c b/src/checkout.c
index 261dee1..cf0a8b8 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -220,19 +220,34 @@ static int checkout_action_wd_only(
 {
 	bool remove = false;
 	git_checkout_notify_t notify = GIT_CHECKOUT_NOTIFY_NONE;
-	const git_index_entry *entry;
 
 	if (!git_pathspec_match_path(
 			pathspec, wd->path, false, workdir->ignore_case))
 		return 0;
 
 	/* check if item is tracked in the index but not in the checkout diff */
-	if (data->index != NULL &&
-		(entry = git_index_get_bypath(data->index, wd->path, 0)) != NULL)
-	{
-		notify = GIT_CHECKOUT_NOTIFY_DIRTY;
-		remove = ((data->strategy & GIT_CHECKOUT_FORCE) != 0);
+	if (data->index != NULL) {
+		if (wd->mode != GIT_FILEMODE_TREE) {
+			if (git_index_get_bypath(data->index, wd->path, 0) != NULL) {
+				notify = GIT_CHECKOUT_NOTIFY_DIRTY;
+				remove = ((data->strategy & GIT_CHECKOUT_FORCE) != 0);
+			}
+		} else {
+			/* for tree entries, we have to see if there are any index
+			 * entries that are contained inside that tree
+			 */
+			size_t pos = git_index__prefix_position(data->index, wd->path);
+			const git_index_entry *e = git_index_get_byindex(data->index, pos);
+
+			if (e != NULL && data->diff->pfxcomp(e->path, wd->path) == 0) {
+				notify = GIT_CHECKOUT_NOTIFY_DIRTY;
+				remove = ((data->strategy & GIT_CHECKOUT_FORCE) != 0);
+			}
+		}
 	}
+
+	if (notify != GIT_CHECKOUT_NOTIFY_NONE)
+		/* found in index */;
 	else if (git_iterator_current_is_ignored(workdir)) {
 		notify = GIT_CHECKOUT_NOTIFY_IGNORED;
 		remove = ((data->strategy & GIT_CHECKOUT_REMOVE_IGNORED) != 0);
@@ -418,8 +433,6 @@ static int checkout_action_with_wd_dir(
 	return checkout_action_common(data, action, delta, wd);
 }
 
-#define EXPAND_DIRS_FOR_STRATEGY (GIT_CHECKOUT_FORCE | GIT_CHECKOUT_REMOVE_UNTRACKED | GIT_CHECKOUT_REMOVE_IGNORED)
-
 static int checkout_action(
 	checkout_data *data,
 	git_diff_delta *delta,
@@ -431,7 +444,6 @@ static int checkout_action(
 	int cmp = -1, act;
 	int (*strcomp)(const char *, const char *) = data->diff->strcomp;
 	int (*pfxcomp)(const char *str, const char *pfx) = data->diff->pfxcomp;
-	bool expand_dirs = (data->strategy & EXPAND_DIRS_FOR_STRATEGY) != 0;
 
 	/* move workdir iterator to follow along with deltas */
 
@@ -452,18 +464,21 @@ static int checkout_action(
 		if (cmp < 0) {
 			cmp = pfxcomp(delta->old_file.path, wd->path);
 
-			if (wd->mode == GIT_FILEMODE_TREE && (cmp == 0 || expand_dirs)) {
-				/* case 2 or untracked wd item that might need removal */
-				if (git_iterator_advance_into_directory(workdir, &wd) < 0)
-					goto fail;
-				continue;
-			}
-
 			if (cmp == 0) {
-				/* case 3 -  wd contains non-dir where dir expected */
-				act = checkout_action_with_wd_blocker(data, delta, wd);
-				*wditem_ptr = git_iterator_advance(workdir, &wd) ? NULL : wd;
-				return act;
+				if (wd->mode == GIT_FILEMODE_TREE) {
+					/* case 2 - entry prefixed by workdir tree */
+					if (git_iterator_advance_into_directory(workdir, &wd) < 0)
+						goto fail;
+					continue;
+				}
+
+				/* case 3 maybe - wd contains non-dir where dir expected */
+				if (delta->old_file.path[strlen(wd->path)] == '/') {
+					act = checkout_action_with_wd_blocker(data, delta, wd);
+					*wditem_ptr =
+						git_iterator_advance(workdir, &wd) ? NULL : wd;
+					return act;
+				}
 			}
 
 			/* case 1 - handle wd item (if it matches pathspec) */
@@ -485,8 +500,7 @@ static int checkout_action(
 		cmp = pfxcomp(wd->path, delta->old_file.path);
 
 		if (cmp == 0) { /* case 5 */
-			size_t pathlen = strlen(delta->old_file.path);
-			if (wd->path[pathlen] != '/')
+			if (wd->path[strlen(delta->old_file.path)] != '/')
 				return checkout_action_no_wd(data, delta);
 
 			if (delta->status == GIT_DELTA_TYPECHANGE) {
@@ -529,13 +543,9 @@ static int checkout_remaining_wd_items(
 	git_vector *spec)
 {
 	int error = 0;
-	bool expand_dirs = (data->strategy & EXPAND_DIRS_FOR_STRATEGY) != 0;
 
 	while (wd && !error) {
-		if (wd->mode == GIT_FILEMODE_TREE && expand_dirs)
-			error = git_iterator_advance_into_directory(workdir, &wd);
-
-		else if (!(error = checkout_action_wd_only(data, workdir, wd, spec)))
+		if (!(error = checkout_action_wd_only(data, workdir, wd, spec)))
 			error = git_iterator_advance(workdir, &wd);
 	}
 
@@ -945,7 +955,10 @@ static int checkout_remove_the_old(
 		if ((data->strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) == 0 &&
 			data->index != NULL)
 		{
-			(void)git_index_remove(data->index, str, 0);
+			if (str[strlen(str) - 1] == '/')
+				(void)git_index_remove_directory(data->index, str, 0);
+			else
+				(void)git_index_remove(data->index, str, 0);
 		}
 	}
 
diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c
index b1778a4..fe1f687 100644
--- a/tests-clar/checkout/index.c
+++ b/tests-clar/checkout/index.c
@@ -496,3 +496,28 @@ void test_checkout_index__validates_struct_version(void)
 	err = giterr_last();
 	cl_assert_equal_i(err->klass, GITERR_INVALID);
 }
+
+void test_checkout_index__can_update_prefixed_files(void)
+{
+	git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT;
+
+	cl_git_mkfile("./testrepo/READ", "content\n");
+	cl_git_mkfile("./testrepo/README.after", "content\n");
+	cl_git_pass(p_mkdir("./testrepo/branch_file", 0777));
+	cl_git_pass(p_mkdir("./testrepo/branch_file/contained_dir", 0777));
+	cl_git_mkfile("./testrepo/branch_file/contained_file", "content\n");
+	cl_git_pass(p_mkdir("./testrepo/branch_file.txt.after", 0777));
+
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE | GIT_CHECKOUT_REMOVE_UNTRACKED;
+
+	cl_git_pass(git_checkout_index(g_repo, NULL, &opts));
+
+	test_file_contents("./testrepo/README", "hey there\n");
+	test_file_contents("./testrepo/branch_file.txt", "hi\nbye!\n");
+	test_file_contents("./testrepo/new.txt", "my new file\n");
+
+	cl_assert(!git_path_exists("testrepo/READ"));
+	cl_assert(!git_path_exists("testrepo/README.after"));
+	cl_assert(!git_path_exists("testrepo/branch_file"));
+	cl_assert(!git_path_exists("testrepo/branch_file.txt.after"));
+}