Merge pull request #1090 from arrbee/ignore-invalid-by-default

Ignore invalid entries by default

6 files changed, 165 insertions, 39 deletions

diff --git a/include/git2/ignore.h b/include/git2/ignore.h
index e18615edd..592c96e65 100644
--- a/include/git2/ignore.h
+++ b/include/git2/ignore.h
@@ -41,9 +41,10 @@ GIT_EXTERN(int) git_ignore_add_rule(
 /**
  * Clear ignore rules that were explicitly added.
  *
- * Clears the internal ignore rules that have been set up.  This will not
- * turn off the rules in .gitignore files that actually exist in the
- * filesystem.
+ * Resets to the default internal ignore rules.  This will not turn off
+ * rules in .gitignore files that actually exist in the filesystem.
+ *
+ * The default internal ignores ignore ".", ".." and ".git" entries.
  *
  * @param repo The repository to remove ignore rules from.
  * @return 0 on success
diff --git a/src/ignore.c b/src/ignore.c
index 6a377e60d..5edc5b65b 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -7,6 +7,8 @@
 #define GIT_IGNORE_FILE_INREPO	"info/exclude"
 #define GIT_IGNORE_FILE			".gitignore"
 
+#define GIT_IGNORE_DEFAULT_RULES ".\n..\n.git\n"
+
 static int parse_ignore_file(
 	git_repository *repo, void *parsedata, const char *buffer, git_attr_file *ignores)
 {
@@ -88,6 +90,19 @@ static int push_one_ignore(void *ref, git_buf *path)
 	return push_ignore_file(ign->repo, ign, &ign->ign_path, path->ptr, GIT_IGNORE_FILE);
 }
 
+static int get_internal_ignores(git_attr_file **ign, git_repository *repo)
+{
+	int error;
+
+	if (!(error = git_attr_cache__init(repo)))
+		error = git_attr_cache__internal_file(repo, GIT_IGNORE_INTERNAL, ign);
+
+	if (!error && !(*ign)->rules.length)
+		error = parse_ignore_file(repo, NULL, GIT_IGNORE_DEFAULT_RULES, *ign);
+
+	return error;
+}
+
 int git_ignore__for_path(
 	git_repository *repo,
 	const char *path,
@@ -129,8 +144,7 @@ int git_ignore__for_path(
 		goto cleanup;
 
 	/* set up internals */
-	error = git_attr_cache__internal_file(
-		repo, GIT_IGNORE_INTERNAL, &ignores->ign_internal);
+	error = get_internal_ignores(&ignores->ign_internal, repo);
 	if (error < 0)
 		goto cleanup;
 
@@ -239,16 +253,6 @@ cleanup:
 	return 0;
 }
 
-static int get_internal_ignores(git_attr_file **ign, git_repository *repo)
-{
-	int error;
-
-	if (!(error = git_attr_cache__init(repo)))
-		error = git_attr_cache__internal_file(repo, GIT_IGNORE_INTERNAL, ign);
-
-	return error;
-}
-
 int git_ignore_add_rule(
 	git_repository *repo,
 	const char *rules)
@@ -268,9 +272,13 @@ int git_ignore_clear_internal_rules(
 	int error;
 	git_attr_file *ign_internal;
 
-	if (!(error = get_internal_ignores(&ign_internal, repo)))
+	if (!(error = get_internal_ignores(&ign_internal, repo))) {
 		git_attr_file__clear_rules(ign_internal);
 
+		return parse_ignore_file(
+			repo, NULL, GIT_IGNORE_DEFAULT_RULES, ign_internal);
+	}
+
 	return error;
 }
 
diff --git a/src/iterator.c b/src/iterator.c
index 6be45a4bb..bd586ce99 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -10,6 +10,7 @@
 #include "ignore.h"
 #include "buffer.h"
 #include "git2/submodule.h"
