Commit 2a99df6909af4c93ce2741ddc5d15a7f52270f28

Russell Belfer 2012-05-24T17:14:56

Fix bugs for status with spaces and reloaded attrs This fixes two bugs: * Issue #728 where git_status_file was not working for files that contain spaces. This was caused by reusing the "fnmatch" parsing code from ignore and attribute files to interpret the "pathspec" that constrained the files to apply the status to. In that code, unescaped whitespace was considered terminal to the pattern, so a file with internal whitespace was excluded from the matched files. The fix was to add a mode to that code that allows spaces and tabs inside patterns. This mode only comes into play when parsing in-memory strings. * The other issue was undetected, but it was in the recently added code to reload gitattributes / gitignores when they were changed on disk. That code was not clearing out the old values from the cached file content before reparsing which meant that newly added patterns would be read in, but deleted patterns would not be removed. The fix was to clear the vector of patterns in a cached file before reparsing the file.

diff --git a/src/attr.c b/src/attr.c
index 093f64d..fb66511 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -403,9 +403,14 @@ int git_attr_cache__push_file(
 		goto finish;
 	}
 
-	if (!file &&
-		(error = git_attr_file__new(&file, source, relfile, &cache->pool)) < 0)
-		goto finish;
+	/* if we got here, we have to parse and/or reparse the file */
+	if (file)
+		git_attr_file__clear_rules(file);
+	else {
+		error = git_attr_file__new(&file, source, relfile, &cache->pool);
+		if (error < 0)
+			goto finish;
+	}
 
 	if (parse && (error = parse(repo, content, file)) < 0)
 		goto finish;
diff --git a/src/attr_file.c b/src/attr_file.c
index 5030ad5..ca2f8fb 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -139,18 +139,23 @@ int git_attr_file__new_and_load(
 	return error;
 }
 
