Merge pull request #5156 from pks-t/pks/attr-macros-in-subdir

gitattributes: ignore macros defined in subdirectories

src/attr.c

@@ -252,15 +252,16 @@ static int preload_attr_file(
 	git_attr_session *attr_session,
 	git_attr_file_source source,
 	const char *base,
-	const char *file)
+	const char *file,
+	bool allow_macros)
 {
 	int error;
 	git_attr_file *preload = NULL;
 
 	if (!file)
 		return 0;
-	if (!(error = git_attr_cache__get(
-			&preload, repo, attr_session, source, base, file, git_attr_file__parse_buffer)))
+	if (!(error = git_attr_cache__get(&preload, repo, attr_session, source, base, file,
+					  git_attr_file__parse_buffer, allow_macros)))
 		git_attr_file__free(preload);
 
 	return error;
@@ -324,31 +325,31 @@ static int attr_setup(git_repository *repo, git_attr_session *attr_session)
 
 	if ((error = system_attr_file(&path, attr_session)) < 0 ||
 	    (error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
-				       NULL, path.ptr)) < 0) {
+				       NULL, path.ptr, true)) < 0) {
 		if (error != GIT_ENOTFOUND)
 			goto out;
 	}
 
 	if ((error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
-				       NULL, git_repository_attr_cache(repo)->cfg_attr_file)) < 0)
+				       NULL, git_repository_attr_cache(repo)->cfg_attr_file, true)) < 0)
 		goto out;
 
 	git_buf_clear(&path); /* git_repository_item_path expects an empty buffer, because it uses git_buf_set */
 	if ((error = git_repository_item_path(&path, repo, GIT_REPOSITORY_ITEM_INFO)) < 0 ||
 	    (error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
-				       path.ptr, GIT_ATTR_FILE_INREPO)) < 0) {
+				       path.ptr, GIT_ATTR_FILE_INREPO, true)) < 0) {
 		if (error != GIT_ENOTFOUND)
 			goto out;
 	}
 
 	if ((workdir = git_repository_workdir(repo)) != NULL &&
 	    (error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
-				       workdir, GIT_ATTR_FILE)) < 0)
+				       workdir, GIT_ATTR_FILE, true)) < 0)
 			goto out;
 
 	if ((error = git_repository_index__weakptr(&idx, repo)) < 0 ||
 	    (error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_INDEX,
-				       NULL, GIT_ATTR_FILE)) < 0)
+				       NULL, GIT_ATTR_FILE, true)) < 0)
 			goto out;
 
 	if (attr_session)
@@ -436,13 +437,14 @@ static int push_attr_file(
 	git_vector *list,
 	git_attr_file_source source,
 	const char *base,
-	const char *filename)
+	const char *filename,
+	bool allow_macros)
 {
 	int error = 0;
 	git_attr_file *file = NULL;
 
 	error = git_attr_cache__get(&file, repo, attr_session,
-		source, base, filename, git_attr_file__parse_buffer);
+		source, base, filename, git_attr_file__parse_buffer, allow_macros);
 
 	if (error < 0)
 		return error;
@@ -457,16 +459,18 @@ static int push_attr_file(
 
 static int push_one_attr(void *ref, const char *path)
 {
-	int error = 0, n_src, i;
 	attr_walk_up_info *info = (attr_walk_up_info *)ref;
 	git_attr_file_source src[2];
+	int error = 0, n_src, i;
+	bool allow_macros;
 
 	n_src = attr_decide_sources(
 		info->flags, info->workdir != NULL, info->index != NULL, src);
+	allow_macros = info->workdir ? !strcmp(info->workdir, path) : false;
 
 	for (i = 0; !error && i < n_src; ++i)
-		error = push_attr_file(info->repo, info->attr_session,
-			info->files, src[i], path, GIT_ATTR_FILE);
+		error = push_attr_file(info->repo, info->attr_session, info->files,
+				       src[i], path, GIT_ATTR_FILE, allow_macros);
 
 	return error;
 }
@@ -515,7 +519,7 @@ static int collect_attr_files(
 
 	if ((error = git_repository_item_path(&attrfile, repo, GIT_REPOSITORY_ITEM_INFO)) < 0 ||
 	    (error = push_attr_file(repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
-				    attrfile.ptr, GIT_ATTR_FILE_INREPO)) < 0) {
+				    attrfile.ptr, GIT_ATTR_FILE_INREPO, true)) < 0) {
 		if (error != GIT_ENOTFOUND)
 			goto cleanup;
 	}