+#include <ctype.h>
 
 #define ITERATOR_BASE_INIT(P,NAME_LC,NAME_UC) do { \
 	(P) = git__calloc(1, sizeof(NAME_LC ## _iterator)); \
@@ -465,6 +466,23 @@ static int git_path_with_stat_cmp_icase(const void *a, const void *b)
 	return strcasecmp(path_with_stat_a->path, path_with_stat_b->path);
 }
 
+GIT_INLINE(bool) path_is_dotgit(const git_path_with_stat *ps)
+{
+	if (!ps)
+		return false;
+	else {
+		const char *path = ps->path;
+		size_t len  = ps->path_len;
+
+		return len >= 4 &&
+			tolower(path[len - 1]) == 't' &&
+			tolower(path[len - 2]) == 'i' &&
+			tolower(path[len - 3]) == 'g' &&
+			path[len - 4] == '.' &&
+			(len == 4 || path[len - 5] == '/');
+	}
+}
+
 static workdir_iterator_frame *workdir_iterator__alloc_frame(workdir_iterator *wi)
 {
 	workdir_iterator_frame *wf = git__calloc(1, sizeof(workdir_iterator_frame));
@@ -531,6 +549,9 @@ static int workdir_iterator__expand_dir(workdir_iterator *wi)
 			CASESELECT(wi->base.ignore_case, workdir_iterator__entry_cmp_icase, workdir_iterator__entry_cmp_case),
 			wf->start);
 
+	if (path_is_dotgit(git_vector_get(&wf->entries, wf->index)))
+		wf->index++;
+
 	wf->next  = wi->stack;
 	wi->stack = wf;
 
@@ -574,8 +595,7 @@ static int workdir_iterator__advance(
 		next = git_vector_get(&wf->entries, ++wf->index);
 		if (next != NULL) {
 			/* match git's behavior of ignoring anything named ".git" */
-			if (STRCMP_CASESELECT(wi->base.ignore_case, next->path, DOT_GIT "/") == 0 ||
-				STRCMP_CASESELECT(wi->base.ignore_case, next->path, DOT_GIT) == 0)
+			if (path_is_dotgit(next))
 				continue;
 			/* else found a good entry */
 			break;
@@ -658,8 +678,7 @@ static int workdir_iterator__update_entry(workdir_iterator *wi)
 	wi->entry.path = ps->path;
 
 	/* skip over .git entries */
-	if (STRCMP_CASESELECT(wi->base.ignore_case, ps->path, DOT_GIT "/") == 0 ||
-		STRCMP_CASESELECT(wi->base.ignore_case, ps->path, DOT_GIT) == 0)
+	if (path_is_dotgit(ps))
 		return workdir_iterator__advance((git_iterator *)wi, NULL);
 
 	wi->is_ignored = -1;
diff --git a/src/tree.c b/src/tree.c
index b3f5c302d..fedf4b604 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -26,9 +26,12 @@ static bool valid_filemode(const int filemode)
 
 static int valid_entry_name(const char *filename)
 {
-	return *filename != '\0' && strchr(filename, '/') == NULL &&
-		strcmp(filename, "..") != 0 && strcmp(filename, ".") != 0 &&
-		strcmp(filename, ".git") != 0;
+	return *filename != '\0' &&
+		strchr(filename, '/') == NULL &&
+		(*filename != '.' ||
+		 (strcmp(filename, ".") != 0 &&
+		  strcmp(filename, "..") != 0 &&
+		  strcmp(filename, DOT_GIT) != 0));
 }
 
 static int entry_sort_cmp(const void *a, const void *b)
@@ -299,9 +302,12 @@ size_t git_tree_entrycount(const git_tree *tree)
 	return tree->entries.length;
 }
 
-static int tree_error(const char *str)
+static int tree_error(const char *str, const char *path)
 {
-	giterr_set(GITERR_TREE, "%s", str);
+	if (path)
+		giterr_set(GITERR_TREE, "%s - %s", str, path);
+	else
+		giterr_set(GITERR_TREE, "%s", str);
 	return -1;
 }
 
@@ -316,13 +322,13 @@ static int tree_parse_buffer(git_tree *tree, const char *buffer, const char *buf
 
 		if (git__strtol32(&attr, buffer, &buffer, 8) < 0 ||
 			!buffer || !valid_filemode(attr))
-			return tree_error("Failed to parse tree. Can't parse filemode");
+			return tree_error("Failed to parse tree. Can't parse filemode", NULL);
 
 		if (*buffer++ != ' ')
-			return tree_error("Failed to parse tree. Object is corrupted");
+			return tree_error("Failed to parse tree. Object is corrupted", NULL);
 
 		if (memchr(buffer, 0, buffer_end - buffer) == NULL)
-			return tree_error("Failed to parse tree. Object is corrupted");
+			return tree_error("Failed to parse tree. Object is corrupted", NULL);
 
 		/** Allocate the entry and store it in the entries vector */
 		{
@@ -379,7 +385,7 @@ static int append_entry(
 	git_tree_entry *entry;
 
 	if (!valid_entry_name(filename))
-		return tree_error("Failed to insert entry. Invalid name for a tree entry");
+		return tree_error("Failed to insert entry. Invalid name for a tree entry", filename);
 
 	entry = alloc_entry(filename);
 	GITERR_CHECK_ALLOC(entry);
@@ -453,7 +459,8 @@ static int write_tree(
 			/* Write out the subtree */
 			written = write_tree(&sub_oid, repo, index, subdir, i);
 			if (written < 0) {
-				tree_error("Failed to write subtree");
+				tree_error("Failed to write subtree", subdir);
+				git__free(subdir);
 				goto on_error;
 			} else {
 				i = written - 1; /* -1 because of the loop increment */
@@ -471,18 +478,15 @@ static int write_tree(
 			} else {
 				last_comp = subdir;
 			}
+
 			error = append_entry(bld, last_comp, &sub_oid, S_IFDIR);
 			git__free(subdir);
-			if (error < 0) {
-				tree_error("Failed to insert dir");
+			if (error < 0)
 				goto on_error;
-			}
 		} else {
 			error = append_entry(bld, filename, &entry->oid, entry->mode);
-			if (error < 0) {
-				tree_error("Failed to insert file");
+			if (error < 0)
 				goto on_error;
-			}
 		}
 	}
 
@@ -587,12 +591,12 @@ int git_treebuilder_insert(
 	assert(bld && id && filename);
 
 	if (!valid_filemode(filemode))
-		return tree_error("Failed to insert entry. Invalid filemode");
+		return tree_error("Failed to insert entry. Invalid filemode for file", filename);
 
 	filemode = normalize_filemode(filemode);
 
 	if (!valid_entry_name(filename))
-		return tree_error("Failed to insert entry. Invalid name for a tree entry");
+		return tree_error("Failed to insert entry. Invalid name for a tree entry", filename);
 
 	pos = tree_key_search(&bld->entries, filename, strlen(filename));
 
@@ -648,7 +652,7 @@ int git_treebuilder_remove(git_treebuilder *bld, const char *filename)
 	git_tree_entry *remove_ptr = treebuilder_get(bld, filename);
 
 	if (remove_ptr == NULL || remove_ptr->removed)
-		return tree_error("Failed to remove entry. File isn't in the tree");
+		return tree_error("Failed to remove entry. File isn't in the tree", filename);
 
 	remove_ptr->removed = 1;
 	return 0;
diff --git a/tests-clar/diff/iterator.c b/tests-clar/diff/iterator.c
index 368903200..1d8396099 100644
--- a/tests-clar/diff/iterator.c
+++ b/tests-clar/diff/iterator.c
@@ -668,3 +668,59 @@ void test_diff_iterator__workdir_1_ranged_empty_2(void)
 		"status", NULL, "aaaa_empty_before",
 		0, 0, NULL, NULL);
 }
+
+void test_diff_iterator__workdir_builtin_ignores(void)
+{
+	git_repository *repo = cl_git_sandbox_init("attr");
+	git_iterator *i;
+	const git_index_entry *entry;
+	int idx;
+	static struct {
+		const char *path;
+		bool ignored;
+	} expected[] = {
+		{ "dir/", true },
+		{ "file", false },
+		{ "ign", true },
+		{ "macro_bad", false },
+		{ "macro_test", false },
+		{ "root_test1", false },
+		{ "root_test2", false },
+		{ "root_test3", false },
+		{ "root_test4.txt", false },
+		{ "sub/", false },
+		{ "sub/.gitattributes", false },
+		{ "sub/abc", false },
+		{ "sub/dir/", true },
+		{ "sub/file", false },
+		{ "sub/ign/", true },
+		{ "sub/sub/", false },
+		{ "sub/sub/.gitattributes", false },
+		{ "sub/sub/dir", false }, /* file is not actually a dir */
+		{ "sub/sub/file", false },
+		{ NULL, false }
+	};
+
+	cl_git_pass(p_mkdir("attr/sub/sub/.git", 0777));
+	cl_git_mkfile("attr/sub/.git", "whatever");
+
+	cl_git_pass(
+		git_iterator_for_workdir_range(&i, repo, "dir", "sub/sub/file"));
+	cl_git_pass(git_iterator_current(i, &entry));
+
+	for (idx = 0; entry != NULL; ++idx) {
+		int ignored = git_iterator_current_is_ignored(i);
+
+		cl_assert_equal_s(expected[idx].path, entry->path);
+		cl_assert_(ignored == expected[idx].ignored, expected[idx].path);
+
+		if (!ignored && S_ISDIR(entry->mode))
+			cl_git_pass(git_iterator_advance_into_directory(i, &entry));
+		else
+			cl_git_pass(git_iterator_advance(i, &entry));
+	}
+
+	cl_assert(expected[idx].path == NULL);
+
+	git_iterator_free(i);
+}
diff --git a/tests-clar/status/ignore.c b/tests-clar/status/ignore.c
index ddd0b3d6e..27f9d85b9 100644
--- a/tests-clar/status/ignore.c
+++ b/tests-clar/status/ignore.c
@@ -341,3 +341,41 @@ void test_status_ignore__internal_ignores_inside_deep_paths(void)
 	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "xthis/is/deep"));
 	cl_assert(!ignored);
 }
+
+void test_status_ignore__automatically_ignore_bad_files(void)
+{
+	int ignored;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, ".git"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/file/."));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/../funky"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/whatever.c"));
+	cl_assert(!ignored);
+
+	cl_git_pass(git_ignore_add_rule(g_repo, "*.c\n"));
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, ".git"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/file/."));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/../funky"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/whatever.c"));
+	cl_assert(ignored);
+
+	cl_git_pass(git_ignore_clear_internal_rules(g_repo));
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, ".git"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/file/."));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/../funky"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/whatever.c"));
+	cl_assert(!ignored);
+}