-void git_attr_file__free(git_attr_file *file)
+void git_attr_file__clear_rules(git_attr_file *file)
 {
 	unsigned int i;
 	git_attr_rule *rule;
 
-	if (!file)
-		return;
-
 	git_vector_foreach(&file->rules, i, rule)
 		git_attr_rule__free(rule);
 
 	git_vector_free(&file->rules);
+}
+
+void git_attr_file__free(git_attr_file *file)
+{
+	if (!file)
+		return;
+
+	git_attr_file__clear_rules(file);
 
 	if (file->pool_is_allocated) {
 		git_pool_clear(file->pool);
@@ -338,10 +343,13 @@ int git_attr_fnmatch__parse(
 	const char **base)
 {
 	const char *pattern, *scan;
-	int slash_count;
+	int slash_count, allow_space;
 
 	assert(spec && base && *base);
 
+	spec->flags = (spec->flags & GIT_ATTR_FNMATCH_ALLOWSPACE);
+	allow_space = (spec->flags != 0);
+
 	pattern = *base;
 
 	while (git__isspace(*pattern)) pattern++;
@@ -350,8 +358,6 @@ int git_attr_fnmatch__parse(
 		return GIT_ENOTFOUND;
 	}
 
-	spec->flags = 0;
-
 	if (*pattern == '[') {
 		if (strncmp(pattern, "[attr]", 6) == 0) {
 			spec->flags = spec->flags | GIT_ATTR_FNMATCH_MACRO;
@@ -368,8 +374,10 @@ int git_attr_fnmatch__parse(
 	slash_count = 0;
 	for (scan = pattern; *scan != '\0'; ++scan) {
 		/* scan until (non-escaped) white space */
-		if (git__isspace(*scan) && *(scan - 1) != '\\')
-			break;
+		if (git__isspace(*scan) && *(scan - 1) != '\\') {
+			if (!allow_space || (*scan != ' ' && *scan != '\t'))
+				break;
+		}
 
 		if (*scan == '/') {
 			spec->flags = spec->flags | GIT_ATTR_FNMATCH_FULLPATH;
diff --git a/src/attr_file.h b/src/attr_file.h
index 3718f4b..7939f83 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -22,6 +22,7 @@
 #define GIT_ATTR_FNMATCH_MACRO		(1U << 3)
 #define GIT_ATTR_FNMATCH_IGNORE		(1U << 4)
 #define GIT_ATTR_FNMATCH_HASWILD	(1U << 5)
+#define GIT_ATTR_FNMATCH_ALLOWSPACE	(1U << 6)
 
 typedef struct {
 	char *pattern;
@@ -88,6 +89,8 @@ extern int git_attr_file__new_and_load(
 
 extern void git_attr_file__free(git_attr_file *file);
 
+extern void git_attr_file__clear_rules(git_attr_file *file);
+
 extern int git_attr_file__parse_buffer(
 	git_repository *repo, const char *buf, git_attr_file *file);
 
diff --git a/src/diff.c b/src/diff.c
index 0b2f8fb..90baa95 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -342,6 +342,7 @@ static git_diff_list *git_diff_list_alloc(
 		git_attr_fnmatch *match = git__calloc(1, sizeof(git_attr_fnmatch));
 		if (!match)
 			goto fail;
+		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE;
 		ret = git_attr_fnmatch__parse(match, &diff->pool, NULL, &pattern);
 		if (ret == GIT_ENOTFOUND) {
 			git__free(match);
diff --git a/tests-clar/attr/attr_expect.h b/tests-clar/attr/attr_expect.h
index df1e104..70f1ab4 100644
--- a/tests-clar/attr/attr_expect.h
+++ b/tests-clar/attr/attr_expect.h
@@ -18,19 +18,20 @@ struct attr_expected {
 GIT_INLINE(void) attr_check_expected(
 	enum attr_expect_t expected,
 	const char *expected_str,
+	const char *name,
 	const char *value)
 {
 	switch (expected) {
 	case EXPECT_TRUE:
-		cl_assert(GIT_ATTR_TRUE(value));
+		cl_assert_(GIT_ATTR_TRUE(value), name);
 		break;
 
 	case EXPECT_FALSE:
-		cl_assert(GIT_ATTR_FALSE(value));
+		cl_assert_(GIT_ATTR_FALSE(value), name);
 		break;
 
 	case EXPECT_UNDEFINED:
-		cl_assert(GIT_ATTR_UNSPECIFIED(value));
+		cl_assert_(GIT_ATTR_UNSPECIFIED(value), name);
 		break;
 
 	case EXPECT_STRING:
diff --git a/tests-clar/attr/file.c b/tests-clar/attr/file.c
index d197088..8866fd9 100644
--- a/tests-clar/attr/file.c
+++ b/tests-clar/attr/file.c
@@ -114,7 +114,7 @@ static void check_one_assign(
 	cl_assert_equal_s(name, assign->name);
 	cl_assert(assign->name_hash == git_attr_file__name_hash(assign->name));
 
-	attr_check_expected(expected, expected_str, assign->value);
+	attr_check_expected(expected, expected_str, assign->name, assign->value);
 }
 
 void test_attr_file__assign_variants(void)
diff --git a/tests-clar/attr/lookup.c b/tests-clar/attr/lookup.c
index b2a6aac..40aac0b 100644
--- a/tests-clar/attr/lookup.c
+++ b/tests-clar/attr/lookup.c
@@ -44,7 +44,7 @@ static void run_test_cases(git_attr_file *file, struct attr_expected *cases, int
 		error = git_attr_file__lookup_one(file,&path,c->attr,&value);
 		cl_git_pass(error);
 
-		attr_check_expected(c->expected, c->expected_str, value);
+		attr_check_expected(c->expected, c->expected_str, c->attr, value);
 
 		git_attr_path__free(&path);
 	}
diff --git a/tests-clar/attr/repo.c b/tests-clar/attr/repo.c
index a88dfb3..c37ff54 100644
--- a/tests-clar/attr/repo.c
+++ b/tests-clar/attr/repo.c
@@ -65,7 +65,7 @@ void test_attr_repo__get_one(void)
 	for (scan = test_cases; scan->path != NULL; scan++) {
 		const char *value;
 		cl_git_pass(git_attr_get(&value, g_repo, 0, scan->path, scan->attr));
-		attr_check_expected(scan->expected, scan->expected_str, value);
+		attr_check_expected(scan->expected, scan->expected_str, scan->attr, value);
 	}
 
 	cl_assert(git_attr_cache__is_cached(g_repo, 0, ".git/info/attributes"));
diff --git a/tests-clar/core/env.c b/tests-clar/core/env.c
index 0d58e56..fb483e8 100644
--- a/tests-clar/core/env.c
+++ b/tests-clar/core/env.c
@@ -52,8 +52,11 @@ static int cl_setenv(const char *name, const char *value)
 
 #endif
 
-static char *env_home = NULL;
+#ifdef GIT_WIN32
 static char *env_userprofile = NULL;
+#else
+static char *env_home = NULL;
+#endif
 
 void test_core_env__initialize(void)
 {
diff --git a/tests-clar/status/worktree.c b/tests-clar/status/worktree.c
index 6cc6259..b3ebdb7 100644
--- a/tests-clar/status/worktree.c
+++ b/tests-clar/status/worktree.c
@@ -516,3 +516,68 @@ void test_status_worktree__status_file_with_clean_index_and_empty_workdir(void)
 	cl_git_pass(p_rmdir("wd"));
 	cl_git_pass(p_unlink("my-index"));
 }
+
+
+void test_status_worktree__space_in_filename(void)
+{
+	git_repository *repo;
+	git_index *index;
+	status_entry_single result;
+	unsigned int status_flags;
+
+#define FILE_WITH_SPACE "LICENSE - copy.md"
+
+	cl_git_pass(git_repository_init(&repo, "with_space", 0));
+	cl_git_mkfile("with_space/" FILE_WITH_SPACE, "I have a space in my name\n");
+
+	/* file is new to working directory */
+
+	memset(&result, 0, sizeof(result));
+	cl_git_pass(git_status_foreach(repo, cb_status__single, &result));
+	cl_assert_equal_i(1, result.count);
+	cl_assert(result.status == GIT_STATUS_WT_NEW);
+
+	cl_git_pass(git_status_file(&status_flags, repo, FILE_WITH_SPACE));
+	cl_assert(status_flags == GIT_STATUS_WT_NEW);
+
+	/* ignore the file */
+
+	cl_git_rewritefile("with_space/.gitignore", "*.md\n.gitignore\n");
+
+	memset(&result, 0, sizeof(result));
+	cl_git_pass(git_status_foreach(repo, cb_status__single, &result));
+	cl_assert_equal_i(2, result.count);
+	cl_assert(result.status == GIT_STATUS_IGNORED);
+
+	cl_git_pass(git_status_file(&status_flags, repo, FILE_WITH_SPACE));
+	cl_assert(status_flags == GIT_STATUS_IGNORED);
+
+	/* don't ignore the file */
+
+	cl_git_rewritefile("with_space/.gitignore", ".gitignore\n");
+
+	memset(&result, 0, sizeof(result));
+	cl_git_pass(git_status_foreach(repo, cb_status__single, &result));
+	cl_assert_equal_i(2, result.count);
+	cl_assert(result.status == GIT_STATUS_WT_NEW);
+
+	cl_git_pass(git_status_file(&status_flags, repo, FILE_WITH_SPACE));
+	cl_assert(status_flags == GIT_STATUS_WT_NEW);
+
+	/* add the file to the index */
+
+	cl_git_pass(git_repository_index(&index, repo));
+	cl_git_pass(git_index_add(index, FILE_WITH_SPACE, 0));
+	cl_git_pass(git_index_write(index));
+
+	memset(&result, 0, sizeof(result));
+	cl_git_pass(git_status_foreach(repo, cb_status__single, &result));
+	cl_assert_equal_i(2, result.count);
+	cl_assert(result.status == GIT_STATUS_INDEX_NEW);
+
+	cl_git_pass(git_status_file(&status_flags, repo, FILE_WITH_SPACE));
+	cl_assert(status_flags == GIT_STATUS_INDEX_NEW);
+
+	git_index_free(index);
+	git_repository_free(repo);
+}