Merge pull request #706 from arrbee/fix_592_again

Fix status for files under ignored dirs

src/diff.c

@@ -551,29 +551,27 @@ static int diff_from_iterators(
 		 * matched in old (and/or descend into directories as needed)
 		 */
 		else if (nitem && (!oitem || strcmp(oitem->path, nitem->path) > 0)) {
-			int is_ignored;
-			git_delta_t delta_type = GIT_DELTA_ADDED;
+			git_delta_t delta_type = GIT_DELTA_UNTRACKED;
 
-			/* contained in ignored parent directory, so this can be skipped. */
+			/* check if contained in ignored parent directory */
 			if (git_buf_len(&ignore_prefix) &&
 				git__prefixcmp(nitem->path, git_buf_cstr(&ignore_prefix)) == 0)
-			{
-				if (git_iterator_advance(new_iter, &nitem) < 0)
-					goto fail;
-
-				continue;
-			}
-
-			is_ignored = git_iterator_current_is_ignored(new_iter);
+				delta_type = GIT_DELTA_IGNORED;
 
 			if (S_ISDIR(nitem->mode)) {
-				/* recurse into directory if explicitly requested or
-				 * if there are tracked items inside the directory
+				/* recurse into directory only if there are tracked items in
+				 * it or if the user requested the contents of untracked
+				 * directories and it is not under an ignored directory.
 				 */
-				if ((diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) ||
-					(oitem && git__prefixcmp(oitem->path, nitem->path) == 0))
+				if ((oitem && git__prefixcmp(oitem->path, nitem->path) == 0) ||
+					(delta_type == GIT_DELTA_UNTRACKED &&
+					 (diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) != 0))
 				{
-					if (is_ignored)
+					/* 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 (git_iterator_advance_into_directory(new_iter, &nitem) < 0)
@@ -581,12 +579,34 @@ static int diff_from_iterators(
 
 					continue;
 				}
-				delta_type = GIT_DELTA_UNTRACKED;
 			}
-			else if (is_ignored)
+
+			/* In core git, the next two "else if" clauses are effectively
+			 * reversed -- i.e. when an untracked file contained in an
+			 * ignored directory is individually ignored, it shows up as an
+			 * ignored file in the diff list, even though other untracked
+			 * files in the same directory are skipped completely.
+			 *
+			 * To me, this is odd.  If the directory is ignored and the file
+			 * is untracked, we should skip it consistently, regardless of
+			 * whether it happens to match a pattern in the ignore file.
+			 *
+			 * To match the core git behavior, just reverse the following
+			 * two "else if" cases so that individual file ignores are
+			 * checked before container directory exclusions are used to
+			 * skip the file.
+			 */
+			else if (delta_type == GIT_DELTA_IGNORED) {
+				if (git_iterator_advance(new_iter, &nitem) < 0)
+					goto fail;
+				continue; /* ignored parent directory, so skip completely */
+			}
+
+			else if (git_iterator_current_is_ignored(new_iter))
 				delta_type = GIT_DELTA_IGNORED;
-			else if (new_iter->type == GIT_ITERATOR_WORKDIR)
-				delta_type = GIT_DELTA_UNTRACKED;
+
+			else if (new_iter->type != GIT_ITERATOR_WORKDIR)
+				delta_type = GIT_DELTA_ADDED;
 
 			if (diff_delta__from_one(diff, delta_type, nitem) < 0 ||
 				git_iterator_advance(new_iter, &nitem) < 0)

tests-clar/resources/issue_592b/.gitted/HEAD

+ref: refs/heads/master

tests-clar/resources/issue_592b/.gitted/config

+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = false
+	logallrefupdates = true
+	ignorecase = true

tests-clar/resources/issue_592b/.gitted/description

+Unnamed repository; edit this file 'description' to name the repository.

tests-clar/resources/issue_592b/.gitted/hooks/post-update.sample

+#!/bin/sh
+#
+# An example hook script to prepare a packed repository for use over
+# dumb transports.
+#
+# To enable this hook, rename this file to "post-update".
+
+exec git update-server-info

tests-clar/resources/issue_592b/.gitted/info/exclude

+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~

tests-clar/resources/issue_592b/.gitted/logs/HEAD

+0000000000000000000000000000000000000000 3fbf1852f72fd268e36457b13a18cdd9a4c9ea35 Russell Belfer <rb@github.com> 1337205933 -0700	commit (initial): Initial commit

tests-clar/resources/issue_592b/.gitted/logs/refs/heads/master

+0000000000000000000000000000000000000000 3fbf1852f72fd268e36457b13a18cdd9a4c9ea35 Russell Belfer <rb@github.com> 1337205933 -0700	commit (initial): Initial commit

tests-clar/resources/issue_592b/.gitted/refs/heads/master

+3fbf1852f72fd268e36457b13a18cdd9a4c9ea35

tests-clar/resources/issue_592b/gitignore

+ignored/

tests-clar/resources/issue_592b/ignored/contained/ignored3.txt

+I'm ignored

tests-clar/resources/issue_592b/ignored/contained/tracked3.txt

+You added me anyhow

tests-clar/resources/issue_592b/ignored/ignored2.txt

+I'm ignored

tests-clar/resources/issue_592b/ignored/tracked2.txt

+You like me

tests-clar/resources/issue_592b/ignored1.txt

+I'm ignored

tests-clar/resources/issue_592b/tracked1.txt

+You like me

tests-clar/status/worktree.c

@@ -45,9 +45,9 @@ void test_status_worktree__whole_repository(void)
 		git_status_foreach(repo, cb_status__normal, &counts)
 	);
 
-	cl_assert(counts.entry_count == counts.expected_entry_count);
-	cl_assert(counts.wrong_status_flags_count == 0);
-	cl_assert(counts.wrong_sorted_path == 0);
+	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);
 }
 
 /* this test is equivalent to t18-status.c:statuscb1 */
@@ -58,7 +58,7 @@ void test_status_worktree__empty_repository(void)
 
 	cl_git_pass(git_status_foreach(repo, cb_status__count, &count));
 
-	cl_assert(count == 0);
+	cl_assert_equal_i(0, count);
 }
 
 static int remove_file_cb(void *data, git_buf *file)