@@ -537,9 +541,8 @@ static int collect_attr_files(
 		goto cleanup;
 
 	if (git_repository_attr_cache(repo)->cfg_attr_file != NULL) {
-		error = push_attr_file(
-			repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
-			NULL, git_repository_attr_cache(repo)->cfg_attr_file);
+		error = push_attr_file(repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
+				       NULL, git_repository_attr_cache(repo)->cfg_attr_file, true);
 		if (error < 0)
 			goto cleanup;
 	}
@@ -548,9 +551,8 @@ static int collect_attr_files(
 		error = system_attr_file(&dir, attr_session);
 
 		if (!error)
-			error = push_attr_file(
-				repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
-				NULL, dir.ptr);
+			error = push_attr_file(repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
+					       NULL, dir.ptr, true);
 		else if (error == GIT_ENOTFOUND)
 			error = 0;
 	}

src/attr_file.c

@@ -105,7 +105,8 @@ int git_attr_file__load(
 	git_attr_session *attr_session,
 	git_attr_file_entry *entry,
 	git_attr_file_source source,
-	git_attr_file_parser parser)
+	git_attr_file_parser parser,
+	bool allow_macros)
 {
 	int error = 0;
 	git_blob *blob = NULL;
@@ -177,7 +178,7 @@ int git_attr_file__load(
 	if (attr_session)
 		file->session_key = attr_session->key;
 
-	if (parser && (error = parser(repo, file, content_str)) < 0) {
+	if (parser && (error = parser(repo, file, content_str, allow_macros)) < 0) {
 		git_attr_file__free(file);
 		goto cleanup;
 	}
@@ -249,16 +250,15 @@ static bool parse_optimized_patterns(
 	const char *pattern);
 
 int git_attr_file__parse_buffer(
-	git_repository *repo, git_attr_file *attrs, const char *data)
+	git_repository *repo, git_attr_file *attrs, const char *data, bool allow_macros)
 {
-	int error = 0;
 	const char *scan = data, *context = NULL;
 	git_attr_rule *rule = NULL;
+	int error = 0;
 
-	/* if subdir file path, convert context for file paths */
-	if (attrs->entry &&
-		git_path_root(attrs->entry->path) < 0 &&
-		!git__suffixcmp(attrs->entry->path, "/" GIT_ATTR_FILE))
+	/* If subdir file path, convert context for file paths */
+	if (attrs->entry && git_path_root(attrs->entry->path) < 0 &&
+	    !git__suffixcmp(attrs->entry->path, "/" GIT_ATTR_FILE))
 		context = attrs->entry->path;
 
 	if (git_mutex_lock(&attrs->lock) < 0) {
@@ -267,38 +267,38 @@ int git_attr_file__parse_buffer(
 	}
 
 	while (!error && *scan) {
-		/* allocate rule if needed */
-		if (!rule && !(rule = git__calloc(1, sizeof(*rule)))) {
-			error = -1;
-			break;
-		}
-
-		rule->match.flags =
-			GIT_ATTR_FNMATCH_ALLOWNEG | GIT_ATTR_FNMATCH_ALLOWMACRO;
-
-		/* parse the next "pattern attr attr attr" line */
-		if (!(error = git_attr_fnmatch__parse(
-				&rule->match, &attrs->pool, context, &scan)) &&
-			!(error = git_attr_assignment__parse(
-				repo, &attrs->pool, &rule->assigns, &scan)))
+		/* Allocate rule if needed, otherwise re-use previous rule */
+		if (!rule) {
+			rule = git__calloc(1, sizeof(*rule));
+			GIT_ERROR_CHECK_ALLOC(rule);
+		} else
+			git_attr_rule__clear(rule);
+
+		rule->match.flags = GIT_ATTR_FNMATCH_ALLOWNEG | GIT_ATTR_FNMATCH_ALLOWMACRO;
+
+		/* Parse the next "pattern attr attr attr" line */
+		if ((error = git_attr_fnmatch__parse(&rule->match, &attrs->pool, context, &scan)) < 0 ||
+		    (error = git_attr_assignment__parse(repo, &attrs->pool, &rule->assigns, &scan)) < 0)
 		{
-			if (rule->match.flags & GIT_ATTR_FNMATCH_MACRO)
-				/* TODO: warning if macro found in file below repo root */
-				error = git_attr_cache__insert_macro(repo, rule);
-			else
-				error = git_vector_insert(&attrs->rules, rule);
+			if (error != GIT_ENOTFOUND)
+				goto out;
+			error = 0;
+			continue;
 		}
 
-		/* if the rule wasn't a pattern, on to the next */
-		if (error < 0) {
-			git_attr_rule__clear(rule); /* reset rule contents */
-			if (error == GIT_ENOTFOUND)
-				error = 0;
-		} else {
-			rule = NULL; /* vector now "owns" the rule */
-		}
+		if (rule->match.flags & GIT_ATTR_FNMATCH_MACRO) {
+			/* TODO: warning if macro found in file below repo root */
+			if (!allow_macros)
+				continue;
+			if ((error = git_attr_cache__insert_macro(repo, rule)) < 0)
+				goto out;
+		} else if ((error = git_vector_insert(&attrs->rules, rule)) < 0)
+			goto out;
+
+		rule = NULL;
 	}
 
+out:
 	git_mutex_unlock(&attrs->lock);
 	git_attr_rule__free(rule);
 
@@ -345,33 +345,28 @@ int git_attr_file__lookup_one(
 
 int git_attr_file__load_standalone(git_attr_file **out, const char *path)
 {
-	int error;
-	git_attr_file *file;
 	git_buf content = GIT_BUF_INIT;
+	git_attr_file *file = NULL;
+	int error;
 
-	error = git_attr_file__new(&file, NULL, GIT_ATTR_FILE__FROM_FILE);
-	if (error < 0)
-		return error;
+	if ((error = git_futils_readbuffer(&content, path)) < 0)
+		goto out;
 
-	error = git_attr_cache__alloc_file_entry(
-		&file->entry, NULL, path, &file->pool);
-	if (error < 0) {
-		git_attr_file__free(file);
-		return error;
-	}
-	/* because the cache entry is allocated from the file's own pool, we
+	/*
+	 * Because the cache entry is allocated from the file's own pool, we
 	 * don't have to free it - freeing file+pool will free cache entry, too.
 	 */
 
-	if (!(error = git_futils_readbuffer(&content, path))) {
-		error = git_attr_file__parse_buffer(NULL, file, content.ptr);
-		git_buf_dispose(&content);
-	}
+	if ((error = git_attr_file__new(&file, NULL, GIT_ATTR_FILE__FROM_FILE)) < 0 ||
+	    (error = git_attr_file__parse_buffer(NULL, file, content.ptr, true)) < 0 ||
+	    (error = git_attr_cache__alloc_file_entry(&file->entry, NULL, path, &file->pool)) < 0)
+		goto out;
 
+	*out = file;
+out:
 	if (error < 0)
 		git_attr_file__free(file);
-	else
-		*out = file;
+	git_buf_dispose(&content);
 
 	return error;
 }

src/attr_file.h

@@ -131,7 +131,8 @@ extern int git_attr_get_many_with_session(
 typedef int (*git_attr_file_parser)(
 	git_repository *repo,
 	git_attr_file *file,
-	const char *data);
+	const char *data,
+	bool allow_macros);
 
 /*
  * git_attr_file API
@@ -150,7 +151,8 @@ int git_attr_file__load(
 	git_attr_session *attr_session,
 	git_attr_file_entry *ce,
 	git_attr_file_source source,
-	git_attr_file_parser parser);
+	git_attr_file_parser parser,
+	bool allow_macros);
 
 int git_attr_file__load_standalone(
 	git_attr_file **out, const char *path);
@@ -159,7 +161,7 @@ int git_attr_file__out_of_date(
 	git_repository *repo, git_attr_session *session, git_attr_file *file);
 
 int git_attr_file__parse_buffer(
-	git_repository *repo, git_attr_file *attrs, const char *data);
+	git_repository *repo, git_attr_file *attrs, const char *data, bool allow_macros);
 
 int git_attr_file__clear_rules(
 	git_attr_file *file, bool need_lock);

src/attrcache.c

@@ -208,7 +208,8 @@ int git_attr_cache__get(
 	git_attr_file_source source,
 	const char *base,
 	const char *filename,
-	git_attr_file_parser parser)
+	git_attr_file_parser parser,
+	bool allow_macros)
 {
 	int error = 0;
 	git_attr_cache *cache = git_repository_attr_cache(repo);
@@ -221,7 +222,7 @@ int git_attr_cache__get(
 
 	/* load file if we don't have one or if existing one is out of date */
 	if (!file || (error = git_attr_file__out_of_date(repo, attr_session, file)) > 0)
-		error = git_attr_file__load(&updated, repo, attr_session, entry, source, parser);
+		error = git_attr_file__load(&updated, repo, attr_session, entry, source, parser, allow_macros);
 
 	/* if we loaded the file, insert into and/or update cache */
 	if (updated) {
@@ -424,21 +425,36 @@ void git_attr_cache_flush(git_repository *repo)
 int git_attr_cache__insert_macro(git_repository *repo, git_attr_rule *macro)
 {
 	git_attr_cache *cache = git_repository_attr_cache(repo);
-	git_strmap *macros = cache->macros;
-	int error;
-
-	/* TODO: generate warning log if (macro->assigns.length == 0) */
-	if (macro->assigns.length == 0)
-		return 0;
+	git_attr_rule *preexisting;
+	bool locked = false;
+	int error = 0;
 
-	if (attr_cache_lock(cache) < 0) {
-		git_error_set(GIT_ERROR_OS, "unable to get attr cache lock");
-		error = -1;
-	} else {
-		error = git_strmap_set(macros, macro->match.pattern, macro);
-		git_mutex_unlock(&cache->lock);
+	/*
+	 * Callers assume that if we return success, that the
+	 * macro will have been adopted by the attributes cache.
+	 * Thus, we have to free the macro here if it's not being
+	 * added to the cache.
+	 *
+	 * TODO: generate warning log if (macro->assigns.length == 0)
+	 */
+	if (macro->assigns.length == 0) {
+		git_attr_rule__free(macro);
+		goto out;
 	}
 
+	if ((error = attr_cache_lock(cache)) < 0)
+		goto out;
+	locked = true;
+
+	if ((preexisting = git_strmap_get(cache->macros, macro->match.pattern)) != NULL)
+	    git_attr_rule__free(preexisting);
+
+	if ((error = git_strmap_set(cache->macros, macro->match.pattern, macro)) < 0)
+	    goto out;
+
+out:
+	if (locked)
+		attr_cache_unlock(cache);
 	return error;
 }
 

src/attrcache.h

@@ -34,7 +34,8 @@ extern int git_attr_cache__get(
 	git_attr_file_source source,
 	const char *base,
 	const char *filename,
-	git_attr_file_parser parser);
+	git_attr_file_parser parser,
+	bool allow_macros);
 
 extern bool git_attr_cache__is_cached(
 	git_repository *repo,

src/ignore.c

@@ -163,13 +163,15 @@ out:
 }
 
 static int parse_ignore_file(
-	git_repository *repo, git_attr_file *attrs, const char *data)
+	git_repository *repo, git_attr_file *attrs, const char *data, bool allow_macros)
 {
 	int error = 0;
 	int ignore_case = false;
 	const char *scan = data, *context = NULL;
 	git_attr_fnmatch *match = NULL;
 
+	GIT_UNUSED(allow_macros);
+
 	if (git_repository__cvar(&ignore_case, repo, GIT_CVAR_IGNORECASE) < 0)
 		git_error_clear();
 
@@ -244,9 +246,8 @@ static int push_ignore_file(
 	int error = 0;
 	git_attr_file *file = NULL;
 
-	error = git_attr_cache__get(
-		&file, ignores->repo, NULL, GIT_ATTR_FILE__FROM_FILE,
-		base, filename, parse_ignore_file);
+	error = git_attr_cache__get(&file, ignores->repo, NULL, GIT_ATTR_FILE__FROM_FILE,
+				    base, filename, parse_ignore_file, false);
 	if (error < 0)
 		return error;
 
@@ -272,12 +273,12 @@ static int get_internal_ignores(git_attr_file **out, git_repository *repo)
 	if ((error = git_attr_cache__init(repo)) < 0)
 		return error;
 
-	error = git_attr_cache__get(
-		out, repo, NULL, GIT_ATTR_FILE__IN_MEMORY, NULL, GIT_IGNORE_INTERNAL, NULL);
+	error = git_attr_cache__get(out, repo, NULL, GIT_ATTR_FILE__IN_MEMORY, NULL,
+				    GIT_IGNORE_INTERNAL, NULL, false);
 
 	/* if internal rules list is empty, insert default rules */
 	if (!error && !(*out)->rules.length)
-		error = parse_ignore_file(repo, *out, GIT_IGNORE_DEFAULT_RULES);
+		error = parse_ignore_file(repo, *out, GIT_IGNORE_DEFAULT_RULES, false);
 
 	return error;
 }
@@ -487,7 +488,7 @@ int git_ignore_add_rule(git_repository *repo, const char *rules)
 	if ((error = get_internal_ignores(&ign_internal, repo)) < 0)
 		return error;
 
-	error = parse_ignore_file(repo, ign_internal, rules);
+	error = parse_ignore_file(repo, ign_internal, rules, false);
 	git_attr_file__free(ign_internal);
 
 	return error;
@@ -503,7 +504,7 @@ int git_ignore_clear_internal_rules(git_repository *repo)
 
 	if (!(error = git_attr_file__clear_rules(ign_internal, true)))
 		error = parse_ignore_file(
-			repo, ign_internal, GIT_IGNORE_DEFAULT_RULES);
+				repo, ign_internal, GIT_IGNORE_DEFAULT_RULES, false);
 
 	git_attr_file__free(ign_internal);
 	return error;

tests/attr/lookup.c

@@ -252,7 +252,7 @@ void test_attr_lookup__from_buffer(void)
 
 	cl_git_pass(git_attr_file__new(&file, NULL, 0));
 
-	cl_git_pass(git_attr_file__parse_buffer(NULL, file, "a* foo\nabc bar\n* baz"));
+	cl_git_pass(git_attr_file__parse_buffer(NULL, file, "a* foo\nabc bar\n* baz", true));
 
 	cl_assert(file->rules.length == 3);
 

tests/attr/macro.c

+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+
+#include "clar_libgit2.h"
+
+#include "git2/sys/repository.h"
+#include "attr.h"
+
+static git_repository *g_repo = NULL;
+
+void test_attr_macro__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+	g_repo = NULL;
+}
+
+void test_attr_macro__macros(void)
+{
+	const char *names[5] = { "rootattr", "binary", "diff", "crlf", "frotz" };
+	const char *names2[5] = { "mymacro", "positive", "negative", "rootattr", "another" };
+	const char *names3[3] = { "macro2", "multi2", "multi3" };
+	const char *values[5];
+
+	g_repo = cl_git_sandbox_init("attr");
+
+	cl_git_pass(git_attr_get_many(values, g_repo, 0, "binfile", 5, names));
+
+	cl_assert(GIT_ATTR_IS_TRUE(values[0]));
+	cl_assert(GIT_ATTR_IS_TRUE(values[1]));
+	cl_assert(GIT_ATTR_IS_FALSE(values[2]));
+	cl_assert(GIT_ATTR_IS_FALSE(values[3]));
+	cl_assert(GIT_ATTR_IS_UNSPECIFIED(values[4]));
+
+	cl_git_pass(git_attr_get_many(values, g_repo, 0, "macro_test", 5, names2));
+
+	cl_assert(GIT_ATTR_IS_TRUE(values[0]));
+	cl_assert(GIT_ATTR_IS_TRUE(values[1]));
+	cl_assert(GIT_ATTR_IS_FALSE(values[2]));
+	cl_assert(GIT_ATTR_IS_UNSPECIFIED(values[3]));
+	cl_assert_equal_s("77", values[4]);
+
+	cl_git_pass(git_attr_get_many(values, g_repo, 0, "macro_test", 3, names3));
+
+	cl_assert(GIT_ATTR_IS_TRUE(values[0]));
+	cl_assert(GIT_ATTR_IS_FALSE(values[1]));
+	cl_assert_equal_s("answer", values[2]);
+}
+
+void test_attr_macro__bad_macros(void)
+{
+	const char *names[6] = { "rootattr", "positive", "negative",
+		"firstmacro", "secondmacro", "thirdmacro" };
+	const char *values[6];
+
+	g_repo = cl_git_sandbox_init("attr");
+
+	cl_git_pass(git_attr_get_many(values, g_repo, 0, "macro_bad", 6, names));
+
+	/* these three just confirm that the "mymacro" rule ran */
+	cl_assert(GIT_ATTR_IS_UNSPECIFIED(values[0]));
+	cl_assert(GIT_ATTR_IS_TRUE(values[1]));
+	cl_assert(GIT_ATTR_IS_FALSE(values[2]));
+
+	/* file contains:
+	 *     # let's try some malicious macro defs
+	 *     [attr]firstmacro -thirdmacro -secondmacro
+	 *     [attr]secondmacro firstmacro -firstmacro
+	 *     [attr]thirdmacro secondmacro=hahaha -firstmacro
+	 *     macro_bad firstmacro secondmacro thirdmacro
+	 *
+	 * firstmacro assignment list ends up with:
+	 *     -thirdmacro -secondmacro
+	 * secondmacro assignment list expands "firstmacro" and ends up with:
+	 *     -thirdmacro -secondmacro -firstmacro
+	 * thirdmacro assignment don't expand so list ends up with:
+	 *     secondmacro="hahaha"
+	 *
+	 * macro_bad assignment list ends up with:
+	 *     -thirdmacro -secondmacro firstmacro &&
+	 *     -thirdmacro -secondmacro -firstmacro secondmacro &&
+	 *     secondmacro="hahaha" thirdmacro
+	 *
+	 * so summary results should be:
+	 *     -firstmacro secondmacro="hahaha" thirdmacro
+	 */
+	cl_assert(GIT_ATTR_IS_FALSE(values[3]));
+	cl_assert_equal_s("hahaha", values[4]);
+	cl_assert(GIT_ATTR_IS_TRUE(values[5]));
+}
+
+void test_attr_macro__macros_in_root_wd_apply(void)
+{
+	const char *value;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_pass(p_mkdir("empty_standard_repo/dir", 0777));
+	cl_git_rewritefile("empty_standard_repo/.gitattributes", "[attr]customattr key=value\n");
+	cl_git_rewritefile("empty_standard_repo/dir/.gitattributes", "file customattr\n");
+
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "dir/file", "key"));
+	cl_assert_equal_s(value, "value");
+}
+
+void test_attr_macro__changing_macro_in_root_wd_updates_attributes(void)
+{
+	const char *value;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_rewritefile("empty_standard_repo/.gitattributes",
+			   "[attr]customattr key=first\n"
+			   "file customattr\n");
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file", "key"));
+	cl_assert_equal_s(value, "first");
+
+	cl_git_rewritefile("empty_standard_repo/.gitattributes",
+			   "[attr]customattr key=second\n"
+			   "file customattr\n");
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file", "key"));
+	cl_assert_equal_s(value, "second");
+}
+
+void test_attr_macro__macros_in_subdir_do_not_apply(void)
+{
+	const char *value;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_pass(p_mkdir("empty_standard_repo/dir", 0777));
+	cl_git_rewritefile("empty_standard_repo/dir/.gitattributes",
+			   "[attr]customattr key=value\n"
+			   "file customattr\n");
+
+	/* This should _not_ pass, as macros in subdirectories shall be ignored */
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "dir/file", "key"));
+	cl_assert_equal_p(value, NULL);
+}
+
+void test_attr_macro__adding_macro_succeeds(void)
+{
+	const char *value;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+	cl_git_pass(git_attr_add_macro(g_repo, "macro", "key=value"));
+	cl_git_rewritefile("empty_standard_repo/.gitattributes", "file.txt macro\n");
+
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "key"));
+	cl_assert_equal_s(value, "value");
+}
+
+void test_attr_macro__adding_boolean_macros_succeeds(void)
+{
+	const char *value;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+	cl_git_pass(git_attr_add_macro(g_repo, "macro-pos", "positive"));
+	cl_git_pass(git_attr_add_macro(g_repo, "macro-neg", "-negative"));
+	cl_git_rewritefile("empty_standard_repo/.gitattributes", "file.txt macro-pos macro-neg\n");
+
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "positive"));
+	cl_assert(GIT_ATTR_IS_TRUE(value));
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "negative"));
+	cl_assert(GIT_ATTR_IS_FALSE(value));
+}
+
+void test_attr_macro__redefining_macro_succeeds(void)
+{
+	const char *value;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+	cl_git_pass(git_attr_add_macro(g_repo, "macro", "key=value1"));
+	cl_git_pass(git_attr_add_macro(g_repo, "macro", "key=value2"));
+	cl_git_rewritefile("empty_standard_repo/.gitattributes", "file.txt macro");
+
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "key"));
+	cl_assert_equal_s(value, "value2");
+}
+
+void test_attr_macro__recursive_macro_resolves(void)
+{
+	const char *value;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+	cl_git_pass(git_attr_add_macro(g_repo, "expandme", "key=value"));
+	cl_git_pass(git_attr_add_macro(g_repo, "macro", "expandme"));
+	cl_git_rewritefile("empty_standard_repo/.gitattributes", "file.txt macro");
+
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "key"));
+	cl_assert_equal_s(value, "value");
+}

