Commit d58336dda873704f8d12a8b78c3191deefa4ec14

Russell Belfer 2012-04-26T10:51:45

Fix leading slash behavior in attrs/ignores We were not following the git behavior for leading slashes in path names when matching git ignores and git attribute file patterns. This should fix issue #638.

diff --git a/src/attr.c b/src/attr.c
index 3e3a7e7..120d127 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -23,10 +23,11 @@ int git_attr_get(
 
 	*value = NULL;
 
-	if ((error = git_attr_path__init(
-			&path, pathname, git_repository_workdir(repo))) < 0 ||
-		(error = collect_attr_files(repo, pathname, &files)) < 0)
-		return error;
+	if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0)
+		return -1;
+
+	if ((error = collect_attr_files(repo, pathname, &files)) < 0)
+		goto cleanup;
 
 	attr.name = name;
 	attr.name_hash = git_attr_file__name_hash(name);
@@ -38,13 +39,14 @@ int git_attr_get(
 			if (pos >= 0) {
 				*value = ((git_attr_assignment *)git_vector_get(
 							  &rule->assigns, pos))->value;
-				goto found;
+				goto cleanup;
 			}
 		}
 	}
 
-found:
+cleanup:
 	git_vector_free(&files);
+	git_attr_path__free(&path);
 
 	return error;
 }
@@ -70,10 +72,11 @@ int git_attr_get_many(
 
 	memset((void *)values, 0, sizeof(const char *) * num_attr);
 
-	if ((error = git_attr_path__init(
-			&path, pathname, git_repository_workdir(repo))) < 0 ||
-		(error = collect_attr_files(repo, pathname, &files)) < 0)
-		return error;
+	if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0)
+		return -1;
+
+	if ((error = collect_attr_files(repo, pathname, &files)) < 0)
+		goto cleanup;
 
 	info = git__calloc(num_attr, sizeof(attr_get_many_info));
 	GITERR_CHECK_ALLOC(info);
