Merge pull request #2691 from libgit2/cmn/submodule-and-dir

submodules: stale module entries

12 files changed, 167 insertions, 31 deletions

diff --git a/src/checkout.c b/src/checkout.c
index 8203c39ea..44e2f3b27 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -2242,6 +2242,7 @@ cleanup:
 
 int git_checkout_iterator(
 	git_iterator *target,
+	git_index *index,
 	const git_checkout_options *opts)
 {
 	int error = 0;
@@ -2278,7 +2279,7 @@ int git_checkout_iterator(
 
 	if ((error = git_iterator_reset(target, data.pfx, data.pfx)) < 0 ||
 		(error = git_iterator_for_workdir_ext(
-			&workdir, data.repo, data.opts.target_directory,
+			&workdir, data.repo, data.opts.target_directory, index, NULL,
 			iterflags | GIT_ITERATOR_DONT_AUTOEXPAND,
 			data.pfx, data.pfx)) < 0 ||
 		(error = git_iterator_for_tree(
@@ -2388,7 +2389,7 @@ int git_checkout_index(
 	GIT_REFCOUNT_INC(index);
 
 	if (!(error = git_iterator_for_index(&index_i, index, 0, NULL, NULL)))
-		error = git_checkout_iterator(index_i, opts);
+		error = git_checkout_iterator(index_i, index, opts);
 
 	if (owned)
 		GIT_REFCOUNT_OWN(index, NULL);
@@ -2405,6 +2406,7 @@ int git_checkout_tree(
 	const git_checkout_options *opts)
 {
 	int error;
+	git_index *index;
 	git_tree *tree = NULL;
 	git_iterator *tree_i = NULL;
 
@@ -2439,10 +2441,14 @@ int git_checkout_tree(
 		}
 	}
 
+	if ((error = git_repository_index(&index, repo)) < 0)
+		return error;
+
 	if (!(error = git_iterator_for_tree(&tree_i, tree, 0, NULL, NULL)))
-		error = git_checkout_iterator(tree_i, opts);
+		error = git_checkout_iterator(tree_i, index, opts);
 
 	git_iterator_free(tree_i);
+	git_index_free(index);
 	git_tree_free(tree);
 
 	return error;
diff --git a/src/checkout.h b/src/checkout.h
index f1fe69628..60aa29b26 100644
--- a/src/checkout.h
+++ b/src/checkout.h
@@ -19,6 +19,7 @@
  */
 extern int git_checkout_iterator(
 	git_iterator *target,
+	git_index *index,
 	const git_checkout_options *opts);
 
 #endif
diff --git a/src/diff.c b/src/diff.c
index 375d4cb13..89b3b77f3 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -1214,7 +1214,7 @@ int git_diff_index_to_workdir(
 	DIFF_FROM_ITERATORS(
 		git_iterator_for_index(&a, index, 0, pfx, pfx),
 		git_iterator_for_workdir(
-			&b, repo, GIT_ITERATOR_DONT_AUTOEXPAND, pfx, pfx)
+			&b, repo, index, NULL, GIT_ITERATOR_DONT_AUTOEXPAND, pfx, pfx)
 	);
 
 	if (!error && DIFF_FLAG_IS_SET(*diff, GIT_DIFF_UPDATE_INDEX))
@@ -1230,15 +1230,20 @@ int git_diff_tree_to_workdir(
 	const git_diff_options *opts)
 {
 	int error = 0;
+	git_index *index;
 
 	assert(diff && repo);
 
+	if ((error = git_repository_index(&index, repo)))
+		return error;
+
 	DIFF_FROM_ITERATORS(
 		git_iterator_for_tree(&a, old_tree, 0, pfx, pfx),
 		git_iterator_for_workdir(
-			&b, repo, GIT_ITERATOR_DONT_AUTOEXPAND, pfx, pfx)
+			&b, repo, index, old_tree, GIT_ITERATOR_DONT_AUTOEXPAND, pfx, pfx)
 	);
 
+	git_index_free(index);
 	return error;
 }
 
diff --git a/src/index.c b/src/index.c
index 8a5bf61c1..d3bc081a5 100644
--- a/src/index.c
+++ b/src/index.c
@@ -2441,7 +2441,7 @@ int git_index_add_all(
 		goto cleanup;
 
 	if ((error = git_iterator_for_workdir(
-			&wditer, repo, 0, ps.prefix, ps.prefix)) < 0)
+			&wditer, repo, NULL, NULL, 0, ps.prefix, ps.prefix)) < 0)
 		goto cleanup;
 
 	while (!(error = git_iterator_advance(&wd, wditer))) {
diff --git a/src/iterator.c b/src/iterator.c
index c664f17cd..d8a17a716 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -1268,6 +1268,16 @@ typedef struct {
 	fs_iterator fi;
 	git_ignores ignores;
 	int is_ignored;
+
+	/*
+	 * We may have a tree or the index+snapshot to compare against
+	 * when checking for submodules.
+	 */
+	git_tree *tree;
+	git_index *index;
+	git_vector index_snapshot;
+	git_vector_cmp entry_srch;
+
 } workdir_iterator;
 
 GIT_INLINE(bool) workdir_path_is_dotgit(const git_buf *path)
@@ -1289,6 +1299,49 @@ GIT_INLINE(bool) workdir_path_is_dotgit(const git_buf *path)
 	return (len == 4 || path->ptr[len - 5] == '/');
 }
 
+/**
+ * Figure out if an entry is a submodule.
+ *
+ * We consider it a submodule if the path is listed as a submodule in
+ * either the tree or the index.
+ */
+static int is_submodule(workdir_iterator *wi, git_path_with_stat *ie)
+{
+	int error, is_submodule = 0;
+
+	if (wi->tree) {
+		git_tree_entry *e;
+
+		/* remove the trailing slash for finding */
+		ie->path[ie->path_len-1] = '\0';
+		error = git_tree_entry_bypath(&e, wi->tree, ie->path);
+		ie->path[ie->path_len-1] = '/';
+		if (error < 0 && error != GIT_ENOTFOUND)
+			return 0;
+		if (!error) {
+			is_submodule = e->attr == GIT_FILEMODE_COMMIT;
+			git_tree_entry_free(e);
+		}
+	}
+
+	if (!is_submodule && wi->index) {
+		git_index_entry *e;
+		size_t pos;
+
+		error = git_index_snapshot_find(&pos, &wi->index_snapshot, wi->entry_srch, ie->path, ie->path_len-1, 0);
+		if (error < 0 && error != GIT_ENOTFOUND)
+			return 0;
+
+		if (!error) {
+			e = git_vector_get(&wi->index_snapshot, pos);
+
+			is_submodule = e->mode == GIT_FILEMODE_COMMIT;
+		}
+	}
+
+	return is_submodule;
+}
+
 static int workdir_iterator__enter_dir(fs_iterator *fi)
 {
 	workdir_iterator *wi = (workdir_iterator *)fi;
@@ -1321,7 +1374,7 @@ static int workdir_iterator__enter_dir(fs_iterator *fi)
 		if (!S_ISDIR(entry->st.st_mode) || !strcmp(GIT_DIR, entry->path))
 			continue;
 
-		if (git_submodule__is_submodule(fi->base.repo, entry->path)) {
+		if (is_submodule(wi, entry)) {
 			entry->st.st_mode = GIT_FILEMODE_COMMIT;
 			entry->path_len--;
 			entry->path[entry->path_len] = '\0';
@@ -1363,6 +1416,8 @@ static int workdir_iterator__update_entry(fs_iterator *fi)
 static void workdir_iterator__free(git_iterator *self)
 {
 	workdir_iterator *wi = (workdir_iterator *)self;
+	if (wi->index)
+		git_index_snapshot_release(&wi->index_snapshot, wi->index);
 	fs_iterator__free(self);
 	git_ignore__free(&wi->ignores);
 }
@@ -1371,6 +1426,8 @@ int git_iterator_for_workdir_ext(
 	git_iterator **out,
 	git_repository *repo,
 	const char *repo_workdir,
+	git_index *index,
+	git_tree *tree,
 	git_iterator_flag_t flags,
 	const char *start,
 	const char *end)
@@ -1402,6 +1459,18 @@ int git_iterator_for_workdir_ext(
 		return error;
 	}
 
+	if (tree && (error = git_object_dup((git_object **)&wi->tree, (git_object *)tree)) < 0)
+		return error;
+
+	wi->index = index;
+	if (index && (error = git_index_snapshot_new(&wi->index_snapshot, index)) < 0) {
+		git_iterator_free((git_iterator *)wi);
+		return error;
+	}
+	wi->entry_srch = iterator__ignore_case(wi) ?
+		git_index_entry_isrch : git_index_entry_srch;
+
+
 	/* try to look up precompose and set flag if appropriate */
 	if (git_repository__cvar(&precompose, repo, GIT_CVAR_PRECOMPOSE) < 0)
 		giterr_clear();
diff --git a/src/iterator.h b/src/iterator.h
index d88ad5191..1520bffc2 100644
--- a/src/iterator.h
+++ b/src/iterator.h
@@ -86,6 +86,8 @@ extern int git_iterator_for_workdir_ext(
 	git_iterator **out,
 	git_repository *repo,
 	const char *repo_workdir,
+	git_index *index,
+	git_tree *tree,
 	git_iterator_flag_t flags,
 	const char *start,
 	const char *end);
@@ -96,11 +98,13 @@ extern int git_iterator_for_workdir_ext(
 GIT_INLINE(int) git_iterator_for_workdir(
 	git_iterator **out,
 	git_repository *repo,
+	git_index *index,
+	git_tree *tree,
 	git_iterator_flag_t flags,
 	const char *start,
 	const char *end)
 {
-	return git_iterator_for_workdir_ext(out, repo, NULL, flags, start, end);
+	return git_iterator_for_workdir_ext(out, repo, NULL, index, tree, flags, start, end);
 }
 
 /* for filesystem iterators, you have to explicitly pass in the ignore_case
diff --git a/src/pathspec.c b/src/pathspec.c
index a01d74f07..8b469f717 100644
--- a/src/pathspec.c
+++ b/src/pathspec.c
@@ -524,7 +524,7 @@ int git_pathspec_match_workdir(
 	assert(repo);
 
 	if (!(error = git_iterator_for_workdir(
-			&iter, repo, pathspec_match_iter_flags(flags), NULL, NULL))) {
+			&iter, repo, NULL, NULL, pathspec_match_iter_flags(flags), NULL, NULL))) {
 
 		error = pathspec_match_from_iterator(out, iter, flags, ps);
 
diff --git a/tests/diff/iterator.c b/tests/diff/iterator.c
index 26f670cfa..6011c6a9b 100644
--- a/tests/diff/iterator.c
+++ b/tests/diff/iterator.c
@@ -586,7 +586,7 @@ static void workdir_iterator_test(
 	git_repository *repo = cl_git_sandbox_init(sandbox);
 
 	cl_git_pass(git_iterator_for_workdir(
-		&i, repo, GIT_ITERATOR_DONT_AUTOEXPAND, start, end));
+		&i, repo, NULL, NULL, GIT_ITERATOR_DONT_AUTOEXPAND, start, end));
 
 	error = git_iterator_current(&entry, i);
 	cl_assert((error == 0 && entry != NULL) ||
@@ -797,7 +797,7 @@ void test_diff_iterator__workdir_builtin_ignores(void)
 	cl_git_mkfile("attr/sub/.git", "whatever");
 
 	cl_git_pass(git_iterator_for_workdir(
-		&i, repo, GIT_ITERATOR_DONT_AUTOEXPAND, "dir", "sub/sub/file"));
+		&i, repo, NULL, NULL, GIT_ITERATOR_DONT_AUTOEXPAND, "dir", "sub/sub/file"));
 	cl_git_pass(git_iterator_current(&entry, i));
 
 	for (idx = 0; entry != NULL; ++idx) {
@@ -832,7 +832,7 @@ static void check_wd_first_through_third_range(
 	static const char *expected[] = { "FIRST", "second", "THIRD", NULL };
 
 	cl_git_pass(git_iterator_for_workdir(
-		&i, repo, GIT_ITERATOR_IGNORE_CASE, start, end));
+		&i, repo, NULL, NULL, GIT_ITERATOR_IGNORE_CASE, start, end));
 	cl_git_pass(git_iterator_current(&entry, i));
 
 	for (idx = 0; entry != NULL; ++idx) {
diff --git a/tests/repo/iterator.c b/tests/repo/iterator.c
index 764c2c6cd..fb5561bc2 100644
--- a/tests/repo/iterator.c
+++ b/tests/repo/iterator.c
@@ -665,19 +665,19 @@ void test_repo_iterator__workdir(void)
 	g_repo = cl_git_sandbox_init("icase");
 
 	/* auto expand with no tree entries */
-	cl_git_pass(git_iterator_for_workdir(&i, g_repo, 0, NULL, NULL));
+	cl_git_pass(git_iterator_for_workdir(&i, g_repo, NULL, NULL, 0, NULL, NULL));
 	expect_iterator_items(i, 20, NULL, 20, NULL);
 	git_iterator_free(i);
 
 	/* auto expand with tree entries */
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, GIT_ITERATOR_INCLUDE_TREES, NULL, NULL));
+		&i, g_repo, NULL, NULL, GIT_ITERATOR_INCLUDE_TREES, NULL, NULL));
 	expect_iterator_items(i, 22, NULL, 22, NULL);
 	git_iterator_free(i);
 
 	/* no auto expand (implies trees included) */
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, GIT_ITERATOR_DONT_AUTOEXPAND, NULL, NULL));
+		&i, g_repo, NULL, NULL, GIT_ITERATOR_DONT_AUTOEXPAND, NULL, NULL));
 	expect_iterator_items(i, 12, NULL, 22, NULL);
 	git_iterator_free(i);
 }
