From d58336dda873704f8d12a8b78c3191deefa4ec14 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 26 Apr 2012 10:51:45 -0700 Subject: [PATCH] Fix leading slash behavior in attrs/ignores We were not following the git behavior for leading slashes in path names when matching git ignores and git attribute file patterns. This should fix issue #638. --- src/attr.c | 34 ++++++++++++++----------- src/attr_file.c | 51 ++++++++++++++++++++++++++++---------- src/attr_file.h | 6 ++++- src/ignore.c | 9 ++++--- tests-clar/attr/lookup.c | 8 ++++-- tests-clar/status/ignore.c | 28 +++++++++++++++++++++ 6 files changed, 103 insertions(+), 33 deletions(-) diff --git a/src/attr.c b/src/attr.c index 3e3a7e749..120d12737 100644 --- a/src/attr.c +++ b/src/attr.c @@ -23,10 +23,11 @@ int git_attr_get( *value = NULL; - if ((error = git_attr_path__init( - &path, pathname, git_repository_workdir(repo))) < 0 || - (error = collect_attr_files(repo, pathname, &files)) < 0) - return error; + if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0) + return -1; + + if ((error = collect_attr_files(repo, pathname, &files)) < 0) + goto cleanup; attr.name = name; attr.name_hash = git_attr_file__name_hash(name); @@ -38,13 +39,14 @@ int git_attr_get( if (pos >= 0) { *value = ((git_attr_assignment *)git_vector_get( &rule->assigns, pos))->value; - goto found; + goto cleanup; } } } -found: +cleanup: git_vector_free(&files); + git_attr_path__free(&path); return error; } @@ -70,10 +72,11 @@ int git_attr_get_many( memset((void *)values, 0, sizeof(const char *) * num_attr); - if ((error = git_attr_path__init( - &path, pathname, git_repository_workdir(repo))) < 0 || - (error = collect_attr_files(repo, pathname, &files)) < 0) - return error; + if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0) + return -1; + + if ((error = collect_attr_files(repo, pathname, &files)) < 0) + goto cleanup; info = git__calloc(num_attr, sizeof(attr_get_many_info)); GITERR_CHECK_ALLOC(info); @@ -108,6 +111,7 @@ int git_attr_get_many( cleanup: git_vector_free(&files); + git_attr_path__free(&path); git__free(info); return error; @@ -128,10 +132,11 @@ int git_attr_foreach( git_attr_assignment *assign; git_strmap *seen = NULL; - if ((error = git_attr_path__init( - &path, pathname, git_repository_workdir(repo))) < 0 || - (error = collect_attr_files(repo, pathname, &files)) < 0) - return error; + if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0) + return -1; + + if ((error = collect_attr_files(repo, pathname, &files)) < 0) + goto cleanup; seen = git_strmap_alloc(); GITERR_CHECK_ALLOC(seen); @@ -158,6 +163,7 @@ int git_attr_foreach( cleanup: git_strmap_free(seen); git_vector_free(&files); + git_attr_path__free(&path); return error; } diff --git a/src/attr_file.c b/src/attr_file.c index e34053fc3..650b58fcc 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -251,27 +251,50 @@ git_attr_assignment *git_attr_rule__lookup_assignment( int git_attr_path__init( git_attr_path *info, const char *path, const char *base) { - assert(info && path); - info->path = path; - info->basename = strrchr(path, '/'); + /* build full path as best we can */ + git_buf_init(&info->full, 0); + + if (base != NULL && git_path_root(path) < 0) { + if (git_buf_joinpath(&info->full, base, path) < 0) + return -1; + info->path = info->full.ptr + strlen(base); + } else { + if (git_buf_sets(&info->full, path) < 0) + return -1; + info->path = info->full.ptr; + } + + /* remove trailing slashes */ + while (info->full.size > 0) { + if (info->full.ptr[info->full.size - 1] != '/') + break; + info->full.size--; + } + info->full.ptr[info->full.size] = '\0'; + + /* skip leading slashes in path */ + while (*info->path == '/') + info->path++; + + /* find trailing basename component */ + info->basename = strrchr(info->path, '/'); if (info->basename) info->basename++; if (!info->basename || !*info->basename) - info->basename = path; + info->basename = info->path; - if (base != NULL && git_path_root(path) < 0) { - git_buf full_path = GIT_BUF_INIT; - if (git_buf_joinpath(&full_path, base, path) < 0) - return -1; - info->is_dir = (int)git_path_isdir(full_path.ptr); - git_buf_free(&full_path); - return 0; - } - info->is_dir = (int)git_path_isdir(path); + info->is_dir = (int)git_path_isdir(info->full.ptr); return 0; } +void git_attr_path__free(git_attr_path *info) +{ + git_buf_free(&info->full); + info->path = NULL; + info->basename = NULL; +} + /* * From gitattributes(5): @@ -353,6 +376,8 @@ int git_attr_fnmatch__parse( if (*scan == '/') { spec->flags = spec->flags | GIT_ATTR_FNMATCH_FULLPATH; slash_count++; + if (pattern == scan) + pattern++; } /* remember if we see an unescaped wildcard in pattern */ else if ((*scan == '*' || *scan == '.' || *scan == '[') && diff --git a/src/attr_file.h b/src/attr_file.h index 677534158..10851bc49 100644 --- a/src/attr_file.h +++ b/src/attr_file.h @@ -10,6 +10,7 @@ #include "git2/attr.h" #include "vector.h" #include "pool.h" +#include "buffer.h" #define GIT_ATTR_FILE ".gitattributes" #define GIT_ATTR_FILE_INREPO "info/attributes" @@ -54,9 +55,10 @@ typedef struct { } git_attr_file; typedef struct { + git_buf full; const char *path; const char *basename; - int is_dir; + int is_dir; } git_attr_path; /* @@ -114,6 +116,8 @@ extern git_attr_assignment *git_attr_rule__lookup_assignment( extern int git_attr_path__init( git_attr_path *info, const char *path, const char *base); +extern void git_attr_path__free(git_attr_path *info); + extern int git_attr_assignment__parse( git_repository *repo, /* needed to expand macros */ git_pool *pool, diff --git a/src/ignore.c b/src/ignore.c index 165754b4d..20b96c602 100644 --- a/src/ignore.c +++ b/src/ignore.c @@ -176,20 +176,23 @@ int git_ignore__lookup(git_ignores *ignores, const char *pathname, int *ignored) /* first process builtins - success means path was found */ if (ignore_lookup_in_rules( &ignores->ign_internal->rules, &path, ignored)) - return 0; + goto cleanup; /* next process files in the path */ git_vector_foreach(&ignores->ign_path, i, file) { if (ignore_lookup_in_rules(&file->rules, &path, ignored)) - return 0; + goto cleanup; } /* last process global ignores */ git_vector_foreach(&ignores->ign_global, i, file) { if (ignore_lookup_in_rules(&file->rules, &path, ignored)) - return 0; + goto cleanup; } *ignored = 0; + +cleanup: + git_attr_path__free(&path); return 0; } diff --git a/tests-clar/attr/lookup.c b/tests-clar/attr/lookup.c index accd617e6..81a4a55d3 100644 --- a/tests-clar/attr/lookup.c +++ b/tests-clar/attr/lookup.c @@ -25,6 +25,7 @@ void test_attr_lookup__simple(void) cl_git_pass(git_attr_file__lookup_one(file,&path,"missing",&value)); cl_assert(!value); + git_attr_path__free(&path); git_attr_file__free(file); } @@ -45,6 +46,8 @@ static void run_test_cases(git_attr_file *file, struct attr_expected *cases, int cl_git_pass(error); attr_check_expected(c->expected, c->expected_str, value); + + git_attr_path__free(&path); } } @@ -83,7 +86,7 @@ void test_attr_lookup__match_variants(void) { "/not/pat2/yousee", "attr2", EXPECT_UNDEFINED, NULL }, /* path match */ { "pat3file", "attr3", EXPECT_UNDEFINED, NULL }, - { "/pat3dir/pat3file", "attr3", EXPECT_UNDEFINED, NULL }, + { "/pat3dir/pat3file", "attr3", EXPECT_TRUE, NULL }, { "pat3dir/pat3file", "attr3", EXPECT_TRUE, NULL }, /* pattern* match */ { "pat4.txt", "attr4", EXPECT_TRUE, NULL }, @@ -101,7 +104,7 @@ void test_attr_lookup__match_variants(void) { "pat6/pat6/.pat6", "attr6", EXPECT_TRUE, NULL }, { "pat6/pat6/extra/foobar.pat6", "attr6", EXPECT_UNDEFINED, NULL }, { "/prefix/pat6/pat6/foobar.pat6", "attr6", EXPECT_UNDEFINED, NULL }, - { "/pat6/pat6/foobar.pat6", "attr6", EXPECT_UNDEFINED, NULL }, + { "/pat6/pat6/foobar.pat6", "attr6", EXPECT_TRUE, NULL }, /* complex pattern */ { "pat7a12z", "attr7", EXPECT_TRUE, NULL }, { "pat7e__x", "attr7", EXPECT_TRUE, NULL }, @@ -139,6 +142,7 @@ void test_attr_lookup__match_variants(void) run_test_cases(file, dir_cases, 1); git_attr_file__free(file); + git_attr_path__free(&path); } void test_attr_lookup__assign_variants(void) diff --git a/tests-clar/status/ignore.c b/tests-clar/status/ignore.c index 5d940077c..94f8c3de3 100644 --- a/tests-clar/status/ignore.c +++ b/tests-clar/status/ignore.c @@ -50,3 +50,31 @@ void test_status_ignore__0(void) cl_assert(git_attr_cache__is_cached(g_repo, ".git/info/exclude")); cl_assert(git_attr_cache__is_cached(g_repo, ".gitignore")); } + + +void test_status_ignore__1(void) +{ + int ignored; + + cl_git_rewritefile("attr/.gitignore", "/*.txt\n/dir/\n"); + git_attr_cache_flush(g_repo); + + cl_git_pass(git_status_should_ignore(g_repo, "root_test4.txt", &ignored)); + cl_assert(ignored); + + cl_git_pass(git_status_should_ignore(g_repo, "sub/subdir_test2.txt", &ignored)); + cl_assert(!ignored); + + cl_git_pass(git_status_should_ignore(g_repo, "dir", &ignored)); + cl_assert(ignored); + + cl_git_pass(git_status_should_ignore(g_repo, "dir/", &ignored)); + cl_assert(ignored); + + cl_git_pass(git_status_should_ignore(g_repo, "sub/dir", &ignored)); + cl_assert(!ignored); + + cl_git_pass(git_status_should_ignore(g_repo, "sub/dir/", &ignored)); + cl_assert(!ignored); +} +