From adad518129818399f5d98676784ea8c9396ac30e Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 May 2015 13:52:47 -0400 Subject: attr: regression tests for ignore matching Ensure that when examining a .gitignore in a subdirectory, we do not erroneously apply the paths contained therein to the root of the repository. (Fixed in c02a0e4). --- tests/attr/ignore.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c index aa5b87098..1f4080084 100644 --- a/tests/attr/ignore.c +++ b/tests/attr/ignore.c @@ -191,3 +191,35 @@ void test_attr_ignore__expand_tilde_to_homedir(void) assert_is_ignored(false, "example.global_with_tilde"); } + +/* Ensure that the .gitignore in the subdirectory only affects + * items in the subdirectory. */ +void test_attr_ignore__gitignore_in_subdir(void) +{ + cl_git_rmfile("attr/.gitignore"); + + cl_must_pass(p_mkdir("attr/dir1", 0777)); + cl_must_pass(p_mkdir("attr/dir1/dir2", 0777)); + cl_must_pass(p_mkdir("attr/dir1/dir2/dir3", 0777)); + + cl_git_mkfile("attr/dir1/dir2/dir3/.gitignore", "dir1/\ndir1/subdir/"); + + assert_is_ignored(false, "dir1/file"); + assert_is_ignored(false, "dir1/dir2/file"); + assert_is_ignored(false, "dir1/dir2/dir3/file"); + assert_is_ignored(false, "dir1/dir2/dir3/dir1"); + assert_is_ignored(true, "dir1/dir2/dir3/dir1/file"); + assert_is_ignored(true, "dir1/dir2/dir3/dir1/subdir/foo"); + + if (cl_repo_get_bool(g_repo, "core.ignorecase")) { + cl_git_mkfile("attr/dir1/dir2/dir3/.gitignore", "DiR1/\nDiR1/subdir/\n"); + + assert_is_ignored(false, "dir1/file"); + assert_is_ignored(false, "dir1/dir2/file"); + assert_is_ignored(false, "dir1/dir2/dir3/file"); + assert_is_ignored(false, "dir1/dir2/dir3/dir1"); + assert_is_ignored(true, "dir1/dir2/dir3/dir1/file"); + assert_is_ignored(true, "dir1/dir2/dir3/dir1/subdir/foo"); + } +} + -- cgit v1.2.1 From 97fb9ac73905f1a3be7c6dbebbfd796315b19d88 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 May 2015 13:54:28 -0400 Subject: attr: test that a file is not ignored for a folder When a .gitignore specifies some folder "foo/", ensure that a file with the same name "foo" is not ignored. --- tests/attr/ignore.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c index 1f4080084..5b64b7d82 100644 --- a/tests/attr/ignore.c +++ b/tests/attr/ignore.c @@ -223,3 +223,20 @@ void test_attr_ignore__gitignore_in_subdir(void) } } +void test_attr_ignore__depth_file_not_ignored_when_folder_specified(void) +{ + cl_git_rmfile("attr/.gitignore"); + + cl_must_pass(p_mkdir("attr/dir1", 0777)); + cl_must_pass(p_mkdir("attr/dir1/dir2", 0777)); + cl_must_pass(p_mkdir("attr/dir1/dir2/dir3", 0777)); + + cl_git_mkfile("attr/dir1/dir2/dir3/.gitignore", "dir1/\n"); + + assert_is_ignored(false, "dir1/dir2/dir3/dir1"); + + if (cl_repo_get_bool(g_repo, "core.ignorecase")) { + assert_is_ignored(false, "dir1/dir2/dir3/DiR1"); + } +} + -- cgit v1.2.1 From 9486d203363eeb07166175818fa361e4e5a34015 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 May 2015 13:07:59 -0400 Subject: attr test: test a file beneath ignored folder --- tests/attr/ignore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c index 5b64b7d82..a55b12394 100644 --- a/tests/attr/ignore.c +++ b/tests/attr/ignore.c @@ -160,7 +160,7 @@ void test_attr_ignore__subdirectory_gitignore(void) assert_is_ignored(true, "file1"); assert_is_ignored(true, "dir/file1"); - assert_is_ignored(true, "dir/file2"); /* in ignored dir */ + assert_is_ignored(true, "dir/file2/actual_file"); /* in ignored dir */ assert_is_ignored(false, "dir/file3"); } -- cgit v1.2.1 From ef6d072236bb049480128046932e560e1335cef8 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 May 2015 13:06:05 -0400 Subject: attr: don't match files for folders When ignoring a path "foo/", ensure that this is actually a directory, and not simply a file named "foo". --- src/attr_file.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/attr_file.c b/src/attr_file.c index ef98aacc2..a7cc5916d 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -410,6 +410,14 @@ bool git_attr_fnmatch__match( else matchpath = path->path; + /* fail match if this is a file with same name as ignored folder */ + bool samename = (match->flags & GIT_ATTR_FNMATCH_ICASE) ? + !strcasecmp(match->pattern, matchpath) : + !strcmp(match->pattern, matchpath); + + if (samename) + return false; + matchval = p_fnmatch(match->pattern, matchpath, flags); path->basename[-1] = '/'; return (matchval != FNM_NOMATCH); -- cgit v1.2.1 From 30e629a0731e205f544205b7900b4d5083e57f08 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 May 2015 17:34:14 -0400 Subject: attr: always return errors --- src/attr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/attr.c b/src/attr.c index 102d0248c..d43a15f50 100644 --- a/src/attr.c +++ b/src/attr.c @@ -309,7 +309,8 @@ static int attr_setup(git_repository *repo, git_attr_session *attr_session) if (error == 0) error = preload_attr_file( repo, attr_session, GIT_ATTR_FILE__FROM_FILE, NULL, sys.ptr); - else if (error != GIT_ENOTFOUND) + + if (error != GIT_ENOTFOUND) return error; git_buf_free(&sys); -- cgit v1.2.1 From 9465bedb097078355ea4131a839d15f242cd4652 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 May 2015 16:02:18 -0400 Subject: attr: don't mangle file path during attr matching When determining whether some file matches an attr pattern, do not try to truncate the path to pass to fnmatch. When there is no containing directory for an item (eg, from a .gitignore in the root) this will cause us to truncate our path, which means that we cannot do meaningful comparisons on it and we may have false positives when trying to determine whether a given file is actually a file or a folder (as we have lost the path's base information.) This mangling was to allow fnmatch to compare a directory on disk to the name of a directory, but it is unnecessary as our fnmatch accepts FNM_LEADING_DIR. --- src/attr_file.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/attr_file.c b/src/attr_file.c index a7cc5916d..5ec5b4408 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -401,10 +401,9 @@ bool git_attr_fnmatch__match( path->basename == path->path) return false; - /* for ignore checks, use container of current item for check */ - path->basename[-1] = '\0'; flags |= FNM_LEADING_DIR; + /* for ignore checks, use container of current item for check */ if (match->containing_dir) matchpath = path->basename; else @@ -419,7 +418,7 @@ bool git_attr_fnmatch__match( return false; matchval = p_fnmatch(match->pattern, matchpath, flags); - path->basename[-1] = '/'; + return (matchval != FNM_NOMATCH); } -- cgit v1.2.1 From 90997e405d6bf7abb3f9ed336f5f99639d52a15e Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 May 2015 12:14:55 -0400 Subject: attr: less path mangling during attribute matching When handling attr matching, simply compare the directory path where the attribute file resides to the path being matched. Skip over commonality to allow us to compare the contents of the attribute file to the remainder of the path. This allows us to more easily compare the pattern directly to the path, instead of trying to guess whether we want to compare the path's basename or the full path based on whether the match was inside a containing directory or not. This also allows us to do fewer translations on the pattern (trying to re-prefix it.) --- src/attr_file.c | 52 ++++++++++++---------------------------------------- 1 file changed, 12 insertions(+), 40 deletions(-) diff --git a/src/attr_file.c b/src/attr_file.c index 5ec5b4408..dcf54fdd4 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -359,6 +359,7 @@ bool git_attr_fnmatch__match( git_attr_fnmatch *match, git_attr_path *path) { + const char *relpath = path->path; const char *filename; int flags = 0; @@ -375,6 +376,8 @@ bool git_attr_fnmatch__match( if (git__prefixcmp(path->path, match->containing_dir)) return 0; } + + relpath += match->containing_dir_length; } if (match->flags & GIT_ATTR_FNMATCH_ICASE) @@ -383,7 +386,7 @@ bool git_attr_fnmatch__match( flags |= FNM_LEADING_DIR; if (match->flags & GIT_ATTR_FNMATCH_FULLPATH) { - filename = path->path; + filename = relpath; flags |= FNM_PATHNAME; } else { filename = path->basename; @@ -393,9 +396,6 @@ bool git_attr_fnmatch__match( } if ((match->flags & GIT_ATTR_FNMATCH_DIRECTORY) && !path->is_dir) { - int matchval; - char *matchpath; - /* for attribute checks or root ignore checks, fail match */ if (!(match->flags & GIT_ATTR_FNMATCH_IGNORE) || path->basename == path->path) @@ -403,32 +403,24 @@ bool git_attr_fnmatch__match( flags |= FNM_LEADING_DIR; - /* for ignore checks, use container of current item for check */ - if (match->containing_dir) - matchpath = path->basename; - else - matchpath = path->path; - /* fail match if this is a file with same name as ignored folder */ bool samename = (match->flags & GIT_ATTR_FNMATCH_ICASE) ? - !strcasecmp(match->pattern, matchpath) : - !strcmp(match->pattern, matchpath); + !strcasecmp(match->pattern, relpath) : + !strcmp(match->pattern, relpath); if (samename) return false; - matchval = p_fnmatch(match->pattern, matchpath, flags); - - return (matchval != FNM_NOMATCH); + return (p_fnmatch(match->pattern, relpath, flags) != FNM_NOMATCH); } /* if path is a directory prefix of a negated pattern, then match */ if ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) && path->is_dir) { - size_t pathlen = strlen(path->path); + size_t pathlen = strlen(relpath); bool prefixed = (pathlen <= match->length) && ((match->flags & GIT_ATTR_FNMATCH_ICASE) ? - !strncasecmp(match->pattern, path->path, pathlen) : - !strncmp(match->pattern, path->path, pathlen)); + !strncasecmp(match->pattern, relpath, pathlen) : + !strncmp(match->pattern, relpath, pathlen)); if (prefixed && git_path_at_end_of_segment(&match->pattern[pathlen])) return true; @@ -647,7 +639,7 @@ int git_attr_fnmatch__parse( } if (context) { - char *slash = strchr(context, '/'); + char *slash = strrchr(context, '/'); size_t len; if (slash) { /* include the slash for easier matching */ @@ -657,27 +649,7 @@ int git_attr_fnmatch__parse( } } - if ((spec->flags & GIT_ATTR_FNMATCH_FULLPATH) != 0 && - context != NULL && git_path_root(pattern) < 0) - { - /* use context path minus the trailing filename */ - char *slash = strrchr(context, '/'); - size_t contextlen = slash ? slash - context + 1 : 0; - - /* given an unrooted fullpath match from a file inside a repo, - * prefix the pattern with the relative directory of the source file - */ - spec->pattern = git_pool_malloc( - pool, (uint32_t)(contextlen + spec->length + 1)); - if (spec->pattern) { - memcpy(spec->pattern, context, contextlen); - memcpy(spec->pattern + contextlen, pattern, spec->length); - spec->length += contextlen; - spec->pattern[spec->length] = '\0'; - } - } else { - spec->pattern = git_pool_strndup(pool, pattern, spec->length); - } + spec->pattern = git_pool_strndup(pool, pattern, spec->length); if (!spec->pattern) { *base = git__next_line(pattern); -- cgit v1.2.1 From 34593aaec01ac68f8f5b75a43a5c097db3769fbe Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 May 2015 17:00:25 -0400 Subject: attr: declare variable at top of block for msvc --- src/attr_file.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/attr_file.c b/src/attr_file.c index dcf54fdd4..89706865a 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -396,6 +396,8 @@ bool git_attr_fnmatch__match( } if ((match->flags & GIT_ATTR_FNMATCH_DIRECTORY) && !path->is_dir) { + bool samename; + /* for attribute checks or root ignore checks, fail match */ if (!(match->flags & GIT_ATTR_FNMATCH_IGNORE) || path->basename == path->path) @@ -404,7 +406,7 @@ bool git_attr_fnmatch__match( flags |= FNM_LEADING_DIR; /* fail match if this is a file with same name as ignored folder */ - bool samename = (match->flags & GIT_ATTR_FNMATCH_ICASE) ? + samename = (match->flags & GIT_ATTR_FNMATCH_ICASE) ? !strcasecmp(match->pattern, relpath) : !strcmp(match->pattern, relpath); -- cgit v1.2.1 From 882cc37f8360ef3a3d4dd60621f78375337df25f Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 13 May 2015 10:56:55 -0400 Subject: attr tests: make explicit our dir/file match tests --- tests/attr/ignore.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c index a55b12394..27fed2539 100644 --- a/tests/attr/ignore.c +++ b/tests/attr/ignore.c @@ -207,7 +207,6 @@ void test_attr_ignore__gitignore_in_subdir(void) assert_is_ignored(false, "dir1/file"); assert_is_ignored(false, "dir1/dir2/file"); assert_is_ignored(false, "dir1/dir2/dir3/file"); - assert_is_ignored(false, "dir1/dir2/dir3/dir1"); assert_is_ignored(true, "dir1/dir2/dir3/dir1/file"); assert_is_ignored(true, "dir1/dir2/dir3/dir1/subdir/foo"); @@ -217,26 +216,39 @@ void test_attr_ignore__gitignore_in_subdir(void) assert_is_ignored(false, "dir1/file"); assert_is_ignored(false, "dir1/dir2/file"); assert_is_ignored(false, "dir1/dir2/dir3/file"); - assert_is_ignored(false, "dir1/dir2/dir3/dir1"); assert_is_ignored(true, "dir1/dir2/dir3/dir1/file"); assert_is_ignored(true, "dir1/dir2/dir3/dir1/subdir/foo"); } } -void test_attr_ignore__depth_file_not_ignored_when_folder_specified(void) +/* Ensure that files do not match folder cases */ +void test_attr_ignore__dont_ignore_files_for_folder(void) { cl_git_rmfile("attr/.gitignore"); - cl_must_pass(p_mkdir("attr/dir1", 0777)); - cl_must_pass(p_mkdir("attr/dir1/dir2", 0777)); - cl_must_pass(p_mkdir("attr/dir1/dir2/dir3", 0777)); + cl_git_mkfile("attr/dir/.gitignore", "test/\n"); - cl_git_mkfile("attr/dir1/dir2/dir3/.gitignore", "dir1/\n"); + /* Create "test" as a file; ensure it is not ignored. */ + cl_git_mkfile("attr/dir/test", "This is a file."); - assert_is_ignored(false, "dir1/dir2/dir3/dir1"); + assert_is_ignored(false, "dir/test"); + if (cl_repo_get_bool(g_repo, "core.ignorecase")) + assert_is_ignored(false, "dir/TeSt"); - if (cl_repo_get_bool(g_repo, "core.ignorecase")) { - assert_is_ignored(false, "dir1/dir2/dir3/DiR1"); - } -} + /* Create "test" as a directory; ensure it is ignored. */ + cl_git_rmfile("attr/dir/test"); + cl_must_pass(p_mkdir("attr/dir/test", 0777)); + + assert_is_ignored(true, "dir/test"); + if (cl_repo_get_bool(g_repo, "core.ignorecase")) + assert_is_ignored(true, "dir/TeSt"); + /* Remove "test" entirely; ensure it is not ignored. + * (As it doesn't exist, it is not a directory.) + */ + cl_must_pass(p_rmdir("attr/dir/test")); + + assert_is_ignored(false, "dir/test"); + if (cl_repo_get_bool(g_repo, "core.ignorecase")) + assert_is_ignored(false, "dir/TeSt"); +} -- cgit v1.2.1