@@ -692,66 +692,66 @@ void test_repo_iterator__workdir_icase(void)
 	flag = GIT_ITERATOR_DONT_IGNORE_CASE;
 
 	/* auto expand with no tree entries */
-	cl_git_pass(git_iterator_for_workdir(&i, g_repo, flag, "c", "k/D"));
+	cl_git_pass(git_iterator_for_workdir(&i, g_repo, NULL, NULL, flag, "c", "k/D"));
 	expect_iterator_items(i, 7, NULL, 7, NULL);
 	git_iterator_free(i);
 
-	cl_git_pass(git_iterator_for_workdir(&i, g_repo, flag, "k", "k/Z"));
+	cl_git_pass(git_iterator_for_workdir(&i, g_repo, NULL, NULL, flag, "k", "k/Z"));
 	expect_iterator_items(i, 3, NULL, 3, NULL);
 	git_iterator_free(i);
 
 	/* auto expand with tree entries */
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, flag | GIT_ITERATOR_INCLUDE_TREES, "c", "k/D"));
+		&i, g_repo, NULL, NULL, flag | GIT_ITERATOR_INCLUDE_TREES, "c", "k/D"));
 	expect_iterator_items(i, 8, NULL, 8, NULL);
 	git_iterator_free(i);
 
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, flag | GIT_ITERATOR_INCLUDE_TREES, "k", "k/Z"));
+		&i, g_repo, NULL, NULL, flag | GIT_ITERATOR_INCLUDE_TREES, "k", "k/Z"));
 	expect_iterator_items(i, 4, NULL, 4, NULL);
 	git_iterator_free(i);
 
 	/* no auto expand (implies trees included) */
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "c", "k/D"));
+		&i, g_repo, NULL, NULL, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "c", "k/D"));
 	expect_iterator_items(i, 5, NULL, 8, NULL);
 	git_iterator_free(i);
 
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "k", "k/Z"));
+		&i, g_repo, NULL, NULL, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "k", "k/Z"));
 	expect_iterator_items(i, 1, NULL, 4, NULL);
 	git_iterator_free(i);
 
 	flag = GIT_ITERATOR_IGNORE_CASE;
 
 	/* auto expand with no tree entries */