@@ -108,6 +111,7 @@ int git_attr_get_many(
 
 cleanup:
 	git_vector_free(&files);
+	git_attr_path__free(&path);
 	git__free(info);
 
 	return error;
@@ -128,10 +132,11 @@ int git_attr_foreach(
 	git_attr_assignment *assign;
 	git_strmap *seen = NULL;
 
-	if ((error = git_attr_path__init(
-			&path, pathname, git_repository_workdir(repo))) < 0 ||
-		(error = collect_attr_files(repo, pathname, &files)) < 0)
-		return error;
+	if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0)
+		return -1;
+
+	if ((error = collect_attr_files(repo, pathname, &files)) < 0)
+		goto cleanup;
 
 	seen = git_strmap_alloc();
 	GITERR_CHECK_ALLOC(seen);
@@ -158,6 +163,7 @@ int git_attr_foreach(
 cleanup:
 	git_strmap_free(seen);
 	git_vector_free(&files);
+	git_attr_path__free(&path);
 
 	return error;
 }
diff --git a/src/attr_file.c b/src/attr_file.c
index e34053f..650b58f 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -251,27 +251,50 @@ git_attr_assignment *git_attr_rule__lookup_assignment(
 int git_attr_path__init(
 	git_attr_path *info, const char *path, const char *base)
 {
-	assert(info && path);
-	info->path = path;
-	info->basename = strrchr(path, '/');
-	if (info->basename)
-		info->basename++;
-	if (!info->basename || !*info->basename)
-		info->basename = path;
+	/* build full path as best we can */
+	git_buf_init(&info->full, 0);
 
 	if (base != NULL && git_path_root(path) < 0) {
-		git_buf full_path = GIT_BUF_INIT;
-		if (git_buf_joinpath(&full_path, base, path) < 0)
+		if (git_buf_joinpath(&info->full, base, path) < 0)
+			return -1;
+		info->path = info->full.ptr + strlen(base);
+	} else {
+		if (git_buf_sets(&info->full, path) < 0)
 			return -1;
-		info->is_dir = (int)git_path_isdir(full_path.ptr);
-		git_buf_free(&full_path);
-		return 0;
+		info->path = info->full.ptr;
 	}
-	info->is_dir = (int)git_path_isdir(path);
+
+	/* remove trailing slashes */
+	while (info->full.size > 0) {
+		if (info->full.ptr[info->full.size - 1] != '/')
+			break;
+		info->full.size--;
+	}
+	info->full.ptr[info->full.size] = '\0';
+
+	/* skip leading slashes in path */
+	while (*info->path == '/')
+		info->path++;
+
+	/* find trailing basename component */
+	info->basename = strrchr(info->path, '/');
+	if (info->basename)
+		info->basename++;
+	if (!info->basename || !*info->basename)
+		info->basename = info->path;
+
+	info->is_dir = (int)git_path_isdir(info->full.ptr);
 
 	return 0;
 }
 
+void git_attr_path__free(git_attr_path *info)
+{
+	git_buf_free(&info->full);
+	info->path = NULL;
+	info->basename = NULL;
+}
+
 
 /*
  * From gitattributes(5):
@@ -353,6 +376,8 @@ int git_attr_fnmatch__parse(
 		if (*scan == '/') {
 			spec->flags = spec->flags | GIT_ATTR_FNMATCH_FULLPATH;
 			slash_count++;
+			if (pattern == scan)
+				pattern++;
 		}
 		/* remember if we see an unescaped wildcard in pattern */
 		else if ((*scan == '*' || *scan == '.' || *scan == '[') &&
diff --git a/src/attr_file.h b/src/attr_file.h
index 6775341..10851bc 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -10,6 +10,7 @@
 #include "git2/attr.h"
 #include "vector.h"
 #include "pool.h"
+#include "buffer.h"
 
 #define GIT_ATTR_FILE			".gitattributes"
 #define GIT_ATTR_FILE_INREPO	"info/attributes"
@@ -54,9 +55,10 @@ typedef struct {
 } git_attr_file;
 
 typedef struct {
+	git_buf     full;
 	const char *path;
 	const char *basename;
-	int is_dir;
+	int         is_dir;
 } git_attr_path;
 
 /*
@@ -114,6 +116,8 @@ extern git_attr_assignment *git_attr_rule__lookup_assignment(
 extern int git_attr_path__init(
 	git_attr_path *info, const char *path, const char *base);
 
+extern void git_attr_path__free(git_attr_path *info);
+
 extern int git_attr_assignment__parse(
 	git_repository *repo, /* needed to expand macros */
 	git_pool *pool,
diff --git a/src/ignore.c b/src/ignore.c
index 165754b..20b96c6 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -176,20 +176,23 @@ int git_ignore__lookup(git_ignores *ignores, const char *pathname, int *ignored)
 	/* first process builtins - success means path was found */
 	if (ignore_lookup_in_rules(
 			&ignores->ign_internal->rules, &path, ignored))
-		return 0;
+		goto cleanup;
 
 	/* next process files in the path */
 	git_vector_foreach(&ignores->ign_path, i, file) {
 		if (ignore_lookup_in_rules(&file->rules, &path, ignored))
-			return 0;
+			goto cleanup;
 	}
 
 	/* last process global ignores */
 	git_vector_foreach(&ignores->ign_global, i, file) {
 		if (ignore_lookup_in_rules(&file->rules, &path, ignored))
-			return 0;
+			goto cleanup;
 	}
 
 	*ignored = 0;
+
+cleanup:
+	git_attr_path__free(&path);
 	return 0;
 }
diff --git a/tests-clar/attr/lookup.c b/tests-clar/attr/lookup.c
index accd617..81a4a55 100644
--- a/tests-clar/attr/lookup.c
+++ b/tests-clar/attr/lookup.c
@@ -25,6 +25,7 @@ void test_attr_lookup__simple(void)
 	cl_git_pass(git_attr_file__lookup_one(file,&path,"missing",&value));
 	cl_assert(!value);
 
+	git_attr_path__free(&path);
 	git_attr_file__free(file);
 }
 
@@ -45,6 +46,8 @@ static void run_test_cases(git_attr_file *file, struct attr_expected *cases, int
 		cl_git_pass(error);
 
 		attr_check_expected(c->expected, c->expected_str, value);
+
+		git_attr_path__free(&path);
 	}
 }
 
