Three submodule status bug fixes

1. Fix sort order problem with submodules where "mod" was sorting
   after "mod-plus" because they were being sorted as "mod/" and
   "mod-plus/".  This involved pushing the "contains a .git entry"
   test significantly lower in the stack.
2. Reinstate behavior that a directory which contains a .git entry
   will be treated as a submodule during iteration even if it is
   not yet added to the .gitmodules.
3. Now that any directory containing .git is reported as submodule,
   we have to be more careful checking for GIT_EEXISTS when we
   do a submodule lookup, because that is the error code that is
   returned by git_submodule_lookup when you try to look up a
   directory containing .git that has no record in gitmodules or
   the index.

20 files changed, 104 insertions, 19 deletions

diff --git a/tests-clar/diff/iterator.c b/tests-clar/diff/iterator.c
index f1efdfbba..15b10465a 100644
--- a/tests-clar/diff/iterator.c
+++ b/tests-clar/diff/iterator.c
@@ -720,13 +720,13 @@ void test_diff_iterator__workdir_builtin_ignores(void)
 		{ "root_test2", false },
 		{ "root_test3", false },
 		{ "root_test4.txt", false },
-		{ "sub/", false },
+		{ "sub", false },
 		{ "sub/.gitattributes", false },
 		{ "sub/abc", false },
 		{ "sub/dir/", true },
 		{ "sub/file", false },
 		{ "sub/ign/", true },
-		{ "sub/sub/", false },
+		{ "sub/sub", false },
 		{ "sub/sub/.gitattributes", false },
 		{ "sub/sub/dir", false }, /* file is not actually a dir */
 		{ "sub/sub/file", false },
@@ -746,9 +746,13 @@ void test_diff_iterator__workdir_builtin_ignores(void)
 		cl_assert_equal_s(expected[idx].path, entry->path);
 		cl_assert_(ignored == expected[idx].ignored, expected[idx].path);
 
-		if (!ignored && S_ISDIR(entry->mode))
+		if (!ignored &&
+			(entry->mode == GIT_FILEMODE_TREE ||
+			 entry->mode == GIT_FILEMODE_COMMIT))
+		{
+			/* it is possible to advance "into" a submodule */
 			cl_git_pass(git_iterator_advance_into(&entry, i));
-		else
+		} else
 			cl_git_pass(git_iterator_advance(&entry, i));
 	}
 
diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c
index 7e8915c4b..983465b29 100644
--- a/tests-clar/diff/workdir.c
+++ b/tests-clar/diff/workdir.c
@@ -936,7 +936,8 @@ void test_diff_workdir__submodules(void)
 	p_rename("submod2_target/.gitted", "submod2_target/.git");
 
 	rewrite_gitmodules(git_repository_workdir(g_repo));
-	p_rename("submod2/not_submodule/.gitted", "submod2/not_submodule/.git");
+	p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git");
+	p_rename("submod2/not/.gitted", "submod2/not/.git");
 
 	cl_fixture_cleanup("submod2_target");
 
@@ -954,21 +955,22 @@ void test_diff_workdir__submodules(void)
 	/* essentially doing: git diff 873585b94bdeabccea991ea5e3ec1a277895b698 */
 
 	memset(&exp, 0, sizeof(exp));
+
 	cl_git_pass(git_diff_foreach(
 		diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp));
 
-	/* the following differs from "git diff 873585" by one "untracked" file
-	 * because the diff list includes the "not_submodule/" directory which
-	 * is not displayed in the text diff.
+	/* the following differs from "git diff 873585" by two "untracked" file
+	 * because the diff list includes the "not" and "not-submodule" dirs which
+	 * are not displayed in the text diff.
 	 */
 
-	cl_assert_equal_i(10, exp.files);
+	cl_assert_equal_i(11, exp.files);
 
 	cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]);
 	cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]);
 	cl_assert_equal_i(1, exp.file_status[GIT_DELTA_MODIFIED]);
 	cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]);