-	cl_git_pass(git_iterator_for_workdir(&i, g_repo, flag, "c", "k/D"));
+	cl_git_pass(git_iterator_for_workdir(&i, g_repo, NULL, NULL, flag, "c", "k/D"));
 	expect_iterator_items(i, 13, NULL, 13, NULL);
 	git_iterator_free(i);
 
-	cl_git_pass(git_iterator_for_workdir(&i, g_repo, flag, "k", "k/Z"));
+	cl_git_pass(git_iterator_for_workdir(&i, g_repo, NULL, NULL, flag, "k", "k/Z"));
 	expect_iterator_items(i, 5, NULL, 5, NULL);
 	git_iterator_free(i);
 
 	/* auto expand with tree entries */
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, flag | GIT_ITERATOR_INCLUDE_TREES, "c", "k/D"));
+		&i, g_repo, NULL, NULL, flag | GIT_ITERATOR_INCLUDE_TREES, "c", "k/D"));
 	expect_iterator_items(i, 14, NULL, 14, NULL);
 	git_iterator_free(i);
 
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, flag | GIT_ITERATOR_INCLUDE_TREES, "k", "k/Z"));
+		&i, g_repo, NULL, NULL, flag | GIT_ITERATOR_INCLUDE_TREES, "k", "k/Z"));
 	expect_iterator_items(i, 6, NULL, 6, NULL);
 	git_iterator_free(i);
 
 	/* no auto expand (implies trees included) */
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "c", "k/D"));
+		&i, g_repo, NULL, NULL, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "c", "k/D"));
 	expect_iterator_items(i, 9, NULL, 14, NULL);
 	git_iterator_free(i);
 
 	cl_git_pass(git_iterator_for_workdir(
-		&i, g_repo, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "k", "k/Z"));
+		&i, g_repo, NULL, NULL, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "k", "k/Z"));
 	expect_iterator_items(i, 1, NULL, 6, NULL);
 	git_iterator_free(i);
 }
