Commit adc9bdb3b1428b8edf067ab17c26ef15ec1ac8a7

Russell Belfer 2012-01-31T13:59:32

Fix attr path is_dir check When building an attr path object, the code that checks if the file is a directory was evaluating the file as a relative path to the current working directory, instead of using the repo root. This lead to inconsistent behavior.

diff --git a/src/attr.c b/src/attr.c
index da0f723..3fc01e8 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -21,7 +21,8 @@ int git_attr_get(
 
 	*value = NULL;
 
-	if ((error = git_attr_path__init(&path, pathname)) < GIT_SUCCESS ||
+	if ((error = git_attr_path__init(
+			&path, pathname, git_repository_workdir(repo))) < GIT_SUCCESS ||
 		(error = collect_attr_files(repo, pathname, &files)) < GIT_SUCCESS)
 		return git__rethrow(error, "Could not get attribute for %s", pathname);
 
@@ -69,7 +70,8 @@ int git_attr_get_many(
 
 	memset((void *)values, 0, sizeof(const char *) * num_attr);
 
-	if ((error = git_attr_path__init(&path, pathname)) < GIT_SUCCESS ||
+	if ((error = git_attr_path__init(
+			&path, pathname, git_repository_workdir(repo))) < GIT_SUCCESS ||
 		(error = collect_attr_files(repo, pathname, &files)) < GIT_SUCCESS)
 		return git__rethrow(error, "Could not get attributes for %s", pathname);
 
@@ -130,7 +132,8 @@ int git_attr_foreach(
 	git_attr_assignment *assign;
 	git_hashtable *seen = NULL;
 
-	if ((error = git_attr_path__init(&path, pathname)) < GIT_SUCCESS ||
+	if ((error = git_attr_path__init(
+			&path, pathname, git_repository_workdir(repo))) < GIT_SUCCESS ||
 		(error = collect_attr_files(repo, pathname, &files)) < GIT_SUCCESS)
 		return git__rethrow(error, "Could not get attributes for %s", pathname);
 
diff --git a/src/attr_file.c b/src/attr_file.c
index 5fd136c..74b2b6d 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -234,7 +234,7 @@ git_attr_assignment *git_attr_rule__lookup_assignment(
 }
 
 int git_attr_path__init(
-	git_attr_path *info, const char *path)
+	git_attr_path *info, const char *path, const char *base)
 {
 	assert(info && path);
 	info->path = path;
@@ -243,7 +243,17 @@ int git_attr_path__init(
 		info->basename++;
 	if (!info->basename || !*info->basename)
 		info->basename = path;
+
+	if (base != NULL && git_path_root(path) < 0) {
+		git_buf full_path = GIT_BUF_INIT;
+		int error = git_buf_joinpath(&full_path, base, path);
+		if (error == GIT_SUCCESS)
+			info->is_dir = (git_path_isdir(full_path.ptr) == GIT_SUCCESS);
+		git_buf_free(&full_path);
+		return error;
+	}
 	info->is_dir = (git_path_isdir(path) == GIT_SUCCESS);
+
 	return GIT_SUCCESS;
 }
 
diff --git a/src/attr_file.h b/src/attr_file.h
index 304c7a8..dcb66c5 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -110,7 +110,7 @@ extern git_attr_assignment *git_attr_rule__lookup_assignment(
 	git_attr_rule *rule, const char *name);
 
 extern int git_attr_path__init(
-	git_attr_path *info, const char *path);
+	git_attr_path *info, const char *path, const char *base);
 
 extern int git_attr_assignment__parse(
 	git_repository *repo, /* needed to expand macros */
diff --git a/src/ignore.c b/src/ignore.c
index 516da64..9690eba 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -66,24 +66,20 @@ static int load_ignore_file(
 #define push_ignore(R,S,B,F) \
 	git_attr_cache__push_file((R),(S),(B),(F),load_ignore_file)
 
-typedef struct {
-	git_repository *repo;
-	git_vector *stack;
-} ignore_walk_up_info;
-
 static int push_one_ignore(void *ref, git_buf *path)
 {
-	ignore_walk_up_info *info = (ignore_walk_up_info *)ref;
-	return push_ignore(info->repo, info->stack, path->ptr, GIT_IGNORE_FILE);
+	git_ignores *ign = (git_ignores *)ref;
+	return push_ignore(ign->repo, &ign->stack, path->ptr, GIT_IGNORE_FILE);
 }
 
-int git_ignore__for_path(git_repository *repo, const char *path, git_vector *stack)
+int git_ignore__for_path(git_repository *repo, const char *path, git_ignores *ignores)
 {
 	int error = GIT_SUCCESS;
 	git_buf dir = GIT_BUF_INIT;
 	git_config *cfg;
 	const char *workdir = git_repository_workdir(repo);
-	ignore_walk_up_info info;
+
+	assert(ignores);
 
 	if ((error = git_attr_cache__init(repo)) < GIT_SUCCESS)
 		goto cleanup;
@@ -91,18 +87,20 @@ int git_ignore__for_path(git_repository *repo, const char *path, git_vector *sta
 	if ((error = git_path_find_dir(&dir, path, workdir)) < GIT_SUCCESS)
 		goto cleanup;
 
+	ignores->repo = repo;
+	ignores->dir  = NULL;
+	git_vector_init(&ignores->stack, 2, NULL);
+
 	/* insert internals */
-	if ((error = push_ignore(repo, stack, NULL, GIT_IGNORE_INTERNAL)) < GIT_SUCCESS)
+	if ((error = push_ignore(repo, &ignores->stack, NULL, GIT_IGNORE_INTERNAL)) < GIT_SUCCESS)
 		goto cleanup;
 
 	/* load .gitignore up the path */
-	info.repo = repo;
-	info.stack = stack;
-	if ((error = git_path_walk_up(&dir, workdir, push_one_ignore, &info)) < GIT_SUCCESS)
+	if ((error = git_path_walk_up(&dir, workdir, push_one_ignore, ignores)) < GIT_SUCCESS)
 		goto cleanup;
 
 	/* load .git/info/exclude */
-	if ((error = push_ignore(repo, stack, repo->path_repository, GIT_IGNORE_FILE_INREPO)) < GIT_SUCCESS)
+	if ((error = push_ignore(repo, &ignores->stack, repo->path_repository, GIT_IGNORE_FILE_INREPO)) < GIT_SUCCESS)
 		goto cleanup;
 
 	/* load core.excludesfile */
@@ -110,7 +108,7 @@ int git_ignore__for_path(git_repository *repo, const char *path, git_vector *sta
 		const char *core_ignore;
 		error = git_config_get_string(cfg, GIT_IGNORE_CONFIG, &core_ignore);
 		if (error == GIT_SUCCESS && core_ignore != NULL)
-			error = push_ignore(repo, stack, NULL, core_ignore);
+			error = push_ignore(repo, &ignores->stack, NULL, core_ignore);
 		else {
 			error = GIT_SUCCESS;
 			git_clearerror(); /* don't care if attributesfile is not set */
@@ -121,18 +119,22 @@ int git_ignore__for_path(git_repository *repo, const char *path, git_vector *sta
 cleanup:
 	if (error < GIT_SUCCESS)
 		git__rethrow(error, "Could not get ignore files for '%s'", path);
+	else
+		ignores->dir = git_buf_detach(&dir);
 
 	git_buf_free(&dir);
 
 	return error;
 }
 
-void git_ignore__free(git_vector *stack)
+void git_ignore__free(git_ignores *ignores)
 {
-	git_vector_free(stack);
+	git__free(ignores->dir);
+	ignores->dir = NULL;
+	git_vector_free(&ignores->stack);
 }
 
-int git_ignore__lookup(git_vector *stack, const char *pathname, int *ignored)
+int git_ignore__lookup(git_ignores *ignores, const char *pathname, int *ignored)
 {
 	int error;
 	unsigned int i, j;
@@ -140,12 +142,13 @@ int git_ignore__lookup(git_vector *stack, const char *pathname, int *ignored)
 	git_attr_path path;
 	git_attr_fnmatch *match;
 
-	if ((error = git_attr_path__init(&path, pathname)) < GIT_SUCCESS)
+	if ((error = git_attr_path__init(
+		&path, pathname, git_repository_workdir(ignores->repo))) < GIT_SUCCESS)
 		return git__rethrow(error, "Could not get attribute for '%s'", pathname);
 
 	*ignored = 0;
 
-	git_vector_foreach(stack, i, file) {
+	git_vector_foreach(&ignores->stack, i, file) {
 		git_vector_rforeach(&file->rules, j, match) {
 			if (git_attr_fnmatch__match(match, &path) == GIT_SUCCESS) {
 				*ignored = ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0);
diff --git a/src/ignore.h b/src/ignore.h
index 2954445..9f87ae5 100644
--- a/src/ignore.h
+++ b/src/ignore.h
@@ -10,8 +10,14 @@
 #include "repository.h"
 #include "vector.h"
 
-extern int git_ignore__for_path(git_repository *repo, const char *path, git_vector *stack);
-extern void git_ignore__free(git_vector *stack);
-extern int git_ignore__lookup(git_vector *stack, const char *path, int *ignored);
+typedef struct {
+	git_repository *repo;
+	char *dir;
+	git_vector stack;
+} git_ignores;
+
+extern int git_ignore__for_path(git_repository *repo, const char *path, git_ignores *stack);
+extern void git_ignore__free(git_ignores *stack);
+extern int git_ignore__lookup(git_ignores *stack, const char *path, int *ignored);
 
 #endif
diff --git a/src/status.c b/src/status.c
index ae833ea..d10491c 100644
--- a/src/status.c
+++ b/src/status.c
@@ -124,7 +124,7 @@ static int status_entry_is_ignorable(struct status_entry *e)
 	return (e->status_flags == GIT_STATUS_WT_NEW);
 }
 
-static int status_entry_update_ignore(struct status_entry *e, git_vector *ignores, const char *path)
+static int status_entry_update_ignore(struct status_entry *e, git_ignores *ignores, const char *path)
 {
 	int error, ignored;
 
@@ -141,7 +141,7 @@ struct status_st {
 	git_vector *vector;
 	git_index *index;
 	git_tree *tree;
-	git_vector *ignores;
+	git_ignores *ignores;
 
 	int workdir_path_len;
 	git_buf head_tree_relative_path;
@@ -233,7 +233,7 @@ static int process_folder(
 
 
 	if (full_path != NULL && path_type == GIT_STATUS_PATH_FOLDER) {
-		git_vector ignores = GIT_VECTOR_INIT, *old_ignores;
+		git_ignores ignores, *old_ignores;
 
 		if ((error = git_ignore__for_path(st->repo,
 			full_path->ptr + st->workdir_path_len, &ignores)) == GIT_SUCCESS)
@@ -461,7 +461,8 @@ int git_status_foreach(
 	int (*callback)(const char *, unsigned int, void *),
 	void *payload)
 {
-	git_vector entries, ignores = GIT_VECTOR_INIT;
+	git_vector entries;
+	git_ignores ignores;
 	git_index *index = NULL;
 	git_buf temp_path = GIT_BUF_INIT;
 	struct status_st dirent_st = {0};
@@ -543,7 +544,7 @@ exit:
 	git_buf_free(&dirent_st.head_tree_relative_path);
 	git_buf_free(&temp_path);
 	git_vector_free(&entries);
-	git_vector_free(&ignores);
+	git_ignore__free(&ignores);
 	git_tree_free(tree);
 	return error;
 }
@@ -661,7 +662,7 @@ int git_status_file(unsigned int *status_flags, git_repository *repo, const char
 	}
 
 	if (status_entry_is_ignorable(e)) {
-		git_vector ignores = GIT_VECTOR_INIT;
+		git_ignores ignores;
 
 		if ((error = git_ignore__for_path(repo, path, &ignores)) == GIT_SUCCESS)
 			error = status_entry_update_ignore(e, &ignores, path);
@@ -776,7 +777,7 @@ static int alphasorted_futils_direach(
 int git_status_should_ignore(git_repository *repo, const char *path, int *ignored)
 {
 	int error;
-	git_vector ignores = GIT_VECTOR_INIT;
+	git_ignores ignores;
 
 	if ((error = git_ignore__for_path(repo, path, &ignores)) == GIT_SUCCESS)
 		error = git_ignore__lookup(&ignores, path, ignored);
diff --git a/tests-clar/attr/lookup.c b/tests-clar/attr/lookup.c
index 7779e04..9462bbe 100644
--- a/tests-clar/attr/lookup.c
+++ b/tests-clar/attr/lookup.c
@@ -12,7 +12,7 @@ void test_attr_lookup__simple(void)
 	cl_assert_strequal(cl_fixture("attr/attr0"), file->path);
 	cl_assert(file->rules.length == 1);
 
-	cl_git_pass(git_attr_path__init(&path, "test"));
+	cl_git_pass(git_attr_path__init(&path, "test", NULL));
 	cl_assert_strequal("test", path.path);
 	cl_assert_strequal("test", path.basename);
 	cl_assert(!path.is_dir);
@@ -42,7 +42,7 @@ static void run_test_cases(git_attr_file *file, test_case *cases)
 	int error;
 
 	for (c = cases; c->path != NULL; c++) {
-		cl_git_pass(git_attr_path__init(&path, c->path));
+		cl_git_pass(git_attr_path__init(&path, c->path, NULL));
 
 		if (c->force_dir)
 			path.is_dir = 1;
@@ -138,7 +138,7 @@ void test_attr_lookup__match_variants(void)
 	cl_assert_strequal(cl_fixture("attr/attr1"), file->path);
 	cl_assert(file->rules.length == 10);
 
-	cl_git_pass(git_attr_path__init(&path, "/testing/for/pat0"));
+	cl_git_pass(git_attr_path__init(&path, "/testing/for/pat0", NULL));
 	cl_assert_strequal("pat0", path.basename);
 
 	run_test_cases(file, cases);