Merge pull request #1439 from arrbee/recurse-ignored-dirs

Several diff and status fixes

include/git2/diff.h

@@ -123,7 +123,7 @@ typedef enum {
 	 *  will be marked with only a single entry in the diff list; this flag
 	 *  adds all files under the directory as IGNORED entries, too.
 	 */
-	GIT_DIFF_RECURSE_IGNORED_DIRS = (1 << 10),
+	GIT_DIFF_RECURSE_IGNORED_DIRS = (1 << 18),
 } git_diff_option_t;
 
 /**

include/git2/oid.h

@@ -51,7 +51,7 @@ GIT_EXTERN(int) git_oid_fromstr(git_oid *out, const char *str);
  *
  * @param out oid structure the result is written into.
  * @param str input hex string; must be at least 4 characters
- *      long and null-terminated. 
+ *      long and null-terminated.
  * @return 0 or an error code
  */
 GIT_EXTERN(int) git_oid_fromstrp(git_oid *out, const char *str);

include/git2/status.h

@@ -127,20 +127,30 @@ typedef enum {
  *   will.
  * - GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH indicates that the given path
  *   will be treated as a literal path, and not as a pathspec.
+ * - GIT_STATUS_OPT_RECURSE_IGNORED_DIRS indicates that the contents of
+ *   ignored directories should be included in the status.  This is like
+ *   doing `git ls-files -o -i --exclude-standard` with core git.
  *
  * Calling `git_status_foreach()` is like calling the extended version
  * with: GIT_STATUS_OPT_INCLUDE_IGNORED, GIT_STATUS_OPT_INCLUDE_UNTRACKED,
- * and GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS.
+ * and GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS.  Those options are bundled
+ * together as `GIT_STATUS_OPT_DEFAULTS` if you want them as a baseline.
  */
 typedef enum {
-	GIT_STATUS_OPT_INCLUDE_UNTRACKED = (1 << 0),
-	GIT_STATUS_OPT_INCLUDE_IGNORED = (1 << 1),
-	GIT_STATUS_OPT_INCLUDE_UNMODIFIED = (1 << 2),
-	GIT_STATUS_OPT_EXCLUDE_SUBMODULES = (1 << 3),
-	GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS = (1 << 4),
-	GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH = (1 << 5),
+	GIT_STATUS_OPT_INCLUDE_UNTRACKED      = (1u << 0),
+	GIT_STATUS_OPT_INCLUDE_IGNORED        = (1u << 1),
+	GIT_STATUS_OPT_INCLUDE_UNMODIFIED     = (1u << 2),
+	GIT_STATUS_OPT_EXCLUDE_SUBMODULES     = (1u << 3),
+	GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS = (1u << 4),
+	GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH = (1u << 5),
+	GIT_STATUS_OPT_RECURSE_IGNORED_DIRS   = (1u << 6),
 } git_status_opt_t;
 
+#define GIT_STATUS_OPT_DEFAULTS \
+	(GIT_STATUS_OPT_INCLUDE_IGNORED | \
+	GIT_STATUS_OPT_INCLUDE_UNTRACKED | \
+	GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS)
+
 /**
  * Options to control how `git_status_foreach_ext()` will issue callbacks.
  *

src/diff.c

@@ -12,6 +12,9 @@
 #include "filter.h"
 #include "pathspec.h"
 
+#define DIFF_FLAG_IS_SET(DIFF,FLAG) (((DIFF)->opts.flags & (FLAG)) != 0)
+#define DIFF_FLAG_ISNT_SET(DIFF,FLAG) (((DIFF)->opts.flags & (FLAG)) == 0)
+
 static git_diff_delta *diff_delta__alloc(
 	git_diff_list *diff,
 	git_delta_t status,
@@ -29,7 +32,7 @@ static git_diff_delta *diff_delta__alloc(
 
 	delta->new_file.path = delta->old_file.path;
 
-	if (diff->opts.flags & GIT_DIFF_REVERSE) {
+	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) {
 		switch (status) {
 		case GIT_DELTA_ADDED:   status = GIT_DELTA_DELETED; break;
 		case GIT_DELTA_DELETED: status = GIT_DELTA_ADDED; break;
@@ -63,17 +66,22 @@ static int diff_delta__from_one(
 	int notify_res;
 
 	if (status == GIT_DELTA_IGNORED &&
-		(diff->opts.flags & GIT_DIFF_INCLUDE_IGNORED) == 0)
+		DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_INCLUDE_IGNORED))
 		return 0;
 
 	if (status == GIT_DELTA_UNTRACKED &&
-		(diff->opts.flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0)
+		DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_INCLUDE_UNTRACKED))
+		return 0;
+
+	if (entry->mode == GIT_FILEMODE_COMMIT &&
+		DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES))
 		return 0;
 
 	if (!git_pathspec_match_path(
 			&diff->pathspec, entry->path,
-			(diff->opts.flags & GIT_DIFF_DISABLE_PATHSPEC_MATCH) != 0,
-			(diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0, &matched_pathspec))
+			DIFF_FLAG_IS_SET(diff, GIT_DIFF_DISABLE_PATHSPEC_MATCH),
+			DIFF_FLAG_IS_SET(diff, GIT_DIFF_DELTAS_ARE_ICASE),
+			&matched_pathspec))
 		return 0;
 
 	delta = diff_delta__alloc(diff, status, entry->path);
@@ -124,10 +132,15 @@ static int diff_delta__from_two(
 	int notify_res;
 
 	if (status == GIT_DELTA_UNMODIFIED &&
-		(diff->opts.flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0)
+		DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_INCLUDE_UNMODIFIED))
 		return 0;
 
-	if ((diff->opts.flags & GIT_DIFF_REVERSE) != 0) {
+	if (old_entry->mode == GIT_FILEMODE_COMMIT &&
+		new_entry->mode == GIT_FILEMODE_COMMIT &&
+		DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES))
+		return 0;
+
+	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) {
 		uint32_t temp_mode = old_mode;
 		const git_index_entry *temp_entry = old_entry;
 		old_entry = new_entry;
@@ -149,7 +162,7 @@ static int diff_delta__from_two(
 	delta->new_file.mode = new_mode;
 
 	if (new_oid) {
-		if ((diff->opts.flags & GIT_DIFF_REVERSE) != 0)
+		if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE))
 			git_oid_cpy(&delta->old_file.oid, new_oid);
 		else
 			git_oid_cpy(&delta->new_file.oid, new_oid);
@@ -316,14 +329,14 @@ static git_diff_list *git_diff_list_alloc(
 	if (!diff->opts.old_prefix || !diff->opts.new_prefix)
 		goto fail;
 
-	if (diff->opts.flags & GIT_DIFF_REVERSE) {
+	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) {
 		const char *swap = diff->opts.old_prefix;
 		diff->opts.old_prefix = diff->opts.new_prefix;
 		diff->opts.new_prefix = swap;
 	}
 
 	/* INCLUDE_TYPECHANGE_TREES implies INCLUDE_TYPECHANGE */
-	if (diff->opts.flags & GIT_DIFF_INCLUDE_TYPECHANGE_TREES)
+	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_INCLUDE_TYPECHANGE_TREES))
 		diff->opts.flags |= GIT_DIFF_INCLUDE_TYPECHANGE;
 
 	return diff;