@@ -804,13 +804,13 @@ void test_repo_iterator__workdir_depth(void)
 	build_workdir_tree("icase/dir02/sUB01", 50, 0);
 
 	/* auto expand with no tree entries */
-	cl_git_pass(git_iterator_for_workdir(&iter, g_repo, 0, NULL, NULL));
+	cl_git_pass(git_iterator_for_workdir(&iter, g_repo, NULL, NULL, 0, NULL, NULL));
 	expect_iterator_items(iter, 125, NULL, 125, NULL);
 	git_iterator_free(iter);
 
 	/* auto expand with tree entries (empty dirs silently skipped) */
 	cl_git_pass(git_iterator_for_workdir(
-		&iter, g_repo, GIT_ITERATOR_INCLUDE_TREES, NULL, NULL));
+		&iter, g_repo, NULL, NULL, GIT_ITERATOR_INCLUDE_TREES, NULL, NULL));
 	expect_iterator_items(iter, 337, NULL, 337, NULL);
 	git_iterator_free(iter);
 }
diff --git a/tests/status/submodules.c b/tests/status/submodules.c
index 63cf73f36..b0bb4524f 100644
--- a/tests/status/submodules.c
+++ b/tests/status/submodules.c
@@ -476,3 +476,51 @@ void test_status_submodules__broken_stuff_that_git_allows(void)
 	cl_assert_equal_i(7, counts.entry_count);
 }
 