tests/attr/repo.c

@@ -219,76 +219,6 @@ void test_attr_repo__manpage_example(void)
 	cl_assert(GIT_ATTR_IS_UNSPECIFIED(value));
 }
 
-void test_attr_repo__macros(void)
-{
-	const char *names[5] = { "rootattr", "binary", "diff", "crlf", "frotz" };
-	const char *names2[5] = { "mymacro", "positive", "negative", "rootattr", "another" };
-	const char *names3[3] = { "macro2", "multi2", "multi3" };
-	const char *values[5];
-
-	cl_git_pass(git_attr_get_many(values, g_repo, 0, "binfile", 5, names));
-
-	cl_assert(GIT_ATTR_IS_TRUE(values[0]));
-	cl_assert(GIT_ATTR_IS_TRUE(values[1]));
-	cl_assert(GIT_ATTR_IS_FALSE(values[2]));
-	cl_assert(GIT_ATTR_IS_FALSE(values[3]));
-	cl_assert(GIT_ATTR_IS_UNSPECIFIED(values[4]));
-
-	cl_git_pass(git_attr_get_many(values, g_repo, 0, "macro_test", 5, names2));
-
-	cl_assert(GIT_ATTR_IS_TRUE(values[0]));
-	cl_assert(GIT_ATTR_IS_TRUE(values[1]));
-	cl_assert(GIT_ATTR_IS_FALSE(values[2]));
-	cl_assert(GIT_ATTR_IS_UNSPECIFIED(values[3]));
-	cl_assert_equal_s("77", values[4]);
-
-	cl_git_pass(git_attr_get_many(values, g_repo, 0, "macro_test", 3, names3));
-
-	cl_assert(GIT_ATTR_IS_TRUE(values[0]));
-	cl_assert(GIT_ATTR_IS_FALSE(values[1]));
-	cl_assert_equal_s("answer", values[2]);
-}
-
-void test_attr_repo__bad_macros(void)
-{
-	const char *names[6] = { "rootattr", "positive", "negative",
-		"firstmacro", "secondmacro", "thirdmacro" };
-	const char *values[6];
-
-	cl_git_pass(git_attr_get_many(values, g_repo, 0, "macro_bad", 6, names));
-
-	/* these three just confirm that the "mymacro" rule ran */
-	cl_assert(GIT_ATTR_IS_UNSPECIFIED(values[0]));
-	cl_assert(GIT_ATTR_IS_TRUE(values[1]));
-	cl_assert(GIT_ATTR_IS_FALSE(values[2]));
-
-	/* file contains:
-	 *     # let's try some malicious macro defs
-	 *     [attr]firstmacro -thirdmacro -secondmacro
-	 *     [attr]secondmacro firstmacro -firstmacro
-	 *     [attr]thirdmacro secondmacro=hahaha -firstmacro
-	 *     macro_bad firstmacro secondmacro thirdmacro
-	 *
-	 * firstmacro assignment list ends up with:
-	 *     -thirdmacro -secondmacro
-	 * secondmacro assignment list expands "firstmacro" and ends up with:
-	 *     -thirdmacro -secondmacro -firstmacro
-	 * thirdmacro assignment don't expand so list ends up with:
-	 *     secondmacro="hahaha"
-	 *
-	 * macro_bad assignment list ends up with:
-	 *     -thirdmacro -secondmacro firstmacro &&
-	 *     -thirdmacro -secondmacro -firstmacro secondmacro &&
-	 *     secondmacro="hahaha" thirdmacro
-	 *
-	 * so summary results should be:
-	 *     -firstmacro secondmacro="hahaha" thirdmacro
-	 */
-	cl_assert(GIT_ATTR_IS_FALSE(values[3]));
-	cl_assert_equal_s("hahaha", values[4]);
-	cl_assert(GIT_ATTR_IS_TRUE(values[5]));
-}
-
 #define CONTENT "I'm going to be dynamically processed\r\n" \
 	"And my line endings...\r\n" \
 	"...are going to be\n" \
