Merge pull request #1773 from arrbee/fix-fnmatch-prefix

Revert PR #1462 and provide alternative fix

src/attr_file.c

@@ -79,13 +79,17 @@ int git_attr_file__parse_buffer(
 
 	while (!error && *scan) {
 		/* allocate rule if needed */
-		if (!rule && !(rule = git__calloc(1, sizeof(git_attr_rule)))) {
+		if (!rule) {
+			if (!(rule = git__calloc(1, sizeof(git_attr_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_gitattr_format(
+		if (!(error = git_attr_fnmatch__parse(
 				&rule->match, attrs->pool, context, &scan)) &&
 			!(error = git_attr_assignment__parse(
 				repo, attrs->pool, &rule->assigns, &scan)))
@@ -337,16 +341,23 @@ void git_attr_path__free(git_attr_path *info)
  * GIT_ENOTFOUND if the fnmatch does not require matching, or
  * another error code there was an actual problem.
  */
-int git_attr_fnmatch__parse_gitattr_format(
+int git_attr_fnmatch__parse(
 	git_attr_fnmatch *spec,
 	git_pool *pool,
 	const char *source,
 	const char **base)
 {
-	const char *pattern;
+	const char *pattern, *scan;
+	int slash_count, allow_space;
 
 	assert(spec && base && *base);
 
+	if (parse_optimized_patterns(spec, pool, *base))
+		return 0;
+
+	spec->flags = (spec->flags & GIT_ATTR_FNMATCH__INCOMING);
+	allow_space = ((spec->flags & GIT_ATTR_FNMATCH_ALLOWSPACE) != 0);
+
 	pattern = *base;
 
 	while (git__isspace(*pattern)) pattern++;
@@ -355,7 +366,7 @@ int git_attr_fnmatch__parse_gitattr_format(
 		return GIT_ENOTFOUND;
 	}
 
-	if (*pattern == '[') {
+	if (*pattern == '[' && (spec->flags & GIT_ATTR_FNMATCH_ALLOWMACRO) != 0) {
 		if (strncmp(pattern, "[attr]", 6) == 0) {
 			spec->flags = spec->flags | GIT_ATTR_FNMATCH_MACRO;
 			pattern += 6;
@@ -363,44 +374,11 @@ int git_attr_fnmatch__parse_gitattr_format(
 		/* else a character range like [a-e]* which is accepted */
 	}
 
-	if (*pattern == '!') {
+	if (*pattern == '!' && (spec->flags & GIT_ATTR_FNMATCH_ALLOWNEG) != 0) {
 		spec->flags = spec->flags | GIT_ATTR_FNMATCH_NEGATIVE;
 		pattern++;
 	}
 
-	if (git_attr_fnmatch__parse_shellglob_format(spec, pool, 
-		source, &pattern) < 0)
-			return -1;
-
-	*base = pattern;
-
-	return 0;
-}
-
-/*
- * Fills a spec for the purpose of pure pathspec matching, not
- * related to a gitattribute file parsing.
- *
- * This will return 0 if the spec was filled out, or
- * another error code there was an actual problem.
- */
-int git_attr_fnmatch__parse_shellglob_format(
-	git_attr_fnmatch *spec,
-	git_pool *pool,
-	const char *source,
-	const char **base)
-{
-	const char *pattern, *scan;
-	int slash_count, allow_space;
-
-	assert(spec && base && *base);
-
-	if (parse_optimized_patterns(spec, pool, *base))
-		return 0;
-
-	allow_space = (spec->flags & GIT_ATTR_FNMATCH_ALLOWSPACE) != 0;
-	pattern = *base;
-
 	slash_count = 0;
 	for (scan = pattern; *scan != '\0'; ++scan) {
 		/* scan until (non-escaped) white space */
@@ -636,7 +614,6 @@ static void git_attr_rule__clear(git_attr_rule *rule)
 	/* match.pattern is stored in a git_pool, so no need to free */
 	rule->match.pattern = NULL;
 	rule->match.length = 0;
-	rule->match.flags = 0;
 }
 
 void git_attr_rule__free(git_attr_rule *rule)

src/attr_file.h

@@ -28,6 +28,12 @@
 #define GIT_ATTR_FNMATCH_ALLOWSPACE	(1U << 6)
 #define GIT_ATTR_FNMATCH_ICASE		(1U << 7)
 #define GIT_ATTR_FNMATCH_MATCH_ALL	(1U << 8)
+#define GIT_ATTR_FNMATCH_ALLOWNEG   (1U << 9)
+#define GIT_ATTR_FNMATCH_ALLOWMACRO (1U << 10)
+
+#define GIT_ATTR_FNMATCH__INCOMING \
+	(GIT_ATTR_FNMATCH_ALLOWSPACE | \
+	 GIT_ATTR_FNMATCH_ALLOWNEG | GIT_ATTR_FNMATCH_ALLOWMACRO)
 
 extern const char *git_attr__true;
 extern const char *git_attr__false;
@@ -115,13 +121,7 @@ extern uint32_t git_attr_file__name_hash(const char *name);
  * other utilities
  */
 
-extern int git_attr_fnmatch__parse_gitattr_format(
-	git_attr_fnmatch *spec,
-	git_pool *pool,
-	const char *source,
-	const char **base);
-
-extern int git_attr_fnmatch__parse_shellglob_format(
+extern int git_attr_fnmatch__parse(
 	git_attr_fnmatch *spec,
 	git_pool *pool,
 	const char *source,

src/ignore.c

@@ -37,9 +37,9 @@ static int parse_ignore_file(
 			GITERR_CHECK_ALLOC(match);
 		}
 
-		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE;
+		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE | GIT_ATTR_FNMATCH_ALLOWNEG;
 
-		if (!(error = git_attr_fnmatch__parse_gitattr_format(
+		if (!(error = git_attr_fnmatch__parse(
 			match, ignores->pool, context, &scan)))
 		{
 			match->flags |= GIT_ATTR_FNMATCH_IGNORE;

src/pathspec.c

@@ -83,9 +83,9 @@ int git_pathspec__vinit(
 		if (!match)
 			return -1;
 
-		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE;
+		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE | GIT_ATTR_FNMATCH_ALLOWNEG;
 
-		ret = git_attr_fnmatch__parse_shellglob_format(match, strpool, NULL, &pattern);
+		ret = git_attr_fnmatch__parse(match, strpool, NULL, &pattern);
 		if (ret == GIT_ENOTFOUND) {
 			git__free(match);
 			continue;
@@ -160,6 +160,16 @@ static int pathspec_match_one(
 		path[match->length] == '/')
 		result = 0;
 
+	/* if we didn't match and this is a negative match, check for exact
+	 * match of filename with leading '!'
+	 */
+	if (result == FNM_NOMATCH &&
+		(match->flags & GIT_ATTR_FNMATCH_NEGATIVE) != 0 &&
+		*path == '!' &&
+		ctxt->strncomp(path + 1, match->pattern, match->length) == 0 &&
+		(!path[match->length + 1] || path[match->length + 1] == '/'))
+		return 1;
+
 	if (result == 0)
 		return (match->flags & GIT_ATTR_FNMATCH_NEGATIVE) ? 0 : 1;
 	return -1;

tests-clar/attr/file.c

@@ -49,7 +49,6 @@ void test_attr_file__match_variants(void)
 	cl_assert(rule);
 	cl_assert_equal_s("pat0", rule->match.pattern);
 	cl_assert(rule->match.length == strlen("pat0"));
-	cl_assert(rule->match.flags == 0);
 	cl_assert(rule->assigns.length == 1);
 	assign = get_assign(rule,0);
 	cl_assert_equal_s("attr0", assign->name);
@@ -59,16 +58,16 @@ void test_attr_file__match_variants(void)
 	rule = get_rule(1);
 	cl_assert_equal_s("pat1", rule->match.pattern);
 	cl_assert(rule->match.length == strlen("pat1"));
-	cl_assert(rule->match.flags == GIT_ATTR_FNMATCH_NEGATIVE);
+	cl_assert((rule->match.flags & GIT_ATTR_FNMATCH_NEGATIVE) != 0);
 
 	rule = get_rule(2);
 	cl_assert_equal_s("pat2", rule->match.pattern);
 	cl_assert(rule->match.length == strlen("pat2"));
-	cl_assert(rule->match.flags == GIT_ATTR_FNMATCH_DIRECTORY);
+	cl_assert((rule->match.flags & GIT_ATTR_FNMATCH_DIRECTORY) != 0);
 
 	rule = get_rule(3);
 	cl_assert_equal_s("pat3dir/pat3file", rule->match.pattern);
-	cl_assert(rule->match.flags == GIT_ATTR_FNMATCH_FULLPATH);
+	cl_assert((rule->match.flags & GIT_ATTR_FNMATCH_FULLPATH) != 0);
 
 	rule = get_rule(4);
 	cl_assert_equal_s("pat4.*", rule->match.pattern);
@@ -89,7 +88,6 @@ void test_attr_file__match_variants(void)
 	rule = get_rule(8);
 	cl_assert_equal_s("pat8 with spaces", rule->match.pattern);
 	cl_assert(rule->match.length == strlen("pat8 with spaces"));
-	cl_assert(rule->match.flags == 0);
 
 	rule = get_rule(9);
 	cl_assert_equal_s("pat9", rule->match.pattern);