+void test_status_submodules__entry_but_dir_tracked(void)
+{
+	git_repository *repo;
+	git_status_list *status;
+	git_diff *diff;
+	git_index *index;
+	git_tree *tree;
+
+	cl_git_pass(git_repository_init(&repo, "mixed-submodule", 0));
+	cl_git_mkfile("mixed-submodule/.gitmodules", "[submodule \"sub\"]\n path = sub\n url = ../foo\n");
+	cl_git_pass(p_mkdir("mixed-submodule/sub", 0777));
+	cl_git_mkfile("mixed-submodule/sub/file", "");
+
+	/* Create the commit with sub/file as a file, and an entry for sub in the modules list */
+	{
+		git_oid tree_id, commit_id;
+		git_signature *sig;
+		git_reference *ref;
+
+		cl_git_pass(git_repository_index(&index, repo));
+		cl_git_pass(git_index_add_bypath(index, ".gitmodules"));
+		cl_git_pass(git_index_add_bypath(index, "sub/file"));
+		cl_git_pass(git_index_write(index));
+		cl_git_pass(git_index_write_tree(&tree_id, index));
+		cl_git_pass(git_signature_now(&sig, "Sloppy Submoduler", "sloppy@example.com"));
+		cl_git_pass(git_tree_lookup(&tree, repo, &tree_id));
+		cl_git_pass(git_commit_create(&commit_id, repo, NULL, sig, sig, NULL, "message", tree, 0, NULL));
+		cl_git_pass(git_reference_create(&ref, repo, "refs/heads/master", &commit_id, 1, sig, "commit: foo"));
+		git_reference_free(ref);
+		git_signature_free(sig);
+	}
+
+	cl_git_pass(git_diff_tree_to_index(&diff, repo, tree, index, NULL));
+	cl_assert_equal_i(0, git_diff_num_deltas(diff));
+	git_diff_free(diff);
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, repo, index, NULL));
+	cl_assert_equal_i(0, git_diff_num_deltas(diff));
+	git_diff_free(diff);
+
+	cl_git_pass(git_status_list_new(&status, repo, NULL));
+	cl_assert_equal_i(0, git_status_list_entrycount(status));
+
+	git_status_list_free(status);
+	git_index_free(index);
+	git_tree_free(tree);
+	git_repository_free(repo);
+}
diff --git a/tests/submodule/status.c b/tests/submodule/status.c
index 6473491b9..6efae35c6 100644
--- a/tests/submodule/status.c
+++ b/tests/submodule/status.c
@@ -317,14 +317,17 @@ void test_submodule_status__iterator(void)
 	};
 	submodule_expectations exp = { 0, expected, expected_flags };
 	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	git_index *index;
 