@@ -83,7 +86,7 @@ void test_attr_lookup__match_variants(void)
 		{ "/not/pat2/yousee", "attr2", EXPECT_UNDEFINED, NULL },
 		/* path match */
 		{ "pat3file", "attr3", EXPECT_UNDEFINED, NULL },
-		{ "/pat3dir/pat3file", "attr3", EXPECT_UNDEFINED, NULL },
+		{ "/pat3dir/pat3file", "attr3", EXPECT_TRUE, NULL },
 		{ "pat3dir/pat3file", "attr3", EXPECT_TRUE, NULL },
 		/* pattern* match */
 		{ "pat4.txt", "attr4", EXPECT_TRUE, NULL },
@@ -101,7 +104,7 @@ void test_attr_lookup__match_variants(void)
 		{ "pat6/pat6/.pat6", "attr6", EXPECT_TRUE, NULL },
 		{ "pat6/pat6/extra/foobar.pat6", "attr6", EXPECT_UNDEFINED, NULL },
 		{ "/prefix/pat6/pat6/foobar.pat6", "attr6", EXPECT_UNDEFINED, NULL },
-		{ "/pat6/pat6/foobar.pat6", "attr6", EXPECT_UNDEFINED, NULL },
+		{ "/pat6/pat6/foobar.pat6", "attr6", EXPECT_TRUE, NULL },
 		/* complex pattern */
 		{ "pat7a12z", "attr7", EXPECT_TRUE, NULL },
 		{ "pat7e__x", "attr7", EXPECT_TRUE, NULL },
@@ -139,6 +142,7 @@ void test_attr_lookup__match_variants(void)
 	run_test_cases(file, dir_cases, 1);
 
 	git_attr_file__free(file);
+	git_attr_path__free(&path);
 }
 
 void test_attr_lookup__assign_variants(void)
diff --git a/tests-clar/status/ignore.c b/tests-clar/status/ignore.c
index 5d94007..94f8c3d 100644
--- a/tests-clar/status/ignore.c
+++ b/tests-clar/status/ignore.c
@@ -50,3 +50,31 @@ void test_status_ignore__0(void)
 	cl_assert(git_attr_cache__is_cached(g_repo, ".git/info/exclude"));
 	cl_assert(git_attr_cache__is_cached(g_repo, ".gitignore"));
 }
+
+
+void test_status_ignore__1(void)
+{
+	int ignored;
+
+	cl_git_rewritefile("attr/.gitignore", "/*.txt\n/dir/\n");
+	git_attr_cache_flush(g_repo);
+
+	cl_git_pass(git_status_should_ignore(g_repo, "root_test4.txt", &ignored));
+	cl_assert(ignored);
+
+	cl_git_pass(git_status_should_ignore(g_repo, "sub/subdir_test2.txt", &ignored));
+	cl_assert(!ignored);
+
+	cl_git_pass(git_status_should_ignore(g_repo, "dir", &ignored));
+	cl_assert(ignored);
+
+	cl_git_pass(git_status_should_ignore(g_repo, "dir/", &ignored));
+	cl_assert(ignored);
+
+	cl_git_pass(git_status_should_ignore(g_repo, "sub/dir", &ignored));
+	cl_assert(!ignored);
+
+	cl_git_pass(git_status_should_ignore(g_repo, "sub/dir/", &ignored));
+	cl_assert(!ignored);
+}
+