@@ -421,3 +351,55 @@ void test_attr_repo__sysdir_with_session(void)
 	git_buf_dispose(&sysdir);
 	git_attr_session__free(&session);
 }
+
+void test_attr_repo__rewrite(void)
+{
+	const char *value;
+
+	cl_git_rewritefile("attr/.gitattributes", "file.txt foo=first\n");
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "foo"));
+	cl_assert_equal_s(value, "first");
+
+	cl_git_rewritefile("attr/.gitattributes", "file.txt foo=second\n");
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "foo"));
+	cl_assert_equal_s(value, "second");
+
+	cl_git_rewritefile("attr/.gitattributes", "file.txt other=value\n");
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "foo"));
+	cl_assert_equal_p(value, NULL);
+}
+
+void test_attr_repo__rewrite_sysdir(void)
+{
+	git_buf sysdir = GIT_BUF_INIT;
+	const char *value;
+
+	cl_git_pass(p_mkdir("system", 0777));
+	cl_git_pass(git_buf_joinpath(&sysdir, clar_sandbox_path(), "system"));
+	cl_git_pass(git_sysdir_set(GIT_SYSDIR_SYSTEM, sysdir.ptr));
+	g_repo = cl_git_sandbox_reopen();
+
+	cl_git_rewritefile("system/gitattributes", "file foo=first");
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file", "foo"));
+	cl_assert_equal_s(value, "first");
+
+	cl_git_rewritefile("system/gitattributes", "file foo=second");
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file", "foo"));
+	cl_assert_equal_s(value, "second");
+
+	git_buf_dispose(&sysdir);
+}
+
+void test_attr_repo__unlink(void)
+{
+	const char *value;
+
+	cl_git_rewritefile("attr/.gitattributes", "file.txt foo=value1\n");
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "foo"));
+	cl_assert_equal_s(value, "value1");
+
+	cl_git_pass(p_unlink("attr/.gitattributes"));
+
+	cl_git_pass(git_attr_get(&value, g_repo, 0, "file.txt", "foo"));
+	cl_assert_equal_p(value, NULL);
+}