-	cl_git_pass(git_iterator_for_workdir(&iter, g_repo,
+	cl_git_pass(git_repository_index(&index, g_repo));
+	cl_git_pass(git_iterator_for_workdir(&iter, g_repo, index, NULL,
 		GIT_ITERATOR_IGNORE_CASE | GIT_ITERATOR_INCLUDE_TREES, NULL, NULL));
 
 	for (i = 0; !git_iterator_advance(&entry, iter); ++i)
 		cl_assert_equal_s(expected[i], entry->path);
 
 	git_iterator_free(iter);
+	git_index_free(index);
 
 	opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED |
 		GIT_STATUS_OPT_INCLUDE_UNMODIFIED |
diff --git a/tests/threads/iterator.c b/tests/threads/iterator.c
index 8aeae1a6c..8a2d79c2e 100644
--- a/tests/threads/iterator.c
+++ b/tests/threads/iterator.c
@@ -16,7 +16,7 @@ static void *run_workdir_iterator(void *arg)
 	const git_index_entry *entry = NULL;
 
 	cl_git_pass(git_iterator_for_workdir(
-		&iter, _repo, GIT_ITERATOR_DONT_AUTOEXPAND, NULL, NULL));
+		&iter, _repo, NULL, NULL, GIT_ITERATOR_DONT_AUTOEXPAND, NULL, NULL));
 
 	while (!error) {
 		if (entry && entry->mode == GIT_FILEMODE_TREE) {