@@ -100,9 +100,9 @@ void test_status_worktree__purged_worktree(void)
 		git_status_foreach(repo, cb_status__normal, &counts)
 	);
 
-	cl_assert(counts.entry_count == counts.expected_entry_count);
-	cl_assert(counts.wrong_status_flags_count == 0);
-	cl_assert(counts.wrong_sorted_path == 0);
+	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);
 }
 
 /* this test is similar to t18-status.c:statuscb3 */
@@ -135,10 +135,9 @@ void test_status_worktree__swap_subdir_and_file(void)
 		git_status_foreach_ext(repo, &opts, cb_status__normal, &counts)
 	);
 
-	cl_assert(counts.entry_count == counts.expected_entry_count);
-	cl_assert(counts.wrong_status_flags_count == 0);
-	cl_assert(counts.wrong_sorted_path == 0);
-
+	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_worktree__swap_subdir_with_recurse_and_pathspec(void)
@@ -171,9 +170,9 @@ void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void)
 		git_status_foreach_ext(repo, &opts, cb_status__normal, &counts)
 	);
 
-	cl_assert(counts.entry_count == counts.expected_entry_count);
-	cl_assert(counts.wrong_status_flags_count == 0);
-	cl_assert(counts.wrong_sorted_path == 0);
+	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);
 }
 
 /* this test is equivalent to t18-status.c:singlestatus0 */
@@ -347,6 +346,65 @@ void test_status_worktree__issue_592_5(void)
 	git_buf_free(&path);
 }
 
+void test_status_worktree__issue_592_ignores_0(void)
+{
+	int count = 0;
+	status_entry_single st;
+	git_repository *repo = cl_git_sandbox_init("issue_592");
+
+	cl_git_pass(git_status_foreach(repo, cb_status__count, &count));
+	cl_assert_equal_i(0, count);
+
+	cl_git_rewritefile("issue_592/.gitignore",
+		".gitignore\n*.txt\nc/\n[tT]*/\n");
+
+	cl_git_pass(git_status_foreach(repo, cb_status__count, &count));
+	cl_assert_equal_i(1, count);
+
+	/* This is a situation where the behavior of libgit2 is
+	 * different from core git.  Core git will show ignored.txt
+	 * in the list of ignored files, even though the directory
+	 * "t" is ignored and the file is untracked because we have
+	 * the explicit "*.txt" ignore rule.  Libgit2 just excludes
+	 * all untracked files that are contained within ignored
+	 * directories without explicitly listing them.
+	 */
+	cl_git_rewritefile("issue_592/t/ignored.txt", "ping");
+
+	memset(&st, 0, sizeof(st));
+	cl_git_pass(git_status_foreach(repo, cb_status__single, &st));
+	cl_assert_equal_i(1, st.count);
+	cl_assert(st.status == GIT_STATUS_IGNORED);
+
+	cl_git_rewritefile("issue_592/c/ignored_by_dir", "ping");
+
+	memset(&st, 0, sizeof(st));
+	cl_git_pass(git_status_foreach(repo, cb_status__single, &st));
+	cl_assert_equal_i(1, st.count);
+	cl_assert(st.status == GIT_STATUS_IGNORED);
+
+	cl_git_rewritefile("issue_592/t/ignored_by_dir_pattern", "ping");
+
+	memset(&st, 0, sizeof(st));
+	cl_git_pass(git_status_foreach(repo, cb_status__single, &st));
+	cl_assert_equal_i(1, st.count);
+	cl_assert(st.status == GIT_STATUS_IGNORED);
+}
+
+void test_status_worktree__issue_592_ignored_dirs_with_tracked_content(void)
+{
+	int count = 0;
+	git_repository *repo = cl_git_sandbox_init("issue_592b");
+
+	cl_git_pass(git_status_foreach(repo, cb_status__count, &count));
+	cl_assert_equal_i(1, count);
+
+	/* if we are really mimicking core git, then only ignored1.txt
+	 * at the top level will show up in the ignores list here.
+	 * everything else will be unmodified or skipped completely.
+	 */
+}
+
 void test_status_worktree__cannot_retrieve_the_status_of_a_bare_repository(void)
 {
 	git_repository *repo;
@@ -374,7 +432,7 @@ void test_status_worktree__first_commit_in_progress(void)
 
 	memset(&result, 0, sizeof(result));
 	cl_git_pass(git_status_foreach(repo, cb_status__single, &result));
-	cl_assert(result.count == 1);
+	cl_assert_equal_i(1, result.count);
 	cl_assert(result.status == GIT_STATUS_WT_NEW);
 
 	cl_git_pass(git_repository_index(&index, repo));
@@ -383,7 +441,7 @@ void test_status_worktree__first_commit_in_progress(void)
 
 	memset(&result, 0, sizeof(result));
 	cl_git_pass(git_status_foreach(repo, cb_status__single, &result));
-	cl_assert(result.count == 1);
+	cl_assert_equal_i(1, result.count);
 	cl_assert(result.status == GIT_STATUS_INDEX_NEW);
 
 	git_index_free(index);