From 4ba64794aee983483eef659623a765d61311ded1 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 9 Aug 2013 10:52:35 -0700 Subject: [PATCH 1/2] Revert PR #1462 and provide alternative fix This rolls back the changes to fnmatch parsing from commit 2e40a60e847d6c128af23e24ea7a8efebd2427da except for the tests that were added. Instead this adds couple of new flags that can be passed in when attempting to parse an fnmatch pattern. Also, this changes the pathspec match logic to special case matching a filename with a '!' prefix against a negative pattern. This fixes the build. --- src/attr_file.c | 85 +++++++++++++++--------------------------- src/attr_file.h | 14 +++---- src/ignore.c | 4 +- src/pathspec.c | 13 ++++++- tests-clar/attr/file.c | 8 ++-- 5 files changed, 54 insertions(+), 70 deletions(-) diff --git a/src/attr_file.c b/src/attr_file.c index ca5f2137c..92702df98 100644 --- a/src/attr_file.c +++ b/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)))) { - error = -1; - break; + 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,54 +341,7 @@ 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( - git_attr_fnmatch *spec, - git_pool *pool, - const char *source, - const char **base) -{ - const char *pattern; - - assert(spec && base && *base); - - pattern = *base; - - while (git__isspace(*pattern)) pattern++; - if (!*pattern || *pattern == '#') { - *base = git__next_line(pattern); - return GIT_ENOTFOUND; - } - - if (*pattern == '[') { - if (strncmp(pattern, "[attr]", 6) == 0) { - spec->flags = spec->flags | GIT_ATTR_FNMATCH_MACRO; - pattern += 6; - } - /* else a character range like [a-e]* which is accepted */ - } - - if (*pattern == '!') { - 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( +int git_attr_fnmatch__parse( git_attr_fnmatch *spec, git_pool *pool, const char *source, @@ -398,9 +355,30 @@ int git_attr_fnmatch__parse_shellglob_format( if (parse_optimized_patterns(spec, pool, *base)) return 0; - allow_space = (spec->flags & GIT_ATTR_FNMATCH_ALLOWSPACE) != 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++; + if (!*pattern || *pattern == '#') { + *base = git__next_line(pattern); + return GIT_ENOTFOUND; + } + + 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; + } + /* else a character range like [a-e]* which is accepted */ + } + + if (*pattern == '!' && (spec->flags & GIT_ATTR_FNMATCH_ALLOWNEG) != 0) { + spec->flags = spec->flags | GIT_ATTR_FNMATCH_NEGATIVE; + pattern++; + } + 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) diff --git a/src/attr_file.h b/src/attr_file.h index afea1e115..3bc7c6cb8 100644 --- a/src/attr_file.h +++ b/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, diff --git a/src/ignore.c b/src/ignore.c index 7d8280403..3c23158c2 100644 --- a/src/ignore.c +++ b/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; diff --git a/src/pathspec.c b/src/pathspec.c index 4266bb99e..1d7b71a74 100644 --- a/src/pathspec.c +++ b/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,15 @@ 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) + return 1; + if (result == 0) return (match->flags & GIT_ATTR_FNMATCH_NEGATIVE) ? 0 : 1; return -1; diff --git a/tests-clar/attr/file.c b/tests-clar/attr/file.c index 8866fd9bd..4eb1d22fe 100644 --- a/tests-clar/attr/file.c +++ b/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); From b7b77def931be50777884ddd35a8689df0ecbebd Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 9 Aug 2013 11:20:49 -0700 Subject: [PATCH 2/2] Match against file with leading ! was too broad --- src/pathspec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pathspec.c b/src/pathspec.c index 1d7b71a74..d56d03918 100644 --- a/src/pathspec.c +++ b/src/pathspec.c @@ -166,7 +166,8 @@ static int pathspec_match_one( if (result == FNM_NOMATCH && (match->flags & GIT_ATTR_FNMATCH_NEGATIVE) != 0 && *path == '!' && - ctxt->strncomp(path + 1, match->pattern, match->length) == 0) + ctxt->strncomp(path + 1, match->pattern, match->length) == 0 && + (!path[match->length + 1] || path[match->length + 1] == '/')) return 1; if (result == 0)