-	cl_assert_equal_i(9, exp.file_status[GIT_DELTA_UNTRACKED]);
+	cl_assert_equal_i(10, exp.file_status[GIT_DELTA_UNTRACKED]);
 
 	/* the following numbers match "git diff 873585" exactly */
 
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/HEAD b/tests-clar/resources/submod2/not-submodule/.gitted/HEAD
index cb089cd89..cb089cd89 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/HEAD
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/HEAD
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/config b/tests-clar/resources/submod2/not-submodule/.gitted/config
index af107929f..af107929f 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/config
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/config
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/description b/tests-clar/resources/submod2/not-submodule/.gitted/description
index 498b267a8..498b267a8 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/description
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/description
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/index b/tests-clar/resources/submod2/not-submodule/.gitted/index
index f3fafa536..f3fafa536 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/index
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/index
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/info/exclude b/tests-clar/resources/submod2/not-submodule/.gitted/info/exclude
index a5196d1be..a5196d1be 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/info/exclude
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/info/exclude
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/logs/HEAD b/tests-clar/resources/submod2/not-submodule/.gitted/logs/HEAD
index 1749e7dff..1749e7dff 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/logs/HEAD
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/logs/HEAD
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/logs/refs/heads/master b/tests-clar/resources/submod2/not-submodule/.gitted/logs/refs/heads/master
index 1749e7dff..1749e7dff 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/logs/refs/heads/master
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/logs/refs/heads/master
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/objects/68/e92c611b80ee1ed8f38314ff9577f0d15b2444 b/tests-clar/resources/submod2/not-submodule/.gitted/objects/68/e92c611b80ee1ed8f38314ff9577f0d15b2444
index 8892531a7..8892531a7 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/objects/68/e92c611b80ee1ed8f38314ff9577f0d15b2444
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/objects/68/e92c611b80ee1ed8f38314ff9577f0d15b2444
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/objects/71/ff9927d7c8a5639e062c38a7d35c433c424627 b/tests-clar/resources/submod2/not-submodule/.gitted/objects/71/ff9927d7c8a5639e062c38a7d35c433c424627
index c4e1a77d7..c4e1a77d7 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/objects/71/ff9927d7c8a5639e062c38a7d35c433c424627
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/objects/71/ff9927d7c8a5639e062c38a7d35c433c424627
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/objects/f0/1d56b18efd353ef2bb93a4585d590a0847195e b/tests-clar/resources/submod2/not-submodule/.gitted/objects/f0/1d56b18efd353ef2bb93a4585d590a0847195e
index e9f1942a9..e9f1942a9 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/objects/f0/1d56b18efd353ef2bb93a4585d590a0847195e
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/objects/f0/1d56b18efd353ef2bb93a4585d590a0847195e
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/refs/heads/master b/tests-clar/resources/submod2/not-submodule/.gitted/refs/heads/master
index 0bd8514bd..0bd8514bd 100644
--- a/tests-clar/resources/submod2/not_submodule/.gitted/refs/heads/master
+++ b/tests-clar/resources/submod2/not-submodule/.gitted/refs/heads/master
diff --git a/tests-clar/resources/submod2/not_submodule/README.txt b/tests-clar/resources/submod2/not-submodule/README.txt
index 71ff9927d..71ff9927d 100644
--- a/tests-clar/resources/submod2/not_submodule/README.txt
+++ b/tests-clar/resources/submod2/not-submodule/README.txt
diff --git a/tests-clar/resources/submod2/not/.gitted/notempty b/tests-clar/resources/submod2/not/.gitted/notempty
new file mode 100644
index 000000000..9b33ac4e4
--- /dev/null
+++ b/tests-clar/resources/submod2/not/.gitted/notempty
@@ -0,0 +1 @@
+fooled you
diff --git a/tests-clar/resources/submod2/not/README.txt b/tests-clar/resources/submod2/not/README.txt
new file mode 100644
index 000000000..4f6935b98
--- /dev/null
+++ b/tests-clar/resources/submod2/not/README.txt
@@ -0,0 +1 @@
+what am I really
diff --git a/tests-clar/resources/submod2/not_submodule/.gitted/COMMIT_EDITMSG b/tests-clar/resources/submod2/not_submodule/.gitted/COMMIT_EDITMSG
deleted file mode 100644
index 5852f4463..000000000
--- a/tests-clar/resources/submod2/not_submodule/.gitted/COMMIT_EDITMSG
+++ /dev/null
@@ -1 +0,0 @@
-Initial commit
diff --git a/tests-clar/submodule/lookup.c b/tests-clar/submodule/lookup.c
index 868b51e55..acf8f6462 100644
--- a/tests-clar/submodule/lookup.c
+++ b/tests-clar/submodule/lookup.c
@@ -13,7 +13,7 @@ void test_submodule_lookup__initialize(void)
 
 	/* must create submod2_target before rewrite so prettify will work */
 	rewrite_gitmodules(git_repository_workdir(g_repo));
