Merge pull request #2226 from libgit2/rb/submodule-sorting-fix

Fix submodule sort order during iteration

5 files changed, 52 insertions, 19 deletions

diff --git a/src/iterator.c b/src/iterator.c
index e9ec65250..a7a44914c 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -10,7 +10,7 @@
 #include "index.h"
 #include "ignore.h"
 #include "buffer.h"
-#include "git2/submodule.h"
+#include "submodule.h"
 #include <ctype.h>
 
 #define ITERATOR_SET_CB(P,NAME_LC) do { \
@@ -1275,14 +1275,38 @@ GIT_INLINE(bool) workdir_path_is_dotgit(const git_buf *path)
 
 static int workdir_iterator__enter_dir(fs_iterator *fi)
 {
+	fs_iterator_frame *ff = fi->stack;
+	size_t pos;
+	git_path_with_stat *entry;
+	bool found_submodules = false;
+
 	/* only push new ignores if this is not top level directory */
-	if (fi->stack->next != NULL) {
+	if (ff->next != NULL) {
 		workdir_iterator *wi = (workdir_iterator *)fi;
 		ssize_t slash_pos = git_buf_rfind_next(&fi->path, '/');
 
 		(void)git_ignore__push_dir(&wi->ignores, &fi->path.ptr[slash_pos + 1]);
 	}
 
+	/* convert submodules to GITLINK and remove trailing slashes */
+	git_vector_foreach(&ff->entries, pos, entry) {
+		if (S_ISDIR(entry->st.st_mode) &&
+			git_submodule__is_submodule(fi->base.repo, entry->path))
+		{
+			entry->st.st_mode = GIT_FILEMODE_COMMIT;
+			entry->path_len--;
+			entry->path[entry->path_len] = '\0';
+			found_submodules = true;
+		}
+	}
+
+	/* if we renamed submodules, re-sort and re-seek to start */
+	if (found_submodules) {
+		git_vector_set_sorted(&ff->entries, 0);
+		git_vector_sort(&ff->entries);
+		fs_iterator__seek_frame_start(fi, ff);
+	}
+
 	return 0;
 }
 
@@ -1295,7 +1319,6 @@ static int workdir_iterator__leave_dir(fs_iterator *fi)
 
 static int workdir_iterator__update_entry(fs_iterator *fi)
 {
-	int error = 0;
 	workdir_iterator *wi = (workdir_iterator *)fi;
 
 	/* skip over .git entries */
@@ -1305,20 +1328,6 @@ static int workdir_iterator__update_entry(fs_iterator *fi)
 	/* reset is_ignored since we haven't checked yet */
 	wi->is_ignored = -1;
 
-	/* check if apparent tree entries are actually submodules */
-	if (fi->entry.mode != GIT_FILEMODE_TREE)
-		return 0;
-
-	error = git_submodule_lookup(NULL, fi->base.repo, fi->entry.path);
-	if (error < 0)
-		giterr_clear();
-
-	/* mark submodule as GITLINK and remove slash */
-	if (!error) {
-		fi->entry.mode = S_IFGITLINK;
-		fi->entry.path[strlen(fi->entry.path) - 1] = '\0';
-	}
-
 	return 0;
 }
 
diff --git a/src/merge_file.c b/src/merge_file.c
index ab9ca4168..ff0364432 100644
--- a/src/merge_file.c
+++ b/src/merge_file.c
@@ -117,7 +117,7 @@ static int git_merge_file__from_inputs(
 
 	memset(out, 0x0, sizeof(git_merge_file_result));
 
-	merge_file_normalize_opts(&options, given_opts);		
+	merge_file_normalize_opts(&options, given_opts);
 
 	memset(&xmparam, 0x0, sizeof(xmparam_t));
 
@@ -165,7 +165,7 @@ static int git_merge_file__from_inputs(
 	}
 
 	out->automergeable = (xdl_result == 0);
-	out->ptr = (unsigned char *)mmbuffer.ptr;
+	out->ptr = (const char *)mmbuffer.ptr;
 	out->len = mmbuffer.size;
 	out->mode = merge_file_best_mode(ancestor, ours, theirs);
 
diff --git a/src/submodule.c b/src/submodule.c
index e1500b847..fdcc2251a 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -135,6 +135,21 @@ static int submodule_lookup(
  * PUBLIC APIS
  */
 
+bool git_submodule__is_submodule(git_repository *repo, const char *name)
+{
+	git_strmap *map;
+
+	if (load_submodule_config(repo, false) < 0) {
+		giterr_clear();
+		return false;
+	}
+
+	if (!(map = repo->submodules))
+		return false;
+
+	return git_strmap_valid_index(map, git_strmap_lookup_index(map, name));
+}
+
 int git_submodule_lookup(
 	git_submodule **out, /* NULL if user only wants to test existence */
 	git_repository *repo,
diff --git a/src/submodule.h b/src/submodule.h
index 053cb61e0..1c41897e3 100644
--- a/src/submodule.h
+++ b/src/submodule.h
@@ -119,6 +119,9 @@ enum {
 #define GIT_SUBMODULE_STATUS__CLEAR_INTERNAL(S) \
 	((S) & ~(0xFFFFFFFFu << 20))
 
+/* Internal submodule check does not attempt to refresh cached data */
+extern bool git_submodule__is_submodule(git_repository *repo, const char *name);
+
 /* Internal status fn returns status and optionally the various OIDs */
 extern int git_submodule__status(
 	unsigned int *out_status,
diff --git a/tests/diff/submodules.c b/tests/diff/submodules.c
index ead5c71b6..2881f74be 100644
--- a/tests/diff/submodules.c
+++ b/tests/diff/submodules.c
@@ -182,6 +182,8 @@ void test_diff_submodules__submod2_index_to_wd(void)
 		"<UNTRACKED>", /* not */
 		"diff --git a/sm_changed_file b/sm_changed_file\nindex 4800958..4800958 160000\n--- a/sm_changed_file\n+++ b/sm_changed_file\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_file */
 		"diff --git a/sm_changed_head b/sm_changed_head\nindex 4800958..3d9386c 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n", /* sm_changed_head */
+		"<UNTRACKED>", /* sm_changed_head- */
+		"<UNTRACKED>", /* sm_changed_head_ */
 		"diff --git a/sm_changed_index b/sm_changed_index\nindex 4800958..4800958 160000\n--- a/sm_changed_index\n+++ b/sm_changed_index\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_index */
 		"diff --git a/sm_changed_untracked_file b/sm_changed_untracked_file\nindex 4800958..4800958 160000\n--- a/sm_changed_untracked_file\n+++ b/sm_changed_untracked_file\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_untracked_file */
 		"diff --git a/sm_missing_commits b/sm_missing_commits\nindex 4800958..5e49635 160000\n--- a/sm_missing_commits\n+++ b/sm_missing_commits\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 5e4963595a9774b90524d35a807169049de8ccad\n", /* sm_missing_commits */
@@ -190,6 +192,10 @@ void test_diff_submodules__submod2_index_to_wd(void)
 
 	g_repo = setup_fixture_submod2();
 
+	/* bracket existing submodule with similarly named items */
+	cl_git_mkfile("submod2/sm_changed_head-", "hello");
+	cl_git_mkfile("submod2/sm_changed_head_", "hello");
+
 	opts.flags = GIT_DIFF_INCLUDE_UNTRACKED;
 	opts.old_prefix = "a"; opts.new_prefix = "b";