@@ -452,8 +465,9 @@ static int maybe_modified(
 
 	if (!git_pathspec_match_path(
 			&diff->pathspec, oitem->path,
-			(diff->opts.flags & GIT_DIFF_DISABLE_PATHSPEC_MATCH) != 0,
-			(diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0, &matched_pathspec))
+			DIFF_FLAG_IS_SET(diff, GIT_DIFF_DISABLE_PATHSPEC_MATCH),
+			DIFF_FLAG_IS_SET(diff, GIT_DIFF_DELTAS_ARE_ICASE),
+			&matched_pathspec))
 		return 0;
 
 	/* on platforms with no symlinks, preserve mode of existing symlinks */
@@ -478,7 +492,7 @@ static int maybe_modified(
 
 	/* if basic type of file changed, then split into delete and add */
 	else if (GIT_MODE_TYPE(omode) != GIT_MODE_TYPE(nmode)) {
-		if ((diff->opts.flags & GIT_DIFF_INCLUDE_TYPECHANGE) != 0)
+		if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_INCLUDE_TYPECHANGE))
 			status = GIT_DELTA_TYPECHANGE;
 		else {
 			if (diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem) < 0 ||
@@ -515,7 +529,7 @@ static int maybe_modified(
 			int err;
 			git_submodule *sub;
 
-			if ((diff->opts.flags & GIT_DIFF_IGNORE_SUBMODULES) != 0)
+			if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES))
 				status = GIT_DELTA_UNMODIFIED;
 			else if ((err = git_submodule_lookup(&sub, diff->repo, nitem->path)) < 0) {
 				if (err == GIT_EEXISTS)
@@ -543,6 +557,11 @@ static int maybe_modified(
 		}
 	}
 