-	p_rename("submod2/not_submodule/.gitted", "submod2/not_submodule/.git");
+	p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git");
 }
 
 void test_submodule_lookup__cleanup(void)
@@ -39,7 +39,7 @@ void test_submodule_lookup__simple_lookup(void)
 	cl_assert(sm);
 
 	/* lookup git repo subdir that is not added as submodule */
-	cl_assert(git_submodule_lookup(&sm, g_repo, "not_submodule") == GIT_EEXISTS);
+	cl_assert(git_submodule_lookup(&sm, g_repo, "not-submodule") == GIT_EEXISTS);
 
 	/* lookup existing directory that is not a submodule */
 	cl_assert(git_submodule_lookup(&sm, g_repo, "just_a_dir") == GIT_ENOTFOUND);
diff --git a/tests-clar/submodule/modify.c b/tests-clar/submodule/modify.c
index f6d41fdf2..94eb3738a 100644
--- a/tests-clar/submodule/modify.c
+++ b/tests-clar/submodule/modify.c
@@ -18,7 +18,7 @@ void test_submodule_modify__initialize(void)
 
 	/* must create submod2_target before rewrite so prettify will work */
 	rewrite_gitmodules(git_repository_workdir(g_repo));
-	p_rename("submod2/not_submodule/.gitted", "submod2/not_submodule/.git");
+	p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git");
 }
 
 void test_submodule_modify__cleanup(void)
diff --git a/tests-clar/submodule/status.c b/tests-clar/submodule/status.c
index 3fd6960c9..282e82758 100644
--- a/tests-clar/submodule/status.c
+++ b/tests-clar/submodule/status.c
@@ -3,6 +3,7 @@
 #include "path.h"
 #include "submodule_helpers.h"
 #include "fileops.h"
+#include "iterator.h"
 
 static git_repository *g_repo = NULL;
 
@@ -15,7 +16,8 @@ void test_submodule_status__initialize(void)
 
 	/* must create submod2_target before rewrite so prettify will work */
 	rewrite_gitmodules(git_repository_workdir(g_repo));
-	p_rename("submod2/not_submodule/.gitted", "submod2/not_submodule/.git");
+	p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git");
+	p_rename("submod2/not/.gitted", "submod2/not/.git");
 }
 
 void test_submodule_status__cleanup(void)
@@ -52,7 +54,12 @@ void test_submodule_status__ignore_none(void)
 	cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "sm_unchanged"));
 	cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_REMOVE_FILES));
 
-	cl_git_fail(git_submodule_lookup(&sm, g_repo, "not_submodule"));
+	cl_assert_equal_i(GIT_ENOTFOUND,
+		git_submodule_lookup(&sm, g_repo, "just_a_dir"));
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_submodule_lookup(&sm, g_repo, "not-submodule"));
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_submodule_lookup(&sm, g_repo, "not"));
 
 	cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_changed_index"));
 	cl_git_pass(git_submodule_status(&status, sm));
@@ -138,7 +145,7 @@ void test_submodule_status__ignore_untracked(void)
 
 	cl_git_pass(git_submodule_foreach(g_repo, set_sm_ignore, &ign));
 
-	cl_git_fail(git_submodule_lookup(&sm, g_repo, "not_submodule"));
+	cl_git_fail(git_submodule_lookup(&sm, g_repo, "not-submodule"));
 
 	cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_changed_index"));
 	cl_git_pass(git_submodule_status(&status, sm));
@@ -198,7 +205,12 @@ void test_submodule_status__ignore_dirty(void)
 
 	cl_git_pass(git_submodule_foreach(g_repo, set_sm_ignore, &ign));
 
-	cl_git_fail(git_submodule_lookup(&sm, g_repo, "not_submodule"));
+	cl_assert_equal_i(GIT_ENOTFOUND,
+		git_submodule_lookup(&sm, g_repo, "just_a_dir"));
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_submodule_lookup(&sm, g_repo, "not-submodule"));
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_submodule_lookup(&sm, g_repo, "not"));
 
 	cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_changed_index"));
 	cl_git_pass(git_submodule_status(&status, sm));
@@ -258,7 +270,12 @@ void test_submodule_status__ignore_all(void)
 
 	cl_git_pass(git_submodule_foreach(g_repo, set_sm_ignore, &ign));
 
-	cl_git_fail(git_submodule_lookup(&sm, g_repo, "not_submodule"));
+	cl_assert_equal_i(GIT_ENOTFOUND,
+		git_submodule_lookup(&sm, g_repo, "just_a_dir"));
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_submodule_lookup(&sm, g_repo, "not-submodule"));
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_submodule_lookup(&sm, g_repo, "not"));
 
 	cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_changed_index"));
 	cl_git_pass(git_submodule_status(&status, sm));
@@ -305,3 +322,64 @@ void test_submodule_status__ignore_all(void)
 
 	git_buf_free(&path);
 }
+
+typedef struct {
+	size_t counter;
+	const char **paths;
+} submodule_expectations;
+
+static int confirm_submodule_status(
+	const char *path, unsigned int status_flags, void *payload)
+{
+	submodule_expectations *exp = payload;
+
+	while (git__suffixcmp(exp->paths[exp->counter], "/") == 0)
+		exp->counter++;
+
+	cl_assert_equal_s(exp->paths[exp->counter++], path);
+
+	GIT_UNUSED(status_flags);
+
+	return 0;
+}
+
+void test_submodule_status__iterator(void)
+{
+	git_iterator *iter;
+	const git_index_entry *entry;
+	size_t i;
+	static const char *expected[] = {
+		".gitmodules",
+		"just_a_dir/",
+		"just_a_dir/contents",
+		"just_a_file",
+		"not",
+		"not-submodule",
+		"README.txt",
+		"sm_added_and_uncommited",
+		"sm_changed_file",
+		"sm_changed_head",
+		"sm_changed_index",
+		"sm_changed_untracked_file",
+		"sm_missing_commits",
+		"sm_unchanged",
+		NULL
+	};
+	submodule_expectations exp = { 0, expected };
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+
+	cl_git_pass(git_iterator_for_workdir(&iter, g_repo,
+		GIT_ITERATOR_IGNORE_CASE | GIT_ITERATOR_INCLUDE_TREES, NULL, NULL));
+	cl_git_pass(git_iterator_current(&entry, iter));
+
+	for (i = 0; entry; ++i) {
+		cl_assert_equal_s(expected[i], entry->path);
+		cl_git_pass(git_iterator_advance(&entry, iter));
+	}
+
+	git_iterator_free(iter);
+
+	opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_INCLUDE_UNMODIFIED | GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
+
+	cl_git_pass(git_status_foreach_ext(g_repo, &opts, confirm_submodule_status, &exp));
+}