Attributes: don't match files for folders

Merge of pull request #3119 from ethomson/ignore as a single commit.

Conflicts:
	src/attr.c
	tests/attr/ignore.c

3 files changed, 100 insertions, 44 deletions

diff --git a/src/attr.c b/src/attr.c
index a02172689..19a5a6157 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -254,13 +254,9 @@ static int attr_setup(git_repository *repo)
 			repo, GIT_ATTR_FILE__FROM_FILE, NULL, sys.ptr);
 		git_buf_free(&sys);
 	}
-	if (error < 0) {
-		if (error == GIT_ENOTFOUND) {
-			giterr_clear();
-			error = 0;
-		} else
-			return error;
-	}
+
+	if (error != GIT_ENOTFOUND)
+		return error;
 
 	if ((error = preload_attr_file(
 			repo, GIT_ATTR_FILE__FROM_FILE,
diff --git a/src/attr_file.c b/src/attr_file.c
index c0c99aa18..48c22e718 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -344,6 +344,7 @@ bool git_attr_fnmatch__match(
 	git_attr_fnmatch *match,
 	git_attr_path *path)
 {
+	const char *relpath = path->path;
 	const char *filename;
 	int flags = 0;
 
@@ -360,6 +361,8 @@ bool git_attr_fnmatch__match(
 			if (git__prefixcmp(path->path, match->containing_dir))
 				return 0;
 		}
+
+		relpath += match->containing_dir_length;
 	}
 
 	if (match->flags & GIT_ATTR_FNMATCH_ICASE)
@@ -368,7 +371,7 @@ bool git_attr_fnmatch__match(
 		flags |= FNM_LEADING_DIR;
 
 	if (match->flags & GIT_ATTR_FNMATCH_FULLPATH) {
-		filename = path->path;
+		filename = relpath;
 		flags |= FNM_PATHNAME;
 	} else {
 		filename = path->basename;
@@ -378,35 +381,33 @@ bool git_attr_fnmatch__match(
 	}
 
 	if ((match->flags & GIT_ATTR_FNMATCH_DIRECTORY) && !path->is_dir) {
-		int matchval;
-		char *matchpath;
+		bool samename;
 
 		/* for attribute checks or root ignore checks, fail match */
 		if (!(match->flags & GIT_ATTR_FNMATCH_IGNORE) ||
 			path->basename == path->path)
 			return false;
 
-		/* for ignore checks, use container of current item for check */
-		path->basename[-1] = '\0';
 		flags |= FNM_LEADING_DIR;
 
-		if (match->containing_dir)
-			matchpath = path->basename;
-		else
-			matchpath = path->path;
+		/* fail match if this is a file with same name as ignored folder */
+		samename = (match->flags & GIT_ATTR_FNMATCH_ICASE) ?
+			!strcasecmp(match->pattern, relpath) :
+			!strcmp(match->pattern, relpath);
+
+		if (samename)
+			return false;
 
-		matchval = p_fnmatch(match->pattern, matchpath, flags);
-		path->basename[-1] = '/';
-		return (matchval != FNM_NOMATCH);
+		return (p_fnmatch(match->pattern, relpath, flags) != FNM_NOMATCH);
 	}
 
 	/* if path is a directory prefix of a negated pattern, then match */
 	if ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) && path->is_dir) {
-		size_t pathlen = strlen(path->path);
+		size_t pathlen = strlen(relpath);
 		bool prefixed = (pathlen <= match->length) &&
 			((match->flags & GIT_ATTR_FNMATCH_ICASE) ?
-			 !strncasecmp(match->pattern, path->path, pathlen) :
-			 !strncmp(match->pattern, path->path, pathlen));
+			!strncasecmp(match->pattern, relpath, pathlen) :
+			!strncmp(match->pattern, relpath, pathlen));
 
 		if (prefixed && git_path_at_end_of_segment(&match->pattern[pathlen]))
 			return true;
@@ -611,7 +612,7 @@ int git_attr_fnmatch__parse(
 	}
 
 	if (context) {
-		char *slash = strchr(context, '/');
+		char *slash = strrchr(context, '/');
 		size_t len;
 		if (slash) {
 			/* include the slash for easier matching */
@@ -621,27 +622,7 @@ int git_attr_fnmatch__parse(
 		}
 	}
 
-	if ((spec->flags & GIT_ATTR_FNMATCH_FULLPATH) != 0 &&
-		context != NULL && git_path_root(pattern) < 0)
-	{
-		/* use context path minus the trailing filename */
-		char *slash = strrchr(context, '/');
-		size_t contextlen = slash ? slash - context + 1 : 0;
-
-		/* given an unrooted fullpath match from a file inside a repo,
-		 * prefix the pattern with the relative directory of the source file
-		 */
-		spec->pattern = git_pool_malloc(
-			pool, (uint32_t)(contextlen + spec->length + 1));
-		if (spec->pattern) {
-			memcpy(spec->pattern, context, contextlen);
-			memcpy(spec->pattern + contextlen, pattern, spec->length);
-			spec->length += contextlen;
-			spec->pattern[spec->length] = '\0';
-		}
-	} else {
-		spec->pattern = git_pool_strndup(pool, pattern, spec->length);
-	}
+	spec->pattern = git_pool_strndup(pool, pattern, spec->length);
 
 	if (!spec->pattern) {
 		*base = git__next_line(pattern);
diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c
index b187db01c..7d1eef593 100644
--- a/tests/attr/ignore.c
+++ b/tests/attr/ignore.c
@@ -146,6 +146,24 @@ void test_attr_ignore__skip_gitignore_directory(void)
 	assert_is_ignored(true, "NewFolder/NewFolder/File.txt");
 }
 
+void test_attr_ignore__subdirectory_gitignore(void)
+{
+	p_unlink("attr/.gitignore");
+	cl_assert(!git_path_exists("attr/.gitignore"));
+	cl_git_mkfile(
+		"attr/.gitignore",
+		"file1\n");
+	p_mkdir("attr/dir", 0777);
+	cl_git_mkfile(
+		"attr/dir/.gitignore",
+		"file2/\n");
+
+	assert_is_ignored(true, "file1");
+	assert_is_ignored(true, "dir/file1");
+	assert_is_ignored(true, "dir/file2/actual_file");  /* in ignored dir */
+	assert_is_ignored(false, "dir/file3");
+}
+
 void test_attr_ignore__expand_tilde_to_homedir(void)
 {
 	git_config *cfg;
@@ -173,3 +191,64 @@ void test_attr_ignore__expand_tilde_to_homedir(void)
 
 	assert_is_ignored(false, "example.global_with_tilde");
 }
+
+/* Ensure that the .gitignore in the subdirectory only affects
+ * items in the subdirectory. */
+void test_attr_ignore__gitignore_in_subdir(void)
+{
+	cl_git_pass(p_unlink("attr/.gitignore"));
+
+	cl_must_pass(p_mkdir("attr/dir1", 0777));
+	cl_must_pass(p_mkdir("attr/dir1/dir2", 0777));
+	cl_must_pass(p_mkdir("attr/dir1/dir2/dir3", 0777));
+
+	cl_git_mkfile("attr/dir1/dir2/dir3/.gitignore", "dir1/\ndir1/subdir/");
+
+	assert_is_ignored(false, "dir1/file");
+	assert_is_ignored(false, "dir1/dir2/file");
+	assert_is_ignored(false, "dir1/dir2/dir3/file");
+	assert_is_ignored(true,  "dir1/dir2/dir3/dir1/file");
+	assert_is_ignored(true,  "dir1/dir2/dir3/dir1/subdir/foo");
+
+	if (cl_repo_get_bool(g_repo, "core.ignorecase")) {
+		cl_git_mkfile("attr/dir1/dir2/dir3/.gitignore", "DiR1/\nDiR1/subdir/\n");
+
+		assert_is_ignored(false, "dir1/file");
+		assert_is_ignored(false, "dir1/dir2/file");
+		assert_is_ignored(false, "dir1/dir2/dir3/file");
+		assert_is_ignored(true,  "dir1/dir2/dir3/dir1/file");
+		assert_is_ignored(true,  "dir1/dir2/dir3/dir1/subdir/foo");
+	}
+}
+
+/* Ensure that files do not match folder cases */
+void test_attr_ignore__dont_ignore_files_for_folder(void)
+{
+	cl_git_pass(p_unlink("attr/.gitignore"));
+
+	cl_git_mkfile("attr/dir/.gitignore", "test/\n");
+
+	/* Create "test" as a file; ensure it is not ignored. */
+	cl_git_mkfile("attr/dir/test", "This is a file.");
+
+	assert_is_ignored(false, "dir/test");
+	if (cl_repo_get_bool(g_repo, "core.ignorecase"))
+		assert_is_ignored(false, "dir/TeSt");
+
+	/* Create "test" as a directory; ensure it is ignored. */
+	cl_git_pass(p_unlink("attr/dir/test"));
+	cl_must_pass(p_mkdir("attr/dir/test", 0777));
+
+	assert_is_ignored(true, "dir/test");
+	if (cl_repo_get_bool(g_repo, "core.ignorecase"))
+		assert_is_ignored(true, "dir/TeSt");
+
+	/* Remove "test" entirely; ensure it is not ignored.
+	 * (As it doesn't exist, it is not a directory.)
+	 */
+	cl_must_pass(p_rmdir("attr/dir/test"));
+
+	assert_is_ignored(false, "dir/test");
+	if (cl_repo_get_bool(g_repo, "core.ignorecase"))
+		assert_is_ignored(false, "dir/TeSt");
+}