+	/* if mode is GITLINK and submodules are ignored, then skip */
+	else if (S_ISGITLINK(nmode) &&
+			 DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES))
+		status = GIT_DELTA_UNMODIFIED;
+
 	/* if we got here and decided that the files are modified, but we
 	 * haven't calculated the OID of the new item, then calculate it now
 	 */
@@ -553,7 +572,13 @@ static int maybe_modified(
 				return -1;
 			use_noid = &noid;
 		}
-		if (omode == nmode && git_oid_equal(&oitem->oid, use_noid))
+
+		/* if oid matches, then mark unmodified (except submodules, where
+		 * the filesystem content may be modified even if the oid still
+		 * matches between the index and the workdir HEAD)
+		 */
+		if (omode == nmode && !S_ISGITLINK(omode) &&
+			git_oid_equal(&oitem->oid, use_noid))
 			status = GIT_DELTA_UNMODIFIED;
 	}
 
@@ -626,7 +651,7 @@ int git_diff__from_iterators(
 	if (!diff || diff_list_init_from_iterators(diff, old_iter, new_iter) < 0)
 		goto fail;
 
-	if (diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) {
+	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_DELTAS_ARE_ICASE)) {
 		if (git_iterator_set_ignore_case(old_iter, true) < 0 ||
 			git_iterator_set_ignore_case(new_iter, true) < 0)
 			goto fail;
@@ -648,7 +673,7 @@ int git_diff__from_iterators(
 			/* if we are generating TYPECHANGE records then check for that
 			 * instead of just generating a DELETE record
 			 */
-			if ((diff->opts.flags & GIT_DIFF_INCLUDE_TYPECHANGE_TREES) != 0 &&
+			if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_INCLUDE_TYPECHANGE_TREES) &&
 				entry_is_prefixed(diff, nitem, oitem))
 			{
 				/* this entry has become a tree! convert to TYPECHANGE */
@@ -663,7 +688,7 @@ int git_diff__from_iterators(
 				 * Unless RECURSE_UNTRACKED_DIRS is set, skip over them...
 				 */
 				if (S_ISDIR(nitem->mode) &&
-					!(diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS))
+					DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_RECURSE_UNTRACKED_DIRS))
 				{
 					if (git_iterator_advance(&nitem, new_iter) < 0)
 						goto fail;
@@ -691,27 +716,35 @@ int git_diff__from_iterators(
 				 * it or if the user requested the contents of untracked
 				 * directories and it is not under an ignored directory.
 				 */
-				bool recurse_untracked =
+				bool recurse_into_dir =
 					(delta_type == GIT_DELTA_UNTRACKED &&
-					 (diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) != 0);
+					 DIFF_FLAG_IS_SET(diff, GIT_DIFF_RECURSE_UNTRACKED_DIRS)) ||
+					(delta_type == GIT_DELTA_IGNORED &&
+					 DIFF_FLAG_IS_SET(diff, GIT_DIFF_RECURSE_IGNORED_DIRS));
 
 				/* do not advance into directories that contain a .git file */
-				if (!contains_oitem && recurse_untracked) {
+				if (!contains_oitem && recurse_into_dir) {
 					git_buf *full = NULL;
 					if (git_iterator_current_workdir_path(&full, new_iter) < 0)
 						goto fail;
 					if (git_path_contains_dir(full, DOT_GIT))
-						recurse_untracked = false;
+						recurse_into_dir = false;
 				}
 
-				if (contains_oitem || recurse_untracked) {
-					/* if this directory is ignored, remember it as the
-					 * "ignore_prefix" for processing contained items
-					 */
-					if (delta_type == GIT_DELTA_UNTRACKED &&
-						git_iterator_current_is_ignored(new_iter))
-						git_buf_sets(&ignore_prefix, nitem->path);
+				/* if directory is ignored, remember ignore_prefix */
+				if ((contains_oitem || recurse_into_dir) &&
+					delta_type == GIT_DELTA_UNTRACKED &&
+					git_iterator_current_is_ignored(new_iter))
+				{
+					git_buf_sets(&ignore_prefix, nitem->path);
+					delta_type = GIT_DELTA_IGNORED;
+
+					/* skip recursion if we've just learned this is ignored */
+					if (DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_RECURSE_IGNORED_DIRS))
+						recurse_into_dir = false;
+				}
 
+				if (contains_oitem || recurse_into_dir) {
 					/* advance into directory */
 					error = git_iterator_advance_into(&nitem, new_iter);
 
@@ -744,7 +777,8 @@ int git_diff__from_iterators(
 			 * checked before container directory exclusions are used to
 			 * skip the file.
 			 */
-			else if (delta_type == GIT_DELTA_IGNORED) {
+			else if (delta_type == GIT_DELTA_IGNORED &&
+					 DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_RECURSE_IGNORED_DIRS)) {
 				if (git_iterator_advance(&nitem, new_iter) < 0)
 					goto fail;
 				continue; /* ignored parent directory, so skip completely */
@@ -763,7 +797,7 @@ int git_diff__from_iterators(
 			 * instead of just generating an ADDED/UNTRACKED record
 			 */
 			if (delta_type != GIT_DELTA_IGNORED &&
-				(diff->opts.flags & GIT_DIFF_INCLUDE_TYPECHANGE_TREES) != 0 &&
+				DIFF_FLAG_IS_SET(diff, GIT_DIFF_INCLUDE_TYPECHANGE_TREES) &&
 				contains_oitem)
 			{
 				/* this entry was prefixed with a tree - make TYPECHANGE */

src/status.c

@@ -80,22 +80,37 @@ static unsigned int workdir_delta2status(git_delta_t workdir_status)
 typedef struct {
 	git_status_cb cb;
 	void *payload;
+	const git_status_options *opts;
 } status_user_callback;
 
 static int status_invoke_cb(
-	git_diff_delta *i2h, git_diff_delta *w2i, void *payload)
+	git_diff_delta *h2i, git_diff_delta *i2w, void *payload)
 {
 	status_user_callback *usercb = payload;
 	const char *path = NULL;
 	unsigned int status = 0;
 
-	if (w2i) {
-		path = w2i->old_file.path;
-		status |= workdir_delta2status(w2i->status);
+	if (i2w) {
+		path = i2w->old_file.path;
+		status |= workdir_delta2status(i2w->status);
 	}
-	if (i2h) {
-		path = i2h->old_file.path;
-		status |= index_delta2status(i2h->status);
+	if (h2i) {
+		path = h2i->old_file.path;
+		status |= index_delta2status(h2i->status);
+	}
+
+	/* if excluding submodules and this is a submodule everywhere */
+	if (usercb->opts &&
+		(usercb->opts->flags & GIT_STATUS_OPT_EXCLUDE_SUBMODULES) != 0)
+	{
+		bool in_tree  = (h2i && h2i->status != GIT_DELTA_ADDED);
+		bool in_index = (h2i && h2i->status != GIT_DELTA_DELETED);
+		bool in_wd    = (i2w && i2w->status != GIT_DELTA_DELETED);
+
+		if ((!in_tree || h2i->old_file.mode == GIT_FILEMODE_COMMIT) &&
+			(!in_index || h2i->new_file.mode == GIT_FILEMODE_COMMIT) &&
+			(!in_wd || i2w->new_file.mode == GIT_FILEMODE_COMMIT))
+			return 0;
 	}
 
 	return usercb->cb(path, status, usercb->payload);
@@ -109,7 +124,7 @@ int git_status_foreach_ext(
 {
 	int err = 0;
 	git_diff_options diffopt = GIT_DIFF_OPTIONS_INIT;
-	git_diff_list *idx2head = NULL, *wd2idx = NULL;
+	git_diff_list *head2idx = NULL, *idx2wd = NULL;
 	git_tree *head = NULL;
 	git_status_show_t show =
 		opts ? opts->show : GIT_STATUS_SHOW_INDEX_AND_WORKDIR;
@@ -142,34 +157,42 @@ int git_status_foreach_ext(
 		diffopt.flags = diffopt.flags | GIT_DIFF_RECURSE_UNTRACKED_DIRS;
 	if ((opts->flags & GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_DISABLE_PATHSPEC_MATCH;
-	/* TODO: support EXCLUDE_SUBMODULES flag */
-
-	if (show != GIT_STATUS_SHOW_WORKDIR_ONLY &&
-		(err = git_diff_tree_to_index(&idx2head, repo, head, NULL, &diffopt)) < 0)
-		goto cleanup;
+	if ((opts->flags & GIT_STATUS_OPT_RECURSE_IGNORED_DIRS) != 0)
+		diffopt.flags = diffopt.flags | GIT_DIFF_RECURSE_IGNORED_DIRS;
+	if ((opts->flags & GIT_STATUS_OPT_EXCLUDE_SUBMODULES) != 0)
+		diffopt.flags = diffopt.flags | GIT_DIFF_IGNORE_SUBMODULES;
+
+	if (show != GIT_STATUS_SHOW_WORKDIR_ONLY) {
+		err = git_diff_tree_to_index(&head2idx, repo, head, NULL, &diffopt);
+		if (err < 0)
+			goto cleanup;
+	}
 
-	if (show != GIT_STATUS_SHOW_INDEX_ONLY &&
-		(err = git_diff_index_to_workdir(&wd2idx, repo, NULL, &diffopt)) < 0)
-		goto cleanup;
+	if (show != GIT_STATUS_SHOW_INDEX_ONLY) {
+		err = git_diff_index_to_workdir(&idx2wd, repo, NULL, &diffopt);
+		if (err < 0)
+			goto cleanup;
+	}
 
 	usercb.cb = cb;
 	usercb.payload = payload;
+	usercb.opts = opts;
 
 	if (show == GIT_STATUS_SHOW_INDEX_THEN_WORKDIR) {
 		if ((err = git_diff__paired_foreach(
-				 idx2head, NULL, status_invoke_cb, &usercb)) < 0)
+				 head2idx, NULL, status_invoke_cb, &usercb)) < 0)
 			goto cleanup;
 
-		git_diff_list_free(idx2head);
-		idx2head = NULL;
+		git_diff_list_free(head2idx);
+		head2idx = NULL;
 	}
 
-	err = git_diff__paired_foreach(idx2head, wd2idx, status_invoke_cb, &usercb);
+	err = git_diff__paired_foreach(head2idx, idx2wd, status_invoke_cb, &usercb);
 
 cleanup:
 	git_tree_free(head);
-	git_diff_list_free(idx2head);
-	git_diff_list_free(wd2idx);
+	git_diff_list_free(head2idx);
+	git_diff_list_free(idx2wd);
 
 	if (err == GIT_EUSER)
 		giterr_clear();

src/submodule.c

@@ -694,7 +694,7 @@ int git_submodule_open(
 	git_buf_free(&path);
 
 	/* if we have opened the submodule successfully, let's grab the HEAD OID */
-	if (!error && !(submodule->flags & GIT_SUBMODULE_STATUS__WD_OID_VALID)) {
+	if (!error) {
 		if (!git_reference_name_to_id(
 				&submodule->wd_oid, *subrepo, GIT_HEAD_FILE))
 			submodule->flags |= GIT_SUBMODULE_STATUS__WD_OID_VALID;

tests-clar/diff/diff_helpers.c

@@ -42,6 +42,16 @@ int diff_file_cb(
 	return 0;
 }
 
+int diff_print_file_cb(
+	const git_diff_delta *delta,
+	float progress,
+	void *payload)
+{
+	fprintf(stderr, "%c %s\n",
+		git_diff_status_char(delta->status), delta->old_file.path);
+	return diff_file_cb(delta, progress, payload);
+}
+
 int diff_hunk_cb(
 	const git_diff_delta *delta,
 	const git_diff_range *range,

tests-clar/diff/diff_helpers.h

@@ -30,6 +30,11 @@ extern int diff_file_cb(
 	float progress,
 	void *cb_data);
 
+extern int diff_print_file_cb(
+	const git_diff_delta *delta,
+	float progress,
+	void *cb_data);
+
 extern int diff_hunk_cb(
 	const git_diff_delta *delta,
 	const git_diff_range *range,

tests-clar/diff/workdir.c

@@ -953,16 +953,31 @@ void test_diff_workdir__submodules(void)
 	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 two "untracked" file
-	 * because the diff list includes the "not" and "not-submodule" dirs which
-	 * are not displayed in the text diff.
+	/* so "git diff 873585" returns:
+	 *  M   .gitmodules
+	 *  A   just_a_dir/contents
+	 *  A   just_a_file
+	 *  A   sm_added_and_uncommited
+	 *  A   sm_changed_file
+	 *  A   sm_changed_head
+	 *  A   sm_changed_index
+	 *  A   sm_changed_untracked_file
+	 *  M   sm_missing_commits
+	 *  A   sm_unchanged
+	 * which is a little deceptive because of the difference between the
+	 * "git diff <treeish>" results from "git_diff_tree_to_workdir".  The
+	 * only significant difference is that those Added items will show up
+	 * as Untracked items in the pure libgit2 diff.
+	 *
+	 * Then add in the two extra untracked items "not" and "not-submodule"
+	 * to get the 12 files reported here.
 	 */
 
-	cl_assert_equal_i(11, exp.files);
+	cl_assert_equal_i(12, 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(2, exp.file_status[GIT_DELTA_MODIFIED]);
 	cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]);
 	cl_assert_equal_i(10, exp.file_status[GIT_DELTA_UNTRACKED]);
 

tests-clar/status/ignore.c

@@ -199,23 +199,26 @@ void test_status_ignore__subdirectories(void)
 	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "ignore_me"));
 	cl_assert(ignored);
 
-
-	/* So, interestingly, as per the comment in diff_from_iterators() the
-	 * following file is ignored, but in a way so that it does not show up
-	 * in status even if INCLUDE_IGNORED is used.  This actually matches
-	 * core git's behavior - if you follow these steps and try running "git
-	 * status -uall --ignored" then the following file and directory will
-	 * not show up in the output at all.
+	/* I've changed libgit2 so that the behavior here now differs from
+	 * core git but seems to make more sense.  In core git, the following
+	 * items are skipped completed, even if --ignored is passed to status.
+	 * It you mirror these steps and run "git status -uall --ignored" then
+	 * you will not see "test/ignore_me/" in the results.
+	 *
+	 * However, we had a couple reports of this as a bug, plus there is a
+	 * similar circumstance where we were differing for core git when you
+	 * used a rooted path for an ignore, so I changed this behavior.
 	 */
-
-	cl_git_pass(
-		git_futils_mkdir_r("empty_standard_repo/test/ignore_me", NULL, 0775));
+	cl_git_pass(git_futils_mkdir_r(
+		"empty_standard_repo/test/ignore_me", NULL, 0775));
 	cl_git_mkfile(
 		"empty_standard_repo/test/ignore_me/file", "I'm going to be ignored!");
+	cl_git_mkfile(
+		"empty_standard_repo/test/ignore_me/file2", "Me, too!");
 
 	memset(&st, 0, sizeof(st));
 	cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st));
-	cl_assert_equal_i(2, st.count);
+	cl_assert_equal_i(3, st.count);
 
 	cl_git_pass(git_status_file(&st.status, g_repo, "test/ignore_me/file"));
 	cl_assert(st.status == GIT_STATUS_IGNORED);
@@ -225,6 +228,91 @@ void test_status_ignore__subdirectories(void)
 	cl_assert(ignored);
 }
 
+void test_status_ignore__subdirectories_recursion(void)
+{
+	/* Let's try again with recursing into ignored dirs turned on */
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	status_entry_counts counts;
+	static const char *paths_r[] = {
+		".gitignore",
+		"ignore_also/file",
+		"ignore_me",
+		"test/ignore_me/and_me/file",
+		"test/ignore_me/file",
+		"test/ignore_me/file2",
+	};
+	static const unsigned int statuses_r[] = {
+		GIT_STATUS_WT_NEW,
+		GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED,
+	};
+	static const char *paths_nr[] = {
+		".gitignore",
+		"ignore_also/",
+		"ignore_me",
+		"test/ignore_me/",
+	};
+	static const unsigned int statuses_nr[] = {
+		GIT_STATUS_WT_NEW,
+		GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED,
+	};
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n/ignore_also\n");
+
+	cl_git_mkfile(
+		"empty_standard_repo/ignore_me", "I'm going to be ignored!");
+	cl_git_pass(git_futils_mkdir_r(
+		"empty_standard_repo/test/ignore_me", NULL, 0775));
+	cl_git_mkfile(
+		"empty_standard_repo/test/ignore_me/file", "I'm going to be ignored!");
+	cl_git_mkfile(
+		"empty_standard_repo/test/ignore_me/file2", "Me, too!");
+	cl_git_pass(git_futils_mkdir_r(
+		"empty_standard_repo/test/ignore_me/and_me", NULL, 0775));
+	cl_git_mkfile(
+		"empty_standard_repo/test/ignore_me/and_me/file", "Deeply ignored");
+	cl_git_pass(git_futils_mkdir_r(
+		"empty_standard_repo/ignore_also", NULL, 0775));
+	cl_git_mkfile(
+		"empty_standard_repo/ignore_also/file", "I'm going to be ignored!");
+
+	memset(&counts, 0x0, sizeof(status_entry_counts));
+	counts.expected_entry_count = 6;
+	counts.expected_paths = paths_r;
+	counts.expected_statuses = statuses_r;
+
+	opts.flags = GIT_STATUS_OPT_DEFAULTS | GIT_STATUS_OPT_RECURSE_IGNORED_DIRS;
+
+	cl_git_pass(git_status_foreach_ext(
+		g_repo, &opts, cb_status__normal, &counts));
+
+	cl_assert_equal_i(counts.expected_entry_count, counts.entry_count);
+	cl_assert_equal_i(0, counts.wrong_status_flags_count);
+	cl_assert_equal_i(0, counts.wrong_sorted_path);
+
+
+	memset(&counts, 0x0, sizeof(status_entry_counts));
+	counts.expected_entry_count = 4;
+	counts.expected_paths = paths_nr;
+	counts.expected_statuses = statuses_nr;
+
+	opts.flags = GIT_STATUS_OPT_DEFAULTS;
+
+	cl_git_pass(git_status_foreach_ext(
+		g_repo, &opts, cb_status__normal, &counts));
+
+	cl_assert_equal_i(counts.expected_entry_count, counts.entry_count);
+	cl_assert_equal_i(0, counts.wrong_status_flags_count);
+	cl_assert_equal_i(0, counts.wrong_sorted_path);
+}
+
 void test_status_ignore__adding_internal_ignores(void)
 {
 	int ignored;

tests-clar/status/status_helpers.c

@@ -47,3 +47,51 @@ int cb_status__single(const char *p, unsigned int s, void *payload)
 
 	return 0;
 }
+
+int cb_status__print(
+	const char *path, unsigned int status_flags, void *payload)
+{
+	char istatus = ' ', wstatus = ' ';
+	int icount = 0, wcount = 0;
+
+	if (status_flags & GIT_STATUS_INDEX_NEW) {
+		istatus = 'A'; icount++;
+	}
+	if (status_flags & GIT_STATUS_INDEX_MODIFIED) {
+		istatus = 'M'; icount++;
+	}
+	if (status_flags & GIT_STATUS_INDEX_DELETED) {
+		istatus = 'D'; icount++;
+	}
+	if (status_flags & GIT_STATUS_INDEX_RENAMED) {
+		istatus = 'R'; icount++;
+	}
+	if (status_flags & GIT_STATUS_INDEX_TYPECHANGE) {
+		istatus = 'T'; icount++;
+	}
+
+	if (status_flags & GIT_STATUS_WT_NEW) {
+		wstatus = 'A'; wcount++;
+	}
+	if (status_flags & GIT_STATUS_WT_MODIFIED) {
+		wstatus = 'M'; wcount++;
+	}
+	if (status_flags & GIT_STATUS_WT_DELETED) {
+		wstatus = 'D'; wcount++;
+	}
+	if (status_flags & GIT_STATUS_WT_TYPECHANGE) {
+		wstatus = 'T'; wcount++;
+	}
+	if (status_flags & GIT_STATUS_IGNORED) {
+		wstatus = 'I'; wcount++;
+	}
+
+	fprintf(stderr, "%c%c %s (%d/%d%s)\n",
+			istatus, wstatus, path, icount, wcount,
+			(icount > 1 || wcount > 1) ? " INVALID COMBO" : "");
+
+	if (payload)
+		*((int *)payload) += 1;
+
+	return 0;
+}

tests-clar/status/status_helpers.h

@@ -30,4 +30,8 @@ typedef struct {
 
 extern int cb_status__single(const char *p, unsigned int s, void *payload);
 
+/* cb_status__print takes optional payload of "int *" */
+
+extern int cb_status__print(const char *p, unsigned int s, void *payload);
+
 #endif

tests-clar/status/submodules.c

@@ -71,31 +71,34 @@ static unsigned int expected_status[] = {
 	GIT_STATUS_WT_NEW
 };
 
-static int
-cb_status__match(const char *p, unsigned int s, void *payload)
+static int cb_status__match(const char *p, unsigned int s, void *payload)
 {
-	volatile int *index = (int *)payload;
+	status_entry_counts *counts = payload;
+	int idx = counts->entry_count++;
 
-	cl_assert_equal_s(expected_files[*index], p);
-	cl_assert(expected_status[*index] == s);
-	(*index)++;
+	cl_assert_equal_s(counts->expected_paths[idx], p);
+	cl_assert(counts->expected_statuses[idx] == s);
 
 	return 0;
 }
 
 void test_status_submodules__1(void)
 {
-	int index = 0;
+	status_entry_counts counts;
 
 	cl_assert(git_path_isdir("submodules/.git"));
 	cl_assert(git_path_isdir("submodules/testrepo/.git"));
 	cl_assert(git_path_isfile("submodules/.gitmodules"));
 
+	memset(&counts, 0, sizeof(counts));
+	counts.expected_paths = expected_files;
+	counts.expected_statuses = expected_status;
+
 	cl_git_pass(
-		git_status_foreach(g_repo, cb_status__match, &index)
+		git_status_foreach(g_repo, cb_status__match, &counts)
 	);
 
-	cl_assert_equal_i(6, index);
+	cl_assert_equal_i(6, counts.entry_count);
 }
 
 void test_status_submodules__single_file(void)
@@ -104,3 +107,113 @@ void test_status_submodules__single_file(void)
 	cl_git_pass( git_status_file(&status, g_repo, "testrepo") );
 	cl_assert(!status);
 }
+
+void test_status_submodules__moved_head(void)
+{
+	git_submodule *sm;
+	git_repository *smrepo;
+	git_oid oid;
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	status_entry_counts counts;
+	static const char *expected_files_with_sub[] = {
+		".gitmodules",
+		"added",
+		"deleted",
+		"ignored",
+		"modified",
+		"testrepo",
+		"untracked"
+	};
+	static unsigned int expected_status_with_sub[] = {
+		GIT_STATUS_WT_MODIFIED,
+		GIT_STATUS_INDEX_NEW,
+		GIT_STATUS_INDEX_DELETED,
+		GIT_STATUS_IGNORED,
+		GIT_STATUS_WT_MODIFIED,
+		GIT_STATUS_WT_MODIFIED,
+		GIT_STATUS_WT_NEW
+	};
+
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "testrepo"));
+	cl_git_pass(git_submodule_open(&smrepo, sm));
+
+	/* move submodule HEAD to c47800c7266a2be04c571c04d5a6614691ea99bd */
+	cl_git_pass(
+		git_oid_fromstr(&oid, "c47800c7266a2be04c571c04d5a6614691ea99bd"));
+	cl_git_pass(git_repository_set_head_detached(smrepo, &oid));
+
+	/* first do a normal status, which should now include the submodule */
+
+	memset(&counts, 0, sizeof(counts));
+	counts.expected_paths = expected_files_with_sub;
+	counts.expected_statuses = expected_status_with_sub;
+
+	opts.flags = GIT_STATUS_OPT_DEFAULTS;
+
+	cl_git_pass(
+		git_status_foreach_ext(g_repo, &opts, cb_status__match, &counts));
+	cl_assert_equal_i(7, counts.entry_count);
+
+	/* try again with EXCLUDE_SUBMODULES which should skip it */
+
+	memset(&counts, 0, sizeof(counts));
+	counts.expected_paths = expected_files;
+	counts.expected_statuses = expected_status;
+
+	opts.flags = GIT_STATUS_OPT_DEFAULTS | GIT_STATUS_OPT_EXCLUDE_SUBMODULES;
+
+	cl_git_pass(
+		git_status_foreach_ext(g_repo, &opts, cb_status__match, &counts));
+	cl_assert_equal_i(6, counts.entry_count);
+}
+
+void test_status_submodules__dirty_workdir_only(void)
+{
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	status_entry_counts counts;
+	static const char *expected_files_with_sub[] = {
+		".gitmodules",
+		"added",
+		"deleted",
+		"ignored",
+		"modified",
+		"testrepo",
+		"untracked"
+	};
+	static unsigned int expected_status_with_sub[] = {
+		GIT_STATUS_WT_MODIFIED,
+		GIT_STATUS_INDEX_NEW,
+		GIT_STATUS_INDEX_DELETED,
+		GIT_STATUS_IGNORED,
+		GIT_STATUS_WT_MODIFIED,
+		GIT_STATUS_WT_MODIFIED,
+		GIT_STATUS_WT_NEW
+	};
+
+	cl_git_rewritefile("submodules/testrepo/README", "heyheyhey");
+	cl_git_mkfile("submodules/testrepo/all_new.txt", "never seen before");
+
+	/* first do a normal status, which should now include the submodule */
+
+	memset(&counts, 0, sizeof(counts));
+	counts.expected_paths = expected_files_with_sub;
+	counts.expected_statuses = expected_status_with_sub;
+
+	opts.flags = GIT_STATUS_OPT_DEFAULTS;
+
+	cl_git_pass(
+		git_status_foreach_ext(g_repo, &opts, cb_status__match, &counts));
+	cl_assert_equal_i(7, counts.entry_count);
+
+	/* try again with EXCLUDE_SUBMODULES which should skip it */
+
+	memset(&counts, 0, sizeof(counts));
+	counts.expected_paths = expected_files;
+	counts.expected_statuses = expected_status;
+
+	opts.flags = GIT_STATUS_OPT_DEFAULTS | GIT_STATUS_OPT_EXCLUDE_SUBMODULES;
+
+	cl_git_pass(
+		git_status_foreach_ext(g_repo, &opts, cb_status__match, &counts));
+	cl_assert_equal_i(6, counts